Terms of Service: Add ability to update TOS and have users re-sign. #1597
Conversation
|
Automated message from Dropbox CLA bot @rishig, it looks like you've already signed the Dropbox CLA. Thanks! |
| settings.EMAIL_HOST_USER, | ||
| ["all@zulip.com"]) | ||
| do_change_full_name(request.user, full_name) | ||
| do_change_tos_version(request.user, settings.TOS_VERSION) |
There was a problem hiding this comment.
probably this piece should be conditional?
rishig
force-pushed
the
tos
branch
2 times, most recently
from
3408bd0
to
dda624b
Compare
|
updated |
|
Cool, looks pretty good. I realized that we can also remove |
|
Oh, also, I updated the default for UserProfile.tos_version to be |
|
The |
|
Well, there are a few scenarios:
So I think my proposed logic is correct -- right? |
| email = request.user.email | ||
| domain = resolve_email_to_domain(email) | ||
| special_message_template = None | ||
| if request.user.tos_version is None and hasattr(settings, 'FIRST_TIME_TOS_TEMPLATE'): |
There was a problem hiding this comment.
This hasattr line is incorrect. You want settings.FIRST_TIME_TOS_TEMPLATE is not None, since we're set a default value of None for this settings in zproject/settings.py.
A design goal of our settings system is to never need to do a hasattr check for a setting -- we always set a default, so you're always guaranteed you can access it (and check for None if appropriate).
|
https://github.com/timabbott/zulip/tree/terms-candidate is my version having squashed the fixes above into your commit, in case the reference is useful for review. |
|
oops, edited my comment as you were writing yours. The logic is correct, though it was more natural for me to think of it as
This way it's clear what happens if e.g. the user gets partway through the registration flow, or if the user is created by some future batch process that avoids the registration flow, etc. |
|
Cool, yeah, I think that's actually what my model does; a few notes:
|
|
I went ahead and merged this, but we should still into those follow-up details.
|
rishig
force-pushed
the
tos
branch
2 times, most recently
from
cf0d3c7
to
b73cced
Compare
|
still working on TOS flow, but reordered commits so privacy policy is first |
rishig
force-pushed
the
tos
branch
2 times, most recently
from
2c4334a
to
09b933f
Compare
Mostly just changes "Zulip Account" to "Kandra Labs Zulip Account" or "Account", changes "Zulip" to "Kandra Labs", and changes "Massachusetts" to "California".
Most directly useful for the migration to zulipchat.com. Creates a new field in UserProfile to store the tos_version, as well as two new settings TOS_VERSION and FIRST_TIME_TOS_TEMPLATE. We check for a version mismatch between what the user has signed and the current settings.TOS_VERSION whenever the user hits the home page, and redirect them if needed. Note that accounts_accept_terms.html and zerver.views.accounts_accept_terms were unused before this commit (I'm guessing they are from the Dropbox migration in 2014.)
|
Merged. |
Most directly useful for the migration to zulipchat.com.
Creates a new field in UserProfile to store the tos_version, as well as a
special MIGRATION_TOS_VERSION that triggers slightly different text on
accounts_accept_terms.html. We check for a version mismatch between what the
user has signed and the current settings.TOS_VERSION whenever the user hits
the home page, and redirect them if needed.
Note that accounts_accept_terms.html and zerver.views.accounts_accept_terms
were unused before this commit (I'm guessing they are from the Dropbox
migration in 2014.)