Merge ~ines-almeida/launchpad:fetch-service-option-refactor into launchpad:master

Proposed by Ines Almeida
Status: Merged
Approved by: Ines Almeida
Approved revision: 844d8ff30d46dc1642a98938ae6bb0e698e83646
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~ines-almeida/launchpad:fetch-service-option-refactor
Merge into: launchpad:master
Diff against target: 59 lines (+23/-7)
2 files modified
lib/lp/services/features/flags.py (+2/-2)
lib/lp/snappy/tests/test_snap.py (+21/-5)
Reviewer Review Type Date Requested Status
Guruprasad Approve
Simone Pelosi Approve
Review via email: mp+462338@code.launchpad.net

Commit message

Refactor snap.use_fetch_service tests and feature flag description

Description of the change

This MP addressed the remaining comments from this other MP: https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461552

To post a comment you must log in.
Revision history for this message
Simone Pelosi (pelpsi) wrote :

LGTM!

review: Approve
Revision history for this message
Guruprasad (lgp171188) wrote :

LGTM 👍 Thanks!

review: Approve
Revision history for this message
Ines Almeida (ines-almeida) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/services/features/flags.py b/lib/lp/services/features/flags.py
2index ae0bf52..9bd290d 100644
3--- a/lib/lp/services/features/flags.py
4+++ b/lib/lp/services/features/flags.py
5@@ -308,8 +308,8 @@ flag_info = sorted(
6 "snap.fetch_service.enable",
7 "boolean",
8 "If set, allow admins to set snap.use_fetch_service field, which "
9- "sets a snap build to use the fetch-service instead of the "
10- "builder-proxy",
11+ "sets up the builds of a snap to use the fetch service instead of "
12+ "the builder-proxy",
13 "",
14 "",
15 "",
16diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
17index 07fc8d2..32a2d5c 100644
18--- a/lib/lp/snappy/tests/test_snap.py
19+++ b/lib/lp/snappy/tests/test_snap.py
20@@ -2358,11 +2358,8 @@ class TestSnapSet(TestCaseWithFactory):
21 self.assertEqual(ref.path, snap.git_path)
22 self.assertEqual(ref, snap.git_ref)
23
24- def test_auth_to_edit_admin_only_fields(self):
25- # The admin fields can only be updated by an admin
26- self.useFixture(
27- FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: "on"})
28- )
29+ def test_non_admins_cannot_update_admin_only_fields(self):
30+ # The admin fields cannot be updated by a non-admin user
31
32 non_admin = self.factory.makePerson()
33 [ref] = self.factory.makeGitRefs(owner=non_admin)
34@@ -2382,6 +2379,25 @@ class TestSnapSet(TestCaseWithFactory):
35 self.assertRaises(
36 Unauthorized, setattr, snap, field_name, True
37 )
38+
39+ def test_admins_can_update_admin_only_fields(self):
40+ # The admin fields can be updated by an admin
41+ self.useFixture(
42+ FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: "on"})
43+ )
44+
45+ [ref] = self.factory.makeGitRefs()
46+ components = self.makeSnapComponents(git_ref=ref)
47+ snap = getUtility(ISnapSet).new(**components)
48+
49+ admin_fields = [
50+ "allow_internet",
51+ "pro_enable",
52+ "require_virtualized",
53+ "use_fetch_service",
54+ ]
55+
56+ for field_name in admin_fields:
57 # exception isn't raised when an admin does the same
58 with admin_logged_in():
59 setattr(snap, field_name, True)

Subscribers

People subscribed via source and target branches

to status/vote changes: