Merge lp:~cjwatson/launchpad/git-mp-basic-browser into lp:launchpad
- git-mp-basic-browser
- Merge into devel
Proposed by
Colin Watson
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 17463 | ||||
Proposed branch: | lp:~cjwatson/launchpad/git-mp-basic-browser | ||||
Merge into: | lp:launchpad | ||||
Prerequisite: | lp:~cjwatson/launchpad/storm-vocabulary-base | ||||
Diff against target: |
1026 lines (+562/-85) 13 files modified
lib/lp/code/browser/branchmergeproposal.py (+95/-27) lib/lp/code/browser/configure.zcml (+3/-0) lib/lp/code/browser/gitref.py (+61/-0) lib/lp/code/browser/tests/test_branchmergeproposal.py (+208/-54) lib/lp/code/errors.py (+9/-2) lib/lp/code/interfaces/gitrepository.py (+34/-0) lib/lp/code/model/gitref.py (+3/-0) lib/lp/code/model/gitrepository.py (+22/-0) lib/lp/code/model/tests/test_gitrepository.py (+74/-0) lib/lp/code/templates/branchmergeproposal-resubmit.pt (+1/-1) lib/lp/code/templates/gitref-index.pt (+7/-0) lib/lp/code/templates/gitref-pending-merges.pt (+43/-0) lib/lp/testing/factory.py (+2/-1) |
||||
To merge this branch: | bzr merge lp:~cjwatson/launchpad/git-mp-basic-browser | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant (community) | code | Approve | |
Review via email: mp+257674@code.launchpad.net |
This proposal supersedes a proposal from 2015-04-28.
Commit message
Add Git support to most of the merge proposal views.
Description of the change
Add Git support to most of the merge proposal views.
GitRef:
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/lp/code/browser/branchmergeproposal.py' | |||
2 | --- lib/lp/code/browser/branchmergeproposal.py 2015-04-22 16:41:41 +0000 | |||
3 | +++ lib/lp/code/browser/branchmergeproposal.py 2015-04-29 12:39:54 +0000 | |||
4 | @@ -86,6 +86,7 @@ | |||
5 | 86 | InvalidBranchMergeProposal, | 86 | InvalidBranchMergeProposal, |
6 | 87 | WrongBranchMergeProposal, | 87 | WrongBranchMergeProposal, |
7 | 88 | ) | 88 | ) |
8 | 89 | from lp.code.interfaces.branch import IBranch | ||
9 | 89 | from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal | 90 | from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal |
10 | 90 | from lp.code.interfaces.codereviewcomment import ICodeReviewComment | 91 | from lp.code.interfaces.codereviewcomment import ICodeReviewComment |
11 | 91 | from lp.code.interfaces.codereviewinlinecomment import ( | 92 | from lp.code.interfaces.codereviewinlinecomment import ( |
12 | @@ -355,7 +356,7 @@ | |||
13 | 355 | @property | 356 | @property |
14 | 356 | def codebrowse_url(self): | 357 | def codebrowse_url(self): |
15 | 357 | """Return the link to codebrowse for this branch.""" | 358 | """Return the link to codebrowse for this branch.""" |
17 | 358 | return self.context.source_branch.getCodebrowseUrl() | 359 | return self.context.merge_source.getCodebrowseUrl() |
18 | 359 | 360 | ||
19 | 360 | 361 | ||
20 | 361 | class BranchMergeProposalRevisionIdMixin: | 362 | class BranchMergeProposalRevisionIdMixin: |
21 | @@ -373,6 +374,9 @@ | |||
22 | 373 | return None | 374 | return None |
23 | 374 | # If the source branch is REMOTE, then there won't be any ids. | 375 | # If the source branch is REMOTE, then there won't be any ids. |
24 | 375 | source_branch = self.context.source_branch | 376 | source_branch = self.context.source_branch |
25 | 377 | if source_branch is None: | ||
26 | 378 | # Git doesn't have revision numbers. Just use the ids. | ||
27 | 379 | return revision_id | ||
28 | 376 | if source_branch.branch_type == BranchType.REMOTE: | 380 | if source_branch.branch_type == BranchType.REMOTE: |
29 | 377 | return revision_id | 381 | return revision_id |
30 | 378 | else: | 382 | else: |
31 | @@ -586,12 +590,18 @@ | |||
32 | 586 | def initialize(self): | 590 | def initialize(self): |
33 | 587 | super(BranchMergeProposalView, self).initialize() | 591 | super(BranchMergeProposalView, self).initialize() |
34 | 588 | cache = IJSONRequestCache(self.request) | 592 | cache = IJSONRequestCache(self.request) |
41 | 589 | cache.objects.update({ | 593 | if IBranch.providedBy(self.context.merge_source): |
42 | 590 | 'branch_diff_link': | 594 | cache.objects.update({ |
43 | 591 | 'https://%s/+loggerhead/%s/diff/' % ( | 595 | 'branch_diff_link': |
44 | 592 | config.launchpad.code_domain, | 596 | 'https://%s/+loggerhead/%s/diff/' % ( |
45 | 593 | self.context.source_branch.unique_name), | 597 | config.launchpad.code_domain, |
46 | 594 | }) | 598 | self.context.source_branch.unique_name), |
47 | 599 | }) | ||
48 | 600 | else: | ||
49 | 601 | # XXX cjwatson 2015-04-29: Unimplemented for Git; this would | ||
50 | 602 | # require something in the webapp which proxies diff requests to | ||
51 | 603 | # the code browser, or an integrated code browser. | ||
52 | 604 | pass | ||
53 | 595 | if getFeatureFlag("longpoll.merge_proposals.enabled"): | 605 | if getFeatureFlag("longpoll.merge_proposals.enabled"): |
54 | 596 | cache.objects['merge_proposal_event_key'] = subscribe( | 606 | cache.objects['merge_proposal_event_key'] = subscribe( |
55 | 597 | self.context).event_key | 607 | self.context).event_key |
56 | @@ -618,7 +628,9 @@ | |||
57 | 618 | # Sort the comments by date order. | 628 | # Sort the comments by date order. |
58 | 619 | merge_proposal = self.context | 629 | merge_proposal = self.context |
59 | 620 | groups = list(merge_proposal.getRevisionsSinceReviewStart()) | 630 | groups = list(merge_proposal.getRevisionsSinceReviewStart()) |
61 | 621 | source = DecoratedBranch(merge_proposal.source_branch) | 631 | source = merge_proposal.merge_source |
62 | 632 | if IBranch.providedBy(source): | ||
63 | 633 | source = DecoratedBranch(source) | ||
64 | 622 | comments = [] | 634 | comments = [] |
65 | 623 | if getFeatureFlag('code.incremental_diffs.enabled'): | 635 | if getFeatureFlag('code.incremental_diffs.enabled'): |
66 | 624 | ranges = [ | 636 | ranges = [ |
67 | @@ -685,6 +697,8 @@ | |||
68 | 685 | 697 | ||
69 | 686 | @property | 698 | @property |
70 | 687 | def pending_diff(self): | 699 | def pending_diff(self): |
71 | 700 | if self.context.source_branch is None: | ||
72 | 701 | return False | ||
73 | 688 | return ( | 702 | return ( |
74 | 689 | self.context.next_preview_diff_job is not None or | 703 | self.context.next_preview_diff_job is not None or |
75 | 690 | self.context.source_branch.pending_writes) | 704 | self.context.source_branch.pending_writes) |
76 | @@ -705,6 +719,10 @@ | |||
77 | 705 | 719 | ||
78 | 706 | @property | 720 | @property |
79 | 707 | def spec_links(self): | 721 | def spec_links(self): |
80 | 722 | if self.context.source_branch is None: | ||
81 | 723 | # XXX cjwatson 2015-04-16: Implement once Git refs have linked | ||
82 | 724 | # blueprints. | ||
83 | 725 | return [] | ||
84 | 708 | return list( | 726 | return list( |
85 | 709 | self.context.source_branch.getSpecificationLinks(self.user)) | 727 | self.context.source_branch.getSpecificationLinks(self.user)) |
86 | 710 | 728 | ||
87 | @@ -748,12 +766,16 @@ | |||
88 | 748 | @property | 766 | @property |
89 | 749 | def status_config(self): | 767 | def status_config(self): |
90 | 750 | """The config to configure the ChoiceSource JS widget.""" | 768 | """The config to configure the ChoiceSource JS widget.""" |
91 | 769 | if IBranch.providedBy(self.context.merge_source): | ||
92 | 770 | source_revid = self.context.merge_source.last_scanned_id | ||
93 | 771 | else: | ||
94 | 772 | source_revid = self.context.merge_source.commit_sha1 | ||
95 | 751 | return simplejson.dumps({ | 773 | return simplejson.dumps({ |
96 | 752 | 'status_widget_items': vocabulary_to_choice_edit_items( | 774 | 'status_widget_items': vocabulary_to_choice_edit_items( |
97 | 753 | self._createStatusVocabulary(), | 775 | self._createStatusVocabulary(), |
98 | 754 | css_class_prefix='mergestatus'), | 776 | css_class_prefix='mergestatus'), |
99 | 755 | 'status_value': self.context.queue_status.title, | 777 | 'status_value': self.context.queue_status.title, |
101 | 756 | 'source_revid': self.context.source_branch.last_scanned_id, | 778 | 'source_revid': source_revid, |
102 | 757 | 'user_can_edit_status': check_permission( | 779 | 'user_can_edit_status': check_permission( |
103 | 758 | 'launchpad.Edit', self.context), | 780 | 'launchpad.Edit', self.context), |
104 | 759 | }) | 781 | }) |
105 | @@ -965,19 +987,35 @@ | |||
106 | 965 | schema = ResubmitSchema | 987 | schema = ResubmitSchema |
107 | 966 | for_input = True | 988 | for_input = True |
108 | 967 | page_title = label = "Resubmit proposal to merge" | 989 | page_title = label = "Resubmit proposal to merge" |
109 | 968 | field_names = [ | ||
110 | 969 | 'source_branch', | ||
111 | 970 | 'target_branch', | ||
112 | 971 | 'prerequisite_branch', | ||
113 | 972 | 'description', | ||
114 | 973 | 'break_link', | ||
115 | 974 | ] | ||
116 | 975 | 990 | ||
117 | 976 | def initialize(self): | 991 | def initialize(self): |
118 | 977 | self.cancel_url = canonical_url(self.context) | 992 | self.cancel_url = canonical_url(self.context) |
119 | 978 | super(BranchMergeProposalResubmitView, self).initialize() | 993 | super(BranchMergeProposalResubmitView, self).initialize() |
120 | 979 | 994 | ||
121 | 980 | @property | 995 | @property |
122 | 996 | def field_names(self): | ||
123 | 997 | if IBranch.providedBy(self.context.merge_source): | ||
124 | 998 | field_names = [ | ||
125 | 999 | 'source_branch', | ||
126 | 1000 | 'target_branch', | ||
127 | 1001 | 'prerequisite_branch', | ||
128 | 1002 | ] | ||
129 | 1003 | else: | ||
130 | 1004 | field_names = [ | ||
131 | 1005 | 'source_git_repository', | ||
132 | 1006 | 'source_git_path', | ||
133 | 1007 | 'target_git_repository', | ||
134 | 1008 | 'target_git_path', | ||
135 | 1009 | 'prerequisite_git_repository', | ||
136 | 1010 | 'prerequisite_git_path', | ||
137 | 1011 | ] | ||
138 | 1012 | field_names.extend([ | ||
139 | 1013 | 'description', | ||
140 | 1014 | 'break_link', | ||
141 | 1015 | ]) | ||
142 | 1016 | return field_names | ||
143 | 1017 | |||
144 | 1018 | @property | ||
145 | 981 | def initial_values(self): | 1019 | def initial_values(self): |
146 | 982 | UNSET = object() | 1020 | UNSET = object() |
147 | 983 | items = ((key, getattr(self.context, key, UNSET)) for key in | 1021 | items = ((key, getattr(self.context, key, UNSET)) for key in |
148 | @@ -988,10 +1026,24 @@ | |||
149 | 988 | def resubmit_action(self, action, data): | 1026 | def resubmit_action(self, action, data): |
150 | 989 | """Resubmit this proposal.""" | 1027 | """Resubmit this proposal.""" |
151 | 990 | try: | 1028 | try: |
152 | 1029 | if IBranch.providedBy(self.context.merge_source): | ||
153 | 1030 | merge_source = data['source_branch'] | ||
154 | 1031 | merge_target = data['target_branch'] | ||
155 | 1032 | merge_prerequisite = data['prerequisite_branch'] | ||
156 | 1033 | else: | ||
157 | 1034 | merge_source = data['source_git_repository'].getRefByPath( | ||
158 | 1035 | data['source_git_path']) | ||
159 | 1036 | merge_target = data['target_git_repository'].getRefByPath( | ||
160 | 1037 | data['target_git_path']) | ||
161 | 1038 | if data['prerequisite_git_repository']: | ||
162 | 1039 | merge_prerequisite = ( | ||
163 | 1040 | data['prerequisite_git_repository'].getRefByPath( | ||
164 | 1041 | data['prerequisite_git_path'])) | ||
165 | 1042 | else: | ||
166 | 1043 | merge_prerequisite = None | ||
167 | 991 | proposal = self.context.resubmit( | 1044 | proposal = self.context.resubmit( |
171 | 992 | self.user, data['source_branch'], data['target_branch'], | 1045 | self.user, merge_source, merge_target, merge_prerequisite, |
172 | 993 | data['prerequisite_branch'], data['description'], | 1046 | data['description'], data['break_link']) |
170 | 994 | data['break_link']) | ||
173 | 995 | except BranchMergeProposalExists as e: | 1047 | except BranchMergeProposalExists as e: |
174 | 996 | message = structured( | 1048 | message = structured( |
175 | 997 | 'Cannot resubmit because <a href="%(url)s">a similar merge' | 1049 | 'Cannot resubmit because <a href="%(url)s">a similar merge' |
176 | @@ -1072,18 +1124,31 @@ | |||
177 | 1072 | """The view to mark a merge proposal as merged.""" | 1124 | """The view to mark a merge proposal as merged.""" |
178 | 1073 | schema = IBranchMergeProposal | 1125 | schema = IBranchMergeProposal |
179 | 1074 | page_title = label = "Edit branch merge proposal" | 1126 | page_title = label = "Edit branch merge proposal" |
180 | 1075 | field_names = ["merged_revno"] | ||
181 | 1076 | for_input = True | 1127 | for_input = True |
182 | 1077 | 1128 | ||
183 | 1078 | @property | 1129 | @property |
184 | 1130 | def field_names(self): | ||
185 | 1131 | if IBranch.providedBy(self.context.merge_target): | ||
186 | 1132 | return ["merged_revno"] | ||
187 | 1133 | else: | ||
188 | 1134 | return ["merged_revision_id"] | ||
189 | 1135 | |||
190 | 1136 | @property | ||
191 | 1079 | def initial_values(self): | 1137 | def initial_values(self): |
192 | 1080 | # Default to the tip of the target branch, on the assumption that the | 1138 | # Default to the tip of the target branch, on the assumption that the |
193 | 1081 | # source branch has just been merged into it. | 1139 | # source branch has just been merged into it. |
196 | 1082 | if self.context.merged_revno is not None: | 1140 | if IBranch.providedBy(self.context.merge_target): |
197 | 1083 | revno = self.context.merged_revno | 1141 | if self.context.merged_revno is not None: |
198 | 1142 | revno = self.context.merged_revno | ||
199 | 1143 | else: | ||
200 | 1144 | revno = self.context.merge_target.revision_count | ||
201 | 1145 | return {'merged_revno': revno} | ||
202 | 1084 | else: | 1146 | else: |
205 | 1085 | revno = self.context.target_branch.revision_count | 1147 | if self.context.merged_revision_id is not None: |
206 | 1086 | return {'merged_revno': revno} | 1148 | revision_id = self.context.merged_revision_id |
207 | 1149 | else: | ||
208 | 1150 | revision_id = self.context.merge_target.commit_sha1 | ||
209 | 1151 | return {'merged_revision_id': revision_id} | ||
210 | 1087 | 1152 | ||
211 | 1088 | @property | 1153 | @property |
212 | 1089 | def next_url(self): | 1154 | def next_url(self): |
213 | @@ -1095,13 +1160,16 @@ | |||
214 | 1095 | @notify | 1160 | @notify |
215 | 1096 | def mark_merged_action(self, action, data): | 1161 | def mark_merged_action(self, action, data): |
216 | 1097 | """Update the whiteboard and go back to the source branch.""" | 1162 | """Update the whiteboard and go back to the source branch.""" |
218 | 1098 | revno = data['merged_revno'] | 1163 | if IBranch.providedBy(self.context.merge_target): |
219 | 1164 | kwargs = {'merged_revno': data['merged_revno']} | ||
220 | 1165 | else: | ||
221 | 1166 | kwargs = {'merged_revision_id': data['merged_revision_id']} | ||
222 | 1099 | if self.context.queue_status == BranchMergeProposalStatus.MERGED: | 1167 | if self.context.queue_status == BranchMergeProposalStatus.MERGED: |
224 | 1100 | self.context.markAsMerged(merged_revno=revno) | 1168 | self.context.markAsMerged(**kwargs) |
225 | 1101 | self.request.response.addNotification( | 1169 | self.request.response.addNotification( |
226 | 1102 | 'The proposal\'s merged revision has been updated.') | 1170 | 'The proposal\'s merged revision has been updated.') |
227 | 1103 | else: | 1171 | else: |
229 | 1104 | self.context.markAsMerged(revno, merge_reporter=self.user) | 1172 | self.context.markAsMerged(merge_reporter=self.user, **kwargs) |
230 | 1105 | self.request.response.addNotification( | 1173 | self.request.response.addNotification( |
231 | 1106 | 'The proposal has now been marked as merged.') | 1174 | 'The proposal has now been marked as merged.') |
232 | 1107 | 1175 | ||
233 | 1108 | 1176 | ||
234 | === modified file 'lib/lp/code/browser/configure.zcml' | |||
235 | --- lib/lp/code/browser/configure.zcml 2015-04-22 16:11:40 +0000 | |||
236 | +++ lib/lp/code/browser/configure.zcml 2015-04-29 12:39:54 +0000 | |||
237 | @@ -824,6 +824,9 @@ | |||
238 | 824 | <browser:page | 824 | <browser:page |
239 | 825 | name="++ref-commits" | 825 | name="++ref-commits" |
240 | 826 | template="../templates/gitref-commits.pt"/> | 826 | template="../templates/gitref-commits.pt"/> |
241 | 827 | <browser:page | ||
242 | 828 | name="++ref-pending-merges" | ||
243 | 829 | template="../templates/gitref-pending-merges.pt"/> | ||
244 | 827 | </browser:pages> | 830 | </browser:pages> |
245 | 828 | <browser:page | 831 | <browser:page |
246 | 829 | for="lp.code.interfaces.gitref.IGitRef" | 832 | for="lp.code.interfaces.gitref.IGitRef" |
247 | 830 | 833 | ||
248 | === modified file 'lib/lp/code/browser/gitref.py' | |||
249 | --- lib/lp/code/browser/gitref.py 2015-04-22 16:11:40 +0000 | |||
250 | +++ lib/lp/code/browser/gitref.py 2015-04-29 12:39:54 +0000 | |||
251 | @@ -10,12 +10,19 @@ | |||
252 | 10 | 'GitRefView', | 10 | 'GitRefView', |
253 | 11 | ] | 11 | ] |
254 | 12 | 12 | ||
255 | 13 | from lp.code.browser.branchmergeproposal import ( | ||
256 | 14 | latest_proposals_for_each_branch, | ||
257 | 15 | ) | ||
258 | 13 | from lp.code.interfaces.gitref import IGitRef | 16 | from lp.code.interfaces.gitref import IGitRef |
259 | 17 | from lp.code.model.gitrepository import GitRepository | ||
260 | 18 | from lp.services.database.bulk import load_related | ||
261 | 19 | from lp.services.propertycache import cachedproperty | ||
262 | 14 | from lp.services.webapp import ( | 20 | from lp.services.webapp import ( |
263 | 15 | LaunchpadView, | 21 | LaunchpadView, |
264 | 16 | Navigation, | 22 | Navigation, |
265 | 17 | stepthrough, | 23 | stepthrough, |
266 | 18 | ) | 24 | ) |
267 | 25 | from lp.services.webapp.authorization import check_permission | ||
268 | 19 | 26 | ||
269 | 20 | 27 | ||
270 | 21 | class GitRefNavigation(Navigation): | 28 | class GitRefNavigation(Navigation): |
271 | @@ -49,3 +56,57 @@ | |||
272 | 49 | "author_date": self.context.author_date, | 56 | "author_date": self.context.author_date, |
273 | 50 | "commit_message": self.context.commit_message, | 57 | "commit_message": self.context.commit_message, |
274 | 51 | } | 58 | } |
275 | 59 | |||
276 | 60 | @property | ||
277 | 61 | def show_merge_links(self): | ||
278 | 62 | """Return whether or not merge proposal links should be shown. | ||
279 | 63 | |||
280 | 64 | Merge proposal links should not be shown if there is only one | ||
281 | 65 | reference in the entire target. | ||
282 | 66 | """ | ||
283 | 67 | if not self.context.namespace.supports_merge_proposals: | ||
284 | 68 | return False | ||
285 | 69 | repositories = self.context.namespace.collection.getRepositories() | ||
286 | 70 | if repositories.count() > 1: | ||
287 | 71 | return True | ||
288 | 72 | repository = repositories.one() | ||
289 | 73 | if repository is None: | ||
290 | 74 | return False | ||
291 | 75 | return repository.refs.count() > 1 | ||
292 | 76 | |||
293 | 77 | @cachedproperty | ||
294 | 78 | def landing_targets(self): | ||
295 | 79 | """Return a filtered list of landing targets.""" | ||
296 | 80 | return latest_proposals_for_each_branch(self.context.landing_targets) | ||
297 | 81 | |||
298 | 82 | @cachedproperty | ||
299 | 83 | def landing_candidates(self): | ||
300 | 84 | """Return a decorated list of landing candidates.""" | ||
301 | 85 | candidates = list(self.context.landing_candidates) | ||
302 | 86 | load_related( | ||
303 | 87 | GitRepository, candidates, | ||
304 | 88 | ["source_git_repositoryID", "prerequisite_git_repositoryID"]) | ||
305 | 89 | return [proposal for proposal in candidates | ||
306 | 90 | if check_permission("launchpad.View", proposal)] | ||
307 | 91 | |||
308 | 92 | def _getBranchCountText(self, count): | ||
309 | 93 | """Help to show user friendly text.""" | ||
310 | 94 | if count == 0: | ||
311 | 95 | return 'No branches' | ||
312 | 96 | elif count == 1: | ||
313 | 97 | return '1 branch' | ||
314 | 98 | else: | ||
315 | 99 | return '%s branches' % count | ||
316 | 100 | |||
317 | 101 | @cachedproperty | ||
318 | 102 | def landing_candidate_count_text(self): | ||
319 | 103 | return self._getBranchCountText(len(self.landing_candidates)) | ||
320 | 104 | |||
321 | 105 | @cachedproperty | ||
322 | 106 | def dependent_landings(self): | ||
323 | 107 | return [proposal for proposal in self.context.dependent_landings | ||
324 | 108 | if check_permission("launchpad.View", proposal)] | ||
325 | 109 | |||
326 | 110 | @cachedproperty | ||
327 | 111 | def dependent_landing_count_text(self): | ||
328 | 112 | return self._getBranchCountText(len(self.dependent_landings)) | ||
329 | 52 | 113 | ||
330 | === modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py' | |||
331 | --- lib/lp/code/browser/tests/test_branchmergeproposal.py 2015-04-19 12:56:32 +0000 | |||
332 | +++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2015-04-29 12:39:54 +0000 | |||
333 | @@ -46,6 +46,7 @@ | |||
334 | 46 | BranchMergeProposalStatus, | 46 | BranchMergeProposalStatus, |
335 | 47 | CodeReviewVote, | 47 | CodeReviewVote, |
336 | 48 | ) | 48 | ) |
337 | 49 | from lp.code.interfaces.gitrepository import GIT_FEATURE_FLAG | ||
338 | 49 | from lp.code.model.diff import PreviewDiff | 50 | from lp.code.model.diff import PreviewDiff |
339 | 50 | from lp.code.tests.helpers import ( | 51 | from lp.code.tests.helpers import ( |
340 | 51 | add_revision_to_branch, | 52 | add_revision_to_branch, |
341 | @@ -55,6 +56,7 @@ | |||
342 | 55 | PersonVisibility, | 56 | PersonVisibility, |
343 | 56 | TeamMembershipPolicy, | 57 | TeamMembershipPolicy, |
344 | 57 | ) | 58 | ) |
345 | 59 | from lp.services.features.testing import FeatureFixture | ||
346 | 58 | from lp.services.librarian.interfaces.client import LibrarianServerError | 60 | from lp.services.librarian.interfaces.client import LibrarianServerError |
347 | 59 | from lp.services.messages.model.message import MessageSet | 61 | from lp.services.messages.model.message import MessageSet |
348 | 60 | from lp.services.webapp import canonical_url | 62 | from lp.services.webapp import canonical_url |
349 | @@ -107,8 +109,8 @@ | |||
350 | 107 | self.assertTrue(d.can_change_review) | 109 | self.assertTrue(d.can_change_review) |
351 | 108 | 110 | ||
352 | 109 | 111 | ||
355 | 110 | class TestBranchMergeProposalMergedView(TestCaseWithFactory): | 112 | class TestBranchMergeProposalMergedViewBzr(TestCaseWithFactory): |
356 | 111 | """Tests for `BranchMergeProposalMergedView`.""" | 113 | """Tests for `BranchMergeProposalMergedView` for Bazaar.""" |
357 | 112 | 114 | ||
358 | 113 | layer = DatabaseFunctionalLayer | 115 | layer = DatabaseFunctionalLayer |
359 | 114 | 116 | ||
360 | @@ -129,6 +131,30 @@ | |||
361 | 129 | view.initial_values) | 131 | view.initial_values) |
362 | 130 | 132 | ||
363 | 131 | 133 | ||
364 | 134 | class TestBranchMergeProposalMergedViewGit(TestCaseWithFactory): | ||
365 | 135 | """Tests for `BranchMergeProposalMergedView` for Git.""" | ||
366 | 136 | |||
367 | 137 | layer = DatabaseFunctionalLayer | ||
368 | 138 | |||
369 | 139 | def setUp(self): | ||
370 | 140 | # Use an admin so we don't have to worry about launchpad.Edit | ||
371 | 141 | # permissions on the merge proposals for adding comments, or | ||
372 | 142 | # nominating reviewers. | ||
373 | 143 | TestCaseWithFactory.setUp(self, user="admin@canonical.com") | ||
374 | 144 | self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"})) | ||
375 | 145 | self.bmp = self.factory.makeBranchMergeProposalForGit() | ||
376 | 146 | |||
377 | 147 | def test_initial_values(self): | ||
378 | 148 | # The default merged_revision_id is the head commit SHA-1 of the | ||
379 | 149 | # target ref. | ||
380 | 150 | view = BranchMergeProposalMergedView(self.bmp, LaunchpadTestRequest()) | ||
381 | 151 | removeSecurityProxy(self.bmp.source_git_ref).commit_sha1 = "0" * 40 | ||
382 | 152 | removeSecurityProxy(self.bmp.target_git_ref).commit_sha1 = "1" * 40 | ||
383 | 153 | self.assertEqual( | ||
384 | 154 | {'merged_revision_id': self.bmp.target_git_ref.commit_sha1}, | ||
385 | 155 | view.initial_values) | ||
386 | 156 | |||
387 | 157 | |||
388 | 132 | class TestBranchMergeProposalAddVoteView(TestCaseWithFactory): | 158 | class TestBranchMergeProposalAddVoteView(TestCaseWithFactory): |
389 | 133 | """Test the AddVote view.""" | 159 | """Test the AddVote view.""" |
390 | 134 | 160 | ||
391 | @@ -661,14 +687,14 @@ | |||
392 | 661 | self.assertEqual(BrowserNotificationLevel.INFO, notification.level) | 687 | self.assertEqual(BrowserNotificationLevel.INFO, notification.level) |
393 | 662 | 688 | ||
394 | 663 | 689 | ||
396 | 664 | class TestBranchMergeProposalResubmitView(TestCaseWithFactory): | 690 | class TestBranchMergeProposalResubmitViewMixin: |
397 | 665 | """Test BranchMergeProposalResubmitView.""" | 691 | """Test BranchMergeProposalResubmitView.""" |
398 | 666 | 692 | ||
399 | 667 | layer = DatabaseFunctionalLayer | 693 | layer = DatabaseFunctionalLayer |
400 | 668 | 694 | ||
401 | 669 | def createView(self): | 695 | def createView(self): |
402 | 670 | """Create the required view.""" | 696 | """Create the required view.""" |
404 | 671 | context = self.factory.makeBranchMergeProposal() | 697 | context = self._makeBranchMergeProposal() |
405 | 672 | self.useContext(person_logged_in(context.registrant)) | 698 | self.useContext(person_logged_in(context.registrant)) |
406 | 673 | view = BranchMergeProposalResubmitView( | 699 | view = BranchMergeProposalResubmitView( |
407 | 674 | context, LaunchpadTestRequest()) | 700 | context, LaunchpadTestRequest()) |
408 | @@ -679,36 +705,34 @@ | |||
409 | 679 | """resubmit_action resubmits the proposal.""" | 705 | """resubmit_action resubmits the proposal.""" |
410 | 680 | view = self.createView() | 706 | view = self.createView() |
411 | 681 | context = view.context | 707 | context = view.context |
419 | 682 | new_proposal = view.resubmit_action.success( | 708 | new_proposal = view.resubmit_action.success(self._getFormValues( |
420 | 683 | {'source_branch': context.source_branch, | 709 | context.merge_source, context.merge_target, |
421 | 684 | 'target_branch': context.target_branch, | 710 | context.merge_prerequisite, { |
422 | 685 | 'prerequisite_branch': context.prerequisite_branch, | 711 | 'description': None, |
423 | 686 | 'description': None, | 712 | 'break_link': False, |
424 | 687 | 'break_link': False, | 713 | })) |
418 | 688 | }) | ||
425 | 689 | self.assertEqual(new_proposal.supersedes, context) | 714 | self.assertEqual(new_proposal.supersedes, context) |
428 | 690 | self.assertEqual(new_proposal.source_branch, context.source_branch) | 715 | self.assertEqual(new_proposal.merge_source, context.merge_source) |
429 | 691 | self.assertEqual(new_proposal.target_branch, context.target_branch) | 716 | self.assertEqual(new_proposal.merge_target, context.merge_target) |
430 | 692 | self.assertEqual( | 717 | self.assertEqual( |
432 | 693 | new_proposal.prerequisite_branch, context.prerequisite_branch) | 718 | new_proposal.merge_prerequisite, context.merge_prerequisite) |
433 | 694 | 719 | ||
434 | 695 | def test_resubmit_action_change_branches(self): | 720 | def test_resubmit_action_change_branches(self): |
435 | 696 | """Changing the branches changes the branches in the new proposal.""" | 721 | """Changing the branches changes the branches in the new proposal.""" |
436 | 697 | view = self.createView() | 722 | view = self.createView() |
447 | 698 | target = view.context.source_branch.target | 723 | new_source = self._makeBranchWithSameTarget(view.context.merge_source) |
448 | 699 | new_source = self.factory.makeBranchTargetBranch(target) | 724 | new_target = self._makeBranchWithSameTarget(view.context.merge_source) |
449 | 700 | new_target = self.factory.makeBranchTargetBranch(target) | 725 | new_prerequisite = self._makeBranchWithSameTarget( |
450 | 701 | new_prerequisite = self.factory.makeBranchTargetBranch(target) | 726 | view.context.merge_source) |
451 | 702 | new_proposal = view.resubmit_action.success( | 727 | new_proposal = view.resubmit_action.success(self._getFormValues( |
452 | 703 | {'source_branch': new_source, 'target_branch': new_target, | 728 | new_source, new_target, new_prerequisite, { |
453 | 704 | 'prerequisite_branch': new_prerequisite, | 729 | 'description': 'description', |
454 | 705 | 'description': 'description', | 730 | 'break_link': False, |
455 | 706 | 'break_link': False, | 731 | })) |
446 | 707 | }) | ||
456 | 708 | self.assertEqual(new_proposal.supersedes, view.context) | 732 | self.assertEqual(new_proposal.supersedes, view.context) |
460 | 709 | self.assertEqual(new_proposal.source_branch, new_source) | 733 | self.assertEqual(new_proposal.merge_source, new_source) |
461 | 710 | self.assertEqual(new_proposal.target_branch, new_target) | 734 | self.assertEqual(new_proposal.merge_target, new_target) |
462 | 711 | self.assertEqual(new_proposal.prerequisite_branch, new_prerequisite) | 735 | self.assertEqual(new_proposal.merge_prerequisite, new_prerequisite) |
463 | 712 | 736 | ||
464 | 713 | def test_resubmit_action_break_link(self): | 737 | def test_resubmit_action_break_link(self): |
465 | 714 | """Enabling break_link prevents linking the old and new proposals.""" | 738 | """Enabling break_link prevents linking the old and new proposals.""" |
466 | @@ -716,24 +740,22 @@ | |||
467 | 716 | new_proposal = self.resubmitDefault(view, break_link=True) | 740 | new_proposal = self.resubmitDefault(view, break_link=True) |
468 | 717 | self.assertIs(None, new_proposal.supersedes) | 741 | self.assertIs(None, new_proposal.supersedes) |
469 | 718 | 742 | ||
472 | 719 | @staticmethod | 743 | @classmethod |
473 | 720 | def resubmitDefault(view, break_link=False, prerequisite_branch=None): | 744 | def resubmitDefault(cls, view, break_link=False, merge_prerequisite=None): |
474 | 721 | context = view.context | 745 | context = view.context |
484 | 722 | if prerequisite_branch is None: | 746 | if merge_prerequisite is None: |
485 | 723 | prerequisite_branch = context.prerequisite_branch | 747 | merge_prerequisite = context.merge_prerequisite |
486 | 724 | return view.resubmit_action.success( | 748 | return view.resubmit_action.success(cls._getFormValues( |
487 | 725 | {'source_branch': context.source_branch, | 749 | context.merge_source, context.merge_target, merge_prerequisite, { |
488 | 726 | 'target_branch': context.target_branch, | 750 | 'description': None, |
489 | 727 | 'prerequisite_branch': prerequisite_branch, | 751 | 'break_link': break_link, |
490 | 728 | 'description': None, | 752 | })) |
482 | 729 | 'break_link': break_link, | ||
483 | 730 | }) | ||
491 | 731 | 753 | ||
492 | 732 | def test_resubmit_existing(self): | 754 | def test_resubmit_existing(self): |
493 | 733 | """Resubmitting a proposal when another is active is a user error.""" | 755 | """Resubmitting a proposal when another is active is a user error.""" |
494 | 734 | view = self.createView() | 756 | view = self.createView() |
495 | 735 | first_bmp = view.context | 757 | first_bmp = view.context |
497 | 736 | with person_logged_in(first_bmp.target_branch.owner): | 758 | with person_logged_in(first_bmp.merge_target.owner): |
498 | 737 | first_bmp.resubmit(first_bmp.registrant) | 759 | first_bmp.resubmit(first_bmp.registrant) |
499 | 738 | self.resubmitDefault(view) | 760 | self.resubmitDefault(view) |
500 | 739 | (notification,) = view.request.response.notifications | 761 | (notification,) = view.request.response.notifications |
501 | @@ -742,19 +764,86 @@ | |||
502 | 742 | ' <a href=.*>a similar merge proposal</a> is already active.')) | 764 | ' <a href=.*>a similar merge proposal</a> is already active.')) |
503 | 743 | self.assertEqual(BrowserNotificationLevel.ERROR, notification.level) | 765 | self.assertEqual(BrowserNotificationLevel.ERROR, notification.level) |
504 | 744 | 766 | ||
505 | 767 | |||
506 | 768 | class TestBranchMergeProposalResubmitViewBzr( | ||
507 | 769 | TestBranchMergeProposalResubmitViewMixin, TestCaseWithFactory): | ||
508 | 770 | """Test BranchMergeProposalResubmitView for Bazaar.""" | ||
509 | 771 | |||
510 | 772 | def _makeBranchMergeProposal(self): | ||
511 | 773 | return self.factory.makeBranchMergeProposal() | ||
512 | 774 | |||
513 | 775 | @staticmethod | ||
514 | 776 | def _getFormValues(source_branch, target_branch, prerequisite_branch, | ||
515 | 777 | extras): | ||
516 | 778 | values = { | ||
517 | 779 | 'source_branch': source_branch, | ||
518 | 780 | 'target_branch': target_branch, | ||
519 | 781 | 'prerequisite_branch': prerequisite_branch, | ||
520 | 782 | } | ||
521 | 783 | values.update(extras) | ||
522 | 784 | return values | ||
523 | 785 | |||
524 | 786 | def _makeBranchWithSameTarget(self, branch): | ||
525 | 787 | return self.factory.makeBranchTargetBranch(branch.target) | ||
526 | 788 | |||
527 | 745 | def test_resubmit_same_target_prerequisite(self): | 789 | def test_resubmit_same_target_prerequisite(self): |
528 | 746 | """User error if same branch is target and prerequisite.""" | 790 | """User error if same branch is target and prerequisite.""" |
529 | 747 | view = self.createView() | 791 | view = self.createView() |
530 | 748 | first_bmp = view.context | 792 | first_bmp = view.context |
533 | 749 | self.resubmitDefault( | 793 | self.resubmitDefault(view, merge_prerequisite=first_bmp.merge_target) |
532 | 750 | view, prerequisite_branch=first_bmp.target_branch) | ||
534 | 751 | self.assertEqual( | 794 | self.assertEqual( |
535 | 752 | view.errors, | 795 | view.errors, |
536 | 753 | ['Target and prerequisite branches must be different.']) | 796 | ['Target and prerequisite branches must be different.']) |
537 | 754 | 797 | ||
538 | 755 | 798 | ||
541 | 756 | class TestResubmitBrowser(BrowserTestCase): | 799 | class TestBranchMergeProposalResubmitViewGit( |
542 | 757 | """Browser tests for resubmitting branch merge proposals.""" | 800 | TestBranchMergeProposalResubmitViewMixin, TestCaseWithFactory): |
543 | 801 | """Test BranchMergeProposalResubmitView for Git.""" | ||
544 | 802 | |||
545 | 803 | def setUp(self): | ||
546 | 804 | super(TestBranchMergeProposalResubmitViewGit, self).setUp() | ||
547 | 805 | self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"})) | ||
548 | 806 | |||
549 | 807 | def _makeBranchMergeProposal(self): | ||
550 | 808 | return self.factory.makeBranchMergeProposalForGit() | ||
551 | 809 | |||
552 | 810 | @staticmethod | ||
553 | 811 | def _getFormValues(source_branch, target_branch, prerequisite_branch, | ||
554 | 812 | extras): | ||
555 | 813 | values = { | ||
556 | 814 | 'source_git_repository': source_branch.repository, | ||
557 | 815 | 'source_git_path': source_branch.path, | ||
558 | 816 | 'target_git_repository': target_branch.repository, | ||
559 | 817 | 'target_git_path': target_branch.path, | ||
560 | 818 | } | ||
561 | 819 | if prerequisite_branch is not None: | ||
562 | 820 | values.update({ | ||
563 | 821 | 'prerequisite_git_repository': prerequisite_branch.repository, | ||
564 | 822 | 'prerequisite_git_path': prerequisite_branch.path, | ||
565 | 823 | }) | ||
566 | 824 | else: | ||
567 | 825 | values.update({ | ||
568 | 826 | 'prerequisite_git_repository': None, | ||
569 | 827 | 'prerequisite_git_path': '', | ||
570 | 828 | }) | ||
571 | 829 | values.update(extras) | ||
572 | 830 | return values | ||
573 | 831 | |||
574 | 832 | def _makeBranchWithSameTarget(self, branch): | ||
575 | 833 | return self.factory.makeGitRefs(target=branch.target)[0] | ||
576 | 834 | |||
577 | 835 | def test_resubmit_same_target_prerequisite(self): | ||
578 | 836 | """User error if same branch is target and prerequisite.""" | ||
579 | 837 | view = self.createView() | ||
580 | 838 | first_bmp = view.context | ||
581 | 839 | self.resubmitDefault(view, merge_prerequisite=first_bmp.merge_target) | ||
582 | 840 | self.assertEqual( | ||
583 | 841 | view.errors, | ||
584 | 842 | ['Target and prerequisite references must be different.']) | ||
585 | 843 | |||
586 | 844 | |||
587 | 845 | class TestResubmitBrowserBzr(BrowserTestCase): | ||
588 | 846 | """Browser tests for resubmitting branch merge proposals for Bazaar.""" | ||
589 | 758 | 847 | ||
590 | 759 | layer = DatabaseFunctionalLayer | 848 | layer = DatabaseFunctionalLayer |
591 | 760 | 849 | ||
592 | @@ -781,6 +870,41 @@ | |||
593 | 781 | self.assertEqual('flibble', bmp.superseded_by.description) | 870 | self.assertEqual('flibble', bmp.superseded_by.description) |
594 | 782 | 871 | ||
595 | 783 | 872 | ||
596 | 873 | class TestResubmitBrowserGit(BrowserTestCase): | ||
597 | 874 | """Browser tests for resubmitting branch merge proposals for Git.""" | ||
598 | 875 | |||
599 | 876 | layer = DatabaseFunctionalLayer | ||
600 | 877 | |||
601 | 878 | def setUp(self): | ||
602 | 879 | super(TestResubmitBrowserGit, self).setUp() | ||
603 | 880 | self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"})) | ||
604 | 881 | |||
605 | 882 | def test_resubmit_text(self): | ||
606 | 883 | """The text of the resubmit page is as expected.""" | ||
607 | 884 | bmp = self.factory.makeBranchMergeProposalForGit(registrant=self.user) | ||
608 | 885 | text = self.getMainText(bmp, '+resubmit') | ||
609 | 886 | expected = ( | ||
610 | 887 | 'Resubmit proposal to merge.*' | ||
611 | 888 | 'Source Git Repository:.*' | ||
612 | 889 | 'Source Git branch path:.*' | ||
613 | 890 | 'Target Git Repository:.*' | ||
614 | 891 | 'Target Git branch path:.*' | ||
615 | 892 | 'Prerequisite Git Repository:.*' | ||
616 | 893 | 'Prerequisite Git branch path:.*' | ||
617 | 894 | 'Description.*' | ||
618 | 895 | 'Start afresh.*') | ||
619 | 896 | self.assertTextMatchesExpressionIgnoreWhitespace(expected, text) | ||
620 | 897 | |||
621 | 898 | def test_resubmit_controls(self): | ||
622 | 899 | """Proposals can be resubmitted using the browser.""" | ||
623 | 900 | bmp = self.factory.makeBranchMergeProposalForGit(registrant=self.user) | ||
624 | 901 | browser = self.getViewBrowser(bmp, '+resubmit') | ||
625 | 902 | browser.getControl('Description').value = 'flibble' | ||
626 | 903 | browser.getControl('Resubmit').click() | ||
627 | 904 | with person_logged_in(self.user): | ||
628 | 905 | self.assertEqual('flibble', bmp.superseded_by.description) | ||
629 | 906 | |||
630 | 907 | |||
631 | 784 | class TestBranchMergeProposalView(TestCaseWithFactory): | 908 | class TestBranchMergeProposalView(TestCaseWithFactory): |
632 | 785 | 909 | ||
633 | 786 | layer = LaunchpadFunctionalLayer | 910 | layer = LaunchpadFunctionalLayer |
634 | @@ -1248,17 +1372,17 @@ | |||
635 | 1248 | self.assertEqual(mp_url, browser.url) | 1372 | self.assertEqual(mp_url, browser.url) |
636 | 1249 | 1373 | ||
637 | 1250 | 1374 | ||
639 | 1251 | class TestLatestProposalsForEachBranch(TestCaseWithFactory): | 1375 | class TestLatestProposalsForEachBranchMixin: |
640 | 1252 | """Confirm that the latest branch is returned.""" | 1376 | """Confirm that the latest branch is returned.""" |
641 | 1253 | 1377 | ||
642 | 1254 | layer = DatabaseFunctionalLayer | 1378 | layer = DatabaseFunctionalLayer |
643 | 1255 | 1379 | ||
644 | 1256 | def test_newest_first(self): | 1380 | def test_newest_first(self): |
645 | 1257 | # If each proposal targets a different branch, each will be returned. | 1381 | # If each proposal targets a different branch, each will be returned. |
647 | 1258 | bmp1 = self.factory.makeBranchMergeProposal( | 1382 | bmp1 = self._makeBranchMergeProposal( |
648 | 1259 | date_created=( | 1383 | date_created=( |
649 | 1260 | datetime(year=2008, month=9, day=10, tzinfo=pytz.UTC))) | 1384 | datetime(year=2008, month=9, day=10, tzinfo=pytz.UTC))) |
651 | 1261 | bmp2 = self.factory.makeBranchMergeProposal( | 1385 | bmp2 = self._makeBranchMergeProposal( |
652 | 1262 | date_created=( | 1386 | date_created=( |
653 | 1263 | datetime(year=2008, month=10, day=10, tzinfo=pytz.UTC))) | 1387 | datetime(year=2008, month=10, day=10, tzinfo=pytz.UTC))) |
654 | 1264 | self.assertEqual( | 1388 | self.assertEqual( |
655 | @@ -1266,27 +1390,57 @@ | |||
656 | 1266 | 1390 | ||
657 | 1267 | def test_visible_filtered_out(self): | 1391 | def test_visible_filtered_out(self): |
658 | 1268 | # If the proposal is not visible to the user, they are not returned. | 1392 | # If the proposal is not visible to the user, they are not returned. |
660 | 1269 | bmp1 = self.factory.makeBranchMergeProposal( | 1393 | bmp1 = self._makeBranchMergeProposal( |
661 | 1270 | date_created=( | 1394 | date_created=( |
662 | 1271 | datetime(year=2008, month=9, day=10, tzinfo=pytz.UTC))) | 1395 | datetime(year=2008, month=9, day=10, tzinfo=pytz.UTC))) |
664 | 1272 | bmp2 = self.factory.makeBranchMergeProposal( | 1396 | bmp2 = self._makeBranchMergeProposal( |
665 | 1273 | date_created=( | 1397 | date_created=( |
666 | 1274 | datetime(year=2008, month=10, day=10, tzinfo=pytz.UTC))) | 1398 | datetime(year=2008, month=10, day=10, tzinfo=pytz.UTC))) |
670 | 1275 | removeSecurityProxy(bmp2.source_branch).transitionToInformationType( | 1399 | self._setBranchInvisible(bmp2.merge_source) |
668 | 1276 | InformationType.USERDATA, bmp2.source_branch.owner, | ||
669 | 1277 | verify_policy=False) | ||
671 | 1278 | self.assertEqual( | 1400 | self.assertEqual( |
672 | 1279 | [bmp1], latest_proposals_for_each_branch([bmp1, bmp2])) | 1401 | [bmp1], latest_proposals_for_each_branch([bmp1, bmp2])) |
673 | 1280 | 1402 | ||
674 | 1281 | def test_same_target(self): | 1403 | def test_same_target(self): |
675 | 1282 | # If the proposals target the same branch, then the most recent is | 1404 | # If the proposals target the same branch, then the most recent is |
676 | 1283 | # returned. | 1405 | # returned. |
678 | 1284 | bmp1 = self.factory.makeBranchMergeProposal( | 1406 | bmp1 = self._makeBranchMergeProposal( |
679 | 1285 | date_created=( | 1407 | date_created=( |
680 | 1286 | datetime(year=2008, month=9, day=10, tzinfo=pytz.UTC))) | 1408 | datetime(year=2008, month=9, day=10, tzinfo=pytz.UTC))) |
683 | 1287 | bmp2 = self.factory.makeBranchMergeProposal( | 1409 | bmp2 = self._makeBranchMergeProposal( |
684 | 1288 | target_branch=bmp1.target_branch, | 1410 | merge_target=bmp1.merge_target, |
685 | 1289 | date_created=( | 1411 | date_created=( |
686 | 1290 | datetime(year=2008, month=10, day=10, tzinfo=pytz.UTC))) | 1412 | datetime(year=2008, month=10, day=10, tzinfo=pytz.UTC))) |
687 | 1291 | self.assertEqual( | 1413 | self.assertEqual( |
688 | 1292 | [bmp2], latest_proposals_for_each_branch([bmp1, bmp2])) | 1414 | [bmp2], latest_proposals_for_each_branch([bmp1, bmp2])) |
689 | 1415 | |||
690 | 1416 | |||
691 | 1417 | class TestLatestProposalsForEachBranchBzr( | ||
692 | 1418 | TestLatestProposalsForEachBranchMixin, TestCaseWithFactory): | ||
693 | 1419 | """Confirm that the latest branch is returned for Bazaar.""" | ||
694 | 1420 | |||
695 | 1421 | def _makeBranchMergeProposal(self, merge_target=None, **kwargs): | ||
696 | 1422 | return self.factory.makeBranchMergeProposal( | ||
697 | 1423 | target_branch=merge_target, **kwargs) | ||
698 | 1424 | |||
699 | 1425 | @staticmethod | ||
700 | 1426 | def _setBranchInvisible(branch): | ||
701 | 1427 | removeSecurityProxy(branch).transitionToInformationType( | ||
702 | 1428 | InformationType.USERDATA, branch.owner, verify_policy=False) | ||
703 | 1429 | |||
704 | 1430 | |||
705 | 1431 | class TestLatestProposalsForEachBranchGit( | ||
706 | 1432 | TestLatestProposalsForEachBranchMixin, TestCaseWithFactory): | ||
707 | 1433 | """Confirm that the latest branch is returned for Bazaar.""" | ||
708 | 1434 | |||
709 | 1435 | def setUp(self): | ||
710 | 1436 | super(TestLatestProposalsForEachBranchGit, self).setUp() | ||
711 | 1437 | self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"})) | ||
712 | 1438 | |||
713 | 1439 | def _makeBranchMergeProposal(self, merge_target=None, **kwargs): | ||
714 | 1440 | return self.factory.makeBranchMergeProposalForGit( | ||
715 | 1441 | target_ref=merge_target, **kwargs) | ||
716 | 1442 | |||
717 | 1443 | @staticmethod | ||
718 | 1444 | def _setBranchInvisible(branch): | ||
719 | 1445 | removeSecurityProxy(branch.repository).transitionToInformationType( | ||
720 | 1446 | InformationType.USERDATA, branch.owner, verify_policy=False) | ||
721 | 1293 | 1447 | ||
722 | === modified file 'lib/lp/code/errors.py' | |||
723 | --- lib/lp/code/errors.py 2015-04-19 12:56:32 +0000 | |||
724 | +++ lib/lp/code/errors.py 2015-04-29 12:39:54 +0000 | |||
725 | @@ -231,11 +231,18 @@ | |||
726 | 231 | """Raised if there is already a matching BranchMergeProposal.""" | 231 | """Raised if there is already a matching BranchMergeProposal.""" |
727 | 232 | 232 | ||
728 | 233 | def __init__(self, existing_proposal): | 233 | def __init__(self, existing_proposal): |
729 | 234 | # Circular import. | ||
730 | 235 | from lp.code.interfaces.branch import IBranch | ||
731 | 236 | # display_name is the newer style, but IBranch uses the older style. | ||
732 | 237 | if IBranch.providedBy(existing_proposal.merge_source): | ||
733 | 238 | display_name = "displayname" | ||
734 | 239 | else: | ||
735 | 240 | display_name = "display_name" | ||
736 | 234 | super(BranchMergeProposalExists, self).__init__( | 241 | super(BranchMergeProposalExists, self).__init__( |
737 | 235 | 'There is already a branch merge proposal registered for ' | 242 | 'There is already a branch merge proposal registered for ' |
738 | 236 | 'branch %s to land on %s that is still active.' % | 243 | 'branch %s to land on %s that is still active.' % |
741 | 237 | (existing_proposal.source_branch.displayname, | 244 | (getattr(existing_proposal.merge_source, display_name), |
742 | 238 | existing_proposal.target_branch.displayname)) | 245 | getattr(existing_proposal.merge_target, display_name))) |
743 | 239 | self.existing_proposal = existing_proposal | 246 | self.existing_proposal = existing_proposal |
744 | 240 | 247 | ||
745 | 241 | 248 | ||
746 | 242 | 249 | ||
747 | === modified file 'lib/lp/code/interfaces/gitrepository.py' | |||
748 | --- lib/lp/code/interfaces/gitrepository.py 2015-04-22 16:11:40 +0000 | |||
749 | +++ lib/lp/code/interfaces/gitrepository.py 2015-04-29 12:39:54 +0000 | |||
750 | @@ -49,6 +49,7 @@ | |||
751 | 49 | Choice, | 49 | Choice, |
752 | 50 | Datetime, | 50 | Datetime, |
753 | 51 | Int, | 51 | Int, |
754 | 52 | List, | ||
755 | 52 | Text, | 53 | Text, |
756 | 53 | TextLine, | 54 | TextLine, |
757 | 54 | ) | 55 | ) |
758 | @@ -607,6 +608,39 @@ | |||
759 | 607 | :return: A collection of `IGitRepository` objects. | 608 | :return: A collection of `IGitRepository` objects. |
760 | 608 | """ | 609 | """ |
761 | 609 | 610 | ||
762 | 611 | @call_with(user=REQUEST_USER) | ||
763 | 612 | @operation_parameters( | ||
764 | 613 | person=Reference( | ||
765 | 614 | title=_("The person whose repository visibility is being " | ||
766 | 615 | "checked."), | ||
767 | 616 | schema=IPerson), | ||
768 | 617 | repository_names=List(value_type=Text(), | ||
769 | 618 | title=_('List of repository unique names'), required=True), | ||
770 | 619 | ) | ||
771 | 620 | @export_read_operation() | ||
772 | 621 | @operation_for_version("devel") | ||
773 | 622 | def getRepositoryVisibilityInfo(user, person, repository_names): | ||
774 | 623 | """Return the named repositories visible to both user and person. | ||
775 | 624 | |||
776 | 625 | Anonymous requesters don't get any information. | ||
777 | 626 | |||
778 | 627 | :param user: The user requesting the information. If the user is | ||
779 | 628 | None then we return an empty dict. | ||
780 | 629 | :param person: The person whose repository visibility we wish to | ||
781 | 630 | check. | ||
782 | 631 | :param repository_names: The unique names of the repositories to | ||
783 | 632 | check. | ||
784 | 633 | |||
785 | 634 | Return a dict with the following values: | ||
786 | 635 | person_name: the displayname of the person. | ||
787 | 636 | visible_repositories: a list of the unique names of the repositories | ||
788 | 637 | which the requester and specified person can both see. | ||
789 | 638 | |||
790 | 639 | This API call is provided for use by the client Javascript. It is | ||
791 | 640 | not designed to efficiently scale to handle requests for large | ||
792 | 641 | numbers of repositories. | ||
793 | 642 | """ | ||
794 | 643 | |||
795 | 610 | @operation_parameters( | 644 | @operation_parameters( |
796 | 611 | target=Reference( | 645 | target=Reference( |
797 | 612 | title=_("Target"), required=True, schema=IHasGitRepositories)) | 646 | title=_("Target"), required=True, schema=IHasGitRepositories)) |
798 | 613 | 647 | ||
799 | === modified file 'lib/lp/code/model/gitref.py' | |||
800 | --- lib/lp/code/model/gitref.py 2015-04-24 12:58:46 +0000 | |||
801 | +++ lib/lp/code/model/gitref.py 2015-04-29 12:39:54 +0000 | |||
802 | @@ -376,3 +376,6 @@ | |||
803 | 376 | 376 | ||
804 | 377 | def __ne__(self, other): | 377 | def __ne__(self, other): |
805 | 378 | return not self == other | 378 | return not self == other |
806 | 379 | |||
807 | 380 | def __hash__(self): | ||
808 | 381 | return hash(self.repository) ^ hash(self.path) ^ hash(self.commit_sha1) | ||
809 | 379 | 382 | ||
810 | === modified file 'lib/lp/code/model/gitrepository.py' | |||
811 | --- lib/lp/code/model/gitrepository.py 2015-04-22 16:42:57 +0000 | |||
812 | +++ lib/lp/code/model/gitrepository.py 2015-04-29 12:39:54 +0000 | |||
813 | @@ -38,6 +38,7 @@ | |||
814 | 38 | from storm.store import Store | 38 | from storm.store import Store |
815 | 39 | from zope.component import getUtility | 39 | from zope.component import getUtility |
816 | 40 | from zope.interface import implements | 40 | from zope.interface import implements |
817 | 41 | from zope.security.interfaces import Unauthorized | ||
818 | 41 | from zope.security.proxy import removeSecurityProxy | 42 | from zope.security.proxy import removeSecurityProxy |
819 | 42 | 43 | ||
820 | 43 | from lp.app.enums import ( | 44 | from lp.app.enums import ( |
821 | @@ -774,6 +775,27 @@ | |||
822 | 774 | collection = IGitCollection(target).visibleByUser(user) | 775 | collection = IGitCollection(target).visibleByUser(user) |
823 | 775 | return collection.getRepositories(eager_load=True) | 776 | return collection.getRepositories(eager_load=True) |
824 | 776 | 777 | ||
825 | 778 | def getRepositoryVisibilityInfo(self, user, person, repository_names): | ||
826 | 779 | """See `IGitRepositorySet`.""" | ||
827 | 780 | if user is None: | ||
828 | 781 | return dict() | ||
829 | 782 | lookup = getUtility(IGitLookup) | ||
830 | 783 | visible_repositories = [] | ||
831 | 784 | for name in repository_names: | ||
832 | 785 | repository = lookup.getByUniqueName(name) | ||
833 | 786 | try: | ||
834 | 787 | if (repository is not None | ||
835 | 788 | and repository.visibleByUser(user) | ||
836 | 789 | and repository.visibleByUser(person)): | ||
837 | 790 | visible_repositories.append(repository.unique_name) | ||
838 | 791 | except Unauthorized: | ||
839 | 792 | # We don't include repositories user cannot see. | ||
840 | 793 | pass | ||
841 | 794 | return { | ||
842 | 795 | 'person_name': person.displayname, | ||
843 | 796 | 'visible_repositories': visible_repositories, | ||
844 | 797 | } | ||
845 | 798 | |||
846 | 777 | def getDefaultRepository(self, target): | 799 | def getDefaultRepository(self, target): |
847 | 778 | """See `IGitRepositorySet`.""" | 800 | """See `IGitRepositorySet`.""" |
848 | 779 | clauses = [GitRepository.target_default == True] | 801 | clauses = [GitRepository.target_default == True] |
849 | 780 | 802 | ||
850 | === modified file 'lib/lp/code/model/tests/test_gitrepository.py' | |||
851 | --- lib/lp/code/model/tests/test_gitrepository.py 2015-04-21 23:05:48 +0000 | |||
852 | +++ lib/lp/code/model/tests/test_gitrepository.py 2015-04-29 12:39:54 +0000 | |||
853 | @@ -1216,6 +1216,80 @@ | |||
854 | 1216 | public_repositories + [private_repository], | 1216 | public_repositories + [private_repository], |
855 | 1217 | self.repository_set.getRepositories(other_person, project)) | 1217 | self.repository_set.getRepositories(other_person, project)) |
856 | 1218 | 1218 | ||
857 | 1219 | def test_getRepositoryVisibilityInfo_empty_repository_names(self): | ||
858 | 1220 | # If repository_names is empty, getRepositoryVisibilityInfo returns | ||
859 | 1221 | # an empty visible_repositories list. | ||
860 | 1222 | person = self.factory.makePerson(name="fred") | ||
861 | 1223 | info = self.repository_set.getRepositoryVisibilityInfo( | ||
862 | 1224 | person, person, repository_names=[]) | ||
863 | 1225 | self.assertEqual("Fred", info["person_name"]) | ||
864 | 1226 | self.assertEqual([], info["visible_repositories"]) | ||
865 | 1227 | |||
866 | 1228 | def test_getRepositoryVisibilityInfo(self): | ||
867 | 1229 | person = self.factory.makePerson(name="fred") | ||
868 | 1230 | owner = self.factory.makePerson() | ||
869 | 1231 | visible_repository = self.factory.makeGitRepository() | ||
870 | 1232 | invisible_repository = self.factory.makeGitRepository( | ||
871 | 1233 | owner=owner, information_type=InformationType.USERDATA) | ||
872 | 1234 | invisible_name = removeSecurityProxy(invisible_repository).unique_name | ||
873 | 1235 | repositories = [visible_repository.unique_name, invisible_name] | ||
874 | 1236 | |||
875 | 1237 | with person_logged_in(owner): | ||
876 | 1238 | info = self.repository_set.getRepositoryVisibilityInfo( | ||
877 | 1239 | owner, person, repository_names=repositories) | ||
878 | 1240 | self.assertEqual("Fred", info["person_name"]) | ||
879 | 1241 | self.assertEqual( | ||
880 | 1242 | [visible_repository.unique_name], info["visible_repositories"]) | ||
881 | 1243 | |||
882 | 1244 | def test_getRepositoryVisibilityInfo_unauthorised_user(self): | ||
883 | 1245 | # If the user making the API request cannot see one of the | ||
884 | 1246 | # repositories, that repository is not included in the results. | ||
885 | 1247 | person = self.factory.makePerson(name="fred") | ||
886 | 1248 | owner = self.factory.makePerson() | ||
887 | 1249 | visible_repository = self.factory.makeGitRepository() | ||
888 | 1250 | invisible_repository = self.factory.makeGitRepository( | ||
889 | 1251 | owner=owner, information_type=InformationType.USERDATA) | ||
890 | 1252 | invisible_name = removeSecurityProxy(invisible_repository).unique_name | ||
891 | 1253 | repositories = [visible_repository.unique_name, invisible_name] | ||
892 | 1254 | |||
893 | 1255 | someone = self.factory.makePerson() | ||
894 | 1256 | with person_logged_in(someone): | ||
895 | 1257 | info = self.repository_set.getRepositoryVisibilityInfo( | ||
896 | 1258 | someone, person, repository_names=repositories) | ||
897 | 1259 | self.assertEqual("Fred", info["person_name"]) | ||
898 | 1260 | self.assertEqual( | ||
899 | 1261 | [visible_repository.unique_name], info["visible_repositories"]) | ||
900 | 1262 | |||
901 | 1263 | def test_getRepositoryVisibilityInfo_anonymous(self): | ||
902 | 1264 | # Anonymous users are not allowed to see any repository visibility | ||
903 | 1265 | # information, even if the repository they are querying about is | ||
904 | 1266 | # public. | ||
905 | 1267 | person = self.factory.makePerson(name="fred") | ||
906 | 1268 | owner = self.factory.makePerson() | ||
907 | 1269 | visible_repository = self.factory.makeGitRepository(owner=owner) | ||
908 | 1270 | repositories = [visible_repository.unique_name] | ||
909 | 1271 | |||
910 | 1272 | with person_logged_in(owner): | ||
911 | 1273 | info = self.repository_set.getRepositoryVisibilityInfo( | ||
912 | 1274 | None, person, repository_names=repositories) | ||
913 | 1275 | self.assertEqual({}, info) | ||
914 | 1276 | |||
915 | 1277 | def test_getRepositoryVisibilityInfo_invalid_repository_name(self): | ||
916 | 1278 | # If an invalid repository name is specified, it is not included. | ||
917 | 1279 | person = self.factory.makePerson(name="fred") | ||
918 | 1280 | owner = self.factory.makePerson() | ||
919 | 1281 | visible_repository = self.factory.makeGitRepository(owner=owner) | ||
920 | 1282 | repositories = [ | ||
921 | 1283 | visible_repository.unique_name, | ||
922 | 1284 | "invalid_repository_name"] | ||
923 | 1285 | |||
924 | 1286 | with person_logged_in(owner): | ||
925 | 1287 | info = self.repository_set.getRepositoryVisibilityInfo( | ||
926 | 1288 | owner, person, repository_names=repositories) | ||
927 | 1289 | self.assertEqual("Fred", info["person_name"]) | ||
928 | 1290 | self.assertEqual( | ||
929 | 1291 | [visible_repository.unique_name], info["visible_repositories"]) | ||
930 | 1292 | |||
931 | 1219 | def test_setDefaultRepository_refuses_person(self): | 1293 | def test_setDefaultRepository_refuses_person(self): |
932 | 1220 | # setDefaultRepository refuses if the target is a person. | 1294 | # setDefaultRepository refuses if the target is a person. |
933 | 1221 | person = self.factory.makePerson() | 1295 | person = self.factory.makePerson() |
934 | 1222 | 1296 | ||
935 | === modified file 'lib/lp/code/templates/branchmergeproposal-resubmit.pt' | |||
936 | --- lib/lp/code/templates/branchmergeproposal-resubmit.pt 2010-11-04 16:56:35 +0000 | |||
937 | +++ lib/lp/code/templates/branchmergeproposal-resubmit.pt 2015-04-29 12:39:54 +0000 | |||
938 | @@ -24,7 +24,7 @@ | |||
939 | 24 | </div> | 24 | </div> |
940 | 25 | </div> | 25 | </div> |
941 | 26 | 26 | ||
943 | 27 | <div id="source-revisions"> | 27 | <div id="source-revisions" tal:condition="context/source_branch"> |
944 | 28 | <tal:history-available condition="context/source_branch/revision_count" | 28 | <tal:history-available condition="context/source_branch/revision_count" |
945 | 29 | define="branch context/source_branch; | 29 | define="branch context/source_branch; |
946 | 30 | revisions view/unlanded_revisions"> | 30 | revisions view/unlanded_revisions"> |
947 | 31 | 31 | ||
948 | === modified file 'lib/lp/code/templates/gitref-index.pt' | |||
949 | --- lib/lp/code/templates/gitref-index.pt 2015-03-19 17:04:22 +0000 | |||
950 | +++ lib/lp/code/templates/gitref-index.pt 2015-04-29 12:39:54 +0000 | |||
951 | @@ -17,6 +17,13 @@ | |||
952 | 17 | <div metal:fill-slot="main"> | 17 | <div metal:fill-slot="main"> |
953 | 18 | 18 | ||
954 | 19 | <div class="yui-g"> | 19 | <div class="yui-g"> |
955 | 20 | <div id="ref-relations" class="portlet"> | ||
956 | 21 | <tal:ref-pending-merges | ||
957 | 22 | replace="structure context/@@++ref-pending-merges" /> | ||
958 | 23 | </div> | ||
959 | 24 | </div> | ||
960 | 25 | |||
961 | 26 | <div class="yui-g"> | ||
962 | 20 | <div id="ref-info" class="portlet"> | 27 | <div id="ref-info" class="portlet"> |
963 | 21 | <h2>Branch information</h2> | 28 | <h2>Branch information</h2> |
964 | 22 | <div class="two-column-list"> | 29 | <div class="two-column-list"> |
965 | 23 | 30 | ||
966 | === added file 'lib/lp/code/templates/gitref-pending-merges.pt' | |||
967 | --- lib/lp/code/templates/gitref-pending-merges.pt 1970-01-01 00:00:00 +0000 | |||
968 | +++ lib/lp/code/templates/gitref-pending-merges.pt 2015-04-29 12:39:54 +0000 | |||
969 | @@ -0,0 +1,43 @@ | |||
970 | 1 | <div | ||
971 | 2 | xmlns:tal="http://xml.zope.org/namespaces/tal" | ||
972 | 3 | xmlns:metal="http://xml.zope.org/namespaces/metal" | ||
973 | 4 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" | ||
974 | 5 | tal:define=" | ||
975 | 6 | context_menu view/context/menu:context; | ||
976 | 7 | features request/features" | ||
977 | 8 | tal:condition="view/show_merge_links"> | ||
978 | 9 | |||
979 | 10 | <h3>Branch merges</h3> | ||
980 | 11 | <div id="merge-links" | ||
981 | 12 | class="actions"> | ||
982 | 13 | <div id="merge-summary"> | ||
983 | 14 | |||
984 | 15 | <div id="landing-candidates" | ||
985 | 16 | tal:condition="view/landing_candidates"> | ||
986 | 17 | <img src="/@@/merge-proposal-icon" /> | ||
987 | 18 | <a href="+activereviews" tal:content="structure view/landing_candidate_count_text"> | ||
988 | 19 | 1 branch | ||
989 | 20 | </a> | ||
990 | 21 | proposed for merging into this one. | ||
991 | 22 | |||
992 | 23 | </div> | ||
993 | 24 | |||
994 | 25 | <div id="dependent-landings" tal:condition="view/dependent_landings"> | ||
995 | 26 | <img src="/@@/merge-proposal-icon" /> | ||
996 | 27 | <a href="+merges" tal:content="structure view/dependent_landing_count_text"> | ||
997 | 28 | 1 branch | ||
998 | 29 | </a> | ||
999 | 30 | dependent on this one. | ||
1000 | 31 | </div> | ||
1001 | 32 | |||
1002 | 33 | <div id="landing-targets" tal:condition="view/landing_targets"> | ||
1003 | 34 | <tal:landing-candidates repeat="mergeproposal view/landing_targets"> | ||
1004 | 35 | <tal:merge-fragment | ||
1005 | 36 | tal:replace="structure mergeproposal/@@+summary-fragment"/> | ||
1006 | 37 | </tal:landing-candidates> | ||
1007 | 38 | </div> | ||
1008 | 39 | |||
1009 | 40 | </div> | ||
1010 | 41 | </div> | ||
1011 | 42 | |||
1012 | 43 | </div> | ||
1013 | 0 | 44 | ||
1014 | === modified file 'lib/lp/testing/factory.py' | |||
1015 | --- lib/lp/testing/factory.py 2015-04-22 16:11:40 +0000 | |||
1016 | +++ lib/lp/testing/factory.py 2015-04-29 12:39:54 +0000 | |||
1017 | @@ -1747,7 +1747,8 @@ | |||
1018 | 1747 | u"type": GitObjectType.COMMIT, | 1747 | u"type": GitObjectType.COMMIT, |
1019 | 1748 | } | 1748 | } |
1020 | 1749 | for path in paths} | 1749 | for path in paths} |
1022 | 1750 | return repository.createOrUpdateRefs(refs_info, get_objects=True) | 1750 | return removeSecurityProxy(repository).createOrUpdateRefs( |
1023 | 1751 | refs_info, get_objects=True) | ||
1024 | 1751 | 1752 | ||
1025 | 1752 | def makeBug(self, target=None, owner=None, bug_watch_url=None, | 1753 | def makeBug(self, target=None, owner=None, bug_watch_url=None, |
1026 | 1753 | information_type=None, date_closed=None, title=None, | 1754 | information_type=None, date_closed=None, title=None, |
One bit of useless code, otherwise looks good.