← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~sinzui/launchpad/mailing-list-ui-0 into lp:launchpad

 

Curtis Hovey has proposed merging lp:~sinzui/launchpad/mailing-list-ui-0 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #297505 Warn users that mailing list archives are public unless the team is actually private
  https://bugs.launchpad.net/bugs/297505


Warn users that mailing list archive is public.

    Launchpad bug: https://bugs.launchpad.net/bugs/297505
    Pre-implementation: No one
    Test command: ./bin/test -vv \
      -t test_mailinglists -t stories/mailinglists

some team administrators don't realise that mailing list archives are visible
by everybody (not just members) even if the team is restricted. They're only
private if the team is private.

--------------------------------------------------------------------

RULES

    * Update the mailing list configuration page to explain that the
      archive will be public or private.
    * Update the mailing list portlet to state if the archive is
      public or private.


QA

    * Visit https://launchpad.net/~launchpad-users
    * Verify that the Mailing list portlet says "View public archive"
    * View the Configure mailing list page.
    * Verify that the page says the messages are in a "public archive"
    * Visit https://launchpad.net/~launchpad/+mailinglist
    * Verify that the page says the messages will be in a "public archive"


LINT

    lib/lp/registry/browser/team.py
    lib/lp/registry/browser/tests/test_mailinglists.py
    lib/lp/registry/stories/mailinglists/lifecycle.txt
    lib/lp/registry/stories/mailinglists/subscriptions.txt
    lib/lp/registry/stories/mailinglists/welcome-message.txt
    lib/lp/registry/templates/team-mailinglist.pt
    lib/lp/registry/templates/team-portlet-mailinglist.pt


IMPLEMENTATION

Updated the mailing list portlet to state the archive's visibility. Use a
list presentation to make it easier to see what can happen. Deleted
duplicate purge tests from lifecycle.txt, but also updated it because the
a template was using <b> instead of <strong>. Deleted a mediocre test from
subscriptions.txt -- updated the name of a test in test_mailinglists to
clarify what is reallying being tested.
    lib/lp/registry/browser/tests/test_mailinglists.py
    lib/lp/registry/stories/mailinglists/lifecycle.txt
    lib/lp/registry/stories/mailinglists/subscriptions.txt
    lib/lp/registry/templates/team-portlet-mailinglist.pt

Updated to form to explain the archive visibility. Added a page_title
that testing reported was missing. Delete the mailing_list_pending_approval
section because it is unreachable; lists are not approved any more. Updated a
test because the a template was using <b> instead of <strong>. 
    lib/lp/registry/browser/tests/test_mailinglists.py
    lib/lp/registry/browser/team.py
    lib/lp/registry/templates/team-mailinglist.pt
    lib/lp/registry/stories/mailinglists/welcome-message.txt
-- 
https://code.launchpad.net/~sinzui/launchpad/mailing-list-ui-0/+merge/43829
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/mailing-list-ui-0 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py	2010-12-10 23:10:09 +0000
+++ lib/lp/registry/browser/team.py	2010-12-15 21:02:35 +0000
@@ -508,6 +508,7 @@
     field_names = ['welcome_message']
     label = "Mailing list configuration"
     custom_widget('welcome_message', TextAreaWidget, width=72, height=10)
