Opened 13 years ago

Closed 7 years ago

#103 closed defect (fixed)

Change user status into a boolean, "active" or "is_active"

Reported by: Christopher Allan Webber Owned by:
Priority: minor Milestone:
Component: programming Keywords: small
Cc: aaronw Parent Tickets:

Description (last modified by Christopher Allan Webber)

Currently we have status being one of "active" or
'needs\_email\_verification'... we could simplify things by just
having email\_verified (which we already have) and "active" be a
boolean.

The role of active will then switch to whether or not the user is
enabled. You might set active to False if a user was abusing their
account, for instance.

The migration on this should be pretty easy... just remove
needs\_email\_verification and make all existing users active,
since currently we don't have any inactive state.

This is a bitesized task.



Change History (11)

comment:1 by Christopher Allan Webber, 13 years ago

-  Update the auth views
-  you should be sure to update tests
-  gmg user commands
-  and the active user decorator.
-  submission views? Might be handled by above.



comment:2 by Aaron Williamson, 12 years ago

Owner: set to Elvenlord Elrond
Status: NewFeedback
I bit this bite-sized task today. Relevant branch is
[https://gitorious.org/\ :sub:`copiesofcopies/mediagoblin/copiesofcopies-mediagoblin/commits/feature390\_user\_status\_to\_boolean](https://gitorious.org/`\ copiesofcopies/mediagoblin/copiesofcopies-mediagoblin/commits/feature390\_user\_status\_to\_boolean)



comment:3 by Elrond, 12 years ago

Owner: changed from Elvenlord Elrond to Aaron Williamson
Status: FeedbackIn Progress
Thanks for starting at this!

I think, there was a misunderstanding somewhere.

We currently have two states: "needs email verification" and
"active". Those are currently mirrored in the .email\_verified
bool. So in theory we could completely drop the .status field and
use the .email\_verified field everywhere instead!
That said: Please really just drop it in your migration.

We could stop here.

Instead we want a new field .is\_active, so that users could be
de-activated later on. That field should be True by default! We
want active users! And it's, what the behaviour is till now.
That said: Just add it as a new field in your migration with a
default of True. Thing done.

Now to the gory details:

Most code probably wants to check for a "fully active user". In the
new modell this is a user with .is\_active==True (can login) and
.email\_verified==True (can do everything, because verified their
email).

So it might make sense to have a "@property is\_full\_user(self):
return self.is\_active && self.email\_verified" on the Mixin class
to allow easy access everywhere.

And a bunch of db queries likely will need to check both
variables.

A few places though (all those dealing with email verification,
likely) will be happy with a .is\_active==True only. Because they
only require a user that has not been actively disabled.

I hope all this makese sense (especialy after the talk on irc).



comment:4 by Will Kahn-Greene, 12 years ago

The original url for this bug was http://bugs.foocorp.net/issues/390 .

comment:5 by Christopher Allan Webber, 11 years ago

Description: modified (diff)

I think we need #697 in order to really do this right.

comment:6 by Christopher Allan Webber, 11 years ago

Cc: aaronw added
Owner: Aaron Williamson removed

Aaron are you actively working on this? I'm assuming not. Anyway, I'm removing ownership, feel free to reclaim!

comment:7 by Elrond, 11 years ago

I think, we should rethink this whole ticket again. And have a proper plan how to continue. This one is not simple.

chris: What ticket are you referring to in comment:5?

comment:8 by Christopher Allan Webber, 11 years ago

Ha. I wrote the wrong ticket number... accidental transposition.

I meant #679... I think in order to really have the right way of showing "states" of users, we want proper "fixture" support as described in that ticket.

comment:9 by Christopher Allan Webber, 11 years ago

Foundations (formerly referred to in this ticket as fixtures) have landed! That means that this ticket is no longer blocked.

Someone could do this by:

  • Writing up foundations for these states
  • Writing migrations to add those states to old databases and update all media entries over to using those states

comment:10 by Ben Sturmfels, 8 years ago

Keywords: small added

comment:11 by Ben Sturmfels, 7 years ago

Resolution: fixed
Status: acceptedclosed

Closing as a believe this was resolved by contributor tilly-Q in October 2013.

The User model no longer has fields email_verified or status. See migration line email_verified.drop() on mediagoblin/db/migrations.py:723. The email verification status is now modelled by the the active privilege.

Unrelated, but noting that if an admin were to remove the active privilege, the user can reinstate it by re-verifying their email - ie. you can't permanently disable an account this way - only banning will do that. Maybe the active privilege should be renamed email verified.

Note: See TracTickets for help on using tickets.