Данило Шеган has proposed merging
lp:~danilo/launchpad/duplicate-pillars-subscriptions into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~danilo/launchpad/duplicate-pillars-subscriptions/+merge/58135
= Group pillar subscriptions =
In testing and QA for bug 728370, we've noticed that sometimes multiple
identical subscription descriptions show up. This turns out to be when there
are several bug tasks for the same pillar (eg. one for Ubuntu Hoary, and
another for Ubuntu Warty). Thus, we need to group them together.
== Proposed fix ==
Treat a list of subscriptions for supervisor as a set.
I am not fixing any of the lint issues yet (I've got a bigger branch up for
review that I'd rather not have hit any conflicts if I fix them in there as
well).
== Tests ==
lib/lp/bugs/javascript/tests/test_subscription.html
== Demo and Q/A ==
https://bugs.launchpad.dev/tomcat/+bug/2/+subscriptions
To facilitate the demo:
- set 'malone.advanced-structural-subscriptions.enabled default 1 on'
on https://launchpad.dev/+feature-rules
- log in as [email protected]:cprov
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/javascript/subscription.js
lib/lp/bugs/javascript/tests/test_subscription.js
./lib/lp/bugs/javascript/subscription.js
99: Unexpected ','.
109: Unexpected ','.
136: Expected '===' and instead saw '=='.
156: Move 'var' declarations to the top of the function.
156: Stopping. (20% scanned).
0: JSLINT had a fatal error.
./lib/lp/bugs/javascript/tests/test_subscription.js
49: Expected ';' and instead saw 'Y'.
182: Unexpected ','.
190: Move 'var' declarations to the top of the function.
190: Stopping. (12% scanned).
0: JSLINT had a fatal error.
--
https://code.launchpad.net/~danilo/launchpad/duplicate-pillars-subscriptions/+merge/58135
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~danilo/launchpad/duplicate-pillars-subscriptions into lp:launchpad.
=== modified file 'lib/lp/bugs/javascript/subscription.js'
--- lib/lp/bugs/javascript/subscription.js 2011-04-14 20:03:32 +0000
+++ lib/lp/bugs/javascript/subscription.js 2011-04-18 14:39:26 +0000
@@ -286,6 +286,35 @@
gather_subscriptions_as_assignee;
/**
+ * Adds a `subscription` to `subscriptions` if it's not in the list already.
+ * Compares reason, action and all the `vars` from existing subscription.
+ */
+function add_subscription_to_set(subscriptions, subscription) {
+ for (var index in subscriptions) {
+ var sub = subscriptions[index];
+ if (sub.reason === subscription.reason &&
+ sub.action === subscription.action) {
+ var are_vars_same = true;
+ for (var param in sub.vars) {
+ // We only check vars from the existing subscription.
+ // Theoretically, there could be a var on `subscription`
+ // not present on `sub`, but we're guarding against that
+ // with reason/action checks.
+ if (sub.vars[param].self !== subscription.vars[param].self) {
+ are_vars_same = false;
+ break;
+ }
+ }
+ if (are_vars_same) {
+ return;
+ }
+ }
+ }
+ // We haven't found matching subscriptions, add it.
+ subscriptions.push(subscription);
+}
+
+/**
* Gather subscription information for implicit bug supervisor.
*/
function gather_subscriptions_as_supervisor(category) {
@@ -294,7 +323,7 @@
for (var index in category.personal) {
var subscription = category.personal[index];
- subscriptions.push({
+ add_subscription_to_set(subscriptions, {
reason: reasons.YOU_OWNER,
vars: {
pillar: get_link_data(subscription.pillar)
@@ -305,7 +334,7 @@
for (var index in category.as_team_member) {
var team_subscription = category.as_team_member[index];
- subscriptions.push({
+ add_subscription_to_set(subscriptions, {
reason: reasons.TEAM_OWNER,
vars: {
team: get_link_data(team_subscription.principal),
@@ -317,7 +346,7 @@
for (var index in category.as_team_admin) {
var team_subscription = category.as_team_admin[index];
- subscriptions.push({
+ add_subscription_to_set(subscriptions, {
reason: reasons.ADMIN_TEAM_OWNER,
vars: {
team: get_link_data(team_subscription.principal),
=== modified file 'lib/lp/bugs/javascript/tests/test_subscription.js'
--- lib/lp/bugs/javascript/tests/test_subscription.js 2011-04-14 20:03:32 +0000
+++ lib/lp/bugs/javascript/tests/test_subscription.js 2011-04-18 14:39:26 +0000
@@ -330,7 +330,8 @@
// supervisor.
var mock_category = {
count: 2,
- personal: [{pillar: 'project'}, {pillar: 'distro'}],
+ personal: [ {pillar: {title: 'project'} },
+ {pillar: {title:'distro'} }],
as_team_member: [],
as_team_admin: []
};
@@ -365,9 +366,9 @@
count: 2,
personal: [],
as_team_member: [{ principal: 'team1',
- pillar: 'project' },
+ pillar: {display_name: 'project'} },
{ principal: 'team2',
- pillar: 'distro' }],
+ pillar: {display_name: 'distro'} }],
as_team_admin: []
};
var subs = module._gather_subscriptions_as_supervisor(mock_category);
@@ -403,14 +404,33 @@
personal: [],
as_team_member: [],
as_team_admin: [{ principal: 'team1',
- pillar: 'project' },
- { principal: 'team2',
- pillar: 'distro' }]
+ pillar: {display_name: 'project'} },
+ { principal: 'team2',
+ pillar: {display_name: 'distro'} }]
};
var subs = module._gather_subscriptions_as_supervisor(mock_category);
Y.Assert.areEqual(2, subs.length);
},
+ test_repeated_pillars: function() {
+ // Different bug tasks might still be on the same pillar,
+ // and we should only get one action.
+ var mock_pillar = { display_name: 'project',
+ web_link: 'http://project/' };
+ var mock_category = {
+ count: 1,
+ personal: [{pillar: mock_pillar},
+ {pillar: mock_pillar}],
+ as_team_member: [],
+ as_team_admin: []
+ };
+ var subs = module._gather_subscriptions_as_supervisor(mock_category);
+ Y.Assert.areEqual(1, subs.length);
+ Y.Assert.areEqual(module._reasons.YOU_OWNER, subs[0].reason);
+ Y.Assert.areEqual(mock_pillar, subs[0].vars.pillar.self);
+ Y.Assert.areEqual(module._actions.SET_BUG_SUPERVISOR, subs[0].action);
+ },
+
test_combined: function() {
// Test that multiple implicit bug supervisor roles
// are all returned.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
