launchpad-reviewers team mailing list archive

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

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:ocirecipe-edit-info-type-ui into launchpad:master with ~pappacena/launchpad:ocirecipe-privacy-banners-ui as a prerequisite.

Commit message:
Allowing selection of information types

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399993
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:ocirecipe-edit-info-type-ui into launchpad:master.

diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
index 4f0a3d9..13b1b27 100644
--- a/lib/lp/oci/browser/ocirecipe.py
+++ b/lib/lp/oci/browser/ocirecipe.py
@@ -56,7 +56,11 @@ from lp.app.browser.tales import (
     )
 from lp.app.errors import UnexpectedFormData
 from lp.app.validators.validation import validate_oci_branch_name
-from lp.app.widgets.itemswidgets import LabeledMultiCheckBoxWidget
+from lp.app.vocabularies import InformationTypeVocabulary
+from lp.app.widgets.itemswidgets import (
+    LabeledMultiCheckBoxWidget,
+    LaunchpadRadioWidgetWithDescription,
+    )
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.code.browser.widgets.gitref import GitRefWidget
 from lp.oci.interfaces.ocipushrule import (
@@ -744,6 +748,7 @@ class IOCIRecipeEditSchema(Interface):
     use_template(IOCIRecipe, include=[
         "name",
         "owner",
+        "information_type",
         "description",
         "git_ref",
         "build_file",
@@ -760,6 +765,26 @@ class OCIRecipeFormMixin:
     custom_widget_build_args = CustomWidgetFactory(
         TextAreaWidget, height=5, width=100)
 
+    custom_widget_information_type = CustomWidgetFactory(
+        LaunchpadRadioWidgetWithDescription,
+        vocabulary=InformationTypeVocabulary(types=[]))
+
+    def setUpInformationTypeWidget(self):
+        info_type_widget = self.widgets['information_type']
+        info_type_widget.vocabulary = InformationTypeVocabulary(
+            types=self.getInformationTypesToShow())
+
+    def getInformationTypesToShow(self):
+        """Get the information types to display on the edit form.
+
+        We display a customised set of information types: anything allowed
+        by the OCI recipe's model, plus the current type.
+        """
+        allowed_types = set(self.context.getAllowedInformationTypes(self.user))
+        if IOCIRecipe.providedBy(self.context):
+            allowed_types.add(self.context.information_type)
+        return allowed_types
+
     def createBuildArgsField(self):
         """Create a form field for OCIRecipe.build_args attribute."""
         if IOCIRecipe.providedBy(self.context):
@@ -833,6 +858,7 @@ class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin,
     field_names = (
         "name",
         "owner",
+        "information_type",
         "description",
         "git_ref",
         "build_file",
@@ -898,6 +924,7 @@ class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin,
     def setUpWidgets(self):
         """See `LaunchpadFormView`."""
         super(OCIRecipeAddView, self).setUpWidgets()
+        self.setUpInformationTypeWidget()
         self.widgets["processors"].widget_class = "processors"
         self.setUpGitRefWidget()
         # disable the official recipe button if the user doesn't have
@@ -1017,6 +1044,7 @@ class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin,
     field_names = (
         "owner",
         "name",
+        "information_type",
         "description",
         "git_ref",
         "build_file",
@@ -1056,6 +1084,7 @@ class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin,
     def setUpWidgets(self):
         """See `LaunchpadFormView`."""
         super(OCIRecipeEditView, self).setUpWidgets()
+        self.setUpInformationTypeWidget()
         self.setUpGitRefWidget()
         # disable the official recipe button if the user doesn't have
         # permissions to change it
diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index 88ec7da..7104610 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -57,6 +57,7 @@ from lp.oci.interfaces.ociregistrycredentials import (
     IOCIRegistryCredentialsSet,
     )
 from lp.oci.tests.helpers import OCIConfigHelperMixin
+from lp.registry.enums import BranchSharingPolicy
 from lp.registry.interfaces.person import IPersonSet
 from lp.services.config import config
 from lp.services.database.constants import UTC_NOW
@@ -231,6 +232,31 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
             "Official recipe:\nNo",
             MatchesTagText(content, "official-recipe"))
 
+    def test_create_new_available_information_types(self):
+        public_pillar = self.factory.makeProduct(owner=self.person)
+        private_pillar = self.factory.makeProduct(
+            owner=self.person,
+            information_type=InformationType.PROPRIETARY,
+            branch_sharing_policy=BranchSharingPolicy.PROPRIETARY)
+        public_oci_project = self.factory.makeOCIProject(
+            registrant=self.person, pillar=public_pillar)
+        private_oci_project = self.factory.makeOCIProject(
+            registrant=self.person, pillar=private_pillar)
+
+        # Public pillar.
+        browser = self.getViewBrowser(
+            public_oci_project, view_name="+new-recipe", user=self.person)
+        self.assertContentEqual(
+            ['PUBLIC', 'PUBLICSECURITY', 'PRIVATESECURITY', 'USERDATA'],
+            browser.getControl(name="field.information_type").options)
+
+        # Proprietary pillar.
+        browser = self.getViewBrowser(
+            private_oci_project, view_name="+new-recipe", user=self.person)
+        self.assertContentEqual(
+            ['PROPRIETARY'],
+            browser.getControl(name="field.information_type").options)
+
     def test_create_new_recipe_invalid_format(self):
         oci_project = self.factory.makeOCIProject()
         [git_ref] = self.factory.makeGitRefs(
@@ -600,12 +626,7 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
             for name in disabled])
         self.assertThat(processors_control.controls, MatchesSetwise(*matchers))
 
-    def test_edit_private_recipe_shows_banner(self):
-        recipe = self.factory.makeOCIRecipe(
-            registrant=self.person, owner=self.person,
-            information_type=InformationType.USERDATA)
-        browser = self.getViewBrowser(recipe, user=self.person)
-        browser.getLink("Edit OCI recipe").click()
+    def assertShowsPrivateBanner(self, browser):
         banners = find_tags_by_class(
             browser.contents, "private_banner_container")
         self.assertEqual(1, len(banners))
@@ -613,6 +634,14 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
             'The information on this page is private.',
             extract_text(banners[0]))
 
+    def test_edit_private_recipe_shows_banner(self):
+        recipe = self.factory.makeOCIRecipe(
+            registrant=self.person, owner=self.person,
+            information_type=InformationType.USERDATA)
+        browser = self.getViewBrowser(recipe, user=self.person)
+        browser.getLink("Edit OCI recipe").click()
+        self.assertShowsPrivateBanner(browser)
+
     def test_edit_recipe(self):
         oci_project = self.factory.makeOCIProject()
         oci_project_display = oci_project.display_name
@@ -677,6 +706,44 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
         browser.getControl("Update OCI recipe").click()
         self.assertIn("Branch does not match format", browser.contents)
 
+    def test_edit_can_make_recipe_private(self):
+        pillar = self.factory.makeProduct(
+            owner=self.person,
+            information_type=InformationType.PUBLIC,
+            branch_sharing_policy=BranchSharingPolicy.PUBLIC_OR_PROPRIETARY)
+        oci_project = self.factory.makeOCIProject(
+            registrant=self.person, pillar=pillar)
+        [git_ref] = self.factory.makeGitRefs(
+            owner=self.person,
+            paths=['/refs/heads/v2.0-20.04'])
+        recipe = self.factory.makeOCIRecipe(
+            registrant=self.person, owner=self.person,
+            oci_project=oci_project, git_ref=git_ref,
+            information_type=InformationType.PUBLIC)
+
+        browser = self.getViewBrowser(recipe, '+edit', user=self.person)
+
+        # Make sure we are showing all available information types:
+        info_type_field = browser.getControl(name="field.information_type")
+        self.assertContentEqual([
+            'PUBLIC', 'PUBLICSECURITY', 'PRIVATESECURITY', 'USERDATA',
+            'PROPRIETARY'],
+            info_type_field.options)
+
+        info_type_field.value = InformationType.PROPRIETARY.name
+        browser.getControl("Update OCI recipe").click()
+        self.assertShowsPrivateBanner(browser)
+
+    def test_edit_recipe_on_public_pillar_information_types(self):
+        recipe = self.factory.makeOCIRecipe(
+            registrant=self.person, owner=self.person)
+        browser = self.getViewBrowser(recipe, '+edit', user=self.person)
+
+        info_type_field = browser.getControl(name="field.information_type")
+        self.assertContentEqual(
+            ['PUBLIC', 'PUBLICSECURITY', 'PRIVATESECURITY', 'USERDATA'],
+            info_type_field.options)
+
     def test_edit_recipe_sets_date_last_modified(self):
         # Editing an OCI recipe sets the date_last_modified property.
         date_created = datetime(2000, 1, 1, tzinfo=pytz.UTC)
diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
index c72394e..c8fb968 100644
--- a/lib/lp/oci/interfaces/ocirecipe.py
+++ b/lib/lp/oci/interfaces/ocirecipe.py
@@ -341,6 +341,12 @@ class IOCIRecipeView(Interface):
         """Get an OCIRecipeBuildRequest object for the given job_id.
         """
 
+    def getAllowedInformationTypes(user):
+        """Get a list of acceptable `InformationType`s for this OCI recipe.
+
+        If the user is a Launchpad admin, any type is acceptable.
+        """
+
     def userCanBeSubscribed(user):
         """Checks if a user can be subscribed to the current OCI recipe."""
 
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index 6128452..3b7fff2 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -285,6 +285,10 @@ class OCIRecipe(Storm, WebhookTargetMixin):
         reconcile_access_for_artifacts([self], self.information_type,
                                        [self.pillar])
 
+    def getAllowedInformationTypes(self, user):
+        """See `IOCIRecipe`."""
+        return self.oci_project.getAllowedInformationTypes(user)
+
     def visibleByUser(self, user):
         """See `IOCIRecipe`."""
         if self.information_type in PUBLIC_INFORMATION_TYPES:
diff --git a/lib/lp/oci/templates/ocirecipe-new.pt b/lib/lp/oci/templates/ocirecipe-new.pt
index 668c33d..fe1c899 100644
--- a/lib/lp/oci/templates/ocirecipe-new.pt
+++ b/lib/lp/oci/templates/ocirecipe-new.pt
@@ -20,6 +20,9 @@
         <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/information_type">
+          <metal:block use-macro="context/@@launchpad_form/widget_row" />
+        </tal:widget>
         <tal:widget define="widget nocall:view/widgets/description">
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>
diff --git a/lib/lp/registry/interfaces/ociproject.py b/lib/lp/registry/interfaces/ociproject.py
index 00a6563..bb0a785 100644
--- a/lib/lp/registry/interfaces/ociproject.py
+++ b/lib/lp/registry/interfaces/ociproject.py
@@ -101,6 +101,13 @@ class IOCIProjectView(IHasGitRepositories, Interface):
     def getUnofficialRecipes(visible_by_user=None):
         """Gets the unofficial recipes for this OCI project."""
 
+    def getAllowedInformationTypes(user):
+        """Get a list of acceptable `InformationType`s for for OCI recipes
+        of this OCI project.
+
+        If the user is a Launchpad admin, any type is acceptable.
+        """
+
     def getDefaultGitRepository(person):
         """Returns the default git repository for the given user under the
         namespace of this OCI project"""
diff --git a/lib/lp/registry/model/ociproject.py b/lib/lp/registry/model/ociproject.py
index 079df1e..e98b3ea 100644
--- a/lib/lp/registry/model/ociproject.py
+++ b/lib/lp/registry/model/ociproject.py
@@ -32,8 +32,17 @@ from zope.component import getUtility
 from zope.interface import implementer
 from zope.security.proxy import removeSecurityProxy
 
+from lp.app.enums import (
+    PRIVATE_INFORMATION_TYPES,
+    PUBLIC_INFORMATION_TYPES,
+    )
+from lp.app.interfaces.services import IService
 from lp.bugs.model.bugtarget import BugTargetBase
 from lp.code.interfaces.gitnamespace import IGitNamespaceSet
+from lp.code.model.branchnamespace import (
+    BRANCH_POLICY_ALLOWED_TYPES,
+    BRANCH_POLICY_REQUIRED_GRANTS,
+    )
 from lp.oci.interfaces.ocirecipe import IOCIRecipeSet
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.ociproject import (
@@ -43,6 +52,7 @@ from lp.registry.interfaces.ociproject import (
 from lp.registry.interfaces.ociprojectname import IOCIProjectNameSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.product import IProduct
+from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.model.accesspolicy import reconcile_access_for_artifacts
 from lp.registry.model.ociprojectname import OCIProjectName
@@ -71,6 +81,17 @@ def oci_project_modified(oci_project, event):
     removeSecurityProxy(oci_project).date_last_modified = UTC_NOW
 
 
+def user_has_special_oci_access(user):
+    """Admins have special access.
+
+    :param user: An `IPerson` or None.
+    """
+    if user is None:
+        return False
+    roles = IPersonRoles(user)
+    return roles.in_admin
+
+
 @implementer(IOCIProject)
 class OCIProject(BugTargetBase, StormBase):
     """See `IOCIProject` and `IOCIProjectSet`."""
@@ -252,6 +273,22 @@ class OCIProject(BugTargetBase, StormBase):
         recipe = removeSecurityProxy(recipe)
         recipe._official = status
 
+    def getAllowedInformationTypes(self, user):
+        """See `IOCIRecipe`."""
+        if user_has_special_oci_access(user):
+            # Admins can set any type.
+            return set(PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES)
+        required_grant = BRANCH_POLICY_REQUIRED_GRANTS[
+            self.pillar.branch_sharing_policy]
+        if (required_grant is not None
+                and not getUtility(IService, 'sharing').checkPillarAccess(
+                    [self.pillar], required_grant, self.registrant)
+                and (user is None
+                     or not getUtility(IService, 'sharing').checkPillarAccess(
+                            [self.pillar], required_grant, user))):
+            return []
+        return BRANCH_POLICY_ALLOWED_TYPES[self.pillar.branch_sharing_policy]
+
     def getDefaultGitRepository(self, person):
         namespace = getUtility(IGitNamespaceSet).get(person, oci_project=self)
         return namespace.getByName(self.name)