Launchpad itself

Merge lp:~bryce/launchpad/lp-617698-forwarding into lp:launchpad/db-devel

  1. lp-617698-forwarding
  2. Merge into db-devel
Proposed by Bryce Harrington
Status: Rejected
Rejected by: Curtis Hovey
Proposed branch: lp:~bryce/launchpad/lp-617698-forwarding
Merge into: lp:launchpad/db-devel
Prerequisite: lp:~bryce/launchpad/lp-617695-linkui
Diff against target: 1109 lines (+705/-45)
17 files modified
lib/canonical/launchpad/scripts/bzremotecomponentfinder.py (+6/-2)
lib/lp/bugs/browser/bugalsoaffects.py (+23/-7)
lib/lp/bugs/browser/bugtracker.py (+106/-4)
lib/lp/bugs/browser/configure.zcml (+9/-0)
lib/lp/bugs/browser/tests/bugtask-adding-views.txt (+1/-1)
lib/lp/bugs/browser/tests/test_bugtracker_component.py (+189/-0)
lib/lp/bugs/browser/widgets/bugtask.py (+20/-0)
lib/lp/bugs/doc/bugtracker.txt (+13/-20)
lib/lp/bugs/interfaces/bugtracker.py (+8/-2)
lib/lp/bugs/model/bugtracker.py (+29/-9)
lib/lp/bugs/templates/bugtracker-edit-component.pt (+16/-0)
lib/lp/bugs/templates/bugtracker-index.pt (+2/-0)
lib/lp/bugs/templates/bugtracker-portlet-components.pt (+36/-0)
lib/lp/registry/configure.zcml (+1/-0)
lib/lp/registry/interfaces/sourcepackage.py (+2/-0)
lib/lp/registry/model/sourcepackage.py (+11/-0)
lib/lp/services/features/scopes.py (+233/-0)
To merge this branch: bzr merge lp:~bryce/launchpad/lp-617698-forwarding
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) code Approve
Bryce Harrington (community) Needs Resubmitting
Review via email: mp+41003@code.launchpad.net

Commit message

Implement remote_component support when forwarding bugs.

Description of the change

Implement remote_component support when forwarding bugs.

This branch is dependent on the lp:~bryce/launchpad/lp-617695-linkui so that branch should be landed first.

This causes the 'component' field to be properly filled in for the bugzilla submission page when forwarding a bug report to a remote project.

lint has been checked. ec2 test has been run and passed successfully.

== Testing ==
For manual testing of this feature locally, I first ran cronscripts/update-bugzilla-remote-components.py to populate the mozilla and gnome bugzillas instances in the sample data.

