Merge ~pappacena/launchpad:snap-create-on-project into launchpad:master

[Thiago F. Pappacena]

Adding +new-snap page for project

The link on the project overview to this new page is proposed in another MP (https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399224) to keep this MP shorter for review.

[Colin Watson (cjwatson)]

Looks generally OK, but I'd like to know what we're planning to do about the store name extraction issue, and I'd also like to see a screenshot or two of the new UI.

review: Needs Information

[Thiago F. Pappacena (pappacena)]

Pushed the requested changes and replied about the store name issue.

Screenshots:
- Create snap from gitref: https://private-fileshare.canonical.com/~pappacena/screenshots/private-oci-recipe/new-snap-gitref.png

- Create snap from project: https://private-fileshare.canonical.com/~pappacena/screenshots/private-oci-recipe/new-snap-project.png

[Colin Watson (cjwatson)]

review: Approve

[Thiago F. Pappacena (pappacena)]

Pushed the requested changes.

[Otto Co-Pilot (otto-copilot)]

Merging failed
https://jenkins.ols.canonical.com/online-services/job/launchpad/1502/

diff --git a/lib/lp/snappy/browser/configure.zcml b/lib/lp/snappy/browser/configure.zcml
index ebbbb66..a21c0b4 100644
--- a/lib/lp/snappy/browser/configure.zcml
+++ b/lib/lp/snappy/browser/configure.zcml
@@ -87,6 +87,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 0d76973..701dc19 100644
--- a/lib/lp/snappy/browser/snap.py
+++ b/lib/lp/snappy/browser/snap.py
@@ -61,11 +61,13 @@ 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.personproduct import IPersonProductFactory
 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
@@ -164,6 +166,29 @@ class SnapNavigation(WebhookTargetNavigationMixin, Navigation):
             return self.context.getSubscription(person)
 
 
+class SnapFormMixin:
+    def validateVCSWidgets(self, cls, data):
+        """Validates if VCS sub-widgets."""
+        # Set widgets as required or optional depending on the vcs field.
+        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.
@@ -183,7 +208,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
@@ -497,28 +525,17 @@ class SnapAuthorizeMixin:
             log_oops(e, self.request)
 
 
-class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
-                  SnapInformationTypeMixin):
+class SnapAddView(SnapAuthorizeMixin, EnableProcessorsMixin,
+                  SnapInformationTypeMixin, SnapFormMixin, LaunchpadFormView):
     """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
@@ -529,6 +546,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()
@@ -540,6 +577,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()
@@ -553,6 +594,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 out 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):
@@ -561,7 +611,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(
@@ -606,6 +656,9 @@ class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
 
     def validate_widgets(self, data, names=None):
         """See `LaunchpadFormView`."""
+        if self.widgets.get('vcs') is not None:
+            super(SnapAddView, self).validate_widgets(data, ['vcs'])
+            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.
@@ -625,9 +678,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
@@ -639,7 +700,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'],
@@ -664,8 +724,8 @@ class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin,
         self.validateInformationType(data)
 
 
-class BaseSnapEditView(LaunchpadEditFormView, SnapAuthorizeMixin,
-                       SnapInformationTypeMixin):
+class BaseSnapEditView(SnapAuthorizeMixin, SnapInformationTypeMixin,
+                       SnapFormMixin, LaunchpadEditFormView):
 
     schema = ISnapEditSchema
 
@@ -675,13 +735,8 @@ class BaseSnapEditView(LaunchpadEditFormView, SnapAuthorizeMixin,
 
     def setUpWidgets(self, context=None):
         """See `LaunchpadFormView`."""
-        super(BaseSnapEditView, self).setUpWidgets(context=None)
-        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)]
+        super(BaseSnapEditView, self).setUpWidgets()
+        self.setUpVCSWidgets()
 
     @property
     def has_snappy_distro_series(self):
@@ -690,18 +745,8 @@ 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.
@@ -747,7 +792,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 894326b..9852eb0 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..480d51c 100644
--- a/lib/lp/snappy/templates/snap-new.pt
+++ b/lib/lp/snappy/templates/snap-new.pt
@@ -30,12 +30,49 @@
         <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>
+
+        <tr tal:condition="view/is_project_context">
+          <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:widget define="widget nocall:view/widgets/store_distro_series">
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>