Merge lp:~wallyworld/launchpad/transitionToInformationType-regression-1024148 into lp:launchpad

Proposed by Ian Booth
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 15634
Proposed branch: lp:~wallyworld/launchpad/transitionToInformationType-regression-1024148
Merge into: lp:launchpad
Diff against target: 75 lines (+25/-11)
3 files modified
lib/lp/bugs/model/bug.py (+8/-7)
lib/lp/code/model/branch.py (+1/-1)
lib/lp/code/model/tests/test_branch.py (+16/-3)
To merge this branch: bzr merge lp:~wallyworld/launchpad/transitionToInformationType-regression-1024148
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+115056@code.launchpad.net

Commit message

Ensure transitionToInformationType() works for branches without subscribers.

Description of the change

== Implementation ==

Add a check so that in branch's transitionToInformationType(), getPeopleWithoutAccess() is only called with a non empty people parameter.
Also do the check for bugs, even though at the moment bugs will always have subscribers. If things change, the code will be robust.

== Tests ==

Add a new test to TestBranchSetPrivate:
- test_can_transition_with_no_subscribers

== Lint ==

Fix some existing lint in test_branch

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/model/bug.py
  lib/lp/code/model/branch.py
  lib/lp/code/model/tests/test_branch.py

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

Thank you.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/model/bug.py'
2--- lib/lp/bugs/model/bug.py 2012-07-10 04:07:25 +0000
3+++ lib/lp/bugs/model/bug.py 2012-07-16 03:02:18 +0000
4@@ -1815,14 +1815,15 @@
5 # information type value.
6 if information_type in PRIVATE_INFORMATION_TYPES:
7 # Grant the subscriber access if they can't see the bug.
8- service = getUtility(IService, 'sharing')
9 subscribers = self.getDirectSubscribers()
10- blind_subscribers = service.getPeopleWithoutAccess(
11- self, subscribers)
12- if len(blind_subscribers):
13- service.ensureAccessGrants(
14- blind_subscribers, who, bugs=[self],
15- ignore_permissions=True)
16+ if subscribers:
17+ service = getUtility(IService, 'sharing')
18+ blind_subscribers = service.getPeopleWithoutAccess(
19+ self, subscribers)
20+ if len(blind_subscribers):
21+ service.ensureAccessGrants(
22+ blind_subscribers, who, bugs=[self],
23+ ignore_permissions=True)
24
25 self.updateHeat()
26
27
28=== modified file 'lib/lp/code/model/branch.py'
29--- lib/lp/code/model/branch.py 2012-07-14 07:50:13 +0000
30+++ lib/lp/code/model/branch.py 2012-07-16 03:02:18 +0000
31@@ -264,7 +264,7 @@
32 raise BranchCannotChangeInformationType()
33 self.information_type = information_type
34 self._reconcileAccess()
35- if information_type in PRIVATE_INFORMATION_TYPES:
36+ if information_type in PRIVATE_INFORMATION_TYPES and self.subscribers:
37 # Grant the subscriber access if they can't see the branch.
38 service = getUtility(IService, 'sharing')
39 blind_subscribers = service.getPeopleWithoutAccess(
40
41=== modified file 'lib/lp/code/model/tests/test_branch.py'
42--- lib/lp/code/model/tests/test_branch.py 2012-07-14 07:50:13 +0000
43+++ lib/lp/code/model/tests/test_branch.py 2012-07-16 03:02:18 +0000
44@@ -2543,6 +2543,19 @@
45 InformationType.EMBARGOEDSECURITY,
46 get_policies_for_artifact(branch)[0].type)
47
48+ def test_can_transition_with_no_subscribers(self):
49+ # Ensure that a branch can transition to another private type when
50+ # there are no subscribers to the branch.
51+ owner = self.factory.makePerson()
52+ branch = self.factory.makeBranch(
53+ owner=owner, information_type=InformationType.USERDATA)
54+ with person_logged_in(owner):
55+ branch.unsubscribe(owner, owner)
56+ branch.transitionToInformationType(
57+ InformationType.EMBARGOEDSECURITY, owner, verify_policy=False)
58+ self.assertEqual(
59+ InformationType.EMBARGOEDSECURITY, branch.information_type)
60+
61
62 class TestBranchCommitsForDays(TestCaseWithFactory):
63 """Tests for `Branch.commitsForDays`."""
64@@ -2896,9 +2909,9 @@
65 else:
66 information_type = InformationType.PUBLIC
67 target_branch = factory.makeAnyBranch(information_type=information_type)
68- revision = factory.makeBranchRevision(revision_id=revision_id,
69- branch=target_branch,
70- sequence=revno)
71+ factory.makeBranchRevision(revision_id=revision_id,
72+ branch=target_branch,
73+ sequence=revno)
74 return factory.makeBranchMergeProposal(merged_revno=revno,
75 target_branch=target_branch)
76