Bug#754873: [tracker.debian.org] subscription from profile email check verification
Control: tags -1 + patch
Hi,
Sorry for the late answer.
I finally made a verification of the checkbox and the field using
jquery (client-side only to reduce the potential impact of the
modification).
I also added a basic subscription test (as this tab had none).
The checkbox is now checked by default to the 1st user email address,
and as soon as a subscription is done with one or more emails these
emails are stored in a session variable (I'm not sure you agree with
this storage, but if you have a better way to do it, please tell me)
and those emails are automatically checked (instead of the default) on
the subscription page next time you'll go there.
This should close both #753828 and #754873.
I separated the tests, jquery modifications and python-side
modifications in 3 different commits (attached patches) so it's easier
to figure out what is what.
Should you have any remarks, please don't hesitate.
Best,
Joseph
From 9eb3a2aa1dc588b52f9b6e3b5806cd04cadf9f98 Mon Sep 17 00:00:00 2001
From: Joseph Herlant <herlantj@gmail.com>
Date: Fri, 25 Jul 2014 22:40:16 +0200
Subject: [PATCH 1/3] Adding functionnal tests about subscription tab in user
space.
---
functional_tests/tests.py | 93 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/functional_tests/tests.py b/functional_tests/tests.py
index ef152fc..ae01db3 100644
--- a/functional_tests/tests.py
+++ b/functional_tests/tests.py
@@ -755,6 +755,9 @@ class SubscribeToPackageTest(UserAccountsTestMixin, SeleniumTestCase):
"""
Tests for stories regarding subscribing to a package over the Web.
"""
+ def get_subscriptions_url(self):
+ return reverse('dtracker-accounts-subscriptions')
+
def test_subscribe_from_package_page(self):
"""
Tests that a user that has only one email address can subscribe to a
@@ -872,6 +875,96 @@ class SubscribeToPackageTest(UserAccountsTestMixin, SeleniumTestCase):
unsub_button = self.get_element_by_id('unsubscribe-button')
self.assertFalse(unsub_button.is_displayed())
+ def test_subscribe_package_from_subscription_tab(self):
+ """
+ This test validates that a user can correctly subscribe to a package
+ from the subscription tab in its personnal space.
+ """
+ # Initially the user is not subscribed to the package
+ self.assertFalse(self.user.is_subscribed_to(self.package))
+ # The user logs in and goes to his subscriptions page
+ self.log_in()
+ self.get_page(self.get_subscriptions_url())
+ # To ensure the subscription page is fully charged
+ self.wait_response(1)
+ self.assert_in_page_body('Subscribe')
+ # Checking that at least one checkbox is checked
+ available_mails = self.browser.find_elements_by_xpath("//input[@type='checkbox'][@name='email']")
+ is_there_checked_emails = False
+ for checkbox in available_mails:
+ if checkbox.is_selected():
+ is_there_checked_emails = True
+ break
+ self.assertTrue(is_there_checked_emails)
+ # Filling the package search field
+ self.assert_element_with_id_in_page('package-search-input')
+ self.input_to_element('package-search-input', self.package.name)
+ # Subscribing to the package and ensuring it's completely done!
+ self.send_enter('package-search-input')
+ self.wait_response(1)
+ self.assertTrue(self.user.is_subscribed_to(self.package))
+
+ def test_package_subscription_no_email_from_subscription_tab_fails(self):
+ """
+ The UI should prevent the user from forgetting to check at least one
+ email checkbox from the subscription tab in its personnal space.
+ """
+ # Initially the user is not subscribed to the package
+ self.assertFalse(self.user.is_subscribed_to(self.package))
+ # The user logs in and goes to his subscriptions page
+ self.log_in()
+ self.get_page(self.get_subscriptions_url())
+ # To ensure the subscription page is fully charged
+ self.wait_response(1)
+ self.assert_in_page_body('Subscribe')
+ # All email checkboxes should be unchecked
+ available_mails = self.browser.find_elements_by_xpath("//input[@type='checkbox'][@name='email']")
+ for checkbox in available_mails:
+ if checkbox.is_selected():
+ checkbox.click()
+ # Filling the package search field
+ self.assert_element_with_id_in_page('package-search-input')
+ self.input_to_element('package-search-input', self.package.name)
+ # Subscribing to the package
+ self.send_enter('package-search-input')
+ self.wait_response(1)
+ # He gets a message informing him that the field is required
+ self.assert_in_page_body('You need to select at least an email')
+ # The scubscription should not have been done!
+ self.assertFalse(self.user.is_subscribed_to(self.package))
+
+ def test_package_subscription_no_package_from_subscription_tab_fails(self):
+ """
+ The UI should prevent the user from forgetting to check at least one
+ email checkbox from the subscription tab in its personnal space.
+ """
+ # Initially the user is not subscribed to the package
+ self.assertFalse(self.user.is_subscribed_to(self.package))
+ # The user logs in and goes to his subscriptions page
+ self.log_in()
+ self.get_page(self.get_subscriptions_url())
+ # To ensure the subscription page is fully charged
+ self.wait_response(1)
+ self.assert_in_page_body('Subscribe')
+ # Checking that at least one checkbox is checked
+ available_mails = self.browser.find_elements_by_xpath("//input[@type='checkbox'][@name='email']")
+ is_there_checked_emails = False
+ for checkbox in available_mails:
+ if checkbox.is_selected():
+ is_there_checked_emails = True
+ break
+ self.assertTrue(is_there_checked_emails)
+ # Filling the package search field
+ self.assert_element_with_id_in_page('package-search-input')
+ self.input_to_element('package-search-input', '')
+ # Subscribing to the package
+ self.send_enter('package-search-input')
+ self.wait_response(1)
+ # He gets a message informing him that the field is required
+ self.assert_in_page_body('This field is required')
+ # The scubscription should not have been done!
+ self.assertFalse(self.user.is_subscribed_to(self.package))
+
class ChangeProfileTest(UserAccountsTestMixin, SeleniumTestCase):
def test_modify_personal_info(self):
--
2.0.1
From 317f3c59bee95396c34cf576d2445cd4d9b2f1f3 Mon Sep 17 00:00:00 2001
From: Joseph Herlant <herlantj@gmail.com>
Date: Mon, 21 Jul 2014 06:27:01 +0200
Subject: [PATCH 2/3] Client side verification for the email and package
fields.
---
.../accounts/static/accounts/js/profile.js | 30 ++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/distro_tracker/accounts/static/accounts/js/profile.js b/distro_tracker/accounts/static/accounts/js/profile.js
index 1da556f..6071813 100644
--- a/distro_tracker/accounts/static/accounts/js/profile.js
+++ b/distro_tracker/accounts/static/accounts/js/profile.js
@@ -172,4 +172,34 @@ $(function() {
$(this).closest('form').submit();
return false;
});
+
+ $('#package-subscribe-form').submit(function(){
+ /* First check that the textbox is filled */
+ var pkg_name = $.trim($('#package-search-input').val());
+ if(pkg_name === ''){
+ $('#package-search-input').parent().addClass('error').addClass('control-group');
+ var helper = $('<span class="help-inline">').text('This field is required.');
+ $('#package-search-input').parent().append(helper);
+ return false;
+ }
+ /* Then check that at least one email is selected */
+ var any_email_checked = false;
+ $("input[type='checkbox'][name='email']").each(function(){
+ if($(this).prop('checked')) any_email_checked = true;
+ });
+ if(!any_email_checked){
+ $("input[type='checkbox'][name='email']").first().parent().parent().addClass('error').addClass('control-group');
+ var helper = $('<span class="help-inline">').text('You need to select at least an email to subscribe.');
+ $("input[type='checkbox'][name='email']").first().parent().parent().append(helper);
+ return false;
+ }
+ });
+ $("input[type='checkbox'][name='email']").change(function(){
+ if(this.checked) {
+ $(this).parent().parent().removeClass('error').removeClass('control-group');
+ if($(this).parent().parent().children('.help-inline').length){
+ $(this).parent().parent().children('.help-inline').remove();
+ }
+ }
+ });
});
--
2.0.1
From 1cb093490a860064648d26bfa64d391d66b67f36 Mon Sep 17 00:00:00 2001
From: Joseph Herlant <herlantj@gmail.com>
Date: Mon, 21 Jul 2014 04:12:04 +0200
Subject: [PATCH 3/3] Initially checked checkbox and remembering email between
pages
---
distro_tracker/accounts/templates/accounts/subscriptions.html | 3 ++-
distro_tracker/accounts/views.py | 8 ++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/distro_tracker/accounts/templates/accounts/subscriptions.html b/distro_tracker/accounts/templates/accounts/subscriptions.html
index 6361504..b7be240 100644
--- a/distro_tracker/accounts/templates/accounts/subscriptions.html
+++ b/distro_tracker/accounts/templates/accounts/subscriptions.html
@@ -208,7 +208,8 @@
<div>Emails which should be subscribed: </div>
{% for email in request.user.emails.all %}
<label class="checkbox inline">
- <input type="checkbox" name="email" value="{{ email }}"> {{ email }}
+ <input type="checkbox" name="email" value="{{ email }}"
+ {% if email.email in checked_emails %} checked="True" {%endif %}> {{ email }}
</label>
{% endfor %}
</div>
diff --git a/distro_tracker/accounts/views.py b/distro_tracker/accounts/views.py
index 1f760fd..39cbf90 100644
--- a/distro_tracker/accounts/views.py
+++ b/distro_tracker/accounts/views.py
@@ -154,8 +154,13 @@ class SubscriptionsView(LoginRequiredMixin, View):
}
for user_email in user_emails
}
+ # Initializing session variable if not set.
+ if('checked_emails' not in request.session.keys()):
+ request.session['checked_emails'] = []
+ request.session['checked_emails'].append(user_emails[0].__str__())
return render(request, self.template_name, {
'subscriptions': subscriptions,
+ 'checked_emails': request.session['checked_emails']
})
@@ -183,6 +188,9 @@ class SubscribeUserToPackageView(LoginRequiredMixin, View):
if not package or not emails:
raise Http404
+ #Remember checked emails via session variable
+ request.session['checked_emails'] = emails
+
# Check whether the logged in user is associated with the given emails
users_emails = [e.email for e in request.user.emails.all()]
for email in emails:
--
2.0.1
Reply to: