launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:snap-create-on-project into launchpad:master with ~pappacena/launchpad:snap-pillar-edit as a prerequisite.

Commit message:
Adding +new-snap page for project

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399184
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:snap-create-on-project into launchpad:master.

diff --git a/lib/lp/snappy/browser/configure.zcml b/lib/lp/snappy/browser/configure.zcml
index b11f797..eafa7a1 100644
--- a/lib/lp/snappy/browser/configure.zcml
+++ b/lib/lp/snappy/browser/configure.zcml
@@ -88,6 +88,12 @@
             name="+new-snap"
             template="../templates/snap-new.pt" />
         <browser:page
+            for="lp.registry.interfaces.product.IProduct"
+            class="lp.snappy.browser.snap.SnapAddView"
+            permission="launchpad.AnyPerson"
+            name="+new-snap"
+            template="../templates/snap-new.pt" />
+        <browser:page
             for="lp.snappy.interfaces.snap.ISnap"
             class="lp.snappy.browser.snap.SnapRequestBuildsView"
             permission="launchpad.Edit"
diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
index d4ba8bd..3a6d0c8 100644
--- a/lib/lp/snappy/browser/snap.py
+++ b/lib/lp/snappy/browser/snap.py
@@ -58,10 +58,12 @@ from lp.app.widgets.itemswidgets import (
     )
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.code.browser.widgets.gitref import GitRefWidget
+from lp.code.interfaces.branch import IBranch
 from lp.code.interfaces.gitref import IGitRef
 from lp.registry.enums import VCSType
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.product import IProduct
 from lp.services.features import getFeatureFlag
 from lp.services.propertycache import cachedproperty
 from lp.services.scripts import log
@@ -138,6 +140,32 @@ class SnapNavigation(WebhookTargetNavigationMixin, Navigation):
             return self.context.getSubscription(person)
 
 
+class SnapFormMixin:
+    def validateVCSWidgets(self, cls, data):
+        """Validates if VCS sub-widgets."""
+        if self.widgets.get('vcs') is not None:
+            # Set widgets as required or optional depending on the vcs
+            # field.
+            super(cls, self).validate_widgets(data, ['vcs'])
+            vcs = data.get('vcs')
+            if vcs == VCSType.BZR:
+                self.widgets['branch'].context.required = True
+                self.widgets['git_ref'].context.required = False
+            elif vcs == VCSType.GIT:
+                self.widgets['branch'].context.required = False
+                self.widgets['git_ref'].context.required = True
+            else:
+                raise AssertionError("Unknown branch type %s" % vcs)
+
+    def setUpVCSWidgets(self):
+        widget = self.widgets.get('vcs')
+        if widget is not None:
+            current_value = widget._getFormValue()
+            self.vcs_bzr_radio, self.vcs_git_radio = [
+                render_radio_widget_part(widget, value, current_value)
+                for value in (VCSType.BZR, VCSType.GIT)]
+
+
 class SnapInformationTypeMixin:
     def getPossibleInformationTypes(self, snap, user):
         """Get the information types to display on the edit form.
@@ -157,7 +185,10 @@ class SnapInformationTypeMixin:
         information types will be calculated based on the project.
         """
         info_type = data.get('information_type')
-        project = data.get('project')
+        if IProduct.providedBy(self.context):
+            project = self.context
+        else:
+            project = data.get('project')
         if info_type is None and project is None:
             # Nothing to validate here. Move on.
             return
@@ -474,27 +505,16 @@ class SnapAuthorizeMixin:
 
 
 class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
-                  SnapInformationTypeMixin):
+                  SnapInformationTypeMixin, SnapFormMixin):
     """View for creating snap packages."""
 
     page_title = label = 'Create a new snap package'
 
     schema = ISnapEditSchema
-    field_names = [
-        'owner',
-        'name',
-        'project',
-        'information_type',
-        'store_distro_series',
-        'build_source_tarball',
-        'auto_build',
-        'auto_build_archive',
-        'auto_build_pocket',
-        'auto_build_channels',
-        'store_upload',
-        'store_name',
-        'store_channels',
-        ]
+
+    custom_widget_vcs = LaunchpadRadioWidget
+    custom_widget_git_ref = CustomWidgetFactory(
+        GitRefWidget, allow_external=True)
     custom_widget_store_distro_series = LaunchpadRadioWidget
     custom_widget_auto_build_archive = SnapArchiveWidget
     custom_widget_auto_build_pocket = LaunchpadDropdownWidget
@@ -505,6 +525,26 @@ class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
         "auto_build_pocket": "/+help-snappy/snap-build-pocket.html",
         }
 
+    @property
+    def field_names(self):
+        fields = ['owner', 'name']
+        if self.is_project_context:
+            fields += ['vcs', 'branch', 'git_ref']
+        else:
+            fields += ['project']
+        return fields + [
+            'information_type',
+            'store_distro_series',
+            'build_source_tarball',
+            'auto_build',
+            'auto_build_archive',
+            'auto_build_pocket',
+            'auto_build_channels',
+            'store_upload',
+            'store_name',
+            'store_channels',
+            ]
+
     def initialize(self):
         """See `LaunchpadView`."""
         super(SnapAddView, self).initialize()
