Merge lp:~benji/launchpad/bug-771231 into lp:launchpad

Proposed by Benji York
Status: Merged
Approved by: Benji York
Approved revision: no longer in the source branch.
Merged at revision: 12964
Proposed branch: lp:~benji/launchpad/bug-771231
Merge into: lp:launchpad
Diff against target: 206 lines (+113/-16)
3 files modified
lib/lp/registry/javascript/structural-subscription.js (+30/-0)
lib/lp/registry/javascript/tests/test_structural_subscription.html (+1/-0)
lib/lp/registry/javascript/tests/test_structural_subscription.js (+82/-16)
To merge this branch: bzr merge lp:~benji/launchpad/bug-771231
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+59555@code.launchpad.net

Commit message

[r=bac][bug=771231] add feedback when creating a structural subscription

Description of the change

Bug 771231 is about getting confirmation that adding a structural
subscription worked and that the new subscription is configured the way
the user intended. This branch adds such a confirmation by building an
informational message box (the same kind of box
response.addNotification(text) results in).

The related JS tests can be run by loading
lib/lp/registry/javascript/tests/test_structural_subscription.html in a
browser. They all pass locally.

The make lint report is clean.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
2--- lib/lp/registry/javascript/structural-subscription.js 2011-04-28 17:07:38 +0000
3+++ lib/lp/registry/javascript/structural-subscription.js 2011-04-29 21:10:54 +0000
4@@ -1567,6 +1567,36 @@
5 config, subscription_info, 0,
6 filter_info, filter_id, anim_node);
7 Y.lazr.anim.green_flash({node: anim_node}).run();
8+ } else {
9+ // Since there is no filter description to update we need another
10+ // way to tell the user that the subscription was sucessfully
11+ // added. We'll do that by creating an informational message box.
12+ var description = filter.get('description');
13+ var header = Y.Node.create('<div/>')
14+ .setStyle('marginBottom', '1em')
15+ .append('The subscription')
16+ .append('&#32;'); // a space
17+ if (description) {
18+ header
19+ .append('named "')
20+ .append(Y.Node.create('<span/>')
21+ .set('text', description))
22+ .append('"')
23+ .append('&#32;'); // a space
24+ }
25+ header.append('has been created.');
26+
27+ Y.one('#request-notifications')
28+ .empty()
29+ .append(Y.Node.create('<div/>')
30+ // We're creating an informational message box.
31+ .addClass('informational')
32+ .addClass('message')
33+ // The box needs to be a little wider to accomodate the
34+ // wordy subscription description.
35+ .setStyle('width', '50em')
36+ .append(header)
37+ .append(create_filter_description(filter.getAttrs())));
38 }
39 };
40 }
41
42=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.html'
43--- lib/lp/registry/javascript/tests/test_structural_subscription.html 2011-04-11 16:12:18 +0000
44+++ lib/lp/registry/javascript/tests/test_structural_subscription.html 2011-04-29 21:10:54 +0000
45@@ -55,6 +55,7 @@
46 </style>
47 </head>
48 <body class="yui3-skin-sam">
49+ <div id="request-notifications"></div>
50 <div id="log"></div>
51 </body>
52 </html>
53
54=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
55--- lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-04-27 22:00:21 +0000
56+++ lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-04-29 21:10:54 +0000
57@@ -907,8 +907,18 @@
58 setUp: function() {
59 var TestBugFilter = function() {};
60 TestBugFilter.prototype = {
61- 'getAttrs': function () {
62- return {};
63+ get: function (name) {
64+ return 'DESCRIPTION';
65+ },
66+ getAttrs: function () {
67+ return {
68+ description: 'DESCRIPTION',
69+ statuses: [],
70+ importances: [],
71+ tags: [],
72+ find_all_tags: true,
73+ bug_notification_level: 'Discussion'
74+ };
75 }
76 };
77 // We need an lp_client that will appear to succesfully create the
78@@ -920,7 +930,7 @@
79 this.post_called = true;
80 },
81 patch: function(uri, representation, config, headers) {
82- config.on.success();
83+ config.on.success(new TestBugFilter());
84 this.patch_called = true;
85 },
86 post_called: false,
87@@ -977,6 +987,49 @@
88 }));
89
90 suite.add(new Y.Test.Case({
91+ // The make_add_subscription_success_handler function constructs a
92+ // function that gives a visual feedback after adding a subscription.
93+
94+ name:
95+ 'Structural Subscription: make_add_subscription_success_handler',
96+
97+ _should: {error: {}},
98+
99+ setUp: function() {
100+ this.TestBugFilter = function() {};
101+ this.TestBugFilter.prototype = {
102+ get: function (name) {
103+ return 'DESCRIPTION';
104+ },
105+ getAttrs: function () {
106+ return {
107+ description: 'DESCRIPTION',
108+ statuses: [],
109+ importances: [],
110+ tags: [],
111+ find_all_tags: true,
112+ bug_notification_level: 'Discussion'
113+ };
114+ }
115+ };
116+ },
117+
118+ test_description_is_added: function() {
119+ // If we add a subscription on a page that doesn't display
120+ // subcription details then we need to add an "informational
121+ // message" describing the just-added subscription.
122+ var handler;
123+ var config = {add_filter_description: false};
124+ handler = module._make_add_subscription_success_handler(config);
125+ var form_data = {};
126+ handler(form_data, new this.TestBugFilter());
127+ var text = Y.one('#request-notifications').get('text');
128+ Assert.isTrue(text.indexOf('DESCRIPTION') !== -1);
129+ }
130+
131+ }));
132+
133+ suite.add(new Y.Test.Case({
134 name: 'Structural Subscription: edit subcription workflow',
135
136 _should: {error: {}},
137@@ -1198,10 +1251,11 @@
138 };
139 this.content_box = create_test_node();
140 Y.one('body').appendChild(this.content_box);
141+ this.original_lp = monkeypatch_LP();
142 },
143
144 tearDown: function() {
145- //delete this.configuration;
146+ window.LP = this.original_lp;
147 remove_test_node();
148 delete this.content_box;
149 },
150@@ -1257,18 +1311,6 @@
151 }, 20);
152 },
153
154- // Success handler for adding a subscription does nothing in
155- // the DOM if config.add_filter_description is not set.
156- test_make_add_subscription_success_handler_nothing: function() {
157- var success_handler =
158- module._make_add_subscription_success_handler(this.config);
159- var subs_list = this.content_box.appendChild(
160- Y.Node.create('<div id="structural-subscriptions"></div>'));
161- success_handler();
162- // No sub-nodes have been created in the subs_list node.
163- Assert.isTrue(subs_list.all('div.subscription-filter').isEmpty());
164- },
165-
166 // Success handler for adding a subscription creates
167 // a subscription listing if there's none and adds a filter to it.
168 test_make_add_subscription_success_handler_empty_list: function() {
169@@ -1286,6 +1328,9 @@
170 url: "http://target/" };
171 window.LP.cache.target_info = target_info;
172 var filter = {
173+ get: function (name) {
174+ return 'DESCRIPTION';
175+ },
176 getAttrs: function() {
177 return {
178 importances: [],
179@@ -1330,6 +1375,27 @@
180 var target_info = {
181 title: "Subscription target",
182 url: "http://target/" };
183+ window.LP.cache.subscription_info = [{
184+ target_url: 'http://example.com',
185+ target_title:'Example project',
186+ filters: [{
187+ filter: {
188+ description: 'DESCRIPTION',
189+ statuses: [],
190+ importances: [],
191+ tags: [],
192+ find_all_tags: true,
193+ bug_notification_level: 'Discussion',
194+ self_link: 'http://example.com/a_filter'
195+ },
196+ can_mute: true,
197+ is_muted: false,
198+ subscriber_is_team: false,
199+ subscriber_url: 'http://example.com/subscriber',
200+ subscriber_title: 'Thidwick',
201+ user_is_team_admin: false
202+ }]
203+ }];
204 window.LP.cache.target_info = target_info;
205 var filter = {
206 getAttrs: function() {