Merge ~cjwatson/launchpad:bugs-admin-registry into launchpad:master

Proposed by Colin Watson on 2019-10-10
Status: Merged
Approved by: Colin Watson on 2019-10-11
Approved revision: 08bf395fa29e13237fccc4a2c44738db445c874c
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:bugs-admin-registry
Merge into: launchpad:master
Diff against target: 191 lines (+28/-59)
5 files modified
lib/lp/bugs/security.py (+2/-3)
lib/lp/bugs/stories/bugtracker/xx-reschedule-all-watches.txt (+11/-21)
lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt (+8/-17)
lib/lp/bugs/tests/test_bugtracker.py (+3/-9)
lib/lp/bugs/tests/test_bugwatch.py (+4/-9)
Reviewer Review Type Date Requested Status
William Grant code 2019-10-10 Approve on 2019-10-11
Review via email: mp+373957@code.launchpad.net

Commit message

Grant bugtracker/bugwatch admin to registry, not LP developers

Launchpad developers don't intrinsically need data admin access to
things like bugtrackers and bugwatches. It makes more sense for this to
be associated with registry experts, like most of our other
semi-privileged data admin operations.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/bugs/security.py b/lib/lp/bugs/security.py
2index e683ecb..4e22be0 100644
3--- a/lib/lp/bugs/security.py
4+++ b/lib/lp/bugs/security.py
5@@ -290,7 +290,7 @@ class AdminBugTracker(AuthorizationBase):
6 return (
7 user.in_janitor or
8 user.in_admin or
9- user.in_launchpad_developers)
10+ user.in_registry_experts)
11
12
13 class AdminBugWatch(AuthorizationBase):
14@@ -298,8 +298,7 @@ class AdminBugWatch(AuthorizationBase):
15 usedfor = IBugWatch
16
17 def checkAuthenticated(self, user):
18- return (
19- user.in_admin or user.in_launchpad_developers)
20+ return user.in_admin or user.in_registry_experts
21
22
23 class EditStructuralSubscription(AuthorizationBase):
24diff --git a/lib/lp/bugs/stories/bugtracker/xx-reschedule-all-watches.txt b/lib/lp/bugs/stories/bugtracker/xx-reschedule-all-watches.txt
25index 9350107..9edc7b3 100644
26--- a/lib/lp/bugs/stories/bugtracker/xx-reschedule-all-watches.txt
27+++ b/lib/lp/bugs/stories/bugtracker/xx-reschedule-all-watches.txt
28@@ -33,38 +33,28 @@ However, the reschedule button will appear to administrators.
29 >>> admin_browser.getControl('Reschedule all watches')
30 <SubmitControl...>
31
32-It will also appear for non-admin members of the Launchpad Developers
33-team.
34+It will also appear for non-admin registry experts.
35
36- >>> from zope.component import getUtility
37- >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
38- >>> from lp.registry.interfaces.person import IPersonSet
39+ >>> from lp.testing import login_celebrity
40
41- >>> login(ADMIN_EMAIL)
42- >>> admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
43- >>> new_lp_developer = factory.makePerson()
44- >>> launchpad_developers = getUtility(
45- ... ILaunchpadCelebrities).launchpad_developers
46- >>> dev_added = launchpad_developers.addMember(
47- ... new_lp_developer, admin_user)
48-
49- >>> lp_dev_browser = setupBrowser(
50- ... auth='Basic %s:test' % new_lp_developer.preferredemail.email)
51+ >>> registry_expert = login_celebrity('registry_experts')
52+ >>> registry_browser = setupBrowser(
53+ ... auth='Basic %s:test' % registry_expert.preferredemail.email)
54 >>> logout()
55
56- >>> lp_dev_browser.open(bug_tracker_edit_url)
57- >>> reschedule_button = lp_dev_browser.getControl(
58+ >>> registry_browser.open(bug_tracker_edit_url)
59+ >>> reschedule_button = registry_browser.getControl(
60 ... 'Reschedule all watches')
61
62 Clicking the button will reschedule the watches for the bug tracker for
63 checking at some future date.
64
65 >>> reschedule_button.click()
66- >>> print(lp_dev_browser.url)
67+ >>> print(registry_browser.url)
68 http://bugs.launchpad.test/bugs/bugtrackers/our-bugtracker
69
70 >>> for message in find_tags_by_class(
71- ... lp_dev_browser.contents, 'informational message'):
72+ ... registry_browser.contents, 'informational message'):
73 ... print(extract_text(message))
74 All bug watches on Our BugTracker have been rescheduled.
75
76@@ -84,8 +74,8 @@ appear on the bugtracker page.
77 >>> bug_watch.destroySelf()
78 >>> logout()
79
80- >>> lp_dev_browser.open(bug_tracker_edit_url)
81- >>> reschedule_button = lp_dev_browser.getControl(
82+ >>> registry_browser.open(bug_tracker_edit_url)
83+ >>> reschedule_button = registry_browser.getControl(
84 ... 'Reschedule all watches')
85 Traceback (most recent call last):
86 ...
87diff --git a/lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt b/lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt
88index 2d02bca..c21f6ed 100644
89--- a/lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt
90+++ b/lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt
91@@ -287,32 +287,23 @@ The "Reset this watch" button will appear for administrators.
92 >>> admin_browser.getControl('Reset this watch')
93 <SubmitControl...>
94
95-It also appears for Launchpad Developers.
96+It also appears for registry experts.
97
98- >>> from zope.component import getUtility
99- >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
100- >>> from lp.registry.interfaces.person import IPersonSet
101+ >>> from lp.testing import login_celebrity
102
103- >>> login(ADMIN_EMAIL)
104- >>> admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
105- >>> new_lp_developer = factory.makePerson()
106- >>> launchpad_developers = getUtility(
107- ... ILaunchpadCelebrities).launchpad_developers
108- >>> dev_added = launchpad_developers.addMember(
109- ... new_lp_developer, admin_user)
110-
111- >>> lp_dev_browser = setupBrowser(
112- ... auth='Basic %s:test' % new_lp_developer.preferredemail.email)
113+ >>> registry_expert = login_celebrity('registry_experts')
114+ >>> registry_browser = setupBrowser(
115+ ... auth='Basic %s:test' % registry_expert.preferredemail.email)
116 >>> logout()
117
118- >>> lp_dev_browser.open(watch_url)
119- >>> reset_button = lp_dev_browser.getControl('Reset this watch')
120+ >>> registry_browser.open(watch_url)
121+ >>> reset_button = registry_browser.getControl('Reset this watch')
122
123 Clicking the button will reset the watch completely.
124
125 >>> reset_button.click()
126 >>> for message in find_tags_by_class(
127- ... lp_dev_browser.contents, 'informational message'):
128+ ... registry_browser.contents, 'informational message'):
129 ... print(extract_text(message))
130 The ... bug watch has been reset.
131
132diff --git a/lib/lp/bugs/tests/test_bugtracker.py b/lib/lp/bugs/tests/test_bugtracker.py
133index 2c4cbde..da07fe6 100644
134--- a/lib/lp/bugs/tests/test_bugtracker.py
135+++ b/lib/lp/bugs/tests/test_bugtracker.py
136@@ -256,15 +256,9 @@ class BugTrackerTestCase(TestCaseWithFactory):
137 self.bug_tracker.resetWatches()
138 self._assertBugWatchesAreCheckedInTheFuture()
139
140- def test_lp_dev_can_reset_watches(self):
141- # Launchpad developers can reset the watches on a bugtracker.
142- login(ADMIN_EMAIL)
143- admin = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
144- launchpad_developers = getUtility(
145- ILaunchpadCelebrities).launchpad_developers
146- lp_dev = self.factory.makePerson()
147- launchpad_developers.addMember(lp_dev, admin)
148- login_person(lp_dev)
149+ def test_registry_can_reset_watches(self):
150+ # Registry experts can reset the watches on a bugtracker.
151+ login_celebrity("registry_experts")
152 self.bug_tracker.resetWatches()
153 self._assertBugWatchesAreCheckedInTheFuture()
154
155diff --git a/lib/lp/bugs/tests/test_bugwatch.py b/lib/lp/bugs/tests/test_bugwatch.py
156index 30d9621..7dd061f 100644
157--- a/lib/lp/bugs/tests/test_bugwatch.py
158+++ b/lib/lp/bugs/tests/test_bugwatch.py
159@@ -24,7 +24,6 @@ from zope.component import getUtility
160 from zope.security.interfaces import Unauthorized
161 from zope.security.proxy import removeSecurityProxy
162
163-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
164 from lp.bugs.interfaces.bugtask import (
165 BugTaskImportance,
166 BugTaskStatus,
167@@ -52,6 +51,7 @@ from lp.services.webapp import urlsplit
168 from lp.testing import (
169 ANONYMOUS,
170 login,
171+ login_celebrity,
172 login_person,
173 TestCase,
174 TestCaseWithFactory,
175@@ -717,14 +717,9 @@ class TestBugWatchResetting(TestCaseWithFactory):
176 login_person(unprivileged_user)
177 self.assertRaises(Unauthorized, getattr, self.bug_watch, 'reset')
178
179- def test_lp_developer_can_reset_watches(self):
180- # A Launchpad developer can call the reset() method on a bug
181- # watch.
182- admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
183- lp_developers = getUtility(ILaunchpadCelebrities).launchpad_developers
184- lp_dev = self.factory.makePerson()
185- lp_developers.addMember(lp_dev, admin_user)
186- login_person(lp_dev)
187+ def test_registry_expert_can_reset_watches(self):
188+ # A registry expert can call the reset() method on a bug watch.
189+ login_celebrity("registry_experts")
190 self.bug_watch.reset()
191 self._assertBugWatchHasBeenChanged()
192

Subscribers

People subscribed via source and target branches