launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/ml-moderate-view-0 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #680009 AssertionError on +mailinglist-moderate page
  https://bugs.launchpad.net/bugs/680009


This is my branch to redirect users from the mailing list moderation view.

    lp:~sinzui/launchpad/ml-moderate-view-0
    Diff size: 60
    Launchpad bug: https://bugs.launchpad.net/bugs/680009
    Test command: ./bin/test -vv \
        -t TestModeration -t stories/mailinglists/moderation
    Pre-implementation: no one
    Target release: 10.12


Redirect users from the mailing list moderation view
-----------------------------------------------------

The user hacked the url to access the +mailinglist-moderate view. I think we
want to replace the assert in TeamMailingListModerationView with a redirect to
the team page and add a notification to explain the team does not have a
mailing list.


Rules
-----

    * Like the +contact-user page, the view should issue a redirect and
      notification to explain why the page is not available.


QA
--

    * Visit https://launchpad.net/~elementaryos/+mailinglist-moderate
    * Verify you are redirected to https://launchpad.net/~elementaryos
    * Verify there is a notification stating that ~elementaryos does not
      have a mailing list.


Lint
----

Linting changed files:
  lib/lp/registry/browser/team.py
  lib/lp/registry/browser/tests/test_team.py


Implementation
--------------

Replaced the assert with a guard. Issue a notification and a redirect when
there is no mailing list.
    lib/lp/registry/browser/team.py
    lib/lp/registry/browser/tests/test_team.py
-- 
https://code.launchpad.net/~sinzui/launchpad/ml-moderate-view-0/+merge/41548
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/ml-moderate-view-0 into lp:launchpad/devel.

=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py	2010-11-11 21:30:09 +0000
+++ lib/lp/registry/browser/team.py	2010-11-23 04:57:01 +0000
@@ -801,8 +801,10 @@
         super(TeamMailingListModerationView, self).__init__(context, request)
         list_set = getUtility(IMailingListSet)
         self.mailing_list = list_set.get(self.context.name)
-        assert(self.mailing_list is not None), (
-            'No mailing list: %s' % self.context.name)
+        if self.mailing_list is None:
+            self.request.response.addInfoNotification(
+                '%s does not have a mailing list.' % self.context.displayname)
+            return self.request.response.redirect(canonical_url(self.context))
 
     @cachedproperty
     def hold_count(self):

=== modified file 'lib/lp/registry/browser/tests/test_team.py'
--- lib/lp/registry/browser/tests/test_team.py	2010-11-11 22:43:01 +0000
+++ lib/lp/registry/browser/tests/test_team.py	2010-11-23 04:57:01 +0000
@@ -3,6 +3,7 @@
 
 __metaclass__ = type
 
+from canonical.launchpad.webapp.publisher import canonical_url
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.registry.browser.person import TeamOverviewMenu
 from lp.testing import (
@@ -12,7 +13,10 @@
     )
 from lp.testing.matchers import IsConfiguredBatchNavigator
 from lp.testing.menu import check_menu_links
-from lp.testing.views import create_initialized_view
+from lp.testing.views import (
+    create_initialized_view,
+    create_view,
+    )
 
 
 class TestTeamMenu(TestCaseWithFactory):
@@ -55,6 +59,18 @@
             view.held_messages,
             IsConfiguredBatchNavigator('message', 'messages'))
 
+    def test_no_mailing_list_redirect(self):
+        team = self.factory.makeTeam()
+        login_person(team.teamowner)
+        view = create_view(team, name='+mailinglist-moderate')
+        response = view.request.response
+        self.assertEqual(302, response.getStatus())
+        self.assertEqual(canonical_url(team), response.getHeader('location'))
+        self.assertEqual(1, len(response.notifications))
+        self.assertEqual(
+            '%s does not have a mailing list.' % (team.displayname),
+            response.notifications[0].message)
+
 
 class TestTeamMemberAddView(TestCaseWithFactory):