+    page_title = label
 
     def __init__(self, context, request):
         """Set feedback messages for users who want to edit the mailing list.

=== modified file 'lib/lp/registry/browser/tests/test_mailinglists.py'
--- lib/lp/registry/browser/tests/test_mailinglists.py	2010-10-26 15:47:24 +0000
+++ lib/lp/registry/browser/tests/test_mailinglists.py	2010-12-15 21:02:35 +0000
@@ -8,13 +8,32 @@
 
 
 from canonical.launchpad.ftests import login_person
-from canonical.launchpad.testing.pages import find_tag_by_id
+from canonical.launchpad.testing.pages import (
+    extract_text,
+    find_tag_by_id,
+    )
 from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.registry.interfaces.person import PersonVisibility
 from lp.testing import (
     person_logged_in,
     TestCaseWithFactory,
     )
-from lp.testing.views import create_view
+from lp.testing.views import (
+    create_initialized_view,
+    create_view,
+    )
+
+
+class MailingListTestCase(TestCaseWithFactory):
+    """Verify the content in +mailing-list-portlet."""
+
+    def makeTeamWithMailingList(self, name=None, owner=None, visibility=None):
+        team = self.factory.makeTeam(
+            name=name, owner=owner, visibility=visibility)
+        login_person(team.teamowner)
+        team_list = self.factory.makeMailingList(
+            team=team, owner=team.teamowner)
+        return team
 
 
 class MailingListSubscriptionControlsTestCase(TestCaseWithFactory):
@@ -41,7 +60,7 @@
         link_tag = find_tag_by_id(content, "link-list-subscribe")
         self.assertNotEqual(None, link_tag)
 
-    def test_subscribe_control_doesnt_render_for_anon(self):
+    def test_subscribe_control_doesnt_render_for_non_member(self):
         other_person = self.factory.makePerson()
         login_person(other_person)
         view = create_view(self.b_team, name='+index',
@@ -51,3 +70,69 @@
         self.assertNotEqual('', content)
         link_tag = find_tag_by_id(content, "link-list-subscribe")
         self.assertEqual(None, link_tag)
+
+
+class TestMailingListPortlet(MailingListTestCase):
+    """Verify the content in +mailing-list-portlet."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_public_archive(self):
+        # Public teams have public archives.
+        team = self.makeTeamWithMailingList()
+        view = create_view(
+            team, name='+portlet-mailinglist',
+            server_url='http://launchpad.dev', path_info='/~%s' % team.name)
+        link = find_tag_by_id(view(), 'mailing-list-archive')
+        self.assertEqual('View public archive', extract_text(link))
+
+    def test_private_archive(self):
+        # Private teams have private archives.
+        team = self.makeTeamWithMailingList(
+            visibility=PersonVisibility.PRIVATE)
+        view = create_view(
+            team, name='+portlet-mailinglist',
+            server_url='http://launchpad.dev', path_info='/~%s' % team.name)
+        link = find_tag_by_id(view(), 'mailing-list-archive')
+        self.assertEqual('View private archive', extract_text(link))
+
+
+class TestTeamMailingListConfigurationView(MailingListTestCase):
+    """Verify the +mailinglist view."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_public_achive_message_with_list(self):
+        # Public teams have public archives.
+        team = self.makeTeamWithMailingList()
+        view = create_initialized_view(
+            team, name='+mailinglist', principal=team.teamowner,)
+        element = find_tag_by_id(view(), 'mailing-list-archive')
+        self.assertEqual('public', extract_text(element))
+
+    def test_private_message_message_with_list(self):
+        # Private teams have private archives.
+        team = self.makeTeamWithMailingList(
+            visibility=PersonVisibility.PRIVATE)
+        view = create_initialized_view(
+            team, name='+mailinglist', principal=team.teamowner)
+        element = find_tag_by_id(view(), 'mailing-list-archive')
+        self.assertEqual('private', extract_text(element))
+
+    def test_public_achive_message_without_list(self):
+        # Public teams have public archives.
+        team = self.factory.makeTeam()
+        view = create_initialized_view(
+            team, name='+mailinglist', principal=team.teamowner,)
+        element = find_tag_by_id(view(), 'mailing-list-archive')
+        self.assertEqual('public', extract_text(element))
+
+    def test_private_message_message_without_list(self):
+        # Private teams have private archives.
+        team = self.factory.makeTeam(
+            visibility=PersonVisibility.PRIVATE)
+        login_person(team.teamowner)
+        view = create_initialized_view(
+            team, name='+mailinglist', principal=team.teamowner)
+        element = find_tag_by_id(view(), 'mailing-list-archive')
+        self.assertEqual('private', extract_text(element))

=== modified file 'lib/lp/registry/stories/mailinglists/lifecycle.txt'
--- lib/lp/registry/stories/mailinglists/lifecycle.txt	2010-12-10 23:10:09 +0000
+++ lib/lp/registry/stories/mailinglists/lifecycle.txt	2010-12-15 21:02:35 +0000
@@ -45,7 +45,7 @@
     ...     """Find out if a mailing list is in an unusual state."""
     ...     tag = find_tag_by_id(contents, 'mailing_list_status_message')
     ...     if tag:
-    ...         return extract_text(tag.b)
+    ...         return extract_text(tag.strong)
     ...     else:
     ...         return ""
 
@@ -83,13 +83,6 @@
     ...     find_tag_by_id(browser.contents, 'mailing-list-archive'))
     http://lists.launchpad.dev/landscape-developers
 
-The archive link is clickable, though we won't click it because Mailman isn't
-running.
-
-    >>> browser.getLink('View archive')
-    <Link text='View archive'
-          url='http://lists.launchpad.dev/landscape-developers'>
-
 The team's overview page also displays the posting address.
 
     >>> print extract_text(
@@ -390,87 +383,3 @@
 The team owner can see that an inactive list can be reactivated or purged.
 
     >>> set_list_state('aardvarks', MailingListStatus.INACTIVE)
-
-
-Actions on purged mailing lists
-===============================
-
-A mailing list expert purges the inactive list.
-
-    >>> expert_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
-    >>> expert_browser.getControl('Purge this Mailing List').click()
-
-Once purged, it's as if the mailing list never existed.
-
-    >>> print expert_browser.getLink('Mailing list archive')
-    Traceback (most recent call last):
-    ...
-    LinkNotFoundError
-
-So now the team owner can re-register the mailing list.
-
-    >>> browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
-    >>> browser.getControl('Apply for Mailing List').click()
-    >>> print browser.title
-    Aardvarks in Launchpad
-    >>> act()
-    >>> browser.open('http://launchpad.dev/~aardvarks')
-    >>> browser.getLink('View archive')
-    <Link text='View archive'
-          url='http://lists.launchpad.dev/aardvarks'>
-
-Once the mailing list has been purged, the team can be renamed.
-
-    >>> user_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
-    >>> user_browser.getControl('Deactivate this Mailing List').click()
-    >>> act()
-    >>> user_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
-    >>> user_browser.getControl('Purge this Mailing List').click()
-
-    >>> user_browser.open('http://launchpad.dev/~aardvarks/+edit')
-    >>> user_browser.getControl(name='field.name').value = 'antelopes'
-    >>> user_browser.getControl(name='field.displayname').value = 'Antelopes'
-    >>> user_browser.getControl('Save').click()
-    >>> print user_browser.title
-    Antelopes in Launchpad
-
-And of course, a mailing list for the newly renamed team can be created.
-
-    >>> user_browser.open('http://launchpad.dev/~antelopes/+mailinglist')
-    >>> user_browser.getControl('Apply for Mailing List').click()
-    >>> print user_browser.title
-    Antelopes in Launchpad
-    >>> act()
-    >>> user_browser.open(user_browser.url) # A `reload` would resubmit.
-    >>> user_browser.getLink('View archive')
-    <Link text='View archive'
-          url='http://lists.launchpad.dev/antelopes'>
-
-A team with an active mailing list cannot be merged with another team.
-
-    >>> admin_browser.open('http://launchpad.dev/people/+adminteammerge')
-    >>> admin_browser.getControl('Duplicated Team').value = 'antelopes'
-    >>> admin_browser.getControl('Target Team').value = 'guadamen'
-    >>> admin_browser.getControl('Merge').click()
-    >>> for message in get_feedback_messages(admin_browser.contents):
-    ...     print message
-    There is 1 error.
-    antelopes is associated with a Launchpad mailing list; we can't merge it.
-
-But if the mailing list for the team being merge has been purged, then the
-merge is allowed.
-
-    >>> admin_browser.open('http://launchpad.dev/~antelopes/+mailinglist')
-    >>> admin_browser.getControl('Deactivate this Mailing List').click()
-    >>> act()
-    >>> admin_browser.open('http://launchpad.dev/~antelopes/+mailinglist')
-    >>> admin_browser.getControl('Purge this Mailing List').click()
-
-    >>> admin_browser.open('http://launchpad.dev/people/+adminteammerge')
-    >>> admin_browser.getControl('Duplicated Team').value = 'antelopes'
-    >>> admin_browser.getControl('Target Team').value = 'guadamen'
-    >>> admin_browser.getControl('Merge').click()
-    >>> admin_browser.getControl('Deactivate Members and Merge').click()
-    >>> for message in get_feedback_messages(admin_browser.contents):
-    ...     print message
-    Merge completed successfully.

=== modified file 'lib/lp/registry/stories/mailinglists/subscriptions.txt'
--- lib/lp/registry/stories/mailinglists/subscriptions.txt	2010-10-17 15:44:08 +0000
+++ lib/lp/registry/stories/mailinglists/subscriptions.txt	2010-12-15 21:02:35 +0000
@@ -186,7 +186,8 @@
 team.  (Otherwise, the mailing list subscription checkbox won't show at
 all.)
 
-    >>> from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+    >>> from canonical.launchpad.interfaces.launchpad import (
+    ...     ILaunchpadCelebrities)
     >>> from lp.registry.interfaces.person import IPersonSet
     >>> from canonical.launchpad.ftests import login, logout
     >>> from zope.component import getUtility
@@ -400,20 +401,9 @@
     rosetta-admins@xxxxxxxxxxxxxxxxxxx
     Policy: You must be a team member to subscribe to the team mailing list.
     Subscribe to mailing list
-    View archive
+    View public archive
     View subscribers...
 
-Non-members are not shown links to subscribe or unsubscribe.
-
-    >>> user_browser.open('http://launchpad.dev/~rosetta-admins')
-    >>> print extract_text(
-    ...     find_portlet(user_browser.contents, 'Mailing list'))
-    Mailing list
-    rosetta-admins@xxxxxxxxxxxxxxxxxxx
-    Policy: You must be a team member to subscribe to the team mailing list.
-    View archive
-    View subscribers
-
 The mailing list for Rosetta Admins has no subscribers.
 (Jeff Waugh has asked to subscribe but he's not considered a subscriber
 because his membership on Rosetta Admins hasn't been approved)

=== modified file 'lib/lp/registry/stories/mailinglists/welcome-message.txt'
--- lib/lp/registry/stories/mailinglists/welcome-message.txt	2009-04-17 10:32:16 +0000
+++ lib/lp/registry/stories/mailinglists/welcome-message.txt	2010-12-15 21:02:35 +0000
@@ -1,4 +1,5 @@
-= Customizing the welcome message =
+Customizing the welcome message
+===============================
 
 A team mailing list can have a custom welcome message.  The welcome message is
 customizable whether or not the mailing list is currently the team's contact
@@ -49,11 +50,12 @@
     ...     MailingListStatus)
     >>> from zope.security.proxy import removeSecurityProxy
     >>> naked_list = removeSecurityProxy(mailing_list)
-    >>> removeSecurityProxy(mailing_list).status = MailingListStatus.MOD_FAILED
+    >>> removeSecurityProxy(
+    ...     mailing_list).status = MailingListStatus.MOD_FAILED
     >>> transaction.commit()
     >>> logout()
 
     >>> user_browser.open('http://launchpad.dev/~aardvarks/+mailinglist')
     >>> print extract_text(find_tag_by_id(
-    ...     user_browser.contents, 'mailing_list_status_message').b)
+    ...     user_browser.contents, 'mailing_list_status_message').strong)
     This team's mailing list is in an inconsistent state...

=== modified file 'lib/lp/registry/templates/team-mailinglist.pt'
--- lib/lp/registry/templates/team-mailinglist.pt	2010-03-18 18:55:02 +0000
+++ lib/lp/registry/templates/team-mailinglist.pt	2010-12-15 21:02:35 +0000
@@ -16,14 +16,19 @@
     <metal:extra-info fill-slot="extra_info">
       <p id="mailing_list_status_message"
          tal:condition="view/mailing_list_status_message">
-        <b tal:content="structure view/mailing_list_status_message">
+        <strong tal:content="structure view/mailing_list_status_message">
           This team's mailing list will be available within a few minutes.
-        </b>
+        </strong>
       </p>
 
       <p tal:condition="view/list_is_usable">
         This team's mailing list is active at
-        <b><address tal:replace="view/mailinglist_address" /></b>.
+        <strong><address tal:replace="view/mailinglist_address" /></strong>.
+        Messages are in a <strong id="mailing-list-archive"
+              tal:content="context/visibility/title/fmt:lower">public</strong>
+        archive at
+        <a tal:attributes="href context/mailing_list/archive_url"
+          tal:content="context/mailing_list/archive_url" />
       </p>
     </metal:extra-info>
 
@@ -31,11 +36,21 @@
       <div id="no_mailing_list"
          tal:condition="view/list_can_be_requested">
         <p>
-          You can request a mailing list for this team. Once a
-          Launchpad administrator approves your request, team members
-          can subscribe and post to the team mailing list. The team
-          mailing list may also be used as the team's contact address.
+          You can create a mailing list for this team. Team members
+          can subscribe and post to the team mailing list.
         </p>
+
+        <ul class="bulleted">
+          <li>
+            The mailing list can be used as the team's contact address
+          </li>
+          <li>
+            Messages will be kept in a
+            <strong id="mailing-list-archive"
+              tal:content="context/visibility/title/fmt:lower" /> archive
+          </li>
+        </ul>
+
         <p id="ubuntu-notice"><strong>Ubuntu mailing lists should not be
           created here.  Create them
           at <a href="https://lists.ubuntu.com";>lists.ubuntu.com</a> instead.
@@ -47,14 +62,6 @@
         <input tal:replace="structure view/request_list_creation/render" />
       </div>
 
-      <div id="mailing_list_pending_approval"
-         tal:condition="view/list_application_can_be_cancelled">
-        <p>The application for this team's mailing list is pending approval.</p>
-        <p>
-          <input tal:replace="structure view/cancel_list_creation/render" />
-        </p>
-      </div>
-
       <div id="mailing_list_purge"
            tal:condition="view/list_can_be_purged">
         <p>You can purge this mailing list so that the team can be merged or

=== modified file 'lib/lp/registry/templates/team-portlet-mailinglist.pt'
--- lib/lp/registry/templates/team-portlet-mailinglist.pt	2010-08-17 19:03:44 +0000
+++ lib/lp/registry/templates/team-portlet-mailinglist.pt	2010-12-15 21:02:35 +0000
@@ -34,8 +34,10 @@
           </form>
           </tal:subscribed-to-list>
       </tal:member>
-      <img src="/@@/mail" alt="email" /> <a id="mailing-list-archive"
-              tal:attributes="href archive_url">View archive</a>
+      <img src="/@@/mail" alt="email" />
+      <a id="mailing-list-archive"
+        tal:attributes="href archive_url">View <tal:visibility
+          replace="context/visibility/title/fmt:lower" /> archive</a>
       <br /><img src="/@@/team" alt="team" /> <a id="mailing-list-subscribers"
               tal:attributes="href context/fmt:url/+mailing-list-subscribers"
               >View subscribers</a>