Merge lp:~sinzui/launchpad/contact-new-maintainer into lp:launchpad

[Curtis Hovey]

Send licensing email to new project maintainer.

Lp is sending licensing emails to the project registrant, but the
registrant may have made someone else the maintainer. The licensing
jobs already use the maintainer, so there is a chance of confusion
when the registrant is not a member of the maintainer team.

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

RULES

    Pre-implementation: abentley
    * Use the project maintainer, not the registrant when sending
      the initial email about licensing issues.
    * The emails already use neutral language to deal with user and teams.

QA

    * Clear the staging inbox so that you can find the email.
    * Visit https://qastaging.launchpad.net/projects/+new
    * Register a project with a proprietary license and set a team as the
      maintainer.
    * Check the staging inbox and verify there is a message addressed
      to the team admins.

LINT

    lib/lp/registry/subscribers.py
    lib/lp/registry/tests/test_subscribers.py

TEST

    ./bin/test -vvc lp.registry.tests.test_subscribers

IMPLEMENTATION

I first updated LicenseNotification to ignore the user and instead use
the product.owner. I then removed the user argument since it was unused.
Since the code does not adapt the event.user, tests do not need to be
logged in.
    lib/lp/registry/subscribers.py
    lib/lp/registry/tests/test_subscribers.py

[Abel Deuring (adeuring)]

review: Approve (code)

=== modified file 'lib/lp/registry/subscribers.py'
--- lib/lp/registry/subscribers.py	2012-06-14 05:18:22 +0000
+++ lib/lp/registry/subscribers.py	2012-10-05 15:09:23 +0000
@@ -14,12 +14,12 @@
 import pytz
 from zope.security.proxy import removeSecurityProxy
 
-from lp.registry.interfaces.person import IPerson
 from lp.registry.interfaces.product import License
 from lp.services.config import config
 from lp.services.mail.helpers import get_email_template
 from lp.services.mail.sendmail import (
     format_address,
+    format_address_for_person,
     simple_sendmail,
     )
 from lp.services.webapp.menu import structured
@@ -32,8 +32,7 @@
 def product_licenses_modified(product, event):
     """Send a notification if licences changed and a licence is special."""
     if LicenseNotification.needs_notification(product):
-        user = IPerson(event.user)
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         notification.send()
         notification.display()
 
@@ -41,9 +40,8 @@
 class LicenseNotification:
     """Send notification about special licences to the user."""
 
-    def __init__(self, product, user):
+    def __init__(self, product):
         self.product = product
-        self.user = user
 
     @staticmethod
     def needs_notification(product):
@@ -85,15 +83,18 @@
         if not self.needs_notification(self.product):
             # The project has a common licence.
             return False