@@ -516,6 +556,10 @@ class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
                 self.context.information_type in PRIVATE_INFORMATION_TYPES):
                 raise SnapPrivateFeatureDisabled
 
+    @property
+    def is_project_context(self):
+        return IProduct.providedBy(self.context)
+
     def setUpFields(self):
         """See `LaunchpadFormView`."""
         super(SnapAddView, self).setUpFields()
@@ -529,6 +573,15 @@ class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
         """See `LaunchpadFormView`."""
         super(SnapAddView, self).setUpWidgets()
         self.widgets['processors'].widget_class = 'processors'
+        if self.is_project_context:
+            # If we are on Project:+new-snap page, we know which information
+            # types the project supports. Let's filter our the ones that are
+            # not supported.
+            types = getUtility(ISnapSet).getPossibleSnapInformationTypes(
+                    self.context)
+            info_type_widget = self.widgets['information_type']
+            info_type_widget.vocabulary = InformationTypeVocabulary(types)
+            self.setUpVCSWidgets()
 
     @property
     def cancel_url(self):
@@ -537,7 +590,7 @@ class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
     @property
     def initial_values(self):
         store_name = None
-        if self.has_snappy_distro_series:
+        if self.has_snappy_distro_series and not self.is_project_context:
             # Try to extract Snap store name from snapcraft.yaml file.
             try:
                 snapcraft_data = getUtility(ISnapSet).getSnapcraftYaml(
@@ -582,6 +635,7 @@ class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
 
     def validate_widgets(self, data, names=None):
         """See `LaunchpadFormView`."""
+        self.validateVCSWidgets(SnapAddView, data)
         if self.widgets.get('auto_build') is not None:
             # Set widgets as required or optional depending on the
             # auto_build field.
@@ -601,9 +655,17 @@ class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
     @action('Create snap package', name='create')
     def create_action(self, action, data):
         if IGitRef.providedBy(self.context):
-            kwargs = {'git_ref': self.context}
+            kwargs = {'git_ref': self.context, 'project': data['project']}
+        elif IBranch.providedBy(self.context):
+            kwargs = {'branch': self.context, 'project': data['project']}
+        elif self.is_project_context:
+            if data['vcs'] == VCSType.GIT:
+                kwargs = {'git_ref': data['git_ref']}
+            else:
+                kwargs = {'branch': data['branch']}
+            kwargs['project'] = self.context
         else:
-            kwargs = {'branch': self.context}
+            raise NotImplementedError("Unknown context for snap creation.")
         if not data.get('auto_build', False):
             data['auto_build_archive'] = None
             data['auto_build_pocket'] = None
@@ -615,7 +677,6 @@ class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
             auto_build_pocket=data['auto_build_pocket'],
             auto_build_channels=data['auto_build_channels'],
             information_type=data['information_type'],
-            project=data['project'],
             processors=data['processors'],
             build_source_tarball=data['build_source_tarball'],
             store_upload=data['store_upload'],
@@ -641,7 +702,7 @@ class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
 
 
 class BaseSnapEditView(LaunchpadEditFormView, SnapAuthorizeMixin,
-                       SnapInformationTypeMixin):
+                       SnapInformationTypeMixin, SnapFormMixin):
 
     schema = ISnapEditSchema
 
