launchpad-reviewers team mailing list archive

[Ines Almeida <mp+462338@xxxxxxxxxxxxxxxxxx>]

Ines Almeida has proposed merging ~ines-almeida/launchpad:fetch-service-option-refactor into launchpad:master.

Commit message:
Refactor snap.use_fetch_service tests and feature flag description

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/462338

This MP addressed the remaining comments from this other MP: https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461552
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:fetch-service-option-refactor into launchpad:master.

diff --git a/lib/lp/services/features/flags.py b/lib/lp/services/features/flags.py
index ae0bf52..3089b66 100644
--- a/lib/lp/services/features/flags.py
+++ b/lib/lp/services/features/flags.py
@@ -308,8 +308,8 @@ flag_info = sorted(
             "snap.fetch_service.enable",
             "boolean",
             "If set, allow admins to set snap.use_fetch_service field, which "
-            "sets a snap build to use the fetch-service instead of the "
-            "builder-proxy",
+            "sets up the builds of a snap to use the fetch-servic instead of "
+            "the builder-proxy",
             "",
             "",
             "",
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index 07fc8d2..d18d2d1 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -2358,11 +2358,8 @@ class TestSnapSet(TestCaseWithFactory):
         self.assertEqual(ref.path, snap.git_path)
         self.assertEqual(ref, snap.git_ref)
 
-    def test_auth_to_edit_admin_only_fields(self):
-        # The admin fields can only be updated by an admin
-        self.useFixture(
-            FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: "on"})
-        )
+    def test_non_admins_cannot_update_admin_only_fields(self):
+        # The admin fields can not be updated by a non-admin
 
         non_admin = self.factory.makePerson()
         [ref] = self.factory.makeGitRefs(owner=non_admin)
@@ -2382,6 +2379,25 @@ class TestSnapSet(TestCaseWithFactory):
                 self.assertRaises(
                     Unauthorized, setattr, snap, field_name, True
                 )
+
+    def test_admins_can_update_admin_only_fields(self):
+        # The admin fields can be updated by an admin
+        self.useFixture(
+            FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: "on"})
+        )
+
+        [ref] = self.factory.makeGitRefs()
+        components = self.makeSnapComponents(git_ref=ref)
+        snap = getUtility(ISnapSet).new(**components)
+
+        admin_fields = [
+            "allow_internet",
+            "pro_enable",
+            "require_virtualized",
+            "use_fetch_service",
+        ]
+
+        for field_name in admin_fields:
             # exception isn't raised when an admin does the same
             with admin_logged_in():
                 setattr(snap, field_name, True)