Fix bug 1335331 - Enable users to change their email address

pontoon/base/forms.py

@@ -159,15 +159,29 @@ def save(self, commit=True):
 )
 
 
-class UserFirstNameForm(forms.ModelForm):
+class UserProfileForm(forms.ModelForm):
     """
     Form is responsible for saving user's name.
     """
     first_name = forms.RegexField(regex='^[^<>"\'&]+$', max_length=30, strip=True)
+    email = forms.EmailField()
 
     class Meta:
         model = User
-        fields = ('first_name',)
+        fields = ('first_name', 'email')
+
+    def clean_email(self):
+        email = self.cleaned_data.get('email')
+        if (
+            email and
+            (
+                User.objects.filter(email=email)
+                .exclude(username=self.instance.username)
+                .exists()
+            )
+        ):
+            raise forms.ValidationError(u'Email address must be unique.')
+        return email
 
 
 class UserCustomHomepageForm(forms.ModelForm):

pontoon/contributors/static/css/contributor.css

@@ -3,7 +3,7 @@
 }
 
 a.avatar {
-  display: inline-block;
+  display: block;
   position: relative;
 }
 
@@ -33,57 +33,9 @@ a.avatar:hover .desc ~ img {
 #username {
   color: #EBEBEB;
   font-size: 48px;
-  letter-spacing: -2px;
-  text-transform: none;
-}
-
-#username .uneditable {
   letter-spacing: normal;
   padding: 8px 0 20px;
-}
-
-#username .container {
-  display: inline-block;
-  position: relative;
-}
-
-#username .container input {
-  background: transparent;
-  border-bottom: 1px dashed transparent;
-  color: #FFFFFF;
-  font-size: 48px;
-  font-weight: 300;
-  padding: 4px 50px;
-  text-align: center;
-  width: 100%;
-
-  -moz-box-sizing: border-box;
-  box-sizing: border-box;
-}
-
-#username .container input:hover,
-#username .container input:focus {
-  border-color: #888888;
-}
-
-#username .container .submit {
-  bottom: -8px;
-  color: #888888;
-  opacity: 0;
-  padding: 20px 0;
-  position: absolute;
-  right: 0;
-  /* Needed or click doesn't trigger */
-  transition: all 0.1s ease-out;
-}
-
-#username .container input:focus + .submit {
-  cursor: pointer;
-  opacity: 1;
-}
-
-#username .container input:focus + .submit:hover {
-  color: #7BC876;
+  text-transform: none;
 }
 
 .info {

pontoon/contributors/static/css/settings.css

@@ -52,3 +52,45 @@
   float: none;
   margin: 9px;
 }
+
+#profile-form {
+  display: block;
+  position: relative;
+  text-align: left;
+  margin: 0 auto 70px auto;
+  width: 620px;
+}
+
+#profile-form .field {
+  margin-top: 20px;
+  text-align: left;
+}
+
+#profile-form .field input {
+  color: #FFFFFF;
+  background: #333941;
+  border: 1px solid #4d5967;
+  border-radius: 3px;
+  float: none;
+  width: 290px;
+  padding: 4px;
+
+  -moz-box-sizing: border-box;
+  box-sizing: border-box;
+}
+
+#profile-form button {
+  margin-top: 10px;
+}
+
+#email-warning {
+  display: none;
+}
+#email-warning,
+.errorlist {
+  color: #F36;
+  list-style: none;
+  margin: 0;
+  margin-top: 5px;
+  text-align: left;
+}

pontoon/contributors/static/js/contributor.js

