Bug 1006569 - [privacy] Add checkbox at sign-up #135
Conversation
| @@ -5,6 +5,7 @@ | |||
| from django.db import models | |||
|
|
|||
| from tower import ugettext_lazy as _lazy | |||
| from random import randint | |||
There was a problem hiding this comment.
Will remove in next commit.
|
I think that once a person accepts the privacy policy via the checkbox they cannot unaccept it, so we should only display the check box when it is empty. Once a user has accepted it the checkbox should no longer appear on the form. You will need to write some custom code to output the form in users/templates/users/profile/edit.html instead of just using You would probably have to do this anyway to get the form to match the wireframe. [1] https://github.com/mozilla/oneanddone/blob/master/oneanddone/tasks/templates/tasks/form.html |
|
It's a good start @bitgeeky. Nice work. Please address the comments above and let me know when it's ready for another review. |
|
Also, @bitgeeky, please attach the PR to the bug and request review from me when it's ready. That is a better method than pinging me on IRC and makes everything easier to track. |
|
Made the changes suggested in comments. @bobsilverberg r ? |
| username = forms.RegexField( | ||
| label= "Username https://oneanddone.mozilla.org/en-US/profile/", | ||
| max_length=30, regex=r'^[a-zA-Z0-9]+$', | ||
| error_messages = {'invalid': "This value may contain only alphanumeric characters. ( r'^[a-zA-Z0-9]+$' )"}) |
There was a problem hiding this comment.
We should not include the regex in the error message. That is going to be meaningless to 99% of the people who see it and will only serve to confuse them.
|
Great work, @bitgeeky, it looks really good. I like the way you solved the duplicate username/recursion issue, and the way you created a separate form to deal with the fact that sometimes we need a checkbox and sometimes we do not. There are a few comments above, and one other thing I wanted to add is that I think it would be nice to show a confirmation message at the top of the screen after a user successfully saves the form, like we do with the Task form. Try creating or saving a task and you'll see what I mean. Let me know when those changes are ready for another review. |
|
Made suggested display changes. @bobsilverberg r ? |
| label = _("You are creating a profile that will include a public username and work history. You will begin to receive email communications from Mozilla once you have completed tasks. You may unsubscribe at anytime by clicking the line at the bottom of these emails."), | ||
| required = True,) | ||
| username = forms.RegexField( | ||
| label= "Username:", |
There was a problem hiding this comment.
Missing i18n for label
|
@bobsilverberg r ? |
Functionally works fine for me and all the tests pass too.
I will add the unit tests and modify helper text later today and need to discuss about the deletion of user email from database usecase.