launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03130
[Merge] lp:~bac/launchpad/bug-745660 into lp:launchpad
Brad Crittenden has proposed merging lp:~bac/launchpad/bug-745660 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #745660 in Launchpad itself: "Attempt to subscribe team that is not associated to distribution fails"
https://bugs.launchpad.net/launchpad/+bug/745660
For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-745660/+merge/55636
= Summary =
If a distribution has a bug supervisor team set then only members of
that team my have a structural subscription to that distro. This branch
filters out those teams from the ones presented in the structural
subscription widget.
== Proposed fix ==
The set of teams exposed to JavaScript is filtered under the conditions
stated above.
== Pre-implementation notes ==
Chats with Gary about the problem.
== Implementation details ==
As above.
== Tests ==
bin/test -vvm lp.bugs -t test_expose
== Demo and Q/A ==
Go to https://launchpad.dev/+feature-rules and set the following rule:
malone.advanced-structural-subscriptions.enabled default 1 on
Login as stevea/test.
Go to https://launchpad.dev/ubuntu and click on 'Subscribe to bug mail'.
Note that 'Ubuntu Gnome Team' is in the list of teams Steve can subscribe.
Go to http://bugs.launchpad.dev/ubuntu and set the Ubuntu Team as the
bug supervisor. You'll note that Ubuntu Gnome Team is not a member of
Ubuntu Team.
Go back to https://launchpad.dev/ubuntu and click on 'Subscribe to bug
mail'. Now you'll see that Ubuntu Gnome Team is not one of his
selections. QED.
= Launchpad lint =
(Will fix as appropriate.)
Checking for conflicts and issues in changed files.
Linting changed files:
.bzrignore
lib/lp/bugs/browser/structuralsubscription.py
lib/lp/bugs/browser/tests/test_expose.py
Makefile
./lib/lp/bugs/browser/tests/test_expose.py
111: E231 missing whitespace after ','
./Makefile
81: Line exceeds 78 characters.
159: Line exceeds 78 characters.
289: Line exceeds 78 characters.
416: Line exceeds 78 characters.
446: Line exceeds 78 characters.
480: Line exceeds 78 characters.
--
https://code.launchpad.net/~bac/launchpad/bug-745660/+merge/55636
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-745660 into lp:launchpad.
=== modified file '.bzrignore'
--- .bzrignore 2011-03-30 19:42:37 +0000
+++ .bzrignore 2011-03-30 21:15:59 +0000
@@ -80,3 +80,4 @@
.idea
run.gdb
lib/canonical/launchpad/icing/icon-sprites
+lib/canonical/launchpad/icing/icon-sprites.positioning
=== modified file 'Makefile'
--- Makefile 2011-03-30 19:42:37 +0000
+++ Makefile 2011-03-30 21:15:59 +0000
@@ -159,9 +159,12 @@
${LP_BUILT_JS_ROOT}/sprite.css: bin/sprite-util ${ICING}/sprite.css.in ${ICING}/icon-sprites.positioning
${SHHH} bin/sprite-util create-css
-sprite_image: ${ICING}/icon-sprites
-
-${ICING}/icon-sprites: bin/sprite-util ${ICING}/icon-sprites.positioning
+sprite_image: ${ICING}/icon-sprites ${ICING}/icon-sprites.positioning
+
+${ICING}/icon-sprites.positioning: bin/sprite-util
+ ${SHHH} bin/sprite-util create-image
+
+${ICING}/icon-sprites: bin/sprite-util
${SHHH} bin/sprite-util create-image
# We absolutely do not want to include the lazr.testing module and
=== removed file 'lib/canonical/launchpad/icing/icon-sprites.positioning'
--- lib/canonical/launchpad/icing/icon-sprites.positioning 2011-03-22 05:56:34 +0000
+++ lib/canonical/launchpad/icing/icon-sprites.positioning 1970-01-01 00:00:00 +0000
@@ -1,484 +0,0 @@
-/* DO NOT EDIT THIS FILE BY HAND!!! */
-/* It is autogenerated by spriteutils. */
-{
- "../images/arrowLeft.png": [
- 0,
- -14918
- ],
- "../images/cancel.png": [
- 0,
- -7540
- ],
- "../images/milestone.png": [
- 0,
- -3440
- ],
- "../images/build-needed.png": [
- 0,
- -13442
- ],
- "../images/team.png": [
- 0,
- -2130
- ],
- "../images/bug-undecided.png": [
- 0,
- -8036
- ],
- "../images/blueprint-low.png": [
- 0,
- -5900
- ],
- "../images/meeting.png": [
- 0,
- -10490
- ],
- "../images/no.png": [
- 0,
- -1312
- ],
- "../images/distribution-badge.png": [
- 0,
- -9184
- ],
- "../images/arrowTop.png": [
- 0,
- -14590
- ],
- "../images/zoom-in.png": [
- 0,
- -11802
- ],
- "../images/team-badge.png": [
- 0,
- -2294
- ],
- "../images/blue-bar.png": [
- 0,
- -15082
- ],
- "../images/arrowStart.png": [
- 0,
- -14262
- ],
- "../images/ppa-icon-inactive.png": [
- 0,
- -12458
- ],
- "../images/zoom-out.png": [
- 0,
- -11966
- ],
- "../images/purple-bar.png": [
- 0,
- -15410
- ],
- "../images/bullet.png": [
- 0,
- -11638
- ],
- "../images/info-large.png": [
- 0,
- -17504
- ],
- "../images/trash-logo.png": [
- 0,
- -20340
- ],
- "../images/warning.png": [
- 0,
- -10162
- ],
- "../images/mail.png": [
- 0,
- -3932
- ],
- "../images/build-failure.png": [
- 0,
- -13606
- ],
- "../images/branch-large.png": [
- 0,
- -16230
- ],
- "../images/download-large.png": [
- 0,
- -17322
- ],
- "../images/private-large.png": [
- 0,
- -18232
- ],
- "../images/launchpad-large.png": [
- 0,
- -17686
- ],
- "../images/translation-file.png": [
- 0,
- -10818
- ],
- "../images/source-package-recipe.png": [
- 0,
- -12622
- ],
- "../images/project-logo.png": [
- 0,
- -18842
- ],
- "../images/bug-medium.png": [
- 0,
- -4752
- ],
- "../images/architecture.png": [
- 0,
- -12130
- ],
- "../images/tour-icon": [
- 0,
- -16066
- ],
- "../images/trash-icon.png": [
- 0,
- -11146
- ],
- "../images/person-inactive.png": [
- 0,
- -6722
- ],
- "../images/arrowBottom.png": [
- 0,
- -14754
- ],
- "../images/project.png": [
- 0,
- -9508
- ],
- "../images/crowd.png": [
- 0,
- -1640
- ],
- "../images/info.png": [
- 0,
- -492
- ],
- "../images/flame-icon.png": [
- 0,
- -7872
- ],
- "../images/ubuntu-icon.png": [
- 0,
- -6556
- ],
- "../images/link.png": [
- 0,
- -3768
- ],
- "../images/person-logo.png": [
- 0,
- -19270
- ],
- "../images/distribution-logo.png": [
- 0,
- -18628
- ],
- "../images/retry.png": [
- 0,
- -9020
- ],
- "../images/rss.png": [
- 0,
- -6392
- ],
- "../images/private.png": [
- 0,
- -10326
- ],
- "../images/merge-proposal-icon.png": [
- 0,
- -12950
- ],
- "../images/download.png": [
- 0,
- -984
- ],
- "../images/arrowDown.png": [
- 0,
- -14098
- ],
- "../images/package-binary.png": [
- 0,
- -8856
- ],
- "../images/maybe.png": [
- 0,
- -7212
- ],
- "../images/bug-status-expand.png": [
- 0,
- -12786
- ],
- "../images/crowd-large.png": [
- 0,
- -16594
- ],
- "../images/blueprint.png": [
- 0,
- -5244
- ],
- "../images/project-badge.png": [
- 0,
- -9346
- ],
- "../images/bug-high.png": [
- 0,
- -4588
- ],
- "../images/blueprint-undefined.png": [
- 0,
- -6064
- ],
- "../images/blueprint-not.png": [
- 0,
- -6228
- ],
- "../images/stop.png": [
- 0,
- -11310
- ],
- "../images/flame-large.png": [
- 0,
- -17140
- ],
- "../images/bug-dupe-icon.png": [
- 0,
- -8528
- ],
- "../images/bug-critical.png": [
- 0,
- -4424
- ],
- "../images/build-success.png": [
- 0,
- -13278
- ],
- "../images/haspatch-icon.png": [
- 0,
- -15902
- ],
- "../images/person-inactive-badge.png": [
- 0,
- -6886
- ],
- "../images/ppa-icon.png": [
- 0,
- -12294
- ],
- "../images/yes.png": [
- 0,
- -1476
- ],
- "../images/team-logo.png": [
- 0,
- -19698
- ],
- "../images/arrowRight.png": [
- 0,
- -2456
- ],
- "../images/blueprint-high.png": [
- 0,
- -5572
- ],
- "../images/product.png": [
- 0,
- -9834
- ],
- "../images/bug-low.png": [
- 0,
- -4916
- ],
- "../images/package-source.png": [
- 0,
- -3276
- ],
- "../images/language.png": [
- 0,
- -3604
- ],
- "../images/person.png": [
- 0,
- -1804
- ],
- "../images/arrowUp.png": [
- 0,
- -13934
- ],
- "../images/distribution.png": [
- 0,
- -3112
- ],
- "../images/error-large.png": [
- 0,
- -16958
- ],
- "../images/news.png": [
- 0,
- -15738
- ],
- "../images/treeExpanded.png": [
- 0,
- -2784
- ],
- "../images/build-depwait.png": [
- 0,
- -13770
- ],
- "../images/blueprint-essential.png": [
- 0,
- -5408
- ],
- "../images/question.png": [
- 0,
- -656
- ],
- "../images/error.png": [
- 0,
- -7376
- ],
- "../images/bug-unknown.png": [
- 0,
- -8364
- ],
- "../images/product-logo.png": [
- 0,
- -19056
- ],
- "../images/blueprint-medium.png": [
- 0,
- -5736
- ],
- "../images/product-badge.png": [
- 0,
- -9672
- ],
- "../images/list.png": [
- 0,
- -11474
- ],
- "../images/launchpad-logo.png": [
- 0,
- -18414
- ],
- "../images/flame-logo.png": [
- 0,
- -20126
- ],
- "../images/translation-template.png": [
- 0,
- -10982
- ],
- "../images/bugtracker-icon.png": [
- 0,
- -8692
- ],
- "../images/meeting-logo.png": [
- 0,
- -19912
- ],
- "../images/treeCollapsed.png": [
- 0,
- -2620
- ],
- "../images/green-bar.png": [
- 0,
- -15246
- ],
- "../images/build-superseded.png": [
- 0,
- -13114
- ],
- "../images/trash-large.png": [
- 0,
- -18050
- ],
- "../images/red-bar.png": [
- 0,
- -15574
- ],
- "../images/add.png": [
- 0,
- 0
- ],
- "../images/remove.png": [
- 0,
- -328
- ],
- "../images/read-only.png": [
- 0,
- -9998
- ],
- "../images/person-inactive-logo.png": [
- 0,
- -19484
- ],
- "../images/edit.png": [
- 0,
- -164
- ],
- "../images/bug-wishlist.png": [
- 0,
- -5080
- ],
- "../images/warning-large.png": [
- 0,
- -16412
- ],
- "../images/arrowEnd.png": [
- 0,
- -14426
- ],
- "../images/cve.png": [
- 0,
- -4096
- ],
- "../images/notification-close.png": [
- 0,
- -20725
- ],
- "../images/merge-proposal-large.png": [
- 0,
- -17868
- ],
- "../images/branch.png": [
- 0,
- -2948
- ],
- "../images/person-badge.png": [
- 0,
- -1968
- ],
- "../images/notification-private.png": [
- 0,
- -20554
- ],
- "../images/bug.png": [
- 0,
- -4260
- ],
- "../images/bug-remote.png": [
- 0,
- -8200
- ],
- "../images/translation.png": [
- 0,
- -10654
- ],
- "../images/confirm.png": [
- 0,
- -7706
- ],
- "../images/search.png": [
- 0,
- -1148
- ]
-}
\ No newline at end of file
=== modified file 'lib/lp/bugs/browser/structuralsubscription.py'
--- lib/lp/bugs/browser/structuralsubscription.py 2011-03-29 22:34:04 +0000
+++ lib/lp/bugs/browser/structuralsubscription.py 2011-03-30 21:15:59 +0000
@@ -58,6 +58,9 @@
IStructuralSubscriptionForm,
IStructuralSubscriptionTarget,
)
+from lp.registry.interfaces.distribution import (
+ IDistribution,
+ )
from lp.registry.interfaces.distributionsourcepackage import (
IDistributionSourcePackage,
)
@@ -376,7 +379,7 @@
def expose_structural_subscription_data_to_js(context, request,
user, subscriptions=None):
"""Expose all of the data for a structural subscription to JavaScript."""
- expose_user_administered_teams_to_js(request, user)
+ expose_user_administered_teams_to_js(request, user, context)
expose_enum_to_js(request, BugTaskImportance, 'importances')
expose_enum_to_js(request, BugTaskStatus, 'statuses')
if subscriptions is None:
@@ -393,13 +396,20 @@
IJSONRequestCache(request).objects[name] = info
-def expose_user_administered_teams_to_js(request, user,
+def expose_user_administered_teams_to_js(request, user, context,
absoluteURL=absoluteURL):
- """Make the list of teams the user adminsters available to JavaScript."""
+ """Make the list of teams the user administers available to JavaScript."""
info = []
api_request = IWebServiceClientRequest(request)
+ is_distro = IDistribution.providedBy(context)
if user is not None:
for team in user.getAdministratedTeams():
+ # If the context is a distro AND a bug supervisor is set AND
+ # the admininistered team is not a member of the bug supervisor
+ # team THEN skip it.
+ if (is_distro and context.bug_supervisor is not None and
+ not team.inTeam(context.bug_supervisor)):
+ continue
info.append({
'link': absoluteURL(team, api_request),
'title': team.title,
=== modified file 'lib/lp/bugs/browser/tests/test_expose.py'
--- lib/lp/bugs/browser/tests/test_expose.py 2011-03-24 15:31:53 +0000
+++ lib/lp/bugs/browser/tests/test_expose.py 2011-03-30 21:15:59 +0000
@@ -86,33 +86,126 @@
return self.return_value
-class TestStructuralSubscriptionHelpers(TestCase):
- """Test the helpers used to add data that the on-page JS can use."""
-
- def test_teams(self):
+class TestExposeAdministeredTeams(TestCaseWithFactory):
+ """Test the function to expose administered team."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestExposeAdministeredTeams, self).setUp()
+ self.request = FakeRequest()
+ self.user = self.factory.makePerson()
+
+ def _setup_teams(self, owner):
+ self.bug_super_team = self.factory.makeTeam(
+ name='bug-supervisor-team', owner=owner)
+ bug_super_subteam = self.factory.makeTeam(
+ name='bug-supervisor-sub-team', owner=owner)
+ self.factory.makeTeam(
+ name='unrelated-team', owner=owner)
+ with person_logged_in(owner):
+ bug_super_subteam.join(
+ self.bug_super_team, self.bug_super_team.teamowner)
+
+ def _sort(self, team_info, key='title'):
+ return sorted(team_info, cmp=lambda a,b: cmp(a[key], b[key]))
+
+ def test_teams_for_non_distro(self):
# The expose_user_administered_teams_to_js function loads some data
# about the teams the requesting user administers into the response to
# be made available to JavaScript.
- request = FakeRequest()
- user = FakeUser()
- expose_user_administered_teams_to_js(request, user,
+ context = self.factory.makeProduct(owner=self.user)
+ self._setup_teams(self.user)
+
+ expose_user_administered_teams_to_js(self.request, self.user, context,
absoluteURL=fake_absoluteURL)
# The team information should have been added to the request.
- self.assertThat(request.objects, Contains('administratedTeams'))
- team_info = request.objects['administratedTeams']
- # Since there are two (fake) teams, there should be two items in the
+ self.assertThat(self.request.objects, Contains('administratedTeams'))
+ team_info = self._sort(self.request.objects['administratedTeams'])
+ # Since there are three teams, there should be three items in the
# list of team info.
- self.assertThat(len(team_info), Equals(2))
- # The items info consist of a dictionary with link and title keys.
- self.assertThat(team_info[0], KeysEqual('link', 'title'))
- self.assertThat(team_info[1], KeysEqual('link', 'title'))
- # The link is the title of the team.
- self.assertThat(team_info[0]['title'], Equals('Team One'))
- # The link is the API link to the team.
- self.assertThat(team_info[0]['link'],
- Equals('http://example.com/TeamOne'))
+ expected_number_teams = 3
+ self.assertThat(len(team_info), Equals(expected_number_teams))
+ # The items info consist of a dictionary with link and title keys.
+ for i in range(expected_number_teams):
+ self.assertThat(team_info[i], KeysEqual('link', 'title'))
+ # The link is the title of the team.
+ self.assertThat(
+ team_info[0]['title'], Equals(u'Bug Supervisor Sub Team'))
+ self.assertThat(
+ team_info[1]['title'], Equals(u'Bug Supervisor Team'))
+ self.assertThat(
+ team_info[2]['title'], Equals(u'Unrelated Team'))
+ # The link is the API link to the team.
+ self.assertThat(team_info[0]['link'],
+ Equals('http://example.com/BugSupervisorSubTeam'))
+
+ def test_teams_for_distro_with_bug_super(self):
+ self._setup_teams(self.user)
+ context = self.factory.makeDistribution(
+ owner=self.user, members=self.bug_super_team)
+ with person_logged_in(self.user):
+ context.setBugSupervisor(
+ self.bug_super_team, self.user)
+
+ expose_user_administered_teams_to_js(self.request, self.user, context,
+ absoluteURL=fake_absoluteURL)
+
+ # The team information should have been added to the request.
+ self.assertThat(self.request.objects, Contains('administratedTeams'))
+ team_info = self._sort(self.request.objects['administratedTeams'])
+
+ # Since the distro only returns teams that are members of the bug
+ # supervisor team, we only expect two.
+ expected_number_teams = 2
+ self.assertThat(len(team_info), Equals(expected_number_teams))
+ # The items info consist of a dictionary with link and title keys.
+ for i in range(expected_number_teams):
+ self.assertThat(team_info[i], KeysEqual('link', 'title'))
+ # The link is the title of the team.
+ self.assertThat(
+ team_info[0]['title'], Equals(u'Bug Supervisor Sub Team'))
+ self.assertThat(
+ team_info[1]['title'], Equals(u'Bug Supervisor Team'))
+ # The link is the API link to the team.
+ self.assertThat(team_info[0]['link'],
+ Equals('http://example.com/BugSupervisorSubTeam'))
+
+ def test_teams_for_distro_with_no_bug_super(self):
+ self._setup_teams(self.user)
+ context = self.factory.makeDistribution(
+ owner=self.user, members=self.bug_super_team)
+
+ expose_user_administered_teams_to_js(self.request, self.user, context,
+ absoluteURL=fake_absoluteURL)
+
+ # The team information should have been added to the request.
+ self.assertThat(self.request.objects, Contains('administratedTeams'))
+ team_info = self._sort(self.request.objects['administratedTeams'])
+
+ # Since the distro has no bug supervisor set, all administered teams
+ # are returned.
+ expected_number_teams = 3
+ self.assertThat(len(team_info), Equals(expected_number_teams))
+ # The items info consist of a dictionary with link and title keys.
+ for i in range(expected_number_teams):
+ self.assertThat(team_info[i], KeysEqual('link', 'title'))
+ # The link is the title of the team.
+ self.assertThat(
+ team_info[0]['title'], Equals(u'Bug Supervisor Sub Team'))
+ self.assertThat(
+ team_info[1]['title'], Equals(u'Bug Supervisor Team'))
+ self.assertThat(
+ team_info[2]['title'], Equals(u'Unrelated Team'))
+ # The link is the API link to the team.
+ self.assertThat(team_info[0]['link'],
+ Equals('http://example.com/BugSupervisorSubTeam'))
+
+
+class TestStructuralSubscriptionHelpers(TestCase):
+ """Test the helpers used to add data that the on-page JS can use."""
def test_expose_enum_to_js(self):
# Loads the titles of an enum into the response.
Follow ups
