launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/sync-sponsored-widget into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #827555 in Launchpad itself: "native syncs have no way to indicate sponsorship"
  https://bugs.launchpad.net/launchpad/+bug/827555

For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/sync-sponsored-widget/+merge/85701

Add a person picker on the package sync pages so that the user can specify a user to sponsor.
-- 
https://code.launchpad.net/~julian-edwards/launchpad/sync-sponsored-widget/+merge/85701
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/sync-sponsored-widget into lp:launchpad.

=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py	2011-11-26 04:03:29 +0000
+++ lib/lp/registry/browser/distroseries.py	2011-12-14 16:46:17 +0000
@@ -77,6 +77,7 @@
     LaunchpadDropdownWidget,
     LaunchpadRadioWidget,
     )
+from lp.app.widgets.popup import PersonPickerWidget
 from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
 from lp.blueprints.browser.specificationtarget import (
     HasSpecificationsMenuMixin,
@@ -881,6 +882,9 @@
         description=_("Select the differences for syncing."),
         required=True)
 
+    sponsored_person = Choice(
+        vocabulary='ValidPerson', required=False,)
+
 
 class DistroSeriesDifferenceBaseView(LaunchpadFormView,
                                      PackageCopyingMixin,
@@ -888,9 +892,12 @@
     """Base class for all pages presenting differences between
     a derived series and its parent."""
     schema = IDifferencesFormSchema
-    field_names = ['selected_differences']
+    field_names = ['selected_differences', 'sponsored_person']
     custom_widget('selected_differences', LabeledMultiCheckBoxWidget)
     custom_widget('package_type', LaunchpadRadioWidget)
+    custom_widget(
+        'sponsored_person', PersonPickerWidget,
+        header="Select person being sponsored", show_assign_me_button=False)
 
     # Differences type to display. Can be overrided by sublasses.
     differences_type = DistroSeriesDifferenceType.DIFFERENT_VERSIONS
@@ -909,7 +916,6 @@
         if not getFeatureFlag('soyuz.derived_series_ui.enabled'):
             self.request.response.redirect(canonical_url(self.context))
             return
-
         super(DistroSeriesDifferenceBaseView, self).initialize()
 
     def initialize_sync_label(self, label):
@@ -977,6 +983,8 @@
         else:
             destination_pocket = PackagePublishingPocket.RELEASE
 
+        sponsored = data.get("sponsored_person")
+
         # When syncing we *must* do it asynchronously so that a package
         # copy job is created.  This gives the job a chance to inspect
         # the copy and create a PackageUpload if required.
@@ -984,7 +992,7 @@
             'selected_differences', sources, self.context.main_archive,
             self.context, destination_pocket, include_binaries=False,
             dest_url=series_url, dest_display_name=series_title,
-            person=self.user, force_async=True):
+            person=self.user, force_async=True, sponsored=sponsored):
             # The copy worked so we redirect back to show the results. Include
             # the query string so that the user ends up on the same batch page
             # with the same filtering parameters as before.
@@ -1010,6 +1018,10 @@
             self.setFieldError(
                 'selected_differences', 'No differences selected.')
 