@@ -652,12 +713,7 @@ class BaseSnapEditView(LaunchpadEditFormView, SnapAuthorizeMixin,
     def setUpWidgets(self, context=None):
         """See `LaunchpadFormView`."""
         super(BaseSnapEditView, self).setUpWidgets()
-        widget = self.widgets.get('vcs')
-        if widget is not None:
-            current_value = widget._getFormValue()
-            self.vcs_bzr_radio, self.vcs_git_radio = [
-                render_radio_widget_part(widget, value, current_value)
-                for value in (VCSType.BZR, VCSType.GIT)]
+        self.setUpVCSWidgets()
 
     @property
     def has_snappy_distro_series(self):
@@ -665,19 +721,7 @@ class BaseSnapEditView(LaunchpadEditFormView, SnapAuthorizeMixin,
 
     def validate_widgets(self, data, names=None):
         """See `LaunchpadFormView`."""
-        if self.widgets.get('vcs') is not None:
-            # Set widgets as required or optional depending on the vcs
-            # field.
-            super(BaseSnapEditView, self).validate_widgets(data, ['vcs'])
-            vcs = data.get('vcs')
-            if vcs == VCSType.BZR:
-                self.widgets['branch'].context.required = True
-                self.widgets['git_ref'].context.required = False
-            elif vcs == VCSType.GIT:
-                self.widgets['branch'].context.required = False
-                self.widgets['git_ref'].context.required = True
-            else:
-                raise AssertionError("Unknown branch type %s" % vcs)
+        self.validateVCSWidgets(BaseSnapEditView, data)
         if self.widgets.get('auto_build') is not None:
             # Set widgets as required or optional depending on the
             # auto_build field.
@@ -723,7 +767,6 @@ class BaseSnapEditView(LaunchpadEditFormView, SnapAuthorizeMixin,
                         'A public snap cannot have a private repository.')
         self.validateInformationType(data, snap=self.context)
 
-
     def _needStoreReauth(self, data):
         """Does this change require reauthorizing to the store?"""
         store_upload = data.get('store_upload', False)
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index be39333..694e595 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -339,6 +339,49 @@ class TestSnapAddView(BaseTestSnapView):
             "the store.\nEdit snap package",
             MatchesTagText(content, "store_upload"))
 
+    def test_create_new_snap_project(self):
+        self.useFixture(GitHostingFixture(blob=b""))
+        project = self.factory.makeProduct()
+        [git_ref] = self.factory.makeGitRefs()
+        source_display = git_ref.display_name
+        browser = self.getViewBrowser(
+            project, view_name="+new-snap", user=self.person)
+        browser.getControl(name="field.name").value = "snap-name"
+        browser.getControl(name="field.vcs").value = "GIT"
+        browser.getControl(name="field.git_ref.repository").value = (
+            git_ref.repository.shortened_path)
+        browser.getControl(name="field.git_ref.path").value = git_ref.path
+        browser.getControl("Create snap package").click()
+
+        content = find_main_content(browser.contents)
+        self.assertEqual("snap-name", extract_text(content.h1))
+        self.assertThat(
+            "Test Person", MatchesPickerText(content, "edit-owner"))
+        self.assertThat(
+            "Distribution series:\n%s\nEdit snap package" %
+            self.distroseries.fullseriesname,
+            MatchesTagText(content, "distro_series"))
+        self.assertThat(
+            "Source:\n%s\nEdit snap package" % source_display,
+            MatchesTagText(content, "source"))
+        self.assertThat(
+            "Build source tarball:\nNo\nEdit snap package",
+            MatchesTagText(content, "build_source_tarball"))
+        self.assertThat(
+            "Build schedule:\n(?)\nBuilt on request\nEdit snap package\n",
+            MatchesTagText(content, "auto_build"))
+        self.assertThat(
+            "Source archive for automatic builds:\n\nEdit snap package\n",
+            MatchesTagText(content, "auto_build_archive"))
+        self.assertThat(
+            "Pocket for automatic builds:\n\nEdit snap package",
+            MatchesTagText(content, "auto_build_pocket"))
+        self.assertIsNone(find_tag_by_id(content, "auto_build_channels"))
+        self.assertThat(
+            "Builds of this snap package are not automatically uploaded to "
+            "the store.\nEdit snap package",
+            MatchesTagText(content, "store_upload"))
+
     def test_create_new_snap_users_teams_as_owner_options(self):
         # Teams that the user is in are options for the snap package owner.
         self.useFixture(BranchHostingFixture(blob=b""))
diff --git a/lib/lp/snappy/templates/snap-new.pt b/lib/lp/snappy/templates/snap-new.pt
index 5334bee..146d2cb 100644
--- a/lib/lp/snappy/templates/snap-new.pt
+++ b/lib/lp/snappy/templates/snap-new.pt
@@ -30,12 +30,52 @@
         <tal:widget define="widget nocall:view/widgets/owner">
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>
-        <tal:widget define="widget nocall:view/widgets/project">
-          <metal:block use-macro="context/@@launchpad_form/widget_row" />
-        </tal:widget>
+
+        <tal:guard condition="not: view/is_project_context">
+          <tal:widget define="widget nocall:view/widgets/project">
+            <metal:block use-macro="context/@@launchpad_form/widget_row" />
+          </tal:widget>
+        </tal:guard>
         <tal:widget define="widget nocall:view/widgets/information_type">
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>
+
+        <tal:guard condition="view/is_project_context">
+          <tr>
+            <td>
+              <div>
+                <label for="field.vcs">Source:</label>
+                <table>
+                  <tr>
+                    <td>
+                      <label tal:replace="structure view/vcs_bzr_radio" />
+                      <table class="subordinate">
+                        <tal:widget define="widget nocall:view/widgets/branch">
+                          <metal:block
+                         use-macro="context/@@launchpad_form/widget_row" />
+                        </tal:widget>
+                      </table>
+                    </td>
+                  </tr>
+
+                  <tr>
+                    <td>
+                      <label tal:replace="structure view/vcs_git_radio" />
+                      <table class="subordinate">
+                        <tal:widget define="widget
+                        nocall:view/widgets/git_ref">
+                          <metal:block
+                         use-macro="context/@@launchpad_form/widget_row" />
+                        </tal:widget>
+                      </table>
+                    </td>
+                  </tr>
+                </table>
+              </div>
+            </td>
+          </tr>
+        </tal:guard>
+
         <tal:widget define="widget nocall:view/widgets/store_distro_series">
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>