Merge lp:~bryce/launchpad/lp-617695-linkui into lp:launchpad/db-devel
- lp-617695-linkui
- Merge into db-devel
Status: | Merged |
---|---|
Merge reported by: | Curtis Hovey |
Merged at revision: | not available |
Proposed branch: | lp:~bryce/launchpad/lp-617695-linkui |
Merge into: | lp:launchpad/db-devel |
Diff against target: |
541 lines (+329/-10) (has conflicts) 12 files modified
lib/canonical/launchpad/interfaces/_schema_circular_imports.py (+3/-0) lib/canonical/launchpad/scripts/bzremotecomponentfinder.py (+0/-1) lib/lp/bugs/browser/bugtracker.py (+103/-4) lib/lp/bugs/browser/configure.zcml (+9/-0) lib/lp/bugs/browser/tests/test_bugtracker_component.py (+105/-0) lib/lp/bugs/browser/widgets/bugtask.py (+7/-0) lib/lp/bugs/configure.zcml (+1/-0) lib/lp/bugs/interfaces/bugtracker.py (+22/-1) lib/lp/bugs/model/bugtracker.py (+28/-4) lib/lp/bugs/templates/bugtracker-index.pt (+3/-0) lib/lp/bugs/templates/bugtracker-portlet-components.pt (+37/-0) lib/lp/bugs/tests/test_bugtracker_components.py (+11/-0) Text conflict in lib/lp/bugs/browser/bugtracker.py |
To merge this branch: | bzr merge lp:~bryce/launchpad/lp-617695-linkui |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Steve Kowalik (community) | Needs Resubmitting | ||
Curtis Hovey (community) | code | Needs Information | |
Tim Penhey (community) | Abstain | ||
Bryce Harrington (community) | Needs Resubmitting | ||
Review via email: mp+39679@code.launchpad.net |
Commit message
Implements UI for displaying components registered at a remote bug tracker, and UI for linking a distro source package to a component.
Description of the change
This branch contains the UI changes to implement display of components and component groups (products) available from a given bug tracker, along with an edit form for linking components to particular source packages.
A preimplementation call was done with Deryck about the design and implementation of the ui and code. He also gave some advice on the tests while we were at UDS.
Lint issues have been checked and fixed.
I test this using the command:
./bin/test -t browser.
I've also run this through ec2 test. The test run failed due to windmill issues; but, these failures seem unrelated to my branch.
Tim Penhey (thumper) wrote : | # |
Why assume ubuntu as the distributions? Do we need to define the distribution at all? Surely the sourcepackagename itself is enough?
Also as a general rule, we should be doing as little as possible in the view code, so the code to link the component to the sourcepackage should really go in the bugtracker code. Warning bells went off when you overloaded updateContextFr
Steve Kowalik (stevenk) wrote : | # |
Bryce is right, SPN isn't enough, since likely the same package exists in Debian, which why I didn't question that bit.
Bryce Harrington (bryce) wrote : | # |
SteveK, thanks I'll look into dropping the unittest stuff.
Tim, yeah the SPN is used to manually construct a distribution_
But yeah, I can move the linking code into the model. I'd considered that but opted to be lazy. ;-)
Curtis Hovey (sinzui) wrote : | # |
Hi Bryce.
How do I see this feature? I added (default, 0, bugtracker_
Tim Penhey (thumper) wrote : | # |
The point that I was trying to make is that a component relates to a source package. By that I mean something like "gstreamer". It doesn't really matter that source package is in ubuntu / debian / distroX (to me), if the component relates to the package "gstreamer" then surely that should be enough.
I wasn't challenging the design of our "SourcePackage" object. Only trying to understand what we are fundamentally trying to model.
Bryce Harrington (bryce) wrote : | # |
@Curtis, running cronscripts/
For manual reviewing of the UI elements, after I ran that to download the gnome bugzilla data, I configured alsa-utils as using the gnome bug tracker as its upstream tracker, and linked one of the gnome packages like zenity to verify the linking behavior was right.
Bryce Harrington (bryce) wrote : | # |
@Tim, I've promoted the Store code from the browser to a proper interface routine, and generalized it a bit so it'll be useful in and of itself. The remaining code in updateContextFr
Regarding the distro source package complexity, yeah I get what you mean and I agree linking to a 'source package' object directly would be a cleaner design. A source package is going to be linked to the same component regardless of the distribution, and it would be better if we could model it without referencing distros at all. However, the way source packages are implemented in launchpad is kind of wonky. There isn't really a 'source package' database table and storm object that this could be hung off of. There *is* a source_package element in the API, so you would think you could link to that, however:
<deryck> bryceh, no, it's ISourcePackage, which is poorly named. That's the package in a distro series.
<deryck> bryceh, so you're only options are to hang this on either SourcePackage (distro series) or DistributionSou
bryceh, right
bryceh, sourcepackagename is just table magic really to enable re-use of names across distros.
bryceh, but you can't really link to it the same way as the other objects. You could, but it's not really used the same way as the other objects.
...
<deryck> bryceh, so you've done it right. DSP is actually the distro source package, as the name suggests, [while] SP would be better named "distro series source package."
Bryce Harrington (bryce) wrote : | # |
Hey, just noticed this branch is still not merged, although there's been no further review comments since my last one. Is there anything still needing done here? Looks like it got stalled out in review...
With me back on the distro team my time's rather short, so if anyone could shepherd this the rest of the way into launchpad it'd be really appreciated.
(I don't really need this UI for my own purposes, I just use the API, but would be nice to have this feature for other folks in general.)
Bryce Harrington (bryce) wrote : | # |
Hello? Anyone?
Curtis Hovey (sinzui) wrote : | # |
Hi Bryce.
I apologise for not following up on this. I will do the code/UI review in a few hours.
Tim Penhey (thumper) wrote : | # |
With me now out of the LP team, I'll leave this to SteveK to do with as he likes.
Curtis Hovey (sinzui) wrote : | # |
> === modified file 'lib/canonical/
Hi Bryce.
I have some questions about some of the methods and properties. I have some suggestions that I think you can make in less than an hour to improve this branch.
> --- lib/canonical/
> +++ lib/canonical/
I think you need to remerge. This module was moved about 4 months ago. I
expect this to be lp/bugs/
> @@ -495,6 +495,25 @@
> return distribution
>
>
> +class UbuntuSourcePac
> + BugTaskSourcePa
> + """Package widget where the distribution can be assumed as Ubuntu
> +
> + This widgets works the same as `BugTaskSourceP
> + except that it assumes the distribution is 'ubuntu'.
> + """
> + distribution_name = "ubuntu"
> +
> + def getDistribution
> + """See `BugTaskSourceP
> + distribution = getUtility(
> + self.distributi
> + if distribution is None:
> + raise UnexpectedFormData(
> + "No such distribution: %s" % self.distributi
> + return distribution
I think this could be expressed differently. We are not /assuming/ that
the distro is Ubuntu, it is. To ensure it is this, the whole class would
look like:
class UbuntuSourcePac
"""A widget to select Ubuntu packages."""
def getDistribution
"""See `BugTaskSourceP
return getUtility(
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> ...
> @@ -231,6 +237,14 @@
> return shortlist(
> self.context.
>
> + @property
> + def related_
> + """Return all component groups and components
> +
> + This property was created for the Related components portlet in
> + the bug tracker's page.
> + """
> + return self.context.
Properties docstring do avoid "return" because they are not intended to be
methods. The call site is not interesting. The docstring could be.
"""All component groups and components."""
> @@ -449,16 +463,111 @@
...
> +class BugTrackerEditC
> + """Provides editing form for setting source packages for components.
> +
> + In this class we assume that bug tracker components are always
> + linked to source packages in the Ubuntu distribution.
> + """
> +
> + schema = IBugTrackerComp
> + custom_
> + UbuntuSourcePac
> +
> + @property
> + def page_title(self):
> + return smartquote(
> + u'Link a distribution source package to the %s component'
> + % self.context.name)
This is an odd breadcrum...
Steve Kowalik (stevenk) wrote : | # |
I think this MP should be re-submitted against devel, I suspect the reasoning for targeting db-devel is long past.
Bryce Harrington (bryce) wrote : | # |
>> + @property
>> + def page_title(self):
>> + return smartquote(
>> + u'Link a distribution source package to the %s component'
>> + % self.context.name)
>
> This is an odd breadcrumb/
> while this attribute is meant to be tearse so that it fits in the breadcrumbs:
>
> Bug trackers > AbiSource bug tracker > Link component
I don't understand this review comment. I modeled the page_title for this off of the BugTrackerEditView classes' page_title() which looks like this:
@property
def page_title(self):
return smartquote(
Can you elaborate on how you think it should be worded?
Curtis Hovey (sinzui) wrote : | # |
Oh no! I did not see any activity on this. I pulled the branch down and have is all cleaned up. I still cannot see the UI. My branch based on yours is
lp:~sinzui/launchpad/remote-bugtracker-components-ui-0
You can see my commits are only hours ahead of yours.
Lets talk about this.
Bryce Harrington (bryce) wrote : | # |
Regarding viewing the UI:
For manual testing of this feature locally, I first ran cronscripts/
Next, I configured the alsa-utils project as using the gnome bugzilla, at https:/
Navigating to https:/
I logged out, and verified as anonymous that the edit links don't show up on https:/
Preview Diff
1 | === modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py' | |||
2 | --- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2011-05-31 12:17:52 +0000 | |||
3 | +++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2011-06-02 23:32:14 +0000 | |||
4 | @@ -624,6 +624,9 @@ | |||
5 | 624 | IBugTracker, 'addRemoteComponentGroup', IBugTrackerComponentGroup) | 624 | IBugTracker, 'addRemoteComponentGroup', IBugTrackerComponentGroup) |
6 | 625 | patch_collection_return_type( | 625 | patch_collection_return_type( |
7 | 626 | IBugTracker, 'getAllRemoteComponentGroups', IBugTrackerComponentGroup) | 626 | IBugTracker, 'getAllRemoteComponentGroups', IBugTrackerComponentGroup) |
8 | 627 | patch_entry_return_type( | ||
9 | 628 | IBugTracker, 'getRemoteComponentForDistroSourcePackage', | ||
10 | 629 | IBugTrackerComponent) | ||
11 | 627 | 630 | ||
12 | 628 | ## IBugTrackerComponent | 631 | ## IBugTrackerComponent |
13 | 629 | patch_reference_property( | 632 | patch_reference_property( |
14 | 630 | 633 | ||
15 | === modified file 'lib/canonical/launchpad/scripts/bzremotecomponentfinder.py' | |||
16 | --- lib/canonical/launchpad/scripts/bzremotecomponentfinder.py 2010-12-20 03:21:03 +0000 | |||
17 | +++ lib/canonical/launchpad/scripts/bzremotecomponentfinder.py 2011-06-02 23:32:14 +0000 | |||
18 | @@ -117,7 +117,6 @@ | |||
19 | 117 | self.static_bugzilla_text = static_bugzilla_text | 117 | self.static_bugzilla_text = static_bugzilla_text |
20 | 118 | 118 | ||
21 | 119 | def getRemoteProductsAndComponents(self, bugtracker_name=None): | 119 | def getRemoteProductsAndComponents(self, bugtracker_name=None): |
22 | 120 | """""" | ||
23 | 121 | lp_bugtrackers = getUtility(IBugTrackerSet) | 120 | lp_bugtrackers = getUtility(IBugTrackerSet) |
24 | 122 | if bugtracker_name is not None: | 121 | if bugtracker_name is not None: |
25 | 123 | lp_bugtrackers = [ | 122 | lp_bugtrackers = [ |
26 | 124 | 123 | ||
27 | === modified file 'lib/lp/bugs/browser/bugtracker.py' | |||
28 | --- lib/lp/bugs/browser/bugtracker.py 2011-05-27 21:12:25 +0000 | |||
29 | +++ lib/lp/bugs/browser/bugtracker.py 2011-06-02 23:32:14 +0000 | |||
30 | @@ -10,6 +10,7 @@ | |||
31 | 10 | 'BugTrackerBreadcrumb', | 10 | 'BugTrackerBreadcrumb', |
32 | 11 | 'BugTrackerComponentGroupNavigation', | 11 | 'BugTrackerComponentGroupNavigation', |
33 | 12 | 'BugTrackerEditView', | 12 | 'BugTrackerEditView', |
34 | 13 | 'BugTrackerEditComponentView', | ||
35 | 13 | 'BugTrackerNavigation', | 14 | 'BugTrackerNavigation', |
36 | 14 | 'BugTrackerNavigationMenu', | 15 | 'BugTrackerNavigationMenu', |
37 | 15 | 'BugTrackerSetBreadcrumb', | 16 | 'BugTrackerSetBreadcrumb', |
38 | @@ -56,6 +57,16 @@ | |||
39 | 56 | from canonical.launchpad.webapp.breadcrumb import Breadcrumb | 57 | from canonical.launchpad.webapp.breadcrumb import Breadcrumb |
40 | 57 | from canonical.launchpad.webapp.menu import NavigationMenu | 58 | from canonical.launchpad.webapp.menu import NavigationMenu |
41 | 58 | from canonical.lazr.utils import smartquote | 59 | from canonical.lazr.utils import smartquote |
42 | 60 | <<<<<<< TREE | ||
43 | 61 | ======= | ||
44 | 62 | from canonical.widgets import ( | ||
45 | 63 | DelimitedListWidget, | ||
46 | 64 | LaunchpadRadioWidget, | ||
47 | 65 | ) | ||
48 | 66 | from canonical.widgets.bugtask import ( | ||
49 | 67 | UbuntuSourcePackageNameWidget, | ||
50 | 68 | ) | ||
51 | 69 | >>>>>>> MERGE-SOURCE | ||
52 | 59 | from lp.app.browser.launchpadform import ( | 70 | from lp.app.browser.launchpadform import ( |
53 | 60 | action, | 71 | action, |
54 | 61 | custom_widget, | 72 | custom_widget, |
55 | @@ -71,7 +82,13 @@ | |||
56 | 71 | IBugTrackerComponentGroup, | 82 | IBugTrackerComponentGroup, |
57 | 72 | IBugTrackerSet, | 83 | IBugTrackerSet, |
58 | 73 | IRemoteBug, | 84 | IRemoteBug, |
59 | 85 | <<<<<<< TREE | ||
60 | 86 | ======= | ||
61 | 87 | IBugTrackerComponent, | ||
62 | 88 | IBugTrackerComponentGroup, | ||
63 | 89 | >>>>>>> MERGE-SOURCE | ||
64 | 74 | ) | 90 | ) |
65 | 91 | from lp.registry.interfaces.distribution import IDistributionSet | ||
66 | 75 | from lp.services.propertycache import cachedproperty | 92 | from lp.services.propertycache import cachedproperty |
67 | 76 | 93 | ||
68 | 77 | # A set of bug tracker types for which there can only ever be one bug | 94 | # A set of bug tracker types for which there can only ever be one bug |
69 | @@ -227,6 +244,11 @@ | |||
70 | 227 | return shortlist(chain(self.context.projects, | 244 | return shortlist(chain(self.context.projects, |
71 | 228 | self.context.products), 100) | 245 | self.context.products), 100) |
72 | 229 | 246 | ||
73 | 247 | @property | ||
74 | 248 | def related_component_groups(self): | ||
75 | 249 | """All component groups and components.""" | ||
76 | 250 | return self.context.getAllRemoteComponentGroups() | ||
77 | 251 | |||
78 | 230 | 252 | ||
79 | 231 | BUG_TRACKER_ACTIVE_VOCABULARY = SimpleVocabulary.fromItems( | 253 | BUG_TRACKER_ACTIVE_VOCABULARY = SimpleVocabulary.fromItems( |
80 | 232 | [('On', True), ('Off', False)]) | 254 | [('On', True), ('Off', False)]) |
81 | @@ -445,16 +467,93 @@ | |||
82 | 445 | return RemoteBug(self.context, remotebug, bugs) | 467 | return RemoteBug(self.context, remotebug, bugs) |
83 | 446 | 468 | ||
84 | 447 | @stepthrough("+components") | 469 | @stepthrough("+components") |
87 | 448 | def component_groups(self, name): | 470 | def component_groups(self, id): |
88 | 449 | return self.context.getRemoteComponentGroup(name) | 471 | # Navigate by id (component group name should work too) |
89 | 472 | return self.context.getRemoteComponentGroup(id) | ||
90 | 473 | |||
91 | 474 | |||
92 | 475 | class BugTrackerEditComponentView(LaunchpadEditFormView): | ||
93 | 476 | """Provides editing form for setting source packages for components. | ||
94 | 477 | |||
95 | 478 | In this class we assume that bug tracker components are always | ||
96 | 479 | linked to source packages in the Ubuntu distribution. | ||
97 | 480 | """ | ||
98 | 481 | |||
99 | 482 | schema = IBugTrackerComponent | ||
100 | 483 | custom_widget('sourcepackagename', | ||
101 | 484 | UbuntuSourcePackageNameWidget) | ||
102 | 485 | |||
103 | 486 | @property | ||
104 | 487 | def page_title(self): | ||
105 | 488 | return smartquote( | ||
106 | 489 | u'Link a distribution source package to the %s component' | ||
107 | 490 | % self.context.name) | ||
108 | 491 | |||
109 | 492 | @property | ||
110 | 493 | def field_names(self): | ||
111 | 494 | field_names = ['sourcepackagename'] | ||
112 | 495 | return field_names | ||
113 | 496 | |||
114 | 497 | @property | ||
115 | 498 | def initial_values(self): | ||
116 | 499 | """See `LaunchpadFormView.`""" | ||
117 | 500 | field_values = {} | ||
118 | 501 | for name in self.field_names: | ||
119 | 502 | if name == 'sourcepackagename': | ||
120 | 503 | pkg = self.context.distro_source_package | ||
121 | 504 | if pkg is not None: | ||
122 | 505 | field_values['sourcepackagename'] = pkg.name | ||
123 | 506 | else: | ||
124 | 507 | field_values['sourcepackagename'] = "" | ||
125 | 508 | else: | ||
126 | 509 | field_values[name] = getattr(self.context, name) | ||
127 | 510 | |||
128 | 511 | return field_values | ||
129 | 512 | |||
130 | 513 | def updateContextFromData(self, data, context=None): | ||
131 | 514 | """Link component to specified distro source package. | ||
132 | 515 | |||
133 | 516 | Get the user-provided source package name from the form widget, | ||
134 | 517 | look it up in Ubuntu to retrieve the distro_source_package | ||
135 | 518 | object, and link it to this component. | ||
136 | 519 | """ | ||
137 | 520 | component = context or self.context | ||
138 | 521 | sourcepackagename = data['sourcepackagename'] | ||
139 | 522 | distribution = self.widgets['sourcepackagename'].getDistribution() | ||
140 | 523 | dsp = distribution.getSourcePackage(sourcepackagename) | ||
141 | 524 | bug_tracker = self.context.component_group.bug_tracker | ||
142 | 525 | |||
143 | 526 | # Has this source package already been assigned to a component? | ||
144 | 527 | link_comp = bug_tracker.getRemoteComponentForDistroSourcePackageName( | ||
145 | 528 | distribution, sourcepackagename) | ||
146 | 529 | if link_comp is not None: | ||
147 | 530 | self.request.response.addNotification( | ||
148 | 531 | "The %s source package is already linked to %s:%s in %s" %( | ||
149 | 532 | sourcepackagename.name, link_comp.component_group.name, | ||
150 | 533 | link_comp.name, distribution.name)) | ||
151 | 534 | return | ||
152 | 535 | |||
153 | 536 | component.distro_source_package = dsp | ||
154 | 537 | self.request.response.addNotification( | ||
155 | 538 | "%s:%s is now linked to the %s source package in %s" %( | ||
156 | 539 | component.component_group.name, component.name, | ||
157 | 540 | sourcepackagename.name, distribution.name)) | ||
158 | 541 | |||
159 | 542 | @action('Save Changes', name='save') | ||
160 | 543 | def save_action(self, action, data): | ||
161 | 544 | """Update the component with the form data.""" | ||
162 | 545 | self.updateContextFromData(data) | ||
163 | 546 | |||
164 | 547 | self.next_url = canonical_url( | ||
165 | 548 | self.context.component_group.bug_tracker) | ||
166 | 450 | 549 | ||
167 | 451 | 550 | ||
168 | 452 | class BugTrackerComponentGroupNavigation(Navigation): | 551 | class BugTrackerComponentGroupNavigation(Navigation): |
169 | 453 | 552 | ||
170 | 454 | usedfor = IBugTrackerComponentGroup | 553 | usedfor = IBugTrackerComponentGroup |
171 | 455 | 554 | ||
174 | 456 | def traverse(self, name): | 555 | def traverse(self, id): |
175 | 457 | return self.context.getComponent(name) | 556 | return self.context.getComponent(id) |
176 | 458 | 557 | ||
177 | 459 | 558 | ||
178 | 460 | class BugTrackerSetBreadcrumb(Breadcrumb): | 559 | class BugTrackerSetBreadcrumb(Breadcrumb): |
179 | 461 | 560 | ||
180 | === modified file 'lib/lp/bugs/browser/configure.zcml' | |||
181 | --- lib/lp/bugs/browser/configure.zcml 2011-05-29 21:18:09 +0000 | |||
182 | +++ lib/lp/bugs/browser/configure.zcml 2011-06-02 23:32:14 +0000 | |||
183 | @@ -819,6 +819,12 @@ | |||
184 | 819 | path_expression="name" | 819 | path_expression="name" |
185 | 820 | attribute_to_parent="component_group" | 820 | attribute_to_parent="component_group" |
186 | 821 | rootsite="bugs"/> | 821 | rootsite="bugs"/> |
187 | 822 | <browser:page | ||
188 | 823 | name="+edit" | ||
189 | 824 | for="lp.bugs.interfaces.bugtracker.IBugTrackerComponent" | ||
190 | 825 | class="lp.bugs.browser.bugtracker.BugTrackerEditComponentView" | ||
191 | 826 | permission="launchpad.AnyPerson" | ||
192 | 827 | template="../../app/templates/generic-edit.pt"/> | ||
193 | 822 | <browser:pages | 828 | <browser:pages |
194 | 823 | for="lp.bugs.interfaces.bugtracker.IBugTracker" | 829 | for="lp.bugs.interfaces.bugtracker.IBugTracker" |
195 | 824 | class="lp.bugs.browser.bugtracker.BugTrackerView" | 830 | class="lp.bugs.browser.bugtracker.BugTrackerView" |
196 | @@ -830,6 +836,9 @@ | |||
197 | 830 | name="+portlet-details" | 836 | name="+portlet-details" |
198 | 831 | template="../templates/bugtracker-portlet-details.pt"/> | 837 | template="../templates/bugtracker-portlet-details.pt"/> |
199 | 832 | <browser:page | 838 | <browser:page |
200 | 839 | name="+portlet-components" | ||
201 | 840 | template="../templates/bugtracker-portlet-components.pt"/> | ||
202 | 841 | <browser:page | ||
203 | 833 | name="+portlet-projects" | 842 | name="+portlet-projects" |
204 | 834 | template="../templates/bugtracker-portlet-projects.pt"/> | 843 | template="../templates/bugtracker-portlet-projects.pt"/> |
205 | 835 | <browser:page | 844 | <browser:page |
206 | 836 | 845 | ||
207 | === added file 'lib/lp/bugs/browser/tests/test_bugtracker_component.py' | |||
208 | --- lib/lp/bugs/browser/tests/test_bugtracker_component.py 1970-01-01 00:00:00 +0000 | |||
209 | +++ lib/lp/bugs/browser/tests/test_bugtracker_component.py 2011-06-02 23:32:14 +0000 | |||
210 | @@ -0,0 +1,105 @@ | |||
211 | 1 | # Copyright 2010 Canonical Ltd. This software is licensed under the | ||
212 | 2 | # GNU Affero General Public License version (see the file LICENSE). | ||
213 | 3 | |||
214 | 4 | """Unit tests for linking bug tracker components to source packages.""" | ||
215 | 5 | |||
216 | 6 | __metaclass__ = type | ||
217 | 7 | |||
218 | 8 | from zope.component import getUtility | ||
219 | 9 | |||
220 | 10 | from canonical.testing.layers import DatabaseFunctionalLayer | ||
221 | 11 | from lp.registry.interfaces.distribution import IDistributionSet | ||
222 | 12 | from lp.testing import ( | ||
223 | 13 | login_person, | ||
224 | 14 | TestCaseWithFactory, | ||
225 | 15 | ) | ||
226 | 16 | from lp.testing.views import create_initialized_view | ||
227 | 17 | |||
228 | 18 | |||
229 | 19 | class TestBugTrackerEditComponentView(TestCaseWithFactory): | ||
230 | 20 | |||
231 | 21 | layer = DatabaseFunctionalLayer | ||
232 | 22 | |||
233 | 23 | def setUp(self): | ||
234 | 24 | super(TestBugTrackerEditComponentView, self).setUp() | ||
235 | 25 | regular_user = self.factory.makePerson() | ||
236 | 26 | login_person(regular_user) | ||
237 | 27 | |||
238 | 28 | self.bug_tracker = self.factory.makeBugTracker() | ||
239 | 29 | self.comp_group = self.factory.makeBugTrackerComponentGroup( | ||
240 | 30 | u'alpha', self.bug_tracker) | ||
241 | 31 | |||
242 | 32 | def _makeForm(self, sourcepackage): | ||
243 | 33 | if sourcepackage is None: | ||
244 | 34 | name = '' | ||
245 | 35 | else: | ||
246 | 36 | name = sourcepackage.name | ||
247 | 37 | return { | ||
248 | 38 | 'field.sourcepackagename': name, | ||
249 | 39 | 'field.actions.save': 'Save', | ||
250 | 40 | } | ||
251 | 41 | |||
252 | 42 | def test_view_attributes(self): | ||
253 | 43 | component = self.factory.makeBugTrackerComponent( | ||
254 | 44 | u'Example', self.comp_group) | ||
255 | 45 | distro = getUtility(IDistributionSet).getByName('ubuntu') | ||
256 | 46 | package = self.factory.makeDistributionSourcePackage( | ||
257 | 47 | sourcepackagename='example', distribution=distro) | ||
258 | 48 | form = self._makeForm(package) | ||
259 | 49 | view = create_initialized_view( | ||
260 | 50 | component, name='+edit', form=form) | ||
261 | 51 | label = 'Link a distribution source package to the Example component' | ||
262 | 52 | self.assertEqual(label, view.page_title) | ||
263 | 53 | fields = ['sourcepackagename'] | ||
264 | 54 | self.assertEqual(fields, view.field_names) | ||
265 | 55 | |||
266 | 56 | def test_linking(self): | ||
267 | 57 | component = self.factory.makeBugTrackerComponent( | ||
268 | 58 | u'Example', self.comp_group) | ||
269 | 59 | distro = getUtility(IDistributionSet).getByName('ubuntu') | ||
270 | 60 | package = self.factory.makeDistributionSourcePackage( | ||
271 | 61 | sourcepackagename='example', distribution=distro) | ||
272 | 62 | |||
273 | 63 | self.assertIs(None, component.distro_source_package) | ||
274 | 64 | form = self._makeForm(package) | ||
275 | 65 | view = create_initialized_view( | ||
276 | 66 | component, name='+edit', form=form) | ||
277 | 67 | self.assertEqual([], view.errors) | ||
278 | 68 | |||
279 | 69 | notifications = view.request.response.notifications | ||
280 | 70 | #self.assertEqual(1, len(notifications)) | ||
281 | 71 | self.assertEqual(component.distro_source_package, package) | ||
282 | 72 | expected = ( | ||
283 | 73 | u"alpha:Example is now linked to the example " | ||
284 | 74 | "source package in ubuntu") | ||
285 | 75 | self.assertEqual(expected, notifications.pop().message) | ||
286 | 76 | |||
287 | 77 | def test_cannot_doublelink_sourcepackages(self): | ||
288 | 78 | # Two components try linking to same package | ||
289 | 79 | component_a = self.factory.makeBugTrackerComponent( | ||
290 | 80 | u'a', self.comp_group) | ||
291 | 81 | component_b = self.factory.makeBugTrackerComponent( | ||
292 | 82 | u'b', self.comp_group) | ||
293 | 83 | distro = getUtility(IDistributionSet).getByName('ubuntu') | ||
294 | 84 | package = self.factory.makeDistributionSourcePackage( | ||
295 | 85 | sourcepackagename='example', distribution=distro) | ||
296 | 86 | |||
297 | 87 | form = self._makeForm(package) | ||
298 | 88 | view = create_initialized_view( | ||
299 | 89 | component_a, name='+edit', form=form) | ||
300 | 90 | notifications = view.request.response.notifications | ||
301 | 91 | self.assertEqual([], view.errors) | ||
302 | 92 | self.assertEqual(1, len(notifications)) | ||
303 | 93 | self.assertEqual(package, component_a.distro_source_package) | ||
304 | 94 | |||
305 | 95 | form = self._makeForm(package) | ||
306 | 96 | view = create_initialized_view( | ||
307 | 97 | component_b, name='+edit', form=form) | ||
308 | 98 | self.assertIs(None, component_b.distro_source_package) | ||
309 | 99 | self.assertEqual([], view.errors) | ||
310 | 100 | notifications = view.request.response.notifications | ||
311 | 101 | self.assertEqual(1, len(notifications)) | ||
312 | 102 | expected = ( | ||
313 | 103 | u"""The example source package is already linked to """ | ||
314 | 104 | """alpha:a in ubuntu""") | ||
315 | 105 | self.assertEqual(expected, notifications.pop().message) | ||
316 | 0 | 106 | ||
317 | === modified file 'lib/lp/bugs/browser/widgets/bugtask.py' | |||
318 | --- lib/lp/bugs/browser/widgets/bugtask.py 2011-02-02 15:43:31 +0000 | |||
319 | +++ lib/lp/bugs/browser/widgets/bugtask.py 2011-06-02 23:32:14 +0000 | |||
320 | @@ -527,6 +527,13 @@ | |||
321 | 527 | return distribution | 527 | return distribution |
322 | 528 | 528 | ||
323 | 529 | 529 | ||
324 | 530 | class UbuntuSourcePackageNameWidget(BugTaskSourcePackageNameWidget): | ||
325 | 531 | """A widget to select Ubuntu packages.""" | ||
326 | 532 | def getDistribution(self): | ||
327 | 533 | """See `BugTaskSourcePackageNameWidget`""" | ||
328 | 534 | return getUtility(ILaunchpadCelebrities).ubuntu | ||
329 | 535 | |||
330 | 536 | |||
331 | 530 | class AssigneeDisplayWidget(BrowserWidget): | 537 | class AssigneeDisplayWidget(BrowserWidget): |
332 | 531 | """A widget for displaying an assignee.""" | 538 | """A widget for displaying an assignee.""" |
333 | 532 | 539 | ||
334 | 533 | 540 | ||
335 | === modified file 'lib/lp/bugs/configure.zcml' | |||
336 | --- lib/lp/bugs/configure.zcml 2011-05-10 15:14:10 +0000 | |||
337 | +++ lib/lp/bugs/configure.zcml 2011-06-02 23:32:14 +0000 | |||
338 | @@ -394,6 +394,7 @@ | |||
339 | 394 | getBugFilingAndSearchLinks | 394 | getBugFilingAndSearchLinks |
340 | 395 | getBugsWatching | 395 | getBugsWatching |
341 | 396 | getLinkedPersonByName | 396 | getLinkedPersonByName |
342 | 397 | getRemoteComponentForDistroSourcePackageName | ||
343 | 397 | getRemoteComponentGroup | 398 | getRemoteComponentGroup |
344 | 398 | has_lp_plugin | 399 | has_lp_plugin |
345 | 399 | id | 400 | id |
346 | 400 | 401 | ||
347 | === modified file 'lib/lp/bugs/interfaces/bugtracker.py' | |||
348 | --- lib/lp/bugs/interfaces/bugtracker.py 2011-02-23 20:26:53 +0000 | |||
349 | +++ lib/lp/bugs/interfaces/bugtracker.py 2011-06-02 23:32:14 +0000 | |||
350 | @@ -390,7 +390,8 @@ | |||
351 | 390 | 390 | ||
352 | 391 | @operation_parameters( | 391 | @operation_parameters( |
353 | 392 | component_group_name=TextLine( | 392 | component_group_name=TextLine( |
355 | 393 | title=u"The name of the remote component group", required=True)) | 393 | title=u"The name of the remote component group", |
356 | 394 | required=True)) | ||
357 | 394 | @operation_returns_entry(Interface) | 395 | @operation_returns_entry(Interface) |
358 | 395 | @export_read_operation() | 396 | @export_read_operation() |
359 | 396 | def getRemoteComponentGroup(component_group_name): | 397 | def getRemoteComponentGroup(component_group_name): |
360 | @@ -399,6 +400,22 @@ | |||
361 | 399 | :param component_group_name: Name of the component group to retrieve. | 400 | :param component_group_name: Name of the component group to retrieve. |
362 | 400 | """ | 401 | """ |
363 | 401 | 402 | ||
364 | 403 | @operation_parameters( | ||
365 | 404 | distro_name=TextLine( | ||
366 | 405 | title=u"The name of the distribution for the source package", | ||
367 | 406 | required=True), | ||
368 | 407 | source_package_name=TextLine( | ||
369 | 408 | title=u"The name of the source package to look for", | ||
370 | 409 | required=True)) | ||
371 | 410 | @operation_returns_entry(Interface) | ||
372 | 411 | @export_read_operation() | ||
373 | 412 | def getRemoteComponentForDistroSourcePackage( | ||
374 | 413 | distro_name, source_package_name): | ||
375 | 414 | """Returns the component linked to this source package, if any. | ||
376 | 415 | |||
377 | 416 | If no components have been linked, returns value of None. | ||
378 | 417 | """ | ||
379 | 418 | |||
380 | 402 | 419 | ||
381 | 403 | class IBugTrackerSet(Interface): | 420 | class IBugTrackerSet(Interface): |
382 | 404 | """A set of IBugTracker's. | 421 | """A set of IBugTracker's. |
383 | @@ -538,6 +555,10 @@ | |||
384 | 538 | title=_('Name'), | 555 | title=_('Name'), |
385 | 539 | description=_("The name of a software component " | 556 | description=_("The name of a software component " |
386 | 540 | "as shown in Launchpad."))) | 557 | "as shown in Launchpad."))) |
387 | 558 | sourcepackagename = Choice( | ||
388 | 559 | title=_("Package"), required=False, vocabulary='SourcePackageName') | ||
389 | 560 | distribution = Choice( | ||
390 | 561 | title=_("Distribution"), required=False, vocabulary='Distribution') | ||
391 | 541 | 562 | ||
392 | 542 | distro_source_package = exported( | 563 | distro_source_package = exported( |
393 | 543 | Reference( | 564 | Reference( |
394 | 544 | 565 | ||
395 | === modified file 'lib/lp/bugs/model/bugtracker.py' | |||
396 | --- lib/lp/bugs/model/bugtracker.py 2011-05-28 04:09:11 +0000 | |||
397 | +++ lib/lp/bugs/model/bugtracker.py 2011-06-02 23:32:14 +0000 | |||
398 | @@ -83,6 +83,7 @@ | |||
399 | 83 | from lp.bugs.model.bugmessage import BugMessage | 83 | from lp.bugs.model.bugmessage import BugMessage |
400 | 84 | from lp.bugs.model.bugtrackerperson import BugTrackerPerson | 84 | from lp.bugs.model.bugtrackerperson import BugTrackerPerson |
401 | 85 | from lp.bugs.model.bugwatch import BugWatch | 85 | from lp.bugs.model.bugwatch import BugWatch |
402 | 86 | from lp.registry.interfaces.distribution import IDistributionSet | ||
403 | 86 | from lp.registry.interfaces.person import ( | 87 | from lp.registry.interfaces.person import ( |
404 | 87 | IPersonSet, | 88 | IPersonSet, |
405 | 88 | validate_public_person, | 89 | validate_public_person, |
406 | @@ -271,10 +272,17 @@ | |||
407 | 271 | 272 | ||
408 | 272 | if component_name is None: | 273 | if component_name is None: |
409 | 273 | return None | 274 | return None |
410 | 275 | elif component_name.isdigit(): | ||
411 | 276 | component_id = int(component_name) | ||
412 | 277 | return Store.of(self).find( | ||
413 | 278 | BugTrackerComponent, | ||
414 | 279 | BugTrackerComponent.id == component_id, | ||
415 | 280 | BugTrackerComponent.component_group == self.id).one() | ||
416 | 274 | else: | 281 | else: |
417 | 275 | return Store.of(self).find( | 282 | return Store.of(self).find( |
418 | 276 | BugTrackerComponent, | 283 | BugTrackerComponent, |
420 | 277 | (BugTrackerComponent.name == component_name)).one() | 284 | BugTrackerComponent.name == component_name, |
421 | 285 | BugTrackerComponent.component_group == self.id).one() | ||
422 | 278 | 286 | ||
423 | 279 | def addCustomComponent(self, component_name): | 287 | def addCustomComponent(self, component_name): |
424 | 280 | """Adds a component locally that isn't synced from a remote tracker | 288 | """Adds a component locally that isn't synced from a remote tracker |
425 | @@ -680,11 +688,27 @@ | |||
426 | 680 | """See `IBugTracker`.""" | 688 | """See `IBugTracker`.""" |
427 | 681 | component_group = None | 689 | component_group = None |
428 | 682 | store = IStore(BugTrackerComponentGroup) | 690 | store = IStore(BugTrackerComponentGroup) |
432 | 683 | component_group = store.find( | 691 | if component_group_name.isdigit(): |
433 | 684 | BugTrackerComponentGroup, | 692 | component_group_id = int(component_group_name) |
434 | 685 | name = component_group_name).one() | 693 | component_group = store.find( |
435 | 694 | BugTrackerComponentGroup, | ||
436 | 695 | id = component_group_id).one() | ||
437 | 696 | else: | ||
438 | 697 | component_group = store.find( | ||
439 | 698 | BugTrackerComponentGroup, | ||
440 | 699 | name = component_group_name).one() | ||
441 | 686 | return component_group | 700 | return component_group |
442 | 687 | 701 | ||
443 | 702 | def getRemoteComponentForDistroSourcePackageName( | ||
444 | 703 | self, distribution, sourcepackagename): | ||
445 | 704 | """See `IBugTracker`.""" | ||
446 | 705 | pkgs = Store.of(self).find( | ||
447 | 706 | BugTrackerComponent, | ||
448 | 707 | BugTrackerComponent.distribution == distribution.id, | ||
449 | 708 | BugTrackerComponent.source_package_name == | ||
450 | 709 | sourcepackagename.id).one() | ||
451 | 710 | return pkgs[0] | ||
452 | 711 | |||
453 | 688 | 712 | ||
454 | 689 | class BugTrackerSet: | 713 | class BugTrackerSet: |
455 | 690 | """Implements IBugTrackerSet for a container or set of BugTrackers, | 714 | """Implements IBugTrackerSet for a container or set of BugTrackers, |
456 | 691 | 715 | ||
457 | === modified file 'lib/lp/bugs/templates/bugtracker-index.pt' | |||
458 | --- lib/lp/bugs/templates/bugtracker-index.pt 2010-10-10 21:54:16 +0000 | |||
459 | +++ lib/lp/bugs/templates/bugtracker-index.pt 2011-06-02 23:32:14 +0000 | |||
460 | @@ -34,6 +34,9 @@ | |||
461 | 34 | <div class="yui-g"> | 34 | <div class="yui-g"> |
462 | 35 | <div class="first yui-u"> | 35 | <div class="first yui-u"> |
463 | 36 | <div tal:replace="structure context/@@+portlet-details" /> | 36 | <div tal:replace="structure context/@@+portlet-details" /> |
464 | 37 | <div tal:condition="features/bugtracker_components"> | ||
465 | 38 | <div tal:replace="structure context/@@+portlet-components" /> | ||
466 | 39 | </div> | ||
467 | 37 | </div> | 40 | </div> |
468 | 38 | <div class="yui-u"> | 41 | <div class="yui-u"> |
469 | 39 | <div tal:replace="structure context/@@+portlet-projects" /> | 42 | <div tal:replace="structure context/@@+portlet-projects" /> |
470 | 40 | 43 | ||
471 | === added file 'lib/lp/bugs/templates/bugtracker-portlet-components.pt' | |||
472 | --- lib/lp/bugs/templates/bugtracker-portlet-components.pt 1970-01-01 00:00:00 +0000 | |||
473 | +++ lib/lp/bugs/templates/bugtracker-portlet-components.pt 2011-06-02 23:32:14 +0000 | |||
474 | @@ -0,0 +1,37 @@ | |||
475 | 1 | <div | ||
476 | 2 | xmlns:tal="http://xml.zope.org/namespaces/tal" | ||
477 | 3 | xmlns:metal="http://xml.zope.org/namespaces/metal" | ||
478 | 4 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" | ||
479 | 5 | class="portlet" id="portlet-components"> | ||
480 | 6 | <h2>Components</h2> | ||
481 | 7 | <p> | ||
482 | 8 | You can link components from this bug tracker to their corresponding | ||
483 | 9 | distribution source packages in the project's “Change | ||
484 | 10 | components” page. | ||
485 | 11 | </p> | ||
486 | 12 | <ul tal:define="related_component_groups view/related_component_groups"> | ||
487 | 13 | <li tal:repeat="component_group related_component_groups"> | ||
488 | 14 | <strong><span tal:replace="component_group/name" /></strong> | ||
489 | 15 | <ul tal:define="components component_group/components"> | ||
490 | 16 | <li tal:repeat="component components"> | ||
491 | 17 | <span tal:replace="component/name" /> | ||
492 | 18 | | ||
493 | 19 | <span tal:condition="component/distro_source_package"> | ||
494 | 20 | <a class="sprite edit" | ||
495 | 21 | tal:attributes="href string:${context/fmt:url}/+components/${component_group/id}/${component/id}/+edit"> | ||
496 | 22 | <span tal:replace="structure component/distro_source_package/name"/></a> | ||
497 | 23 | </span> | ||
498 | 24 | <a class="sprite add" | ||
499 | 25 | tal:condition="not: component/distro_source_package" | ||
500 | 26 | tal:attributes="href string:${context/fmt:url}/+components/${component_group/id}/${component/id}/+edit"></a> | ||
501 | 27 | </li> | ||
502 | 28 | <li tal:condition="not: components"> | ||
503 | 29 | <i>This bug tracker has no components for this group</i> | ||
504 | 30 | </li> | ||
505 | 31 | </ul> | ||
506 | 32 | </li> | ||
507 | 33 | <li tal:condition="not: related_component_groups"> | ||
508 | 34 | <i>This bug tracker has no components</i> | ||
509 | 35 | </li> | ||
510 | 36 | </ul> | ||
511 | 37 | </div> | ||
512 | 0 | 38 | ||
513 | === modified file 'lib/lp/bugs/tests/test_bugtracker_components.py' | |||
514 | --- lib/lp/bugs/tests/test_bugtracker_components.py 2010-10-15 06:01:53 +0000 | |||
515 | +++ lib/lp/bugs/tests/test_bugtracker_components.py 2011-06-02 23:32:14 +0000 | |||
516 | @@ -88,15 +88,26 @@ | |||
517 | 88 | 88 | ||
518 | 89 | def test_link_distro_source_package(self): | 89 | def test_link_distro_source_package(self): |
519 | 90 | """Check that a link can be set to a distro source package""" | 90 | """Check that a link can be set to a distro source package""" |
520 | 91 | ubuntu = getUtility(IDistributionSet).getByName('ubuntu') | ||
521 | 91 | component = self.factory.makeBugTrackerComponent( | 92 | component = self.factory.makeBugTrackerComponent( |
522 | 92 | u'example', self.comp_group) | 93 | u'example', self.comp_group) |
523 | 93 | package = self.factory.makeDistributionSourcePackage() | 94 | package = self.factory.makeDistributionSourcePackage() |
524 | 94 | self.assertIs(None, component.distro_source_package) | 95 | self.assertIs(None, component.distro_source_package) |
525 | 95 | 96 | ||
526 | 97 | # No components link to the source package yet | ||
527 | 98 | link_comp = self.bug_tracker.getRemoteComponentForDistroSourcePackageName( | ||
528 | 99 | ubuntu, component.distro_source_package.name) | ||
529 | 100 | self.assertIs(None, link_comp) | ||
530 | 101 | |||
531 | 96 | # Set the source package on the component | 102 | # Set the source package on the component |
532 | 97 | component.distro_source_package = package | 103 | component.distro_source_package = package |
533 | 98 | self.assertIsNot(None, component.distro_source_package) | 104 | self.assertIsNot(None, component.distro_source_package) |
534 | 99 | 105 | ||
535 | 106 | # Verify we can find the component by the source package now | ||
536 | 107 | link_comp = self.bug_tracker.getRemoteComponentForDistroSourcePackageName( | ||
537 | 108 | ubuntu, component.distro_source_package.name) | ||
538 | 109 | self.assertIsNot(None, link_comp) | ||
539 | 110 | |||
540 | 100 | 111 | ||
541 | 101 | class TestBugTrackerWithComponents(TestCaseWithFactory): | 112 | class TestBugTrackerWithComponents(TestCaseWithFactory): |
542 | 102 | 113 |
Hi Bryce,
Thanks for the excellent work! I only have one comment, which is the unittest machinery in the unittest and the import of unittest isn't needed.