@@ -1,46 +1,5 @@
 $(function() {
 
-  var input = $('#username input');
-
-  // Save user name handler
-  function save() {
-    $.ajax({
-      url: '/save-user-name/',
-      type: 'POST',
-      data: {
-        csrfmiddlewaretoken: $('#server').data('csrf'),
-        first_name: $.trim(input.val())
-      },
-      success: function(data) {
-        if (data === "ok") {
-          input.blur();
-          Pontoon.endLoader('Thank you!');
-        }
-      },
-      error: function(request) {
-        if (request.responseText === "error") {
-          Pontoon.endLoader('Oops, something went wrong.', 'error');
-        } else {
-          Pontoon.endLoader(request.responseText, 'error');
-        }
-      }
-    });
-  }
-
-  // Save user name by mouse or keyboard
-  $('.submit').click(function() {
-    if ($(this).css('opacity') === "0") {
-      return;
-    }
-    save();
-  });
-  input.keydown(function(e) {
-    if (e.which === 13) {
-      e.preventDefault();
-      save();
-    }
-  });
-
   function loadNextEvents(cb) {
     var currentPage = $timeline.data('page'),
       nextPage = parseInt(currentPage, 10) + 1,

pontoon/contributors/static/js/settings.js

@@ -23,4 +23,19 @@ $(function() {
       }
     });
   });
+
+  // Show a warning when a user changes their email address.
+  var emailField = $('#id_email');
+  var warningContent = $('#email-warning');
+  var originalEmail = emailField.val();
+
+  emailField.on('keyup', function () {
+    var newEmail = emailField.val();
+    if (newEmail === originalEmail && warningContent.is(':visible')) {
+      warningContent.hide();
+    }
+    else if (newEmail !== originalEmail && !warningContent.is(':visible')) {
+      warningContent.show();
+    }
+  });
 });

pontoon/contributors/templates/contributors/profile.html

@@ -24,16 +24,7 @@
     <img class="rounded" src="{{ contributor.gravatar_url(200) }}">
   </a>
 
-  <h2 id="username">
-    {% if my_profile %}
-      <div class="container">
-        <input type="text" placeholder="What's your name?" value="{{ contributor.first_name }}" maxlength="30">
-        <div class="submit fa fa-arrow-circle-right"></div>
-      </div>
-    {% else %}
-      <div class="uneditable">{{ contributor.first_name }}</div>
-    {% endif %}
-  </h2>
+  <h2 id="username">{{ contributor.first_name }}</h2>
 
   <ul class="info">
     <li><span class="fa fa-fw fa-envelope"></span><a href="mailto:{{ contributor.email|nospam }}">{{ contributor.email|nospam }}</a></li>

pontoon/contributors/templates/contributors/settings.html

@@ -21,12 +21,7 @@
     <img class="rounded" src="{{ user.gravatar_url(200) }}">
   </a>
 
-  <h2 id="username">
-    <div class="container">
-      <input type="text" placeholder="What's your name?" value="{{ user.first_name }}" maxlength="30">
-      <div class="submit fa fa-arrow-circle-right"></div>
-    </div>
-  </h2>
+  <h2 id="username">{{ user.first_name }}</h2>
 
   <ul class="info">
     {{ Checkbox.checkbox('Quality checks', class='quality-checks', attribute='quality_checks', is_enabled=user.profile.quality_checks, title='Run quality checks before submitting translations') }}
@@ -49,13 +44,31 @@ <h2 id="username">
     {% csrf_token %}
     <input type="hidden" name="return_url" value="{{ return_url(request) }}">
 
+    <div id="profile-form">
+      <h3>Personal infomation</h3>
+      <div class="field">
+          {{ profile_form.first_name.label_tag(label_suffix='') }}
+          {{ profile_form.first_name }}
+          {{ profile_form.first_name.errors }}
+      </div>
+      <div class="field">
+          {{ profile_form.email.label_tag(label_suffix='') }}
+          {{ profile_form.email }}
+          {{ profile_form.email.errors }}
+          <div id="email-warning">
+            Caution: Changing your email address will cause a logout.<br />
+            Make sure the new one is correct before saving!
+          </div>
+      </div>
+    </div>
+
     <h3>Preferred locales <span class="small stress">(to get suggestions from)</span></h3>
 
     {{ multiple_locale_selector.render(available_locales, selected_locales, form_field='locales_order', sortable=True) }}
 
     <div class="controls">
       <a class="cancel" href="{{ return_url(request) }}">Cancel</a>
-      <button class="button active">Save</button>
+      <button class="submit button active">Save</button>
     </div>
   </form>
 </section>

pontoon/contributors/tests.py

@@ -7,12 +7,14 @@
 from random import randint
 from six.moves import range
 
+from django.core.urlresolvers import reverse
 from django.http import HttpResponse
 from django.utils.timezone import now, make_aware
 from django_nose.tools import (
     assert_equal,
     assert_true,
     assert_code,
+    assert_contains
 )
 
 from pontoon.base.models import User
@@ -38,62 +40,78 @@ def commajoin(*items):
 
 class ContributorProfileTests(UserTestCase):
     """Tests related to the saving user profile."""
+    url = reverse('pontoon.contributors.settings')
 
     def test_invalid_first_name(self):
-        response = self.client.post('/save-user-name/', {'first_name': '<aa>"\'"'})
+        response = self.client.post(self.url, {'first_name': '<aa>"\'"'})
 
-        assert_equal(response.status_code, 400)
-        assert_equal(response.content, 'Enter a valid value.')
+        assert_contains(response, 'Enter a valid value.')
 
-    def test_missing_first_name(self):
-        response = self.client.post('/save-user-name/', {})
+    def test_invalid_email(self):
+        response = self.client.post(self.url, {'email': 'usermail'})
 
-        assert_equal(response.status_code, 400)
-        assert_equal(response.content, 'This field is required.')
+        assert_contains(response, 'Enter a valid email address.')
+
+    def test_missing_profile_fields(self):
+        response = self.client.post(self.url, {})
+
+        assert_contains(response, 'This field is required.', count=2)
 
     def test_valid_first_name(self):
-        response = self.client.post('/save-user-name/', {'first_name': 'contributor'})
+        response = self.client.post(
+            self.url,
+            {'first_name': 'contributor', 'email': self.user.email}
+        )
 
-        assert_equal(response.status_code, 200)
-        assert_equal(response.content, 'ok')
+        assert_contains(response, 'Settings saved.')
 
     def test_user_locales_order(self):
         locale1, locale2, locale3 = LocaleFactory.create_batch(3)
-        response = self.client.get('/settings/')
+        response = self.client.get(self.url)
         assert_equal(response.status_code, 200)
 
         response = self.client.post('/settings/', {
+            'first_name': 'contributor',
+            'email': self.user.email,
             'locales_order': commajoin(
                 locale2.pk,
                 locale1.pk,
                 locale3.pk,
-            )
+            ),
         })
 
-        assert_equal(response.status_code, 302)
+        assert_equal(response.status_code, 200)
         assert_equal(
-            list(User.objects.get(pk=self.user.pk).profile.sorted_locales), [
+            list(User.objects.get(pk=self.user.pk).profile.sorted_locales),
+            [
                 locale2,
                 locale1,
                 locale3,
             ]
         )
         # Test if you can clear all locales
         response = self.client.post('/settings/', {
-            'locales_order': ''
+            'first_name': 'contributor',
+            'email': self.user.email,
+            'locales_order': '',
         })
-        assert_equal(response.status_code, 302)
-        assert_equal(list(User.objects.get(pk=self.user.pk).profile.sorted_locales), [])
+        assert_equal(response.status_code, 200)
+        assert_equal(
+            list(User.objects.get(pk=self.user.pk).profile.sorted_locales),
+            []
+        )
 
         # Test if form handles duplicated locales
         response = self.client.post('/settings/', {
+            'first_name': 'contributor',
+            'email': self.user.email,
             'locales_order': commajoin(
                 locale1.pk,
                 locale2.pk,
                 locale2.pk,
             )
         })
-        assert_equal(response.status_code, 302)
+        assert_equal(response.status_code, 200)
         assert_equal(
             list(User.objects.get(pk=self.user.pk).profile.sorted_locales), [
                 locale1,

pontoon/contributors/urls.py

@@ -51,9 +51,6 @@
         name='pontoon.contributors.toggle_user_profile_attribute'),
 
     # AJAX
-    url(r'^save-user-name/$', views.save_user_name,
-        name='pontoon.contributors.save_user_name'),
-
     url(r'^save-custom-homepage/$', views.save_custom_homepage,
         name='pontoon.contributors.save_custom_homepage'),
 ]