Skip to content

Commit

Permalink
Fix bug 1335331 - Enable users to change their email address (#793)
Browse files Browse the repository at this point in the history 
* Bug 1335331: added ability to change email address

* Bug 1335331: add username in read-only mode

* Bug 1335331: remove placeholders

* Bug 1335331: add email validation

* Bug 1335331: adjust contributor profile tests

* Bug 1335331: make profile-form submit more generic

* Bug 1335331: add logout after email change

* Bug 1335331: change email in tests

* Bug 1335331: add email warning message to css

* Bug 1335331: adjust contributor profile test email

* Bug 1335331: shorten the line to pass PEP8

* Bug 1335331: make code style consistent

* Bug 1335331: correct level of indentation

* Fix errors on settings page.
This commit removes the fancy javascript that allowed to save user profile without reloading the page. It was a big source of errors and complexity. The settings page is now much simpler, the custom home page being the only exception. This fixes the CSRF error that was happening as well when saving data by clicking the Save button.

* Show or hide warning using JS.
It was bad that the warning for a changed email would only appear on focus on the input. First it made it difficult to click the save button, as the layout was changed and thus the button moved. Second, it would show up even if the email was not changed.
This commit also moves CSS to the more specific settings.css file, and addresses a few review comments.

* Make message a help text instead, and always show it.

* Styling changes.
  • Loading branch information
@adngdb
adngdb committed Jan 16, 2018
1 parent 17af08a commit 702d1e1
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 152 deletions.
23 changes: 21 additions & 2 deletions pontoon/base/forms.py
View file
Expand Up @@ -159,15 +159,34 @@ 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(
help_text=(
'Changing your email address will cause a logout. '
'Make sure the new one is correct before saving!'
)
)

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):
Expand Down
52 changes: 2 additions & 50 deletions pontoon/contributors/static/css/contributor.css
View file
Expand Up @@ -3,7 +3,7 @@
}

a.avatar {
display: inline-block;
display: block;
position: relative;
}

Expand Down Expand Up @@ -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 {
Expand Down
44 changes: 44 additions & 0 deletions pontoon/contributors/static/css/settings.css
View file
Expand Up @@ -52,3 +52,47 @@
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;
}

#profile-form .help {
color: #888888;
font-style: italic;
margin-top: 5px;
}

.errorlist {
color: #F36;
list-style: none;
margin: 0;
margin-top: 5px;
text-align: left;
}
41 changes: 0 additions & 41 deletions pontoon/contributors/static/js/contributor.js
View file
@@ -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,
Expand Down
11 changes: 1 addition & 10 deletions pontoon/contributors/templates/contributors/profile.html
View file
Expand Up @@ -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>
Expand Down
24 changes: 17 additions & 7 deletions pontoon/contributors/templates/contributors/settings.html
View file
Expand Up @@ -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') }}
Expand All @@ -49,13 +44,28 @@ <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 }}
<p class="help">{{ profile_form.email.help_text }}</p>
</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>
Expand Down
54 changes: 36 additions & 18 deletions pontoon/contributors/tests.py
View file
Expand Up @@ -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
Expand All @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions pontoon/contributors/urls.py
View file
Expand Up @@ -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'),
]

0 comments on commit 702d1e1

Please sign in to comment.