launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03810
[Merge] lp:~stevenk/launchpad/copies-use-notify into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/copies-use-notify into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/copies-use-notify/+merge/62953
Build on the work of abstracting out notifications to use them for copies. I have added a bool to do_copy() that controls if e-mails are sent.
I have also had to rework parts of the notification code to deal with the fact that syncs only feed you an SPR, and tracking back from the SPR to the PackageUpload is fraught with peril, so there is no changes file.
Note that this code currently contains a hack to work out the announcement list, pending another MP landing that removes that functionality from the exposed notify() interface.
--
https://code.launchpad.net/~stevenk/launchpad/copies-use-notify/+merge/62953
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/copies-use-notify into lp:launchpad.
=== modified file 'lib/lp/soyuz/adapters/notification.py'
--- lib/lp/soyuz/adapters/notification.py 2011-05-27 21:12:25 +0000
+++ lib/lp/soyuz/adapters/notification.py 2011-06-02 07:02:28 +0000
@@ -62,7 +62,8 @@
}
template = get_template(archive, 'rejected')
body = template % information
- to_addrs = get_recipients(blamer, archive, distroseries, changes, logger)
+ to_addrs = get_recipients(
+ blamer, archive, distroseries, logger, changes=changes)
logger.debug("Sending rejection email.")
send_mail(None, archive, to_addrs, subject, body, False, logger=logger)
@@ -171,7 +172,8 @@
return
recipients = get_recipients(
- blamer, archive, distroseries, changes, logger)
+ blamer, archive, distroseries, logger, changes=changes, spr=spr,
+ bprs=bprs)
# There can be no recipients if none of the emails are registered
# in LP.
@@ -202,7 +204,7 @@
subject = calculate_subject(
spr, bprs, customfiles, archive, distroseries, pocket, action)
body = assemble_body(
- blamer, spr, archive, distroseries, summarystring, changes,
+ blamer, spr, bprs, archive, distroseries, summarystring, changes,
action, announce_list)
send_mail(
spr, archive, recipients, subject, body, dry_run,
@@ -212,14 +214,15 @@
build_and_send_mail(action, recipients)
+ (changesfile, date, from_addr, maintainer) = fetch_information(
+ spr, bprs, changes)
# If we're sending an acceptance notification for a non-PPA upload,
# announce if possible. Avoid announcing backports, binary-only
# security uploads, or autosync uploads.
if (action == 'accepted' and announce_list and not archive.is_ppa and
pocket != PackagePublishingPocket.BACKPORTS and
not (pocket == PackagePublishingPocket.SECURITY and spr is None) and
- not is_auto_sync_upload(spr, bprs, pocket, changes['Changed-By'])):
- from_addr = sanitize_string(changes['Changed-By'])
+ not is_auto_sync_upload(spr, bprs, pocket, from_addr)):
name = None
bcc_addr = None
if spr:
@@ -233,15 +236,18 @@
'announcement', [str(announce_list)], from_addr, bcc_addr)
-def assemble_body(blamer, spr, archive, distroseries, summary, changes,
+def assemble_body(blamer, spr, bprs, archive, distroseries, summary, changes,
action, announce_list):
"""Assemble the e-mail notification body."""
+ if changes is None:
+ changes = {}
+ (changesfile, date, changedby, maintainer) = fetch_information(
+ spr, bprs, changes)
information = {
'STATUS': ACTION_DESCRIPTIONS[action],
'SUMMARY': summary,
- 'DATE': 'Date: %s' % changes['Date'],
- 'CHANGESFILE': ChangesFile.formatChangesComment(
- sanitize_string(changes.get('Changes'))),
+ 'DATE': 'Date: %s' % date,
+ 'CHANGESFILE': changesfile,
'DISTRO': distroseries.distribution.title,
'ANNOUNCE': 'No announcement sent',
'CHANGEDBY': '',
@@ -253,7 +259,6 @@
}
if spr:
information['SPR_URL'] = canonical_url(spr)
- changedby = sanitize_string(changes.get('Changed-By'))
if changedby:
information['CHANGEDBY'] = '\nChanged-By: %s' % changedby
origin = changes.get('Origin')
@@ -264,13 +269,11 @@
"\nThis upload awaits approval by a distro manager\n")
if announce_list:
information['ANNOUNCE'] = "Announcing to %s" % announce_list
- if blamer is not None:
- signer_signature = '%s <%s>' % (
- blamer.displayname, blamer.preferredemail.email)
+ if blamer is not None and blamer != email_to_person(changedby):
+ signer_signature = person_to_email(blamer)
if signer_signature != changedby:
information['SIGNER'] = '\nSigned-By: %s' % signer_signature
# Add maintainer if present and different from changed-by.
- maintainer = sanitize_string(changes.get('Maintainer'))
if maintainer and maintainer != changedby:
information['MAINTAINER'] = '\nMaintainer: %s' % maintainer
return get_template(archive, action) % information
@@ -407,11 +410,21 @@
logger.debug(msg)
-def get_recipients(blamer, archive, distroseries, changes, logger):
+def get_recipients(blamer, archive, distroseries, logger, changes=None,
+ spr=None, bprs=None):
"""Return a list of recipients for notification emails."""
candidate_recipients = []
debug(logger, "Building recipients list.")
- changer = email_to_person(changes['Changed-By'])
+ (changesfile, date, changedby, maint) = fetch_information(
+ spr, bprs, changes)
+ if changedby:
+ changer = email_to_person(changedby)
+ else:
+ changer = None
+ if maint:
+ maintainer = email_to_person(maint)
+ else:
+ maintainer = None
if blamer:
# This is a signed upload.
@@ -432,7 +445,6 @@
# If this is not a PPA, we also consider maintainer and changed-by.
if blamer and not archive.is_ppa:
- maintainer = email_to_person(changes['Maintainer'])
if (maintainer and maintainer != blamer and
maintainer.isUploader(distroseries.distribution)):
debug(logger, "Adding maintainer to recipients")
@@ -518,6 +530,12 @@
return getUtility(IPersonSet).getByEmail(email)
+def person_to_email(person):
+ """Return a string of full name <e-mail address> given an IPerson."""
+ if person and person.preferredemail:
+ return '%s <%s>' % (person.displayname, person.preferredemail.email)
+
+
def is_auto_sync_upload(spr, bprs, pocket, changed_by_email):
"""Return True if this is a (Debian) auto sync upload.
@@ -531,6 +549,29 @@
spr and not bprs and changed_by == katie and
pocket != PackagePublishingPocket.SECURITY)
+def fetch_information(spr, bprs, changes):
+ changedby = None
+ maintainer = None
+ if changes:
+ changesfile = ChangesFile.formatChangesComment(
+ sanitize_string(changes.get('Changes')))
+ date = changes.get('Date')
+ changedby = sanitize_string(changes.get('Changed-By'))
+ maintainer = sanitize_string(changes.get('Maintainer'))
+ elif spr:
+ changesfile = spr.changelog_entry
+ date = spr.dateuploaded
+ changedby = person_to_email(spr.creator)
+ maintainer = person_to_email(spr.maintainer)
+ elif bprs:
+ changesfile = bprs[0].changelog_entry
+ date = bprs[0].dateuploaded
+ changedby = person_to_email(
+ bprs[0].build.source_package_release.creator)
+ maintainer = person_to_email(
+ bprs[0].build.source_package_release.maintainer)
+ return (changesfile, date, changedby, maintainer)
+
class LanguagePackEncountered(Exception):
"""Thrown when not wanting to email notifications for language packs."""
=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py 2011-05-17 12:38:49 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py 2011-06-02 07:02:28 +0000
@@ -28,6 +28,7 @@
from canonical.librarian.utils import copy_and_close
from lp.app.errors import NotFoundError
from lp.buildmaster.enums import BuildStatus
+from lp.soyuz.adapters.notification import notify
from lp.soyuz.adapters.packagelocation import build_package_location
from lp.soyuz.enums import (
ArchivePurpose,
@@ -509,7 +510,8 @@
def do_copy(sources, archive, series, pocket, include_binaries=False,
- allow_delayed_copies=True, person=None, check_permissions=True):
+ allow_delayed_copies=True, person=None, check_permissions=True,
+ send_email=True):
"""Perform the complete copy of the given sources incrementally.
Verifies if each copy can be performed using `CopyChecker` and
@@ -534,6 +536,7 @@
:param person: the requester `IPerson`.
:param check_permissions: boolean indicating whether or not the
requester's permissions to copy should be checked.
+ :param send_email: Should we notify for the copy performed?
:raise CannotCopy when one or more copies were not allowed. The error
will contain the reason why each copy was denied.
@@ -569,11 +572,22 @@
destination_series = series
if source.delayed:
delayed_copy = _do_delayed_copy(
- source, archive, destination_series, pocket, include_binaries)
+ source, archive, destination_series, pocket,
+ include_binaries)
sub_copies = [delayed_copy]
else:
sub_copies = _do_direct_copy(
- source, archive, destination_series, pocket, include_binaries)
+ source, archive, destination_series, pocket,
+ include_binaries)
+ if send_email:
+ if archive.purpose == ArchivePurpose.PRIMARY:
+ announce_list = destination_series.changeslist
+ else:
+ announce_list = None
+ notify(
+ person, source.sourcepackagerelease, [], [], archive,
+ destination_series, pocket, changes=None,
+ action='accepted', announce_list=announce_list)
copies.extend(sub_copies)
=== modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py 2011-05-25 03:32:48 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py 2011-06-02 07:02:28 +0000
@@ -7,6 +7,7 @@
import os
import subprocess
import sys
+from textwrap import dedent
import unittest
import pytz
@@ -28,6 +29,7 @@
LaunchpadFunctionalLayer,
LaunchpadZopelessLayer,
)
+from lp.archivepublisher.utils import get_ppa_reference
from lp.bugs.interfaces.bug import (
CreateBugParams,
IBugSet,
@@ -80,10 +82,13 @@
)
from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
from lp.testing import (
+ person_logged_in,
StormStatementRecorder,
TestCaseWithFactory,
)
+from lp.testing.mail_helpers import pop_notifications
from lp.testing.matchers import HasQueryCount
+from lp.testing.sampledata import ADMIN_EMAIL
class ReUploadFileTestCase(TestCaseWithFactory):
@@ -1377,6 +1382,81 @@
[copied_source] = self.doCopy(
source, target_archive, dsp.derived_series, source.pocket, False)
+ def test_copy_ppa_generates_notification(self):
+ # When a copy into a PPA is performed, a notification is sent.
+ archive = self.test_publisher.ubuntutest.main_archive
+ source = self.test_publisher.getPubSource(
+ archive=archive, version='1.0-2', architecturehintlist='any')
+ source.sourcepackagerelease.changelog_entry = '* Foo!'
+ nobby = self.createNobby(('i386', 'hppa'))
+ getUtility(ISourcePackageFormatSelectionSet).add(
+ nobby, SourcePackageFormat.FORMAT_1_0)
+ target_archive = self.factory.makeArchive(
+ distribution=self.test_publisher.ubuntutest)
+ [copied_source] = do_copy(
+ [source], target_archive, nobby, source.pocket, False,
+ person=target_archive.owner, check_permissions=False)
+ [notification] = pop_notifications()
+ self.assertEquals(
+ get_ppa_reference(target_archive),
+ notification['X-Launchpad-PPA'])
+ self.assertIn(
+ source.sourcepackagerelease.changelog_entry,
+ notification.as_string())
+
+ def test_copy_generates_notification(self):
+ # When a copy into a primary archive is performed, a notification is
+ # sent.
+ archive = self.test_publisher.ubuntutest.main_archive
+ source = self.test_publisher.getPubSource(
+ archive=archive, version='1.0-2', architecturehintlist='any')
+ source.sourcepackagerelease.changelog_entry = '* Foo!'
+ # Copying to a primary archive reads the changes to close bugs.
+ transaction.commit()
+ nobby = self.createNobby(('i386', 'hppa'))
+ getUtility(ISourcePackageFormatSelectionSet).add(
+ nobby, SourcePackageFormat.FORMAT_1_0)
+ admin = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
+ nobby.changeslist = 'nobby-changes@xxxxxxxxxxx'
+ [copied_source] = do_copy(
+ [source], archive, nobby, source.pocket, False,
+ person=source.sourcepackagerelease.creator,
+ check_permissions=False)
+ [notification, announcement] = pop_notifications()
+ self.assertEquals(
+ 'Foo Bar <foo.bar@xxxxxxxxxxxxx>', notification['To'])
+ self.assertEquals('nobby-changes@xxxxxxxxxxx', announcement['To'])
+ for mail in (notification, announcement):
+ self.assertEquals(
+ '[ubuntutest/nobby] foo 1.0-2 (Accepted)', mail['Subject'])
+ expected_text = dedent("""
+ * Foo!
+
+ Date: %s
+ Changed-By: Foo Bar <foo.bar@xxxxxxxxxxxxx>
+ http://launchpad.dev/ubuntutest/breezy-autotest/+source/foo/1.0-2
+ """ % source.sourcepackagerelease.dateuploaded)
+ self.assertIn(expected_text, notification.as_string())
+ self.assertIn(expected_text, announcement.as_string())
+
+ def test_copy_does_not_generate_notification(self):
+ # When notify = False is passed to do_copy, no notification is
+ # generated.
+ archive = self.test_publisher.ubuntutest.main_archive
+ source = self.test_publisher.getPubSource(
+ archive=archive, version='1.0-2', architecturehintlist='any')
+ source.sourcepackagerelease.changelog_entry = '* Foo!'
+ nobby = self.createNobby(('i386', 'hppa'))
+ getUtility(ISourcePackageFormatSelectionSet).add(
+ nobby, SourcePackageFormat.FORMAT_1_0)
+ target_archive = self.factory.makeArchive(
+ distribution=self.test_publisher.ubuntutest)
+ [copied_source] = do_copy(
+ [source], target_archive, nobby, source.pocket, False,
+ person=target_archive.owner, check_permissions=False,
+ send_email=False)
+ self.assertEquals([], pop_notifications())
+
class TestDoDelayedCopy(TestCaseWithFactory, BaseDoCopyTests):
