Launchpad itself

Merge lp:~jcsackett/launchpad/unsubscription into lp:launchpad

  1. unsubscription
  2. Merge into devel
Proposed by j.c.sackett
Status: Merged
Approved by: j.c.sackett
Approved revision: no longer in the source branch.
Merged at revision: 11381
Proposed branch: lp:~jcsackett/launchpad/unsubscription
Merge into: lp:launchpad
Diff against target: 80 lines (+55/-3)
2 files modified
lib/lp/registry/browser/tests/test_mailinglists.py (+52/-0)
lib/lp/registry/templates/team-portlet-mailinglist.pt (+3/-3)
To merge this branch: bzr merge lp:~jcsackett/launchpad/unsubscription
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+32680@code.launchpad.net

Commit message

Updates conditions on showing subscribe/unsubscribe controls on team pages so all valid participants can see/use the control. Previously only direct members had access.

Description of the change

Summary

Fixes the conditions under which a user can see a subscribe/unsubscribe control on a team page; prior to this fix only direct members could, despite indirect members also being subscribers.

Proposed fix

Change the conditions under which a user sees subscribe/unsubscribe controls so that if they're a team participant they have the controls.

Pre-implementation notes

After looking at the bug with Curtis Hovey (sinzui) we discovered that the unsubscribe directions actually were correct for some teams, but not all where you might be subscribed. We realized the problem was in the template checking for direct membership rather than participation.

Implementation details

The section that displays the subscribe/unsubscribe controls now uses userIsParticipant to determine whether or not to show. test_mailinglists.py was added in browser/tests/ to ensure that the controls were rendering at the right time.

Demo and Q/A

bin/test -vvc -m lp.registry.browser.tests.test_mailinglist

lint
= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/tests/test_mailinglists.py
  lib/lp/registry/templates/team-portlet-mailinglist.pt

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Jon.

This looks good to land after you have addressed my nitpick about the docstring and id.

> === added file 'lib/lp/registry/browser/tests/test_mailinglists.py'
> --- lib/lp/registry/browser/tests/test_mailinglists.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/browser/tests/test_mailinglists.py 2010-08-17 16:07:46 +0000
...

+class MailingListSubscriptionControlsTestCase(TestCaseWithFactory):
+ """Tests to ensure the rendering of subscribe and unsubscribe
+ controls on the team page."""

PEP 257 states that a docstring has a single line synopses.
    """Verify the team index subscribe/unsubscribe to mailing list content."""

...

> === modified file 'lib/lp/registry/templates/team-portlet-mailinglist.pt'
> --- lib/lp/registry/templates/team-portlet-mailinglist.pt 2010-06-04 15:37:24 +0000
> +++ lib/lp/registry/templates/team-portlet-mailinglist.pt 2010-08-17 16:07:46 +0000
> @@ -19,10 +19,10 @@
> <strong>Policy:</strong>
> You must be a team member to subscribe to the team mailing list.
> <br/>
> - <tal:member condition="view/user_is_active_member">
> + <tal:member condition="view/userIsParticipant">
> <tal:can-subscribe-to-list
> condition="view/user_can_subscribe_to_list">
> - <a class="sprite add"
> + <a id="link.list.subscribe" class="sprite add"

This is not a valid CSS3 id use dashes, no dots, no underscores. We know that
zope and some of out code are making bad ids. We need to get the site ready
for the CSS3 standard.

review: Approve (code)
Revision history for this message
j.c.sackett (jcsackett) wrote :

I'm fine with the docstring issue and good with the CSS3 update; I had noticed the '.' ids in other parts of the template and assumed it was a zope template thing.

I'll make both changes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'lib/lp/registry/browser/tests/test_mailinglists.py'
2--- lib/lp/registry/browser/tests/test_mailinglists.py 1970-01-01 00:00:00 +0000
3+++ lib/lp/registry/browser/tests/test_mailinglists.py 2010-08-17 19:13:47 +0000
4@@ -0,0 +1,52 @@
5+
6+# Copyright 2010 Canonical Ltd. This software is licensed under the
7+# GNU Affero General Public License version 3 (see the file LICENSE).
8+
9+from __future__ import with_statement
10+
11+"""Test harness for mailinglist views unit tests."""
12+
13+__metaclass__ = type
14+
15+
16+from canonical.testing.layers import DatabaseFunctionalLayer
17+from canonical.launchpad.ftests import login_person
18+from canonical.launchpad.testing.pages import find_tag_by_id
19+from lp.testing import TestCaseWithFactory, person_logged_in
20+from lp.testing.views import create_view
21+
22+
23+class MailingListSubscriptionControlsTestCase(TestCaseWithFactory):
24+ """Verify the team index subscribe/unsubscribe to mailing list content."""
25+
26+ layer = DatabaseFunctionalLayer
27+
28+ def setUp(self):
29+ super(MailingListSubscriptionControlsTestCase, self).setUp()
30+ self.a_team = self.factory.makeTeam(name='a')
31+ self.b_team = self.factory.makeTeam(name='b', owner=self.a_team)
32+ self.b_team_list = self.factory.makeMailingList(team=self.b_team,
33+ owner=self.b_team.teamowner)
34+ self.user = self.factory.makePerson()
35+ with person_logged_in(self.a_team.teamowner):
36+ self.a_team.addMember(self.user, self.a_team.teamowner)
37+
38+ def test_subscribe_control_renders(self):
39+ login_person(self.user)
40+ view = create_view(self.b_team, name='+index',
41+ principal=self.user, server_url='http://launchpad.dev',
42+ path_info='/~%s' % self.b_team.name)
43+ content = view.render()
44+ link_tag = find_tag_by_id(content, "link-list-subscribe")
45+ self.assertNotEqual(None, link_tag)
46+
47+ def test_subscribe_control_doesnt_render_for_anon(self):
48+ other_person = self.factory.makePerson()
49+ login_person(other_person)
50+ view = create_view(self.b_team, name='+index',
51+ principal=other_person, server_url='http://launchpad.dev',
52+ path_info='/~%s' % self.b_team.name)
53+ content = view.render()
54+ self.assertNotEqual('', content)
55+ link_tag = find_tag_by_id(content, "link-list-subscribe")
56+ self.assertEqual(None, link_tag)
57
58=== modified file 'lib/lp/registry/templates/team-portlet-mailinglist.pt'
59--- lib/lp/registry/templates/team-portlet-mailinglist.pt 2010-06-04 15:37:24 +0000
60+++ lib/lp/registry/templates/team-portlet-mailinglist.pt 2010-08-17 19:13:47 +0000
61@@ -19,16 +19,16 @@
62 <strong>Policy:</strong>
63 You must be a team member to subscribe to the team mailing list.
64 <br/>
65- <tal:member condition="view/user_is_active_member">
66+ <tal:member condition="view/userIsParticipant">
67 <tal:can-subscribe-to-list
68 condition="view/user_can_subscribe_to_list">
69- <a class="sprite add"
70+ <a id="link-list-subscribe" class="sprite add"
71 href="/people/+me/+editemails">Subscribe to mailing list</a>
72 <br />
73 </tal:can-subscribe-to-list>
74 <tal:subscribed-to-list
75 condition="view/user_is_subscribed_to_list">
76- <form id="form.list.unsubscribe" name="unsubscribe"
77+ <form id="form-list-unsubscribe" name="unsubscribe"
78 action="" method="post">
79 <input type="submit" name="unsubscribe" value="Unsubscribe" />
80 </form>