Opened 12 years ago
Closed 6 years ago
#103 closed defect (fixed)
Change user status into a boolean, "active" or "is_active"
|Reported by:||Christopher Allan Webber||Owned by:|
Description (last modified by )
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 , 12 years ago
comment:2 by , 11 years ago
|Status:||New → Feedback|
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 , 11 years ago
|Status:||Feedback → In 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 , 11 years ago
The original url for this bug was http://bugs.foocorp.net/issues/390 .
comment:5 by , 10 years ago
I think we need #697 in order to really do this right.
comment:6 by , 10 years ago
Aaron are you actively working on this? I'm assuming not. Anyway, I'm removing ownership, feel free to reclaim!
comment:7 by , 10 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 , 10 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 , 10 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 , 7 years ago
comment:11 by , 6 years ago
|Status:||accepted → closed|
Closing as a believe this was resolved by contributor tilly-Q in October 2013.
User model no longer has fields
status. See migration line
mediagoblin/db/migrations.py:723. The email verification status is now modelled by the the
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