launchpad-reviewers team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Gary Poster has proposed merging lp:~gary/launchpad/bug792493 into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #792493 in Launchpad itself: "Creating a structural subscription for a team with a preferredemail makes a mute link that oopses"
  https://bugs.launchpad.net/launchpad/+bug/792493
  Bug #792502 in Launchpad itself: "Bug page subscribe/unsubscribe link doesn't work for anonymous users"
  https://bugs.launchpad.net/launchpad/+bug/792502

For more details, see:
https://code.launchpad.net/~gary/launchpad/bug792493/+merge/63431

This branch fixes the two linked bugs.

For the main (critical) bug, I just added information about whether the team had a preferred email.  If a team does, we shouldn't draw the structural subscription mute button (because it won't work, because if it did, it would be a lie).  I added tests for the Python side.  The JS side doesn't have any tests that are close to this part of the world, so I didn't add any. :-/

I also fixed bug 792502, with the simple change in bugtask_index_portlets.js .  The code was never detaching the handler, and now it is.

lint is happy.

Thank you!
-- 
https://code.launchpad.net/~gary/launchpad/bug792493/+merge/63431
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/bug792493 into lp:launchpad/db-devel.

=== modified file 'lib/lp/bugs/browser/structuralsubscription.py'
--- lib/lp/bugs/browser/structuralsubscription.py	2011-05-26 08:03:46 +0000
+++ lib/lp/bugs/browser/structuralsubscription.py	2011-06-03 21:27:26 +0000
@@ -431,8 +431,8 @@
 def expose_user_administered_teams_to_js(request, user, context,
         absoluteURL=absoluteURL):
     """Make the list of teams the user administers available to JavaScript."""
-    # XXX: Robert Collins workaround multiple calls making this cause timeouts:
-    # see bug 788510.
+    # XXX: Robert Collins workaround multiple calls making this cause
+    # timeouts: see bug 788510.
     objects = IJSONRequestCache(request).objects
     if 'administratedTeams' in objects:
         return