+        sponsored = data.get("sponsored_person")
+        if sponsored is None:
+            self.setFieldError("sponsored_person", "Invalid person")
+
     def canPerformSync(self, *args):
         """Return whether a sync can be performed.
 

=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py	2011-11-26 03:55:38 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py	2011-12-14 16:46:17 +0000
@@ -2115,14 +2115,18 @@
 
     def _syncAndGetView(self, derived_series, person, sync_differences,
                         difference_type=None, view_name='+localpackagediffs',
-                        query_string=''):
+                        query_string='', sponsored=None):
         # A helper to get the POST'ed sync view.
         with person_logged_in(person):
+            form = {
+                'field.selected_differences': sync_differences,
+                'field.actions.sync': 'Sync',
+                }
+            if sponsored is not None:
+                form['field.sponsored_person'] = sponsored.name
             view = create_initialized_view(
                 derived_series, view_name,
-                method='POST', form={
-                    'field.selected_differences': sync_differences,
-                    'field.actions.sync': 'Sync'},
+                method='POST', form=form,
                 query_string=query_string)
             return view
 
@@ -2206,6 +2210,26 @@
             "Signer is not permitted to upload to the "
             "component" in view.errors[0])
 
+    def test_sync_with_sponsoring(self):
+        # The requesting user can set a sponsored person on the sync. We
+        # need to make sure the sponsored person ends up on the copy job
+        # metadata.
+        derived_series, parent_series, sp_name, diff_id = self._setUpDSD(
+            'my-src-name')
+        set_derived_series_sync_feature_flag(self)
+        person, _ = self.makePersonWithComponentPermission(
+            derived_series.main_archive,
+            derived_series.getSourcePackage(
+                sp_name).latest_published_component)
+        sponsored_person = self.factory.makePerson()
+        view = self._syncAndGetView(
+            derived_series, person, [diff_id],
+            sponsored=sponsored_person)
+
+        pcj = PlainPackageCopyJob.getActiveJobs(
+            derived_series.main_archive).one()
+        self.assertEqual(pcj.sponsored, sponsored_person)
+
     def assertPackageCopied(self, series, src_name, version, view):
         # Helper to check that a package has been copied by virtue of
         # there being a package copy job ready to run.

=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt	2011-09-05 12:24:33 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt	2011-12-14 16:46:17 +0000
@@ -87,7 +87,7 @@
         </script>
         <div id="launchpad-form-actions" class="actions"
           tal:define="sync view/actions/byname/field.actions.sync|nothing;
-                      upgrade view/actions/byname/field.actions.upgrade|nothing">
+                      upgrade view/actions/byname/field.actions.upgrade|nothing;">
           <input class="button" type="submit" disabled="true"
               title="Please use a Javascript-enabled browser to sync packages."
               tal:condition="python:sync and sync.available()"
@@ -99,6 +99,13 @@
             tal:attributes="value upgrade/label;
                             name upgrade/__name__;
                             id upgrade/__name__;" />
+          <span
+            tal:define="sponsored_person nocall:view/widgets/sponsored_person;
+                        field_name sponsored_person/context/__name__;
+                        error python:view.getFieldError(field_name);">
+            <label>Person being sponsored (optional):</label>
+            <tal:sponsored replace="structure sponsored_person"/>
+          </span>
         </div>
       </div>
 

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2011-12-12 05:09:37 +0000
+++ lib/lp/soyuz/browser/archive.py	2011-12-14 16:46:17 +0000
@@ -1266,7 +1266,7 @@
 def copy_asynchronously(source_pubs, dest_archive, dest_series, dest_pocket,
                         include_binaries, dest_url=None,
                         dest_display_name=None, person=None,
-                        check_permissions=True):
+                        check_permissions=True, sponsored=None):
     """Schedule jobs to copy packages later.
 
     :return: A `structured` with human-readable feedback about the
@@ -1288,7 +1288,7 @@
             dest_pocket, include_binaries=include_binaries,
             package_version=spph.sourcepackagerelease.version,
             copy_policy=PackageCopyPolicy.INSECURE,
-            requester=person)
+            requester=person, sponsored=sponsored)
 
     return copy_asynchronously_message(len(source_pubs))
 
@@ -1352,7 +1352,7 @@
     def do_copy(self, sources_field_name, source_pubs, dest_archive,
                 dest_series, dest_pocket, include_binaries,
                 dest_url=None, dest_display_name=None, person=None,
-                check_permissions=True, force_async=False):
+                check_permissions=True, force_async=False, sponsored=None):
         """Copy packages and add appropriate feedback to the browser page.
 
         This may either copy synchronously, if there are few enough
@@ -1378,9 +1378,14 @@
             requester's permissions to copy should be checked.
         :param force_async: Force the copy to create package copy jobs and
             perform the copy asynchronously.
+        :param sponsored: An IPerson representing the person being sponsored
+            (for asynchronous copies only).
 
         :return: True if the copying worked, False otherwise.
         """
+        assert(
+            force_async or not sponsored,
+            "sponsored must be None for sync copies")
         try:
             if (force_async == False and
                     self.canCopySynchronously(source_pubs)):
@@ -1394,7 +1399,7 @@
                     source_pubs, dest_archive, dest_series, dest_pocket,
                     include_binaries, dest_url=dest_url,
                     dest_display_name=dest_display_name, person=person,
-                    check_permissions=check_permissions)
+                    check_permissions=check_permissions, sponsored=sponsored)
         except CannotCopy, error:
             self.setFieldError(
                 sources_field_name, render_cannotcopy_as_html(error))