-        user_address = format_address(
-            self.user.displayname, self.user.preferredemail.email)
+        maintainer = self.product.owner
+        if maintainer.is_team:
+            user_address = maintainer.getTeamAdminsEmailAddresses()
+        else:
+            user_address = format_address_for_person(maintainer)
         from_address = format_address(
             "Launchpad", config.canonical.noreply_from_address)
         commercial_address = format_address(
             'Commercial', '[e-mail]')
         substitutions = dict(
-            user_displayname=self.user.displayname,
-            user_name=self.user.name,
+            user_displayname=maintainer.displayname,
+            user_name=maintainer.name,
             product_name=self.product.name,
             product_url=canonical_url(self.product),
             commercial_use_expiration=self.getCommercialUseMessage(),

=== modified file 'lib/lp/registry/tests/test_subscribers.py'
--- lib/lp/registry/tests/test_subscribers.py	2012-10-03 11:20:01 +0000
+++ lib/lp/registry/tests/test_subscribers.py	2012-10-05 15:09:23 +0000
@@ -8,18 +8,25 @@
 from datetime import datetime
 
 import pytz
+from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from lp.registry.interfaces.person import TeamMembershipPolicy
 from lp.registry.interfaces.product import License
 from lp.registry.model.product import LicensesModifiedEvent
 from lp.registry.subscribers import (
     LicenseNotification,
     product_licenses_modified,
     )
+from lp.registry.interfaces.teammembership import (
+    ITeamMembershipSet,
+    TeamMembershipStatus,
+    )
 from lp.services.webapp.publisher import get_current_browser_request
 from lp.testing import (
     login_person,
     logout,
+    person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
@@ -32,16 +39,13 @@
 
     def test_init(self):
         product = self.factory.makeProduct()
-        login_person(product.owner)
         event = LicensesModifiedEvent(product)
-        self.assertEqual(product.owner, event.user.person)
         self.assertEqual(product, event.object)
         self.assertEqual(product, event.object_before_modification)
         self.assertEqual([], event.edited_fields)
 
     def test_init_with_user(self):
         product = self.factory.makeProduct()
-        login_person(product.owner)
         event = LicensesModifiedEvent(product, user=product.owner)
         self.assertEqual(product.owner, event.user)
 
@@ -53,7 +57,6 @@
     def make_product_event(self, licenses):
         product = self.factory.makeProduct(licenses=licenses)
         pop_notifications()
-        login_person(product.owner)
         event = LicensesModifiedEvent(product, user=product.owner)
         return product, event
 
@@ -128,7 +131,7 @@
     def test_send_known_license(self):
         # A known licence does not generate an email.
         product, user = self.make_product_user([License.GNU_GPL_V2])
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         result = notification.send()
         self.assertIs(False, result)
         self.assertEqual(0, len(pop_notifications()))
@@ -136,7 +139,7 @@
     def test_send_other_dont_know(self):
         # An Other/I don't know licence sends one email.
         product, user = self.make_product_user([License.DONT_KNOW])
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         result = notification.send()
         self.assertIs(True, result)
         self.verify_whiteboard(product)
@@ -147,7 +150,7 @@
     def test_send_other_open_source(self):
         # An Other/Open Source licence sends one email.
         product, user = self.make_product_user([License.OTHER_OPEN_SOURCE])
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         result = notification.send()
         self.assertIs(True, result)
         self.verify_whiteboard(product)
@@ -158,7 +161,7 @@
     def test_send_other_proprietary(self):
         # An Other/Proprietary licence sends one email.
         product, user = self.make_product_user([License.OTHER_PROPRIETARY])
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         result = notification.send()
         self.assertIs(True, result)
         self.verify_whiteboard(product)
@@ -166,6 +169,28 @@
         self.assertEqual(1, len(notifications))
         self.verify_user_email(notifications.pop())
 
+    def test_send_other_proprietary_team_admins(self):
+        # An Other/Proprietary licence sends one email to the team admins.
+        product, user = self.make_product_user([License.OTHER_PROPRIETARY])
+        owner = self.factory.makePerson(email='[e-mail]')
+        team = self.factory.makeTeam(
+            owner=owner, membership_policy=TeamMembershipPolicy.RESTRICTED)
+        admin = self.factory.makePerson(email='[e-mail]')
+        with person_logged_in(owner):
+            team.addMember(admin, owner)
+            membership_set = getUtility(ITeamMembershipSet)
+            tm = membership_set.getByPersonAndTeam(admin, team)
+            tm.setStatus(TeamMembershipStatus.ADMIN, owner)
+        with person_logged_in(product.owner):
+            product.owner = team
+        pop_notifications()
+        notification = LicenseNotification(product)
+        result = notification.send()
+        self.assertIs(True, result)
+        notifications = pop_notifications()
+        self.assertEqual(1, len(notifications))
+        self.assertEqual('[e-mail],[e-mail]', notifications[0]['To'])
+
     def test_display_no_request(self):
         # If there is no request, there is no reason to show a message in
         # the browser.
@@ -173,7 +198,7 @@
         # Using the proxied product leads to an exeception when
         # notification.display() below is called because the permission
         # checks product require an interaction.
-        notification = LicenseNotification(removeSecurityProxy(product), user)
+        notification = LicenseNotification(removeSecurityProxy(product))
         logout()
         result = notification.display()
         self.assertIs(False, result)
@@ -181,7 +206,7 @@
     def test_display_no_message(self):
         # A notification is not added if there is no message to show.
         product, user = self.make_product_user([License.GNU_GPL_V2])
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         result = notification.display()
         self.assertEqual('', notification.getCommercialUseMessage())
         self.assertIs(False, result)
@@ -189,7 +214,7 @@
     def test_display_has_message(self):
         # A notification is added if there is a message to show.
         product, user = self.make_product_user([License.OTHER_PROPRIETARY])
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         result = notification.display()
         message = notification.getCommercialUseMessage()
         self.assertIs(True, result)
@@ -204,7 +229,7 @@
         # A notification is added if there is a message to show.
         product, user = self.make_product_user([License.OTHER_PROPRIETARY])
         product.displayname = '<b>Look</b>'
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         result = notification.display()
         self.assertIs(True, result)
         request = get_current_browser_request()
@@ -221,33 +246,33 @@
 
     def test_getTemplateName_other_dont_know(self):
         product, user = self.make_product_user([License.DONT_KNOW])
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         self.assertEqual(
             'product-license-dont-know.txt',
             notification.getTemplateName())
 
     def test_getTemplateName_propietary(self):
         product, user = self.make_product_user([License.OTHER_PROPRIETARY])
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         self.assertEqual(
             'product-license-other-proprietary.txt',
             notification.getTemplateName())
 
     def test_getTemplateName_other_open_source(self):
         product, user = self.make_product_user([License.OTHER_OPEN_SOURCE])
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         self.assertEqual(
             'product-license-other-open-source.txt',
             notification.getTemplateName())
 
     def test_getCommercialUseMessage_without_commercial_subscription(self):
         product, user = self.make_product_user([License.MIT])
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         self.assertEqual('', notification.getCommercialUseMessage())
 
     def test_getCommercialUseMessage_with_complimentary_cs(self):
         product, user = self.make_product_user([License.OTHER_PROPRIETARY])
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         message = (
             "Ball's complimentary commercial subscription expires on %s." %
             product.commercial_subscription.date_expires.date().isoformat())
@@ -257,7 +282,7 @@
         product, user = self.make_product_user([License.MIT])
         self.factory.makeCommercialSubscription(product)
         product.licenses = [License.MIT, License.OTHER_PROPRIETARY]
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         message = (
             "Ball's commercial subscription expires on %s." %
             product.commercial_subscription.date_expires.date().isoformat())
@@ -267,7 +292,7 @@
         product, user = self.make_product_user([License.MIT])
         self.factory.makeCommercialSubscription(product, expired=True)
         product.licenses = [License.MIT, License.OTHER_PROPRIETARY]
-        notification = LicenseNotification(product, user)
+        notification = LicenseNotification(product)
         message = (
             "Ball's commercial subscription expired on %s." %
             product.commercial_subscription.date_expires.date().isoformat())