@@ -461,6 +461,7 @@
                     not team.inTeam(bug_supervisor)):
                     continue
                 info.append({
+                    'has_preferredemail': team.preferredemail is not None,
                     'link': absoluteURL(team, api_request),
                     'title': team.title,
                     'url': canonical_url(team),

=== modified file 'lib/lp/bugs/browser/tests/test_expose.py'
--- lib/lp/bugs/browser/tests/test_expose.py	2011-05-26 22:18:33 +0000
+++ lib/lp/bugs/browser/tests/test_expose.py	2011-06-03 21:27:26 +0000
@@ -20,6 +20,7 @@
 from zope.interface import implements
 from zope.traversing.browser import absoluteURL
 
+from canonical.launchpad.interfaces.emailaddress import EmailAddressStatus
 from canonical.launchpad.webapp.publisher import canonical_url
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.testing.layers import DatabaseFunctionalLayer
@@ -116,6 +117,27 @@
     def _sort(self, team_info, key='title'):
         return sorted(team_info, key=itemgetter(key))
 
+    def test_teams_preferredemail(self):
+        # The function includes information about whether the team has a
+        # preferred email.  This gives us information in JavaScript that tells
+        # us whether the subscription can be muted for a particular member of
+        # the team.  (If the team has a preferredemail, muting is not
+        # possible).
+        context = self.factory.makeProduct(owner=self.user)
+        team1 = self.factory.makeTeam(name='team-1', owner=self.user)
+        team2 = self.factory.makeTeam(name='team-2', owner=self.user)
+        self.factory.makeEmail('foo@xxxxxxxxxxx',
+                               team2,
+                               email_status=EmailAddressStatus.PREFERRED)
+
+        expose_user_administered_teams_to_js(self.request, self.user, context,
+            absoluteURL=fake_absoluteURL)
+        team_info = self._sort(self.request.objects['administratedTeams'])
+        self.assertThat(team_info[0]['title'], Equals(u'Team 1'))
+        self.assertThat(team_info[0]['has_preferredemail'], Equals(False))
+        self.assertThat(team_info[1]['title'], Equals(u'Team 2'))
+        self.assertThat(team_info[1]['has_preferredemail'], Equals(True))
+
     def test_teams_for_non_distro(self):
         # The expose_user_administered_teams_to_js function loads some data
         # about the teams the requesting user administers into the response to
@@ -136,7 +158,9 @@
         self.assertThat(len(team_info), Equals(expected_number_teams))
         # The items info consist of a dictionary with link and title keys.
         for i in range(expected_number_teams):
-            self.assertThat(team_info[i], KeysEqual('link', 'title', 'url'))
+            self.assertThat(
+                team_info[i],
+                KeysEqual('has_preferredemail', 'link', 'title', 'url'))
         # The link is the title of the team.
         self.assertThat(
             team_info[0]['title'], Equals(u'Bug Supervisor Sub Team'))
@@ -208,7 +232,9 @@
         self.assertThat(len(team_info), Equals(expected_number_teams))
         # The items info consist of a dictionary with link and title keys.
         for i in range(expected_number_teams):
-            self.assertThat(team_info[i], KeysEqual('link', 'title', 'url'))
+            self.assertThat(
+                team_info[i],
+                KeysEqual('has_preferredemail', 'link', 'title', 'url'))
         # The link is the title of the team.
         self.assertThat(
             team_info[0]['title'], Equals(u'Bug Supervisor Sub Team'))
@@ -236,7 +262,9 @@
         self.assertThat(len(team_info), Equals(expected_number_teams))
         # The items info consist of a dictionary with link and title keys.
         for i in range(expected_number_teams):
-            self.assertThat(team_info[i], KeysEqual('link', 'title', 'url'))
+            self.assertThat(
+                team_info[i],
+                KeysEqual('has_preferredemail', 'link', 'title', 'url'))
         # The link is the title of the team.
         self.assertThat(
             team_info[0]['title'], Equals(u'Bug Supervisor Sub Team'))

=== modified file 'lib/lp/bugs/javascript/bugtask_index_portlets.js'
--- lib/lp/bugs/javascript/bugtask_index_portlets.js	2011-06-02 16:09:55 +0000
+++ lib/lp/bugs/javascript/bugtask_index_portlets.js	2011-06-03 21:27:26 +0000
@@ -109,11 +109,8 @@
      * If the subscribers portlet fails to load, clear any
      * click handlers, so the normal subscribe page can be reached.
      */
-    namespace.portlet.subscribe('bugs:portletloadfailed', function(handlers) {
-        if (Y.Lang.isArray(handlers)) {
-            var click_handler = handlers[0];
-            click_handler.detach();
-        }
+    namespace.portlet.subscribe('bugs:portletloadfailed', function(handler) {
+        handler.detach();
     });
     namespace.portlet.subscribe('bugs:dupeportletloaded', function() {
         setup_subscription_link_handlers();

=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
--- lib/lp/registry/javascript/structural-subscription.js	2011-06-02 04:22:25 +0000
+++ lib/lp/registry/javascript/structural-subscription.js	2011-06-03 21:27:26 +0000
@@ -1655,13 +1655,16 @@
         if (team !== undefined) {
             title = team.title;
             url = team.url;
+            has_preferredemail = team.has_preferredemail;
         } else {
             is_team = false;
             link = LP.links.me;
+            has_preferredemail = true;
         }
     }
     return {
         is_team: is_team,
+        has_preferredemail: has_preferredemail,
         link: link,
         title: title,
         url: url
@@ -1692,7 +1695,7 @@
         filter: filter,
         subscriber_is_team: team_info.is_team,
         user_is_team_admin: team_info.is_team,
-        can_mute: team_info.is_team,
+        can_mute: team_info.is_team && ! team_info.has_preferredemail,
         subscriber_url: team_info.url,
         subscriber_link: team_info.link,
         subscriber_title: team_info.title,