launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/unsubscription into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #604614 Indirect members cannot see the subscribe/unsubscribe mailing list actions
  https://bugs.launchpad.net/bugs/604614


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

-- 
https://code.launchpad.net/~jcsackett/launchpad/unsubscription/+merge/32680
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/unsubscription into lp:launchpad/devel.

=== 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-14 20:25:59 +0000
@@ -0,0 +1,53 @@
+
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import with_statement
+
+"""Test harness for mailinglist views unit tests."""
+
+__metaclass__ = type
+
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from canonical.launchpad.ftests import login_person
+from canonical.launchpad.testing.pages import find_tag_by_id
+from lp.testing import TestCaseWithFactory, person_logged_in
+from lp.testing.views import create_view
+
+
+class MailingListSubscriptionControlsTestCase(TestCaseWithFactory):
+    """Tests to ensure the rendering of subscribe and unsubscribe
+    controls on the team page."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(MailingListSubscriptionControlsTestCase, self).setUp()
+        self.a_team = self.factory.makeTeam(name='a')
+        self.b_team = self.factory.makeTeam(name='b', owner=self.a_team)
+        self.b_team_list = self.factory.makeMailingList(team=self.b_team,
+            owner=self.b_team.teamowner)
+        self.user = self.factory.makePerson()
+        with person_logged_in(self.a_team.teamowner):
+            self.a_team.addMember(self.user, self.a_team.teamowner)
+
+    def test_subscribe_control_renders(self):
+        login_person(self.user)
+        view = create_view(self.b_team, name='+index',
+            principal=self.user, server_url='http://launchpad.dev',
+            path_info='/~%s' % self.b_team.name)
+        content = view.render()
+        link_tag = find_tag_by_id(content, "link.list.subscribe")
+        self.assertNotEqual(None, link_tag)
+
+    def test_subscribe_control_doesnt_render_for_anon(self):
+        other_person = self.factory.makePerson()
+        login_person(other_person)
+        view = create_view(self.b_team, name='+index',
+            principal=other_person, server_url='http://launchpad.dev',
+            path_info='/~%s' % self.b_team.name)
+        content = view.render()
+        self.assertNotEqual('', content)
+        link_tag = find_tag_by_id(content, "link.list.subscribe")
+        self.assertEqual(None, link_tag)

=== 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-14 20:25:59 +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"
               href="/people/+me/+editemails">Subscribe to mailing list</a>
           <br />
         </tal:can-subscribe-to-list>