[ Next I turned the feature flag on via the psql statement 'insert into featureflag (scope, priority, flag, value) values ('default', 1, 'bugtracker_components', 'on');'. However I had so much trouble getting feature flags to work properly that I've dropped this from the branch. ]

Next, I configured the alsa-utils project as using the gnome bugzilla, at https://bugs.launchpad.dev/alsa-utils/+configure-bugtracker by selecting "In a registered bug tracker" and typing in "gnome-bugs"; for the field "Project ID in bug tracker" I typed in "zenity" just to pick a random existing project.

Navigating to https://bugs.launchpad.dev/bugs/bugtrackers/gnome-bugs I scrolled down to the end and under the 'zenity' component group I clicked the icon next to 'general'. On the linking form I specified 'alsa-utils' as the package. I doublechecked that zenity/general now showed a link to 'alsa-utils'.

I logged out, and verified as anonymous that the edit links don't show up on https://bugs.launchpad.dev/bugs/bugtrackers/gnome-bugs.

Finally, after re-logging in, I filed a new bug report against alsa-utils, clicked "Also affects project", clicked the "bug filing form" link, and verified it filled in the component field in the bugzilla form properly.

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :
Download full text (25.6 KiB)

Hi Bryce,

There's lots of stuff that I'm unfamiliar with here, so please forgive me for holding you up with a "Needs Information" vote. I have some big questions about the UI and test coverage.

Otherwise it would be Approved, with some smaller notes and superficial issues.

> === modified file 'lib/canonical/launchpad/scripts/bzremotecomponentfinder.py'
> --- lib/canonical/launchpad/scripts/bzremotecomponentfinder.py 2010-10-20 00:24:50 +0000
> +++ lib/canonical/launchpad/scripts/bzremotecomponentfinder.py 2010-11-16 19:54:52 +0000
> @@ -117,7 +117,6 @@
> self.static_bugzilla_text = static_bugzilla_text
>
> def getRemoteProductsAndComponents(self, bugtracker_name=None):
> - """"""

There's supposed to be a docstring. It looks as if this one was put in
as a placeholder: "I'll write one later, and if I forget, the reviewer
will spot it." Can you think of what they mant to write?

The fact that this got into the codebase may be a clue that we're
being too sloppy in our reviews.

> === modified file 'lib/canonical/widgets/bugtask.py'
> --- lib/canonical/widgets/bugtask.py 2010-11-08 12:52:43 +0000
> +++ lib/canonical/widgets/bugtask.py 2010-11-16 19:54:52 +0000
> @@ -495,6 +495,25 @@
> return distribution
>
>
> +class UbuntuSourcePackageNameWidget(
> + BugTaskSourcePackageNameWidget):

Why is this line broken? There's plenty of space. Did you rename this
from a longer original name?

> + """Package widget where the distribution can be assumed as Ubuntu

There should be a full stop at the end of the sentence.

> + distribution_name = "ubuntu"
> +
> + def getDistribution(self):
> + """See `BugTaskSourcePackageNameWidget`"""
> + distribution = getUtility(IDistributionSet).getByName(
> + self.distribution_name)
> + if distribution is None:
> + raise UnexpectedFormData(
> + "No such distribution: %s" % self.distribution_name)
> + return distribution

Would it be worth having a comment here to say why you have to override
this method?

Also, doesn't this method amount to a difficult spelling for

    return getUtility(ILaunchpadCelebrities).ubuntu

?

> === modified file 'lib/lp/bugs/browser/bugalsoaffects.py'
> --- lib/lp/bugs/browser/bugalsoaffects.py 2010-09-03 03:12:39 +0000
> +++ lib/lp/bugs/browser/bugalsoaffects.py 2010-11-16 19:54:52 +0000
> @@ -664,8 +664,20 @@
> title = bug.title
> description = u"Originally reported at:\n %s\n\n%s" % (
> canonical_url(bug), bug.description)
> + remote_component = ""
> + comp_group = target.bugtracker.getRemoteComponentGroup(
> + target.remote_product)
> +
> + # Look up the remote component if we have the necessary info
> + if comp_group is not None and data.get('add_packaging', False):
> + package_name = self.context.target.sourcepackagename
> + for component in comp_group.components:
> + if (component.distro_source_package is not None and
> + component.distro_source_package.name == package_name):
> + ...

review: Needs Information
lp:~bryce/launchpad/lp-617698-forwarding updated
9926. By Bryce Harrington

Review comments by Jeroen.

9927. By Bryce Harrington

Review by Jerone: Code can be simplified using the ubuntu celebrity

9928. By Bryce Harrington

Almost all of the method is now an "else" clause after an early
return; we can drop the else and make the code stand on its own.

9929. By Bryce Harrington

Review by Jerone: Make if statement easier to read

9930. By Bryce Harrington

Review by Jeroen: Split out lookup functionality to separate routine

9931. By Bryce Harrington

Review by Jeroen: Find way to maintain visual separation between the two
if conditions and the body

9932. By Bryce Harrington

Review by Jeroen: Note interface change in docstring

9933. By Bryce Harrington

Review by Jeroen: Paragraph is too detailed for a doctest and mostly is
just stating obvious stuff about a function's parameters that doesn't
belong here.

9934. By Bryce Harrington

Review by Jeroen: Simplify setup by using helper functions.

9935. By Bryce Harrington

Review by Jeroen: Split up linking test into two tests

9936. By Bryce Harrington

Review by Jeroen: Use dict to eliminate some if clauses

9937. By Bryce Harrington

Review by Jeroen: Put a space in "% ("

9938. By Bryce Harrington

Review by Jeroen: Construct full name in a variable to make easier to follow

9939. By Bryce Harrington

Review by Jeroen: Use more verbose but conventional way of calling find.

9940. By Bryce Harrington

Re-merge db-devel trunk

9941. By Bryce Harrington

Review by Jeroen: State how the view should behave in this situation.

9942. By Bryce Harrington

Unnecessary to re-test output from the first component since this was
tested previously and is just setup here.

9943. By Bryce Harrington

Review by Jeroen: It doesn't matter how the first component got linked
so just treat it as setup and link the package directly, so that this
test focuses just on testing one thing.

9944. By Bryce Harrington

Review by Jeroen: You could use
self.assertTextMatchesExpressionIgnoreWhitespace for the
verification.

9945. By Bryce Harrington

Avoid potential extra database call when there are no components

9946. By Bryce Harrington

Review by Jeroen: Use URL formatter for links

9947. By Bryce Harrington

Review by Jeroen: Only display editing links if there is a logged in user

9948. By Bryce Harrington

Omit the nonbreaking space for anonymous users

9949. By Bryce Harrington

Preliminary implementation of test for checking anonymous edits

9950. By Bryce Harrington

Re-merge db-devel trunk

9951. By Bryce Harrington

Fix typo in find conditional

9952. By Bryce Harrington

ILaunchpadCelebrities needs declared

9953. By Bryce Harrington

Fix up several test errors

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Hi Bryce,

I see that you've done a lot of work on this branch since my initial review. Looks a lot better (though watch that double blank line below test_no_anonymous_edits ☺).

Please let me know when it's ready for the next round and hopefully we can get it landed!

lp:~bryce/launchpad/lp-617698-forwarding updated
9954. By Bryce Harrington

Use a store call to delete components.

9955. By Bryce Harrington

Enable testing generated links

Featureflags and testing view/user are interfering, so disable them
temporarily.

9956. By Bryce Harrington

Adjust create_initialized_view() call to set up proper logged in user

9957. By Bryce Harrington

Further experimentation with feature flags.

Just can't sort out how to make it work in a unittest. :-/

9958. By Bryce Harrington

Re-merge trunk

9959. By Bryce Harrington

Giving up on feature flags stuff and ripping it out.

The flags work fine in the browser when setting the flag manually, but
I just can't sort out how to get the unittests to recognize it. This
issue has been blocking progress on this branch for months now, and I've
failed to find anyone who can explain how to make it work properly.
Martin Pool suggested just dropping it and moving on, which sounds good
to me.

9960. By Bryce Harrington

Re-merge db-devel trunk

9961. By Bryce Harrington

Adjust to new widgets location

Revision history for this message
Bryce Harrington (bryce) wrote :

Hi Jeroen, yeah the extensiveness of the review made for a rather Herculean labor...

I'm afraid since returning from launchpad X.org has monopolized my attention, however I think this branch is more or less done and ready to go, but I haven't looked at it in a while; I'll check it over again.

lp:~bryce/launchpad/lp-617698-forwarding updated
9962. By Bryce Harrington

Compare string against the '.name' field of DSP rather than object itself.

9963. By Bryce Harrington

Revert Jeroen's change for component form so editing components works.

In review, Jeroen noticed I had an ugly if clause to look up the source
package name field for the component edit form, and suggested replacing
it with a dict to look up the fields.

Unfortunately, it's not so simple. The SourcePackageName object is not
a string property, so has to be handled differently than other kinds of
objects (we have to explicitly reference it's .name field to get the
string out of it). So the None check and special handling of this field
is required. Otherwise, an Oops occurs when it tries to fill in the
package name field with the object rather than the string.

9964. By Bryce Harrington

Re-merge db-devel trunk

Revision history for this message
Bryce Harrington (bryce) wrote :

Alright, I exercised the code for a while, found a couple issues that were introduced due to changes from the review and got them fixed up.

I also re-merged db-devel and fixed a minor conflict; unfortunately it seems launchpad has moved from postgres 8.3 to 8.4, and the upgrade scripts seem to have borked up my system, so I can no longer make schema or run tests locally. Hopefully it's correct though.

I don't know when I'll next get some spare moments to get back to launchpad work, so if there are any remaining minor niggles like whitespace or formatting or UI tweaks or whatnot, if you don't mind please feel free to correct those. Also, if someone else could handle testing and landing this baby I would *really* appreciate getting it off my plate.

review: Needs Resubmitting
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Bryce,

Thanks for sticking with it! I'm re-reviewing it. Meanwhile, to get your Launchpad instance working again:

 * Make sure your postgres config has 8.4 running on port 5432. If you install it side by side with an existing 8.3, it'll try to evade to port 5433 instead. Get rid of the 8.3.

 * There's a script called launchpad-database-setup that re-creates your entire database. This is highly destructive (so better not do it if you have any other postgres databases on that system) but tends to fix these problems.

Jeroen

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :
Download full text (4.8 KiB)

= Approving =

This looks like Pretty Good Stuff now. The commit logs show lots of changes (and actually explain them, which is a nice touch).

I see just 2 things that need fixing, but I'm sure that won't be helped by any further nannying from me. See "Important" below. Some other notes that you may want to consider or improve at your option: nice to have but not worth further delay.

== Design ==

I see a few documented assumptions (very thoughtful, by the way!) about Ubuntu being involved. Does that fit with the ongoing work for supporting derived distributions? Probably depends on the requirements for derived distros.

If not, it may present some new development tasks for derived distros. I wouldn't hold up your branch for it unless it poses some kind of serious problem.

== Important ==

=== Jack-in-the-box file ===

According to the MP diff, you're _adding_ lib/lp/services/features/scopes.py. Which already exists, with contents apparently similar or identical to what's in the diff. How can that happen!?

=== Model/view boundaries ===

In lib/lp/bugs/browser/bugtracker.py, you query the database directly:

188 + def updateContextFromData(self, data, context=None):
[...]
208 + # Has this source package already been assigned to a component?
209 + pkgs = Store.of(component).find(
210 + BugTrackerComponent,
211 + BugTrackerComponent.distribution == distribution.id,
212 + BugTrackerComponent.source_package_name ==
213 + pkg.sourcepackagename.id)
214 + if pkgs.count() > 0:
215 + self.request.response.addNotification(
216 + "The %s source package is already linked to %s in %s" % (
217 + sourcepackagename, component_full_name, distro_name))
218 + return

Querying the database from browser code is frowned upon: it's a job for the model, not for the view.

This particular query does a job that can be concisely defined, so that's good. (In fact some say that a block of code like this with a defining comment on top should almost certainly be a method of its own). I didn't find any method that does exactly this, but there's no reason why there shouldn't be. I think SourcePackage would be a good place.

(Also note that a count() on the query can be relatively expensive and deliver more information than you need. If all you need to know is whether there are any at all, use is_empty() instead.)

== Style ==

In lib/lp/bugs/browser/bugalsoaffects.py:

62 + # Look up the remote component if we have the necessary info
63 + #if data.get('add_packaging', False):
64 + comp_group = target.bugtracker.getRemoteComponentGroup(
65 + target.remote_product)

There's still a commented-out line in there.

In lib/lp/bugs/browser/tests/test_bugtracker_component.py: setUp() still creates a few objects that most of the tests don't actually care about.

Also in lib/lp/bugs/browser/tests/test_bugtracker_component.py, test_no_anonymous_edits:

353 + expected_html = """
354 + <h2>Components</h2>
355 + <ul>
356 + <li>
357 + <strong>alpha</strong>
358 + <ul>
359 + <li>
360 + ...

Read more...

review: Approve (code)
Revision history for this message
Bryce Harrington (bryce) wrote :

"Finally, a question: in lib/lp/bugs/templates/bugtracker-edit-component.pt I notice that the root tag is bug-tracker-edit-component. Can we do that!? My TAL knowledge is mostly cribbed off existing templates, but I thought that for whatever reason we only make up our own tags in the tal/metal namespaces."

==> product-cvereport.pt <==
<product-cve-report

==> remotebug-index.pt <==
<bug-tracker-remote-bug-index

==> malone-index.pt <==
<malone-index

==> distribution-cvereport.pt <==
<distribution-cve-report

==> distribution-upstream-bug-report.pt <==
<distribution-upstream-bug-report

==> distroseries-cvereport.pt <==
<distroseries-cve-report

Seems there are a number of other instances where this is done... I was cribbing off the cve templates since they seemed close to what I was trying to do. If this style is wrong it looks like it'll need fixed in a number of places.

lp:~bryce/launchpad/lp-617698-forwarding updated
9965. By Bryce Harrington

Review from jeroen: "Also note that a count() on the query can be
relatively expensive and deliver more information than you need. If all
you need to know is whether there are any at all, use is_empty() instead"

9966. By Bryce Harrington

Review from jeroen: "Commented-out code is a bad thing."

9967. By Bryce Harrington

Review from jeroen: Typo for "searched"?

9968. By Bryce Harrington

Review from jeroen: Move component/sourcepackage check to a new
member function of SourcePackage.

Revision history for this message
Bryce Harrington (bryce) wrote :

"According to the MP diff, you're _adding_ lib/lp/services/features/scopes.py. Which already exists, with contents apparently similar or identical to what's in the diff. How can that happen!?"

According to my local tree, this only came in as part of a re-merge of db-devel and is not created locally. I certainly never added it myself. I have no idea why it's showing up in the merge proposal diff, nor any clue on how to correct that.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

To be honest, I get these things as well sometimes and don't know how to explain them. Often I'll just produce a diff, clean it up, and patch it into a new branch.

Revision history for this message
Robert Collins (lifeless) wrote :

File a bug about the odd diff?

Revision history for this message
Bryce Harrington (bryce) wrote :

I think there may have been some overlap in the review comments for this MP and the MP for the branch this depends on, https://code.launchpad.net/~bryce/launchpad/lp-617695-linkui/+merge/39679, so I'm going to wait until that branch lands (perhaps tomorrow?) and then do as Jeroen suggests and just generate a new diff and re-merge it into a new branch.

Revision history for this message
Curtis Hovey (sinzui) wrote :

I marked this old review rejected because I think I see code that has already landed in devel. If I am wrong, please submit a new merge proposal with devel.

Unmerged revisions

9968. By Bryce Harrington

Review from jeroen: Move component/sourcepackage check to a new
member function of SourcePackage.

9967. By Bryce Harrington

Review from jeroen: Typo for "searched"?

9966. By Bryce Harrington

Review from jeroen: "Commented-out code is a bad thing."

9965. By Bryce Harrington

Review from jeroen: "Also note that a count() on the query can be
relatively expensive and deliver more information than you need. If all
you need to know is whether there are any at all, use is_empty() instead"

9964. By Bryce Harrington

Re-merge db-devel trunk

9963. By Bryce Harrington

Revert Jeroen's change for component form so editing components works.

In review, Jeroen noticed I had an ugly if clause to look up the source
package name field for the component edit form, and suggested replacing
it with a dict to look up the fields.

Unfortunately, it's not so simple. The SourcePackageName object is not
a string property, so has to be handled differently than other kinds of
objects (we have to explicitly reference it's .name field to get the
string out of it). So the None check and special handling of this field
is required. Otherwise, an Oops occurs when it tries to fill in the
package name field with the object rather than the string.

9962. By Bryce Harrington

Compare string against the '.name' field of DSP rather than object itself.

9961. By Bryce Harrington

Adjust to new widgets location

9960. By Bryce Harrington

Re-merge db-devel trunk

9959. By Bryce Harrington

Giving up on feature flags stuff and ripping it out.

The flags work fine in the browser when setting the flag manually, but
I just can't sort out how to get the unittests to recognize it. This
issue has been blocking progress on this branch for months now, and I've
failed to find anyone who can explain how to make it work properly.
Martin Pool suggested just dropping it and moving on, which sounds good
to me.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/scripts/bzremotecomponentfinder.py'
2--- lib/canonical/launchpad/scripts/bzremotecomponentfinder.py 2010-12-20 03:21:03 +0000
3+++ lib/canonical/launchpad/scripts/bzremotecomponentfinder.py 2011-05-19 01:14:21 +0000
4@@ -117,7 +117,7 @@
5 self.static_bugzilla_text = static_bugzilla_text
6
7 def getRemoteProductsAndComponents(self, bugtracker_name=None):
8- """"""
9+ """Retrieves, parses, and stores component data for each bugtracker"""
10 lp_bugtrackers = getUtility(IBugTrackerSet)
11 if bugtracker_name is not None:
12 lp_bugtrackers = [
13@@ -182,7 +182,11 @@
14 else:
15 # Component is now missing from Bugzilla,
16 # so drop it here too
17- component.remove()
18+ store = IStore(BugTrackerComponent)
19+ store.find(
20+ BugTrackerComponent,
21+ BugTrackerComponent.id == component.id,
22+ ).remove()
23
24 # The remaining components in the collection will need to be
25 # added to launchpad. Record them for now.
26
27=== modified file 'lib/lp/bugs/browser/bugalsoaffects.py'
28--- lib/lp/bugs/browser/bugalsoaffects.py 2011-03-23 16:28:51 +0000
29+++ lib/lp/bugs/browser/bugalsoaffects.py 2011-05-19 01:14:21 +0000
30@@ -646,6 +646,14 @@
31 packaging=PackagingType.PRIME, owner=self.user)
32 return super(ProductBugTaskCreationStep, self).main_action(data)
33
34+ def _lookup_remote_component_name(self, comp_group, package_name):
35+ if comp_group is None:
36+ return ""
37+ for component in comp_group.components:
38+ package = component.distro_source_package
39+ if (package is not None and package.name == package_name.name):
40+ return component.name
41+
42 @property
43 def upstream_bugtracker_links(self):
44 """Return the upstream bugtracker links for the current target.
45@@ -658,13 +666,21 @@
46
47 if not target.bugtracker:
48 return None
49- else:
50- bug = self.context.bug
51- title = bug.title
52- description = u"Originally reported at:\n %s\n\n%s" % (
53- canonical_url(bug), bug.description)
54- return target.bugtracker.getBugFilingAndSearchLinks(
55- target.remote_product, title, description)
56+
57+ bug = self.context.bug
58+ title = bug.title
59+ description = u"Originally reported at:\n %s\n\n%s" % (
60+ canonical_url(bug), bug.description)
61+
62+ # Look up the remote component if we have the necessary info
63+ comp_group = target.bugtracker.getRemoteComponentGroup(
64+ target.remote_product)
65+ package_name = self.context.target.sourcepackagename
66+ remote_component = self._lookup_remote_component_name(
67+ comp_group, package_name)
68+
69+ return target.bugtracker.getBugFilingAndSearchLinks(
70+ target.remote_product, remote_component, title, description)
71
72
73 class BugTrackerCreationStep(AlsoAffectsStep):
74
75=== modified file 'lib/lp/bugs/browser/bugtracker.py'
76--- lib/lp/bugs/browser/bugtracker.py 2011-02-02 15:43:31 +0000
77+++ lib/lp/bugs/browser/bugtracker.py 2011-05-19 01:14:21 +0000
78@@ -10,6 +10,7 @@
79 'BugTrackerBreadcrumb',
80 'BugTrackerComponentGroupNavigation',
81 'BugTrackerEditView',
82+ 'BugTrackerEditComponentView',
83 'BugTrackerNavigation',
84 'BugTrackerNavigationMenu',
85 'BugTrackerSetBreadcrumb',
86@@ -21,6 +22,7 @@
87 ]
88
89 from itertools import chain
90+from storm.locals import Store
91
92 from zope.app.form.browser import TextAreaWidget
93 from zope.component import getUtility
94@@ -59,6 +61,7 @@
95 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
96 from canonical.launchpad.webapp.menu import NavigationMenu
97 from canonical.lazr.utils import smartquote
98+from lp.bugs.browser.widgets.bugtask import UbuntuSourcePackageNameWidget
99 from lp.app.browser.launchpadform import (
100 action,
101 custom_widget,
102@@ -73,7 +76,11 @@
103 IBugTrackerComponentGroup,
104 IBugTrackerSet,
105 IRemoteBug,
106+ IBugTrackerComponent,
107+ IBugTrackerComponentGroup,
108 )
109+from lp.bugs.model.bugtracker import BugTrackerComponent
110+from lp.registry.interfaces.distribution import IDistributionSet
111 from lp.services.propertycache import cachedproperty
112
113 # A set of bug tracker types for which there can only ever be one bug
114@@ -229,6 +236,14 @@
115 return shortlist(chain(self.context.projects,
116 self.context.products), 100)
117
118+ @property
119+ def related_component_groups(self):
120+ """Return all component groups and components.
121+
122+ This property was created for the Related components portlet in
123+ the bug tracker's page.
124+ """
125+ return self.context.getAllRemoteComponentGroups()
126
127 BUG_TRACKER_ACTIVE_VOCABULARY = SimpleVocabulary.fromItems(
128 [('On', True), ('Off', False)])
129@@ -447,16 +462,103 @@
130 return RemoteBug(self.context, remotebug, bugs)
131
132 @stepthrough("+components")
133- def component_groups(self, name):
134- return self.context.getRemoteComponentGroup(name)
135+ def component_groups(self, name_or_id):
136+ """Navigate by id (component group name should work too)"""
137+ return self.context.getRemoteComponentGroup(name_or_id)
138+
139+
140+class BugTrackerEditComponentView(LaunchpadEditFormView):
141+ """Provides editing form for setting source packages for components.
142+
143+ This class assumes that bug tracker components are always
144+ linked to source packages in the Ubuntu distribution.
145+ """
146+
147+ schema = IBugTrackerComponent
148+ custom_widget('sourcepackagename', UbuntuSourcePackageNameWidget)
149+
150+ @property
151+ def page_title(self):
152+ return smartquote(
153+ u'Link a distribution source package to the %s component'
154+ % self.context.name)
155+
156+ @property
157+ def field_names(self):
158+ field_names = ['sourcepackagename']
159+ return field_names
160+
161+ def setUpWidgets(self, context=None):
162+ for field in self.form_fields:
163+ if (field.custom_widget is None):
164+ if (field.__name__ in self.custom_widgets):
165+ field.custom_widget = self.custom_widgets[field.__name__]
166+ self.widgets = form.setUpWidgets(
167+ self.form_fields, self.prefix, self.context, self.request,
168+ data=self.initial_values, adapters=self.adapters,
169+ ignore_request=False)
170+
171+ @property
172+ def initial_values(self):
173+ """See `LaunchpadFormView.`"""
174+ field_values = {}
175+ for name in self.field_names:
176+ if name == 'sourcepackagename':
177+ pkg = self.context.distro_source_package
178+ if pkg is not None:
179+ field_values['sourcepackagename'] = pkg.name
180+ else:
181+ field_values['sourcepackagename'] = ""
182+ else:
183+ field_values[name] = getattr(self.context, name)
184+
185+ return field_values
186+
187+ def updateContextFromData(self, data, context=None):
188+ """Link component to specified distro source package.
189+
190+ Get the user-provided source package name from the form widget,
191+ look it up in Ubuntu to retrieve the distro_source_package
192+ object, and link it to this component.
193+ """
194+ if context is None:
195+ context = self.context
196+ component = context
197+
198+ sourcepackagename = self.request.form.get(
199+ self.widgets['sourcepackagename'].name)
200+
201+ component_full_name = "%s:%s" % (
202+ component.component_group.name, component.name)
203+ distribution = self.widgets['sourcepackagename'].getDistribution()
204+ distro_name = distribution.name
205+ pkg = distribution.getSourcePackage(sourcepackagename)
206+ if pkg.has_component:
207+ self.request.response.addNotification(
208+ "The %s source package is already linked to a component in %s" % (
209+ sourcepackagename, distro_name))
210+ return
211+
212+ component.distro_source_package = pkg
213+ self.request.response.addNotification(
214+ "%s is now linked to the %s source package in %s" % (
215+ component_full_name, sourcepackagename, distro_name))
216+
217+ @action('Save Changes', name='save')
218+ def save_action(self, action, data):
219+ """Update the component with the form data."""
220+ self.updateContextFromData(data)
221+
222+ self.next_url = canonical_url(
223+ self.context.component_group.bug_tracker)
224
225
226 class BugTrackerComponentGroupNavigation(Navigation):
227
228 usedfor = IBugTrackerComponentGroup
229
230- def traverse(self, name):
231- return self.context.getComponent(name)
232+ def traverse(self, id):
233+ return self.context.getComponent(id)
234
235
236 class BugTrackerSetBreadcrumb(Breadcrumb):
237
238=== modified file 'lib/lp/bugs/browser/configure.zcml'
239--- lib/lp/bugs/browser/configure.zcml 2011-04-28 12:51:56 +0000
240+++ lib/lp/bugs/browser/configure.zcml 2011-05-19 01:14:21 +0000
241@@ -813,6 +813,12 @@
242 path_expression="name"
243 attribute_to_parent="component_group"
244 rootsite="bugs"/>
245+ <browser:page
246+ name="+edit"
247+ for="lp.bugs.interfaces.bugtracker.IBugTrackerComponent"
248+ class="lp.bugs.browser.bugtracker.BugTrackerEditComponentView"
249+ permission="launchpad.AnyPerson"
250+ template="../templates/bugtracker-edit-component.pt"/>
251 <browser:pages
252 for="lp.bugs.interfaces.bugtracker.IBugTracker"
253 class="lp.bugs.browser.bugtracker.BugTrackerView"
254@@ -824,6 +830,9 @@
255 name="+portlet-details"
256 template="../templates/bugtracker-portlet-details.pt"/>
257 <browser:page
258+ name="+portlet-components"
259+ template="../templates/bugtracker-portlet-components.pt"/>
260+ <browser:page
261 name="+portlet-projects"
262 template="../templates/bugtracker-portlet-projects.pt"/>
263 <browser:page
264
265=== modified file 'lib/lp/bugs/browser/tests/bugtask-adding-views.txt'
266--- lib/lp/bugs/browser/tests/bugtask-adding-views.txt 2011-03-28 20:54:50 +0000
267+++ lib/lp/bugs/browser/tests/bugtask-adding-views.txt 2011-05-19 01:14:21 +0000
268@@ -618,7 +618,7 @@
269
270 >>> print_links(add_task_view.upstream_bugtracker_links)
271 bug_filing_url:
272- ...?product=foo&short_desc=Reflow%20...&long_desc=Originally%20...
273+ ...?product=foo...&short_desc=Reflow%20...&long_desc=Originally%20...
274 bug_search_url: ...query.cgi?product=foo&short_desc=Reflow%20problems...
275
276 If the product's `bugtracker` isn't specified its
277
278=== added file 'lib/lp/bugs/browser/tests/test_bugtracker_component.py'
279--- lib/lp/bugs/browser/tests/test_bugtracker_component.py 1970-01-01 00:00:00 +0000
280+++ lib/lp/bugs/browser/tests/test_bugtracker_component.py 2011-05-19 01:14:21 +0000
281@@ -0,0 +1,189 @@
282+# Copyright 2010 Canonical Ltd. This software is licensed under the
283+# GNU Affero General Public License version (see the file LICENSE).
284+
285+"""Unit tests for linking bug tracker components to source packages."""
286+
287+__metaclass__ = type
288+
289+import unittest
290+from zope.component import getUtility
291+
292+from canonical.launchpad.testing.pages import find_tag_by_id
293+from canonical.testing.layers import DatabaseFunctionalLayer
294+from lp.registry.interfaces.distribution import IDistributionSet
295+from lp.testing import (
296+ ANONYMOUS,
297+ login,
298+ login_person,
299+ person_logged_in,
300+ TestCaseWithFactory,
301+ )
302+from lp.testing.views import create_initialized_view
303+from storm.store import Store
304+
305+
306+class TestBugTrackerEditComponentView(TestCaseWithFactory):
307+
308+ layer = DatabaseFunctionalLayer
309+
310+ def setUp(self):
311+ super(TestBugTrackerEditComponentView, self).setUp()
312+ self.regular_user = self.factory.makePerson()
313+ login_person(self.regular_user)
314+
315+ self.bug_tracker = self.factory.makeBugTracker(
316+ title=u'Example Bug Tracker',
317+ name=u'example-bug-tracker'
318+ )
319+ self.comp_group = self.factory.makeBugTrackerComponentGroup(
320+ u'alpha', self.bug_tracker)
321+
322+ def _makeForm(self, sourcepackage):
323+ if sourcepackage is None:
324+ name = ''
325+ else:
326+ name = sourcepackage.name
327+ return {
328+ 'field.sourcepackagename': name,
329+ 'field.actions.save': 'Save',
330+ }
331+
332+ def _makeComponent(self, name):
333+ return self.factory.makeBugTrackerComponent(name, self.comp_group)
334+
335+ def _makeUbuntuSourcePackage(self, package_name):
336+ distro = getUtility(IDistributionSet).getByName('ubuntu')
337+ return self.factory.makeDistributionSourcePackage(
338+ sourcepackagename=package_name, distribution=distro)
339+
340+ def test_no_anonymous_edits(self):
341+ '''Only logged in users should see the edit buttons'''
342+ login(ANONYMOUS)
343+ component = self._makeComponent(u'Example')
344+
345+ expected_html = """
346+ <h2>Components</h2>
347+ <ul>
348+ <li>
349+ <strong>alpha</strong>
350+ <ul>
351+ <li>
352+ Example
353+ </li>
354+ </ul>
355+ </li>
356+"""
357+
358+ view = create_initialized_view(
359+ self.bug_tracker, name='+index', rootsite='bugs',
360+ current_request=True)
361+ actual_html = view.render()
362+ self.assertIsNot(None, find_tag_by_id(actual_html, 'portlet-components'))
363+ self.assertTextMatchesExpressionIgnoreWhitespace(
364+ expected_html, actual_html)
365+
366+ def test_generated_links(self):
367+ """Validates hyperlinks generated in the TAL"""
368+ component = self._makeComponent(u'Example')
369+ package = self._makeUbuntuSourcePackage('example')
370+
371+ projectgroup = self.factory.makeProject(name=u'sample-project')
372+ product = self.factory.makeProduct()
373+ with person_logged_in(product.owner):
374+ product.project = projectgroup
375+ product.bugtracker = self.bug_tracker
376+ product.remote_product = u'sample-product'
377+
378+ expected_html = """<strong>alpha</strong>
379+ .*Example.*
380+ .*href=".*/bugs/bugtrackers/example-bug-tracker/.components/alpha/Example/.edit"></a>"""
381+
382+ login_person(self.regular_user)
383+ view = create_initialized_view(
384+ self.bug_tracker, name='+index', principal=self.regular_user)
385+ actual_html = view.render()
386+
387+ self.assertTextMatchesExpressionIgnoreWhitespace(
388+ expected_html, actual_html)
389+
390+ def test_view_attributes(self):
391+ component = self._makeComponent(u'Example')
392+ package = self._makeUbuntuSourcePackage('example-package')
393+ form = self._makeForm(package)
394+ view = create_initialized_view(
395+ component, name='+edit', form=form)
396+ label = 'Link a distribution source package to the Example component'
397+ self.assertEqual(label, view.page_title)
398+ fields = ['sourcepackagename']
399+ self.assertEqual(fields, view.field_names)
400+
401+ def test_linking(self):
402+ component = self._makeComponent(u'Example')
403+ package = self._makeUbuntuSourcePackage('example-package')
404+ form = self._makeForm(package)
405+
406+ self.assertIs(None, component.distro_source_package)
407+ view = create_initialized_view(
408+ component, name='+edit', form=form)
409+ notifications = view.request.response.notifications
410+ self.assertEqual(package, component.distro_source_package)
411+
412+ def test_linking_html(self):
413+ package = self._makeUbuntuSourcePackage('example-package')
414+ component = self._makeComponent(u'Example')
415+ component.distro_source_package = package
416+
417+ expected_html = """<strong>alpha</strong>
418+ .*Example.*
419+ href=".*example-bug-tracker/.components/alpha/Example/.edit">
420+ example-package</a>"""
421+ login_person(self.regular_user)
422+ view = create_initialized_view(
423+ self.bug_tracker, name='+index', principal=self.regular_user)
424+ actual_html = view.render()
425+
426+ self.assertIsNot(None, find_tag_by_id(actual_html, 'portlet-components'))
427+ self.assertTextMatchesExpressionIgnoreWhitespace(
428+ expected_html, actual_html)
429+
430+ def test_linking_notifications(self):
431+ component = self._makeComponent(u'Example')
432+ package = self._makeUbuntuSourcePackage('example-package')
433+ form = self._makeForm(package)
434+
435+ view = create_initialized_view(
436+ component, name='+edit', form=form)
437+ self.assertEqual([], view.errors)
438+ notifications = view.request.response.notifications
439+ expected = """
440+ alpha:Example is now linked to the example-package
441+ source package in ubuntu"""
442+ self.assertTextMatchesExpressionIgnoreWhitespace(
443+ expected, notifications.pop().message)
444+
445+ def test_cannot_doublelink_sourcepackages(self):
446+ ''' Two components try linking to same same package
447+
448+ We must maintain a one-to-one relationship between components
449+ and source packages. However, users are bound to attempt to try
450+ to make multiple components linked to the same source package,
451+ so the view needs to be sure to not allow this to be done and
452+ pop up a friendly error message instead.
453+ '''
454+ component_a = self._makeComponent(u'a')
455+ component_b = self._makeComponent(u'b')
456+ package = self._makeUbuntuSourcePackage('example-package')
457+ component_a.distro_source_package = package
458+
459+ form = self._makeForm(package)
460+ view = create_initialized_view(
461+ component_b, name='+edit', form=form)
462+ self.assertIs(None, component_b.distro_source_package)
463+ self.assertEqual([], view.errors)
464+ notifications = view.request.response.notifications
465+ self.assertEqual(1, len(notifications))
466+ expected = """
467+ The example-package source package is already linked to
468+ alpha:b in ubuntu"""
469+ self.assertTextMatchesExpressionIgnoreWhitespace(
470+ expected, notifications.pop().message)
471
472=== modified file 'lib/lp/bugs/browser/widgets/bugtask.py'
473--- lib/lp/bugs/browser/widgets/bugtask.py 2011-02-02 15:43:31 +0000
474+++ lib/lp/bugs/browser/widgets/bugtask.py 2011-05-19 01:14:21 +0000
475@@ -14,6 +14,7 @@
476 "DBItemDisplayWidget",
477 "NewLineToSpacesWidget",
478 "NominationReviewActionWidget",
479+ "UbuntuSourcePackageNameWidget"
480 ]
481
482 from xml.sax.saxutils import escape
483@@ -47,6 +48,7 @@
484 )
485
486 from canonical.launchpad import _
487+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
488 from canonical.launchpad.webapp import canonical_url
489 from canonical.launchpad.webapp.interfaces import ILaunchBag
490 from lp.app.browser.tales import TeamFormatterAPI
491@@ -527,6 +529,24 @@
492 return distribution
493
494
495+class UbuntuSourcePackageNameWidget(BugTaskSourcePackageNameWidget):
496+ """Package widget where the distribution can be assumed as Ubuntu.
497+
498+ This widgets works the same as `BugTaskSourcePackageNameWidget`,
499+ except that it assumes the distribution is 'ubuntu'.
500+ """
501+ distribution_name = "ubuntu"
502+
503+ def getDistribution(self):
504+ """See `BugTaskSourcePackageNameWidget`
505+
506+ _toFieldValue() in BugTaskSourcePackageNameWidget needs this
507+ routine to determine the distribution. We just hardcode it
508+ to Ubuntu.
509+ """
510+ return getUtility(ILaunchpadCelebrities).ubuntu
511+
512+
513 class AssigneeDisplayWidget(BrowserWidget):
514 """A widget for displaying an assignee."""
515
516
517=== modified file 'lib/lp/bugs/doc/bugtracker.txt'
518--- lib/lp/bugs/doc/bugtracker.txt 2011-03-23 16:28:51 +0000
519+++ lib/lp/bugs/doc/bugtracker.txt 2011-05-19 01:14:21 +0000
520@@ -324,26 +324,20 @@
521 Filing a bug on the remote tracker
522 ----------------------------------
523
524-The IBugTracker interface defines a method, getBugFilingAndSearchLinks(),
525-which returns the URLs of the bug filing form and the bug search on
526-the remote bug tracker as a dict. It accepts three parameters:
527-remote_product, which is the name of the product on the remote
528-tracker; summary, which is the bug summary to be passed to the remote
529-bug tracker for searching or filing and description, which is the full
530-description of the bug. This is only passed to the bug filing form as
531-it is too specific for the search form.
532+The IBugTracker interface defines a method to convert product,
533+component, summary, and description strings into URLs for filing and/or
534+searching bugs.
535
536 >>> def print_links(links_dict):
537 ... for key in sorted(links_dict):
538 ... print "%s: %s" % (key, links_dict[key])
539
540 >>> links = mozilla_bugzilla.getBugFilingAndSearchLinks(
541- ... remote_product='testproduct', summary="Foo", description="Bar")
542+ ... remote_product='testproduct', remote_component='testcomponent',
543+ ... summary="Foo", description="Bar")
544 >>> print_links(links)
545- bug_filing_url:
546- https://.../enter_bug.cgi?product=testproduct&short_desc=Foo&long_desc=Bar
547- bug_search_url:
548- https://.../query.cgi?product=testproduct&short_desc=Foo
549+ bug_filing_url: https://.../enter_bug.cgi?product=testproduct&component=testcomponent&short_desc=Foo&long_desc=Bar
550+ bug_search_url: https://.../query.cgi?product=testproduct&short_desc=Foo
551
552 For the RT tracker we specify a Queue in which to file a ticket.
553
554@@ -430,7 +424,8 @@
555 >>> example_roundup = factory.makeBugTracker(
556 ... 'http://roundup.example.com', BugTrackerType.ROUNDUP)
557 >>> links = example_roundup.getBugFilingAndSearchLinks(
558- ... remote_product='testproduct', summary="Foo", description="Bar")
559+ ... remote_product='testproduct', remote_component='testcomponent',
560+ ... summary="Foo", description="Bar")
561 >>> print_links(links)
562 bug_filing_url: http://.../issue?@template=item&title=Foo&@note=Bar
563 bug_search_url: http://.../issue?@template=search&@search_text=Foo
564@@ -473,7 +468,7 @@
565 >>> links = mozilla_bugzilla.getBugFilingAndSearchLinks(
566 ... 'test', None, None)
567 >>> print_links(links)
568- bug_filing_url: ...?product=test&short_desc=&long_desc=
569+ bug_filing_url: ...?product=test&component=&short_desc=&long_desc=
570 bug_search_url: ...?product=test&short_desc=
571
572 The remote_product, summary and description values are URL-encoded to ensure
573@@ -482,7 +477,7 @@
574 >>> links = mozilla_bugzilla.getBugFilingAndSearchLinks(
575 ... remote_product='@test&', summary="%&", description="()")
576 >>> print_links(links)
577- bug_filing_url: ...?product=%40test%26&short_desc=%25%26&long_desc=%28%29
578+ bug_filing_url: ...?product=%40test%26&component=&short_desc=%25%26&long_desc=%28%29
579 bug_search_url: ...?product=%40test%26&short_desc=%25%26
580
581 getBugFilingAndSearchLinks() will also handle unicode values in the
582@@ -551,8 +546,6 @@
583 ... remote_product='testproduct', summary="Foo", description="Bar")
584
585 >>> print_links(links)
586- bug_filing_url:
587- http://.../enter_bug.cgi?product=testproduct&short_desc=Foo&comment=Bar
588- bug_search_url:
589- http://.../query.cgi?product=testproduct&short_desc=Foo
590+ bug_filing_url: http://.../enter_bug.cgi?product=testproduct&component=&short_desc=Foo&comment=Bar
591+ bug_search_url: http://.../query.cgi?product=testproduct&short_desc=Foo
592
593
594=== modified file 'lib/lp/bugs/interfaces/bugtracker.py'
595--- lib/lp/bugs/interfaces/bugtracker.py 2011-02-23 20:26:53 +0000
596+++ lib/lp/bugs/interfaces/bugtracker.py 2011-05-19 01:14:21 +0000
597@@ -306,12 +306,14 @@
598 watches_needing_update = Attribute(
599 "The set of bug watches that need updating.")
600
601- def getBugFilingAndSearchLinks(remote_product, summary=None,
602- description=None):
603+ def getBugFilingAndSearchLinks(remote_product, remote_component=None,
604+ summary=None, description=None):
605 """Return the bug filing and search links for the tracker.
606
607 :param remote_product: The name of the product on which the bug
608 is to be filed or search for.
609+ :param remote_component: The name of the component on which the bug
610+ is to be filed or searched for.
611 :param summary: The string with which to pre-filly the summary
612 field of the upstream bug tracker's search and bug filing forms.
613 :param description: The string with which to pre-filly the description
614@@ -538,6 +540,10 @@
615 title=_('Name'),
616 description=_("The name of a software component "
617 "as shown in Launchpad.")))
618+ sourcepackagename = Choice(
619+ title=_("Package"), required=False, vocabulary='SourcePackageName')
620+ distribution = Choice(
621+ title=_("Distribution"), required=False, vocabulary='Distribution')
622
623 distro_source_package = exported(
624 Reference(
625
626=== modified file 'lib/lp/bugs/model/bugtracker.py'
627--- lib/lp/bugs/model/bugtracker.py 2011-03-23 16:28:51 +0000
628+++ lib/lp/bugs/model/bugtracker.py 2011-05-19 01:14:21 +0000
629@@ -251,7 +251,6 @@
630
631 def addComponent(self, component_name):
632 """Adds a component that is synced from a remote bug tracker"""
633-
634 component = BugTrackerComponent()
635 component.name = component_name
636 component.component_group = self
637@@ -263,23 +262,28 @@
638 return component
639
640 def getComponent(self, component_name):
641- """Retrieves a component by the given name.
642+ """Retrieves a component by the given name or id number.
643
644 None is returned if there is no component by that name in the
645 group.
646 """
647-
648 if component_name is None:
649 return None
650+ elif component_name.isdigit():
651+ component_id = int(component_name)
652+ return Store.of(self).find(
653+ BugTrackerComponent,
654+ BugTrackerComponent.id == component_id,
655+ BugTrackerComponent.component_group == self.id).one()
656 else:
657 return Store.of(self).find(
658 BugTrackerComponent,
659- (BugTrackerComponent.name == component_name)).one()
660+ BugTrackerComponent.name == component_name,
661+ BugTrackerComponent.component_group == self.id).one()
662
663 def addCustomComponent(self, component_name):
664 """Adds a component locally that isn't synced from a remote tracker
665 """
666-
667 component = BugTrackerComponent()
668 component.name = component_name
669 component.component_group = self
670@@ -329,6 +333,7 @@
671 _filing_url_patterns = {
672 BugTrackerType.BUGZILLA: (
673 "%(base_url)s/enter_bug.cgi?product=%(remote_product)s"
674+ "&component=%(remote_component)s"
675 "&short_desc=%(summary)s&long_desc=%(description)s"),
676 BugTrackerType.GOOGLE_CODE: (
677 "%(base_url)s/entry?summary=%(summary)s&"
678@@ -387,6 +392,7 @@
679 return {
680 gnome_bugzilla: (
681 "%(base_url)s/enter_bug.cgi?product=%(remote_product)s"
682+ "&component=%(remote_component)s"
683 "&short_desc=%(summary)s&comment=%(description)s"),
684 }
685
686@@ -403,7 +409,8 @@
687 else:
688 return False
689
690- def getBugFilingAndSearchLinks(self, remote_product, summary=None,
691+ def getBugFilingAndSearchLinks(self, remote_product,
692+ remote_component=None, summary=None,
693 description=None):
694 """See `IBugTracker`."""
695 bugtracker_urls = {'bug_filing_url': None, 'bug_search_url': None}
696@@ -418,6 +425,10 @@
697 # quote() doesn't blow up later on.
698 remote_product = ''
699
700+ if remote_component is None:
701+ # Ditto for remote component.
702+ remote_component = ''
703+
704 if self in self._custom_filing_url_patterns:
705 # Some bugtrackers are customised to accept different
706 # querystring parameters from the default. We special-case
707@@ -475,6 +486,7 @@
708 url_components = {
709 'base_url': base_url,
710 'remote_product': quote(remote_product),
711+ 'remote_component': quote(remote_component),
712 'summary': quote(summary),
713 'description': quote(description),
714 }
715@@ -675,9 +687,17 @@
716 """See `IBugTracker`."""
717 component_group = None
718 store = IStore(BugTrackerComponentGroup)
719- component_group = store.find(
720- BugTrackerComponentGroup,
721- name = component_group_name).one()
722+ if component_group_name is None:
723+ return None
724+ elif component_group_name.isdigit():
725+ component_group_id = int(component_group_name)
726+ component_group = store.find(
727+ BugTrackerComponentGroup,
728+ BugTrackerComponentGroup.id==component_group_id).one()
729+ else:
730+ component_group = store.find(
731+ BugTrackerComponentGroup,
732+ BugTrackerComponentGroup.name==component_group_name).one()
733 return component_group
734
735
736
737=== added file 'lib/lp/bugs/templates/bugtracker-edit-component.pt'
738--- lib/lp/bugs/templates/bugtracker-edit-component.pt 1970-01-01 00:00:00 +0000
739+++ lib/lp/bugs/templates/bugtracker-edit-component.pt 2011-05-19 01:14:21 +0000
740@@ -0,0 +1,16 @@
741+<bug-tracker-edit-component
742+ xmlns="http://www.w3.org/1999/xhtml"
743+ xmlns:tal="http://xml.zope.org/namespaces/tal"
744+ xmlns:metal="http://xml.zope.org/namespaces/metal"
745+ xmlns:i18n="http://xml.zope.org/namespaces/i18n"
746+ metal:use-macro="view/macro:page/main_only"
747+ i18n:domain="malone">
748+
749+ <div metal:fill-slot="main">
750+ <h1>Link <span tal:replace="context/component_group/name"/> component '<span tal:replace="context/name"/>'</h1>
751+ <div metal:use-macro="context/@@launchpad_form/form">
752+ <p>Configure component</p>
753+ </div>
754+ </div>
755+
756+</bug-tracker-edit-component>
757
758=== modified file 'lib/lp/bugs/templates/bugtracker-index.pt'
759--- lib/lp/bugs/templates/bugtracker-index.pt 2010-10-10 21:54:16 +0000
760+++ lib/lp/bugs/templates/bugtracker-index.pt 2011-05-19 01:14:21 +0000
761@@ -4,6 +4,7 @@
762 xmlns:metal="http://xml.zope.org/namespaces/metal"
763 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
764 metal:use-macro="view/macro:page/main_side"
765+ tal:define="features request/features"
766 i18n:domain="malone">
767
768 <metal:heading fill-slot="heading">
769@@ -34,6 +35,7 @@
770 <div class="yui-g">
771 <div class="first yui-u">
772 <div tal:replace="structure context/@@+portlet-details" />
773+ <div tal:replace="structure context/@@+portlet-components" />
774 </div>
775 <div class="yui-u">
776 <div tal:replace="structure context/@@+portlet-projects" />
777
778=== added file 'lib/lp/bugs/templates/bugtracker-portlet-components.pt'
779--- lib/lp/bugs/templates/bugtracker-portlet-components.pt 1970-01-01 00:00:00 +0000
780+++ lib/lp/bugs/templates/bugtracker-portlet-components.pt 2011-05-19 01:14:21 +0000
781@@ -0,0 +1,36 @@
782+<div
783+ xmlns:tal="http://xml.zope.org/namespaces/tal"
784+ xmlns:metal="http://xml.zope.org/namespaces/metal"
785+ xmlns:i18n="http://xml.zope.org/namespaces/i18n"
786+ class="portlet" id="portlet-components">
787+ <h2>Components</h2>
788+ <p tal:condition="view/user">
789+ You can link components from this bug tracker to their corresponding
790+ distribution source packages in the project's &ldquo;Change
791+ components&rdquo; page.
792+ </p>
793+ <ul tal:define="related_component_groups view/related_component_groups">
794+ <li tal:repeat="component_group related_component_groups">
795+ <strong><span tal:replace="component_group/name" /></strong>
796+ <ul tal:define="components component_group/components">
797+ <li tal:repeat="component components">
798+ <span tal:replace="component/name" />
799+ <tal:logged-in condition="view/user">
800+ &nbsp;
801+ <span tal:condition="component/distro_source_package">
802+ <a class="sprite edit"
803+ tal:attributes="href component/fmt:url/+edit">
804+ <span tal:replace="structure component/distro_source_package/name"/></a>
805+ </span>
806+ <a class="sprite add"
807+ tal:condition="not: component/distro_source_package"
808+ tal:attributes="href component/fmt:url/+edit"></a>
809+ </tal:logged-in>
810+ </li>
811+ </ul>
812+ </li>
813+ <li tal:condition="not: related_component_groups">
814+ <em>This bug tracker has no components</em>
815+ </li>
816+ </ul>
817+</div>
818
819=== modified file 'lib/lp/registry/configure.zcml'
820--- lib/lp/registry/configure.zcml 2011-05-13 03:46:29 +0000
821+++ lib/lp/registry/configure.zcml 2011-05-19 01:14:21 +0000
822@@ -482,6 +482,7 @@
823 getVersion
824 get_distroseries_packages
825 has_bugtasks
826+ has_component
827 high_bugtasks
828 inprogress_bugtasks
829 latest_overall_publication
830
831=== modified file 'lib/lp/registry/interfaces/sourcepackage.py'
832--- lib/lp/registry/interfaces/sourcepackage.py 2011-05-12 14:55:54 +0000
833+++ lib/lp/registry/interfaces/sourcepackage.py 2011-05-19 01:14:21 +0000
834@@ -328,6 +328,8 @@
835 :return: A {`Pocket`-name : `IBranch`} dict.
836 """
837
838+ def has_component():
839+ """Indicates if the source package has a bug tracker component."""
840
841 class ISourcePackageFactory(Interface):
842 """A creator of source packages."""
843
844=== modified file 'lib/lp/registry/model/sourcepackage.py'
845--- lib/lp/registry/model/sourcepackage.py 2011-05-12 14:55:54 +0000
846+++ lib/lp/registry/model/sourcepackage.py 2011-05-19 01:14:21 +0000
847@@ -45,6 +45,7 @@
848 HasBugHeatMixin,
849 )
850 from lp.bugs.model.bugtask import BugTask
851+from lp.bugs.model.bugtracker import BugTrackerComponent
852 from lp.buildmaster.enums import BuildStatus
853 from lp.code.interfaces.seriessourcepackagebranch import (
854 IMakeOfficialBranchLinks,
855@@ -501,6 +502,16 @@
856 BugTask.sourcepackagename == self.sourcepackagename),
857 user, wanted_tags=wanted_tags)
858
859+ def has_component(self):
860+ """Indicates if the source package has a bug tracker component."""
861+ store = Store.of(self.sourcepackagename)
862+ pkgs = store.find(
863+ BugTrackerComponent,
864+ BugTrackerComponent.distribution == self.distribution.id,
865+ BugTrackerComponent.source_package_name ==
866+ self.sourcepackagename.id)
867+ return not pkgs.is_empty()
868+
869 @property
870 def max_bug_heat(self):
871 """See `IHasBugs`."""
872
873=== added file 'lib/lp/services/features/scopes.py'
874--- lib/lp/services/features/scopes.py 1970-01-01 00:00:00 +0000
875+++ lib/lp/services/features/scopes.py 2011-05-07 15:31:21 +0000
876@@ -0,0 +1,233 @@
877+# Copyright 2011 Canonical Ltd. This software is licensed under the
878+# GNU Affero General Public License version 3 (see the file LICENSE).
879+
880+"""Connect feature flags into scopes where they can be used.
881+
882+The most common is flags scoped by some attribute of a web request, such as
883+the page ID or the server name. But other types of scope can also match code
884+run from cron scripts and potentially also other places.
885+"""
886+
887+__all__ = [
888+ 'HANDLERS',
889+ 'ScopesForScript',
890+ 'ScopesFromRequest',
891+ 'undocumented_scopes',
892+ ]
893+
894+__metaclass__ = type
895+
896+import re
897+
898+from zope.component import getUtility
899+
900+from canonical.launchpad.webapp.interfaces import ILaunchBag
901+from lp.services.propertycache import cachedproperty
902+import canonical.config
903+
904+
905+undocumented_scopes = set()
906+
907+
908+class BaseScope():
909+ """A base class for scope handlers.
910+
911+ The docstring of subclasses is used on the +feature-info page as
912+ documentation, so write them accordingly.
913+ """
914+
915+ # The regex pattern used to decide if a handler can evaluate a particular
916+ # scope. Also used on +feature-info.
917+ pattern = None
918+
919+ @cachedproperty
920+ def compiled_pattern(self):
921+ """The compiled scope matching regex. A small optimization."""
922+ return re.compile(self.pattern)
923+
924+ def lookup(self, scope_name):
925+ """Returns true if the given scope name is "active"."""
926+ raise NotImplementedError('Subclasses of BaseScope must implement '
927+ 'lookup.')
928+
929+
930+class DefaultScope(BaseScope):
931+ """The default scope. Always active."""
932+
933+ pattern = r'default$'
934+
935+ def lookup(self, scope_name):
936+ return True
937+
938+
939+class BaseWebRequestScope(BaseScope):
940+ """Base class for scopes that key off web request attributes."""
941+
942+ def __init__(self, request):
943+ self.request = request
944+
945+
946+class PageScope(BaseWebRequestScope):
947+ """The current page ID.
948+
949+ Pageid scopes are written as 'pageid:' + the pageid to match. Pageids
950+ are treated as a namespace with : and # delimiters.
951+
952+ For example, the scope 'pageid:Foo' will be active on pages with pageids:
953+ Foo
954+ Foo:Bar
955+ Foo#quux
956+ """
957+
958+ pattern = r'pageid:'
959+
960+ def lookup(self, scope_name):
961+ """Is the given scope match the current pageid?"""
962+ pageid_scope = scope_name[len('pageid:'):]
963+ scope_segments = self._pageid_to_namespace(pageid_scope)
964+ request_segments = self._request_pageid_namespace
965+ # In 2.6, this can be replaced with izip_longest
966+ for pos, name in enumerate(scope_segments):
967+ if pos == len(request_segments):
968+ return False
969+ if request_segments[pos] != name:
970+ return False
971+ return True
972+
973+ @staticmethod
974+ def _pageid_to_namespace(pageid):
975+ """Return a list of namespace elements for pageid."""
976+ # Normalise delimiters.
977+ pageid = pageid.replace('#', ':')
978+ # Create a list to walk, empty namespaces are elided.
979+ return [name for name in pageid.split(':') if name]
980+
981+ @cachedproperty
982+ def _request_pageid_namespace(self):
983+ return tuple(self._pageid_to_namespace(
984+ self.request._orig_env.get('launchpad.pageid', '')))
985+
986+
987+class TeamScope(BaseScope):
988+ """The current user's team memberships.
989+
990+ Team ID scopes are written as 'team:' + the team name to match.
991+
992+ The scope 'team:launchpad-beta-users' will match members of the team
993+ 'launchpad-beta-users'.
994+ """
995+
996+ pattern = r'team:'
997+
998+ def lookup(self, scope_name):
999+ """Is the given scope a team membership?
1000+
1001+ This will do a two queries, so we probably want to keep the number of
1002+ team based scopes in use to a small number. (Person.inTeam could be
1003+ fixed to reduce this to one query).
1004+ """
1005+ team_name = scope_name[len('team:'):]
1006+ person = getUtility(ILaunchBag).user
1007+ if person is None:
1008+ return False
1009+ return person.inTeam(team_name)
1010+
1011+
1012+class ServerScope(BaseScope):
1013+ """Matches the current server.
1014+
1015+ For example, the scope server.lpnet is active when is_lpnet is set to True
1016+ in the Launchpad configuration.
1017+ """
1018+
1019+ pattern = r'server\.'
1020+
1021+ def lookup(self, scope_name):
1022+ """Match the current server as a scope."""
1023+ server_name = scope_name.split('.', 1)[1]
1024+ try:
1025+ return canonical.config.config['launchpad']['is_' + server_name]
1026+ except KeyError:
1027+ pass
1028+ return False
1029+
1030+
1031+class ScriptScope(BaseScope):
1032+ """Matches the name of the currently running script.
1033+
1034+ For example, the scope script:embroider is active in a script called
1035+ "embroider."
1036+ """
1037+
1038+ pattern = r'script:'
1039+
1040+ def __init__(self, script_name):
1041+ self.script_scope = self.pattern + script_name
1042+
1043+ def lookup(self, scope_name):
1044+ """Match the running script as a scope."""
1045+ return scope_name == self.script_scope
1046+
1047+
1048+# These are the handlers for all of the allowable scopes, listed here so that
1049+# we can for example show all of them in an admin page. Any new scope will
1050+# need a scope handler and that scope handler has to be added to this list.
1051+# See BaseScope for hints as to what a scope handler should look like.
1052+HANDLERS = set([DefaultScope, PageScope, TeamScope, ServerScope, ScriptScope])
1053+
1054+
1055+class MultiScopeHandler():
1056+ """A scope handler that combines multiple `BaseScope`s.
1057+
1058+ The ordering in which they're added is arbitrary, because precedence is
1059+ determined by the ordering of rules.
1060+ """
1061+
1062+ def __init__(self, scopes):
1063+ self.handlers = scopes
1064+
1065+ def _findMatchingHandlers(self, scope_name):
1066+ """Find any handlers that match `scope_name`."""
1067+ return [
1068+ handler
1069+ for handler in self.handlers
1070+ if handler.compiled_pattern.match(scope_name)]
1071+
1072+ def lookup(self, scope_name):
1073+ """Determine if scope_name applies.
1074+
1075+ This method iterates over the configured scope handlers until it
1076+ either finds one that claims the requested scope name matches,
1077+ or the handlers are exhausted, in which case the
1078+ scope name is not a match.
1079+ """
1080+ matching_handlers = self._findMatchingHandlers(scope_name)
1081+ for handler in matching_handlers:
1082+ if handler.lookup(scope_name):
1083+ return True
1084+
1085+ # If we didn't find at least one matching handler, then the
1086+ # requested scope is unknown and we want to record the scope for
1087+ # the +flag-info page to display.
1088+ if len(matching_handlers) == 0:
1089+ undocumented_scopes.add(scope_name)
1090+
1091+
1092+class ScopesFromRequest(MultiScopeHandler):
1093+ """Identify feature scopes based on request state."""
1094+
1095+ def __init__(self, request):
1096+ super(ScopesFromRequest, self).__init__([
1097+ DefaultScope(),
1098+ PageScope(request),
1099+ TeamScope(),
1100+ ServerScope()])
1101+
1102+
1103+class ScopesForScript(MultiScopeHandler):
1104+ """Identify feature scopes for a given script."""
1105+
1106+ def __init__(self, script_name):
1107+ super(ScopesForScript, self).__init__([
1108+ DefaultScope(),
1109+ ScriptScope(script_name)])

Subscribers

People subscribed via source and target branches

to status/vote changes: