launchpad-reviewers team mailing list archive

[Vaishnavi Asawale <mp+490259@xxxxxxxxxxxxxxxxxx>]

Vaishnavi Asawale has proposed merging ~vaishnavi-asawale/launchpad/+git/launchpad-1:add-fetch-service-policy-dropdown into launchpad:master.

Commit message:
Add choice of permissive or strict mode to the fetch service policy on snap Edit recipe

Enable a dropdown for Snaps to select between 'permissive' or 'strict' mode
for the fetch service if 'use_fetch_service' is selected. The default mode is
'strict'.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~vaishnavi-asawale/launchpad/+git/launchpad-1/+merge/490259
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~vaishnavi-asawale/launchpad/+git/launchpad-1:add-fetch-service-policy-dropdown into launchpad:master.

diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
index a65b679..d5ba5e9 100644
--- a/lib/lp/snappy/browser/snap.py
+++ b/lib/lp/snappy/browser/snap.py
@@ -45,6 +45,7 @@ from lp.app.widgets.itemswidgets import (
     LaunchpadRadioWidgetWithDescription,
 )
 from lp.app.widgets.snapbuildchannels import SnapBuildChannelsWidget
+from lp.buildmaster.builderproxy import FetchServicePolicy
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.code.browser.widgets.gitref import GitRefWidget
 from lp.code.interfaces.branch import IBranch
@@ -526,6 +527,7 @@ class ISnapEditSchema(Interface):
             "store_upload",
             "pro_enable",
             "use_fetch_service",
+            "fetch_service_policy",
         ],
     )
 
@@ -550,6 +552,9 @@ class ISnapEditSchema(Interface):
     store_channels = copy_field(ISnap["store_channels"], required=True)
 
     use_fetch_service = copy_field(ISnap["use_fetch_service"], required=True)
+    fetch_service_policy = copy_field(
+        ISnap["fetch_service_policy"], required=False
+    )
 
 
 def log_oops(error, request):
@@ -987,6 +992,7 @@ class SnapEditView(BaseSnapEditView, EnableProcessorsMixin):
         "store_name",
         "store_channels",
         "use_fetch_service",
+        "fetch_service_policy",
     ]
     custom_widget_store_distro_series = LaunchpadRadioWidget
     custom_widget_vcs = LaunchpadRadioWidget
@@ -1075,6 +1081,8 @@ class SnapEditView(BaseSnapEditView, EnableProcessorsMixin):
         if "project" in data:
             project = data.pop("project")
             self.context.setProject(project)
+            if not data.get("use_fetch_service"):
+                data["fetch_service_policy"] = FetchServicePolicy.STRICT
         super().updateContextFromData(data, context, notify_modified)
 
 
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index cd3f5af..a7cb83a 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -34,6 +34,7 @@ from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.widgets.tests.test_snapbuildchannels import (
     TestSnapBuildChannelsWidget,
 )
+from lp.buildmaster.builderproxy import FetchServicePolicy
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.code.errors import BranchHostingFault, GitRepositoryScanFault
@@ -1452,6 +1453,37 @@ class TestSnapEditView(BaseTestSnapView):
         with person_logged_in(self.person):
             self.assertTrue(snap.use_fetch_service)
 
+    def test_edit_fetch_service_policy(self):
+        series = self.factory.makeUbuntuDistroSeries()
+        with admin_logged_in():
+            snappy_series = self.factory.makeSnappySeries(
+                usable_distro_series=[series]
+            )
+        login_person(self.person)
+        snap = self.factory.makeSnap(
+            registrant=self.person,
+            owner=self.person,
+            distroseries=series,
+            branch=self.factory.makeAnyBranch(),
+            store_series=snappy_series,
+        )
+        browser = self.getViewBrowser(snap, user=self.person)
+        browser.getLink("Edit snap package").click()
+        browser.getControl("Use fetch service").selected = True
+        with person_logged_in(self.person):
+            self.assertEqual(
+                snap.fetch_service_policy, FetchServicePolicy.STRICT
+            )
+        browser.getControl("Fetch service policy").value = ["PERMISSIVE"]
+        browser.getControl("Update snap package").click()
+
+        browser = self.getViewBrowser(snap, user=self.person)
+        browser.getLink("Edit snap package").click()
+        with person_logged_in(self.person):
+            self.assertEqual(
+                snap.fetch_service_policy, FetchServicePolicy.PERMISSIVE
+            )
+
     def setUpSeries(self):
         """Set up {distro,snappy}series with some available processors."""
         distroseries = self.factory.makeUbuntuDistroSeries()
diff --git a/lib/lp/snappy/templates/snap-edit.pt b/lib/lp/snappy/templates/snap-edit.pt
index 5f911e5..bf0f36f 100644
--- a/lib/lp/snappy/templates/snap-edit.pt
+++ b/lib/lp/snappy/templates/snap-edit.pt
@@ -137,6 +137,12 @@
         <tal:widget define="widget nocall:view/widgets/use_fetch_service">
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>
+
+        <div id="fetch-service-policy-container">
+          <tal:widget define="widget nocall:view/widgets/fetch_service_policy">
+            <metal:block use-macro="context/@@launchpad_form/widget_row" />
+          </tal:widget>
+        </div>
       </table>
     </metal:formbody>
   </div>
@@ -148,6 +154,52 @@
       }, window);
     });
   </script>
+
+  <!-- Script to enable/disable the fetch_service_policy dropdown -->
+  <script type="text/javascript">
+    LPJS.use('node', function(Y) {
+      Y.on('domready', function () {
+        var checkbox = Y.one("input[name='field.use_fetch_service']");
+        var select = Y.one("select[name='field.fetch_service_policy']");
+        var container = Y.one("#fetch-service-policy-container");
+
+        if (!checkbox || !select || !container) return;
+
+        if (!checkbox.get('checked')) {
+          select.set('value', 'STRICT');
+        }
+
+        var initialSelectedValue = select.get('value');
+
+        function toggleFetchPolicy() {
+          var isChecked = checkbox.get('checked');
+
+          container.setStyle('display', 'block'); 
+
+          if (isChecked) {
+
+            // Remove the "(nothing selected)" option which appears
+            // because fetch_service_policy is set to 'required = False'
+            var options = select.all("option");
+            options.each(function(option) {
+              if (option.get('value') == "") {
+                option.remove();
+              }
+            });
+
+            select.set('disabled', false);
+            select.set('value', initialSelectedValue);
+          } else {
+            initialSelectedValue = select.get('value');
+            select.set('disabled', true);
+          }
+        }
+
+        toggleFetchPolicy(); // On page load
+        checkbox.on('change', toggleFetchPolicy);
+      });
+    });
+  </script>
 </div>
 
 </body>
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index b77b02e..affb4e1 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -1948,6 +1948,20 @@ class TestSnap(TestCaseWithFactory):
             snap.use_fetch_service = True
             self.assertTrue(snap.use_fetch_service)
 
+    def test_snap_fetch_service_policy(self):
+        person = self.factory.makePerson()
+        with person_logged_in(person):
+            snap = self.factory.makeSnap(registrant=person, owner=person)
+
+            snap.use_fetch_service = True
+            self.assertEqual(
+                snap.fetch_service_policy, FetchServicePolicy.STRICT
+            )
+            snap.fetch_service_policy = FetchServicePolicy.PERMISSIVE
+            self.assertEqual(
+                snap.fetch_service_policy, FetchServicePolicy.PERMISSIVE
+            )
+
 
 class TestSnapDeleteWithBuilds(TestCaseWithFactory):
     layer = LaunchpadFunctionalLayer