[Users] Add delete user mutation fixes(#134) #138

jugshaurya · 2020-06-10T13:45:07Z

Issue

fixes: #134

Solution

deletedAt property is used to soft delete the fields and can be used to restore them.

Screenshots (Desktop)

Screenshots (Mobile)

Checklist

PR will only be reviewed if the checklist is completed

I have done extensive testing for my changes.
I have self-reviewed my PR and removed all un-necessary changes.
My changes works well on both desktop and mobile environments.
I have made sure that my changes solves the actual problem mentioned in the issue.
The PR contains only 1 commit and have been updated with master.

boss-contributions-bot · 2020-06-10T13:45:09Z

Thanks @jugshaurya, for opening the pull request! 🙌

One of our mentors will review the pull request soon. ✅

Star ⭐ this project and tweet 🐦 about your contributions.

jugshaurya · 2020-06-10T13:45:22Z

@hereisnaman please review,

thenamankumar · 2020-06-10T16:42:38Z

api/services/User/index.js

@@ -10,7 +10,7 @@ import Availability from './Loaders/Availability';

 export default class User extends BaseModelService {
  static findByUsername(username) {
-    return Models.User.findOne({ where: { username } });
+    return Models.User.findOne({ where: { username }, paranoid: false });


expect options as a second arg and spread it. While calling from the login callback pass the paranoid as false from there inside the options object.

jugshaurya · 2020-06-10T17:29:15Z

@hereisnaman, please review. Changes done

jugshaurya · 2020-06-11T05:11:33Z

@hereisnaman , testing required means, you are going to test it or I am?

jugshaurya · 2020-06-12T14:57:17Z

@hereisnaman, can you help me out how can I test it as no frontend for it is written yet? maybe a new issue should be raised to add this feature with a checklist so that I can add that as well accordingly and test later on! and move on to issue #135 and assign that to me.

thenamankumar · 2020-06-12T16:30:01Z

@hereisnaman, can you help me out how can I test it as no frontend for it is written yet? maybe a new issue should be raised to add this feature with a checklist so that I can add that as well accordingly and test later on! and move on to issue #135 and assign that to me.

visit localhost:3000/api/graphql in browser and test from there.

jugshaurya · 2020-06-13T06:18:18Z

@hereisnaman Testing please review:

Test a: Add a mutation userDisable to delete the user.

deleted_at got filled

Test b: The action should only be allowed to admins.
non-admin-viewer

unauthorized

Test c: Update login callback to not create a user if a deleted entity for that user is present
How to test this ?
By the way in impersonating form:- once a user is deleted it is not showing there to log in as him/her.

thenamankumar · 2020-06-14T06:20:25Z

@jugshaurya for impersonation and user listing, you'll need to handle few tasks from #134

Lets include the last 2 tasks from that issue in this PR.

jugshaurya · 2020-06-14T06:43:42Z

@jugshaurya for impersonation and user listing, you'll need to handle few tasks from #134

Lets include the last 2 tasks from that issue in this PR.

@hereisnaman, i already included the last 2 tasks from issue #134 in this PR where if deleted_at property is not null, we redirect to homepage. And for cheking for admins test-b is also passing. You can see that in above screenshots

thenamankumar · 2020-06-14T15:11:29Z

@jugshaurya for impersonation and user listing, you'll need to handle few tasks from #134
Lets include the last 2 tasks from that issue in this PR.

@hereisnaman, i already included the last 2 tasks from issue #134 in this PR where if deleted_at property is not null, we redirect to homepage. And for cheking for admins test-b is also passing. You can see that in above screenshots

Then why is " By the way in impersonating form:- once a user is deleted it is not showing there to log in as him/her." this an issue. You can pass the required arguments in the query to display those right.

jugshaurya · 2020-06-14T15:48:27Z

@hereisnaman, i already included the last 2 tasks from issue #134 in this PR where if deleted_at property is not null, we redirect to homepage. And for cheking for admins test-b is also passing. You can see that in above screenshots

Then why is " By the way in impersonating form:- once a user is deleted it is not showing there to log in as him/her." this an issue. You can pass the required arguments in the query to display those right.

@hereisnaman , No that was not an issue, actually, i was saying for the test case c . Once the user is deleted and delete_at is filled, the users that are deleted are not showing in the form , which is what we want. So everything is fine. I was just giving explaination for test case c.
also for the point 3 in the checklist, I just used these two lines

   await user.loadByUsername(profile.username, { paranoid: false });
    if (user.deleted_at) return App.redirectToHome(res);

thenamankumar · 2020-06-15T11:24:49Z

app/mutations/userDisable.graphql

@@ -0,0 +1,5 @@
+mutation UserDisable($id: ID!) {
+  disableUser: UserDisable(id: $id) {


The mutation name is userDisable, fix the casing.

oh sorry for that typo

- Add a mutation userDisable to delete the user. - The action should only be allowed to admins. - Update login callback to not create a user, if a deleted entity for that user is present

jugshaurya · 2020-06-15T17:28:00Z

@hereisnaman, please review

jugshaurya · 2020-06-16T12:24:45Z

@hereisnaman, require some more changes?

boss-contributions-bot · 2020-06-18T09:08:45Z

Congratualtions @jugshaurya, your pull request is merged! 🎉

Thanks for your contributions and participating in BOSS 2020. 🙌

You can claim your bounty points here. 💰

thenamankumar approved these changes Jun 10, 2020

View reviewed changes

jugshaurya force-pushed the userDisable-mutation branch from f5726f4 to fcd00c2 Compare June 10, 2020 17:26

jugshaurya requested a review from thenamankumar June 10, 2020 17:29

thenamankumar added the Testing Required label Jun 10, 2020

thenamankumar suggested changes Jun 15, 2020

View reviewed changes

[Users] Add delete user mutation fixes(coding-blocks#134)

d785ec9

- Add a mutation userDisable to delete the user. - The action should only be allowed to admins. - Update login callback to not create a user, if a deleted entity for that user is present

jugshaurya force-pushed the userDisable-mutation branch from fcd00c2 to d785ec9 Compare June 15, 2020 17:26

jugshaurya requested a review from thenamankumar June 15, 2020 17:28

jugshaurya mentioned this pull request Jun 16, 2020

[Users] Implement undo user delete feature #135

Open

4 tasks

thenamankumar approved these changes Jun 18, 2020

View reviewed changes

thenamankumar merged commit 87a464f into coding-blocks:master Jun 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Users] Add delete user mutation fixes(#134) #138

[Users] Add delete user mutation fixes(#134) #138

jugshaurya commented Jun 10, 2020 •

edited

Loading

boss-contributions-bot bot commented Jun 10, 2020

jugshaurya commented Jun 10, 2020 •

edited

Loading

thenamankumar Jun 10, 2020

jugshaurya Jun 11, 2020

jugshaurya commented Jun 10, 2020

jugshaurya commented Jun 11, 2020 •

edited

Loading

jugshaurya commented Jun 12, 2020 •

edited

Loading

thenamankumar commented Jun 12, 2020

jugshaurya commented Jun 13, 2020 •

edited

Loading

thenamankumar commented Jun 14, 2020

jugshaurya commented Jun 14, 2020 •

edited

Loading

thenamankumar commented Jun 14, 2020

jugshaurya commented Jun 14, 2020 •

edited

Loading

thenamankumar Jun 15, 2020

jugshaurya Jun 15, 2020

jugshaurya commented Jun 15, 2020

jugshaurya commented Jun 16, 2020

boss-contributions-bot bot commented Jun 18, 2020

		@@ -0,0 +1,5 @@
		mutation UserDisable($id: ID!) {
		disableUser: UserDisable(id: $id) {

[Users] Add delete user mutation fixes(#134) #138

[Users] Add delete user mutation fixes(#134) #138

Conversation

jugshaurya commented Jun 10, 2020 • edited Loading

Issue

Solution

Screenshots (Desktop)

Screenshots (Mobile)

Checklist

boss-contributions-bot bot commented Jun 10, 2020

jugshaurya commented Jun 10, 2020 • edited Loading

thenamankumar Jun 10, 2020

Choose a reason for hiding this comment

jugshaurya Jun 11, 2020

Choose a reason for hiding this comment

jugshaurya commented Jun 10, 2020

jugshaurya commented Jun 11, 2020 • edited Loading

jugshaurya commented Jun 12, 2020 • edited Loading

thenamankumar commented Jun 12, 2020

jugshaurya commented Jun 13, 2020 • edited Loading

thenamankumar commented Jun 14, 2020

jugshaurya commented Jun 14, 2020 • edited Loading

thenamankumar commented Jun 14, 2020

jugshaurya commented Jun 14, 2020 • edited Loading

thenamankumar Jun 15, 2020

Choose a reason for hiding this comment

jugshaurya Jun 15, 2020

Choose a reason for hiding this comment

jugshaurya commented Jun 15, 2020

jugshaurya commented Jun 16, 2020

boss-contributions-bot bot commented Jun 18, 2020

jugshaurya commented Jun 10, 2020 •

edited

Loading

jugshaurya commented Jun 10, 2020 •

edited

Loading

jugshaurya commented Jun 11, 2020 •

edited

Loading

jugshaurya commented Jun 12, 2020 •

edited

Loading

jugshaurya commented Jun 13, 2020 •

edited

Loading

jugshaurya commented Jun 14, 2020 •

edited

Loading

jugshaurya commented Jun 14, 2020 •

edited

Loading