Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable admins to activate users #3015

Merged

Conversation

seanh
Copy link
Contributor

@seanh seanh commented Feb 24, 2016

Add an "Activate" button next to users on the /admin/users page so that
admins can activate user accounts.

Some refactoring to avoid code duplication between activating by users
clicking on an activation link and admins clicking the new activate button:
move the code that actually activates the user onto the User model where
both views can call it.

@seanh
Copy link
Contributor Author

seanh commented Feb 24, 2016

screenshot from 2016-02-24 17-50-02


screenshot from 2016-02-24 17-50-17

@lenazun
Copy link
Contributor

lenazun commented Feb 24, 2016

👏

@@ -132,6 +133,11 @@ def is_activated(self):

return False

def activate(self, request):
"""Activate the user by deleting any activation they have."""
request.db.delete(self.activation)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably better done as:

session = sa.orm.object_session(self)
session.delete(self.activation)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although we need request anyway to send the notification, but see below...

@nickstenning
Copy link
Contributor

Looks good, although I don't think moving the triggering of the ActivationEvent into the model is the right thing to do. Move that out and you can just have an .activate() method on the user that doesn't require any arguments.

@seanh seanh force-pushed the b9PFMDM7-add-activate-button-to-admin-users-page branch 2 times, most recently from 73e1617 to 71714a0 Compare February 25, 2016 19:37
Add an "Activate" button next to users on the /admin/users page so that
admins can activate user accounts.

Some refactoring to avoid code duplication between activating by users
clicking on an activation link and admins clicking the new activate button:
move the code that actually activates the user onto the User model where
both views can call it.
@seanh seanh force-pushed the b9PFMDM7-add-activate-button-to-admin-users-page branch from 71714a0 to cff6f6d Compare February 25, 2016 19:42
@seanh
Copy link
Contributor Author

seanh commented Feb 25, 2016

Ok, this is done again

@nickstenning
Copy link
Contributor

Looks fab. Sorry for the misunderstanding earlier!

nickstenning added a commit that referenced this pull request Feb 25, 2016
…-to-admin-users-page

Enable admins to activate users
@nickstenning nickstenning merged commit 9b0aa63 into master Feb 25, 2016
@nickstenning nickstenning deleted the b9PFMDM7-add-activate-button-to-admin-users-page branch February 25, 2016 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants