Merge lp:~allenap/launchpad/show-me-too-counts into lp:launchpad
- show-me-too-counts
- Merge into devel
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Approved by: | Gavin Panella | ||||||||
Approved revision: | not available | ||||||||
Merged at revision: | not available | ||||||||
Proposed branch: | lp:~allenap/launchpad/show-me-too-counts | ||||||||
Merge into: | lp:launchpad | ||||||||
Diff against target: |
604 lines (+380/-64) 7 files modified
lib/canonical/launchpad/javascript/bugs/bugtask-index.js (+63/-9) lib/canonical/launchpad/javascript/bugs/tests/test_me_too.js (+34/-1) lib/lp/bugs/browser/bugtask.py (+56/-0) lib/lp/bugs/browser/tests/test_bugtask.py (+136/-0) lib/lp/bugs/stories/bugs/xx-bug-affects-me-too.txt (+26/-4) lib/lp/bugs/templates/bugtasks-and-nominations-table.pt (+63/-48) lib/lp/bugs/windmill/tests/test_bug_me_too.py (+2/-2) |
||||||||
To merge this branch: | bzr merge lp:~allenap/launchpad/show-me-too-counts | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | js | Abstain | |
Abel Deuring (community) | code | Approve | |
Michael Nelson (community) | ui | Approve | |
Review via email: mp+15947@code.launchpad.net |
Commit message
Description of the change
Gavin Panella (allenap) wrote : | # |
Michael Nelson (michael.nelson) wrote : | # |
> This branch does two things: shows the count of users affected by a
> bug (previously this has not been shown), and moves the "Does this bug
> affect you" line to above the bug task table.
Wow - that's really nice Gavin, especially the way you've ensure the language is very natural in every situation! (eg. "This bug affects you and 3 other people" versus "This bug affects 3 people, but not you")
We chatted about included the count for anon users:
<noodles775> allenap: wow, that feature is great!
<noodles775> allenap: I was wondering whether there was a decision behind *not* displaying the count for anon users?
<allenap> noodles775: No deliberate decision, just didn't actually think about it, doh.
<allenap> noodles775: I should probably add it!
<noodles775> allenap: that'd be great!
<allenap> noodles775: I'll make it simply say "This bug affects 27 people".
<noodles775> Perfect
Gavin Panella (allenap) wrote : | # |
Thanks for the UI review Michael! I've implemented that. It now says
"This bug affects 1 person" or "This bug affects x people" when x > 1.
If no one is affected, nothing is shown and there's no extraneous
white-space left in its wake.
I've attached a diff of the changes, though perhaps that's more for
the benefit of the code and js reviewer.
1 | === modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js' |
2 | --- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-12-10 14:23:23 +0000 |
3 | +++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-12-10 16:36:41 +0000 |
4 | @@ -1791,7 +1791,7 @@ |
5 | */ |
6 | function setup_add_attachment() { |
7 | // Find zero or more links to modify. |
8 | - var attachment_link = Y.get('.menu-link-addcomment'); |
9 | + var attachment_link = Y.all('.menu-link-addcomment'); |
10 | attachment_link.on('click', function(e) { |
11 | var comment_input = Y.one('[id="field.comment"]'); |
12 | if (comment_input.get('value') !== '') { |
13 | |
14 | === modified file 'lib/lp/bugs/browser/bugtask.py' |
15 | --- lib/lp/bugs/browser/bugtask.py 2009-12-10 11:51:24 +0000 |
16 | +++ lib/lp/bugs/browser/bugtask.py 2009-12-10 15:41:16 +0000 |
17 | @@ -3134,6 +3134,17 @@ |
18 | else: |
19 | return "This bug doesn't affect you" |
20 | |
21 | + @property |
22 | + def anon_affected_statement(self): |
23 | + """The "this bug affects" statement to show to anonymous users.""" |
24 | + if self.context.users_affected_count == 1: |
25 | + return "This bug affects 1 person" |
26 | + elif self.context.users_affected_count > 1: |
27 | + return "This bug affects %d people" % ( |
28 | + self.context.users_affected_count) |
29 | + else: |
30 | + return None |
31 | + |
32 | |
33 | class BugTaskTableRowView(LaunchpadView): |
34 | """Browser class for rendering a bugtask row on the bug page.""" |
35 | |
36 | === modified file 'lib/lp/bugs/browser/tests/test_bugtask.py' |
37 | --- lib/lp/bugs/browser/tests/test_bugtask.py 2009-12-10 11:51:24 +0000 |
38 | +++ lib/lp/bugs/browser/tests/test_bugtask.py 2009-12-10 15:46:47 +0000 |
39 | @@ -186,6 +186,24 @@ |
40 | "This bug affects 2 people, but not you", |
41 | self.view.affected_statement) |
42 | |
43 | + def test_anon_affected_statement_no_one_affected(self): |
44 | + self.bug.markUserAffected(self.bug.owner, False) |
45 | + self.failUnlessEqual(0, self.bug.users_affected_count) |
46 | + self.assertIs(None, self.view.anon_affected_statement) |
47 | + |
48 | + def test_anon_affected_statement_1_user_affected(self): |
49 | + self.failUnlessEqual(1, self.bug.users_affected_count) |
50 | + self.failUnlessEqual( |
51 | + "This bug affects 1 person", |
52 | + self.view.anon_affected_statement) |
53 | + |
54 | + def test_anon_affected_statement_2_users_affected(self): |
55 | + self.view.context.markUserAffected(self.view.user, True) |
56 | + self.failUnlessEqual(2, self.bug.users_affected_count) |
57 | + self.failUnlessEqual( |
58 | + "This bug affects 2 people", |
59 | + self.view.anon_affected_statement) |
60 | + |
61 | |
62 | def test_suite(): |
63 | suite = unittest.TestSuite() |
64 | |
65 | === modified file 'lib/lp/bugs/stories/bugs/xx-bug-affects-me-too.txt' |
66 | --- lib/lp/bugs/stories/bugs/xx-bug-affects-me-too.txt 2009-12-10 11:54:48 +0000 |
67 | +++ lib/lp/bugs/stories/bugs/xx-bug-affects-me-too.txt 2009-12-10 16:12:43 +0000 |
68 | @@ -73,6 +73,27 @@ |
69 | This bug affects 1 person, but not you |
70 | |
71 | |
72 | +== Anonymous users == |
73 | + |
74 | +Anonymous users just see the number of affected users. |
75 | + |
76 | + >>> anon_browser.open(test_bug_url) |
77 | + >>> print extract_text(find_tag_by_id( |
78 | + ... anon_browser.contents, 'affectsmetoo')) |
79 | + This bug affects 1 person |
80 | + |
81 | +If no one is marked as affected by the bug, the message does not |
82 | +appear at all to anonymous users. |
83 | + |
84 | + >>> login('test@canonical.com') |
85 | + >>> test_bug.markUserAffected(test_bug.owner, False) |
86 | + >>> logout() |
87 | + |
88 | + >>> anon_browser.open(test_bug_url) |
89 | + >>> print find_tag_by_id(anon_browser.contents, 'affectsmetoo') |
90 | + None |
91 | + |
92 | + |
93 | == Static and dynamic support == |
94 | |
95 | A bug page contains markup to support both static (no Javascript) and |
96 | |
97 | === modified file 'lib/lp/bugs/templates/bugtasks-and-nominations-table.pt' |
98 | --- lib/lp/bugs/templates/bugtasks-and-nominations-table.pt 2009-12-10 13:20:50 +0000 |
99 | +++ lib/lp/bugs/templates/bugtasks-and-nominations-table.pt 2009-12-10 16:04:44 +0000 |
100 | @@ -4,53 +4,60 @@ |
101 | define="context_menu context/menu:context" |
102 | omit-tag=""> |
103 | |
104 | -<div class="actions"> |
105 | - <span id="affectsmetoo" style="display: inline" |
106 | - tal:condition="link/enabled" |
107 | - tal:define="link context_menu/affectsmetoo; |
108 | - affected view/current_user_affected_status"> |
109 | - |
110 | - <tal:comment condition="nothing"> |
111 | - This .static section is shown in browsers with javascript |
112 | - enabled, and before setup_me_too is run. |
113 | - </tal:comment> |
114 | - <span class="static"> |
115 | - <tal:affected condition="affected"> |
116 | - <img width="14" height="14" src="/@@/flame-icon" alt="" /> |
117 | - <tal:statement replace="view/affected_statement" /> |
118 | - </tal:affected> |
119 | - <tal:not-affected condition="not:affected"> |
120 | - <tal:statement replace="view/affected_statement" /> |
121 | - </tal:not-affected> |
122 | - <a href="+affectsmetoo"> |
123 | +<tal:affects-editable condition="context_menu/affectsmetoo/enabled"> |
124 | + <div class="actions"> |
125 | + <span id="affectsmetoo" style="display: inline" |
126 | + tal:define="affected view/current_user_affected_status"> |
127 | + |
128 | + <tal:comment condition="nothing"> |
129 | + This .static section is shown in browsers with javascript |
130 | + enabled, and before setup_me_too is run. |
131 | + </tal:comment> |
132 | + <span class="static"> |
133 | + <tal:affected condition="affected"> |
134 | + <img width="14" height="14" src="/@@/flame-icon" alt="" /> |
135 | + <tal:statement replace="view/affected_statement" /> |
136 | + </tal:affected> |
137 | + <tal:not-affected condition="not:affected"> |
138 | + <tal:statement replace="view/affected_statement" /> |
139 | + </tal:not-affected> |
140 | + <a href="+affectsmetoo"> |
141 | + <img class="editicon" src="/@@/edit" alt="Edit" /> |
142 | + </a> |
143 | + </span> |
144 | + |
145 | + <tal:comment condition="nothing"> |
146 | + This .dynamic section is used by setup_me_too to display |
147 | + controls and information in the correct places. |
148 | + </tal:comment> |
149 | + <span class="dynamic unseen"> |
150 | + <img src="/@@/flame-icon" alt=""/> |
151 | + <a href="+affectsmetoo" class="js-action" |
152 | + ><span class="value" tal:content="view/affected_statement" /></a> |
153 | <img class="editicon" src="/@@/edit" alt="Edit" /> |
154 | - </a> |
155 | - </span> |
156 | - |
157 | - <tal:comment condition="nothing"> |
158 | - This .dynamic section is used by setup_me_too to display |
159 | - controls and information in the correct places. |
160 | - </tal:comment> |
161 | - <span class="dynamic unseen"> |
162 | - <img src="/@@/flame-icon" alt=""/> |
163 | - <a href="+affectsmetoo" class="js-action" |
164 | - ><span class="value" tal:content="view/affected_statement" /></a> |
165 | - <img class="editicon" src="/@@/edit" alt="Edit" /> |
166 | - </span> |
167 | - |
168 | - <script type="text/javascript" tal:content="string: |
169 | - LPS.use('event', 'bugs.bugtask_index', function(Y) { |
170 | - Y.on('load', function(e) { |
171 | - Y.bugs.setup_me_too( |
172 | - ${view/current_user_affected_js_status}, |
173 | - ${view/other_users_affected_count}); |
174 | - }, window); |
175 | - }); |
176 | - "> |
177 | - </script> |
178 | - |
179 | - </span> |
180 | -</div> |
181 | + </span> |
182 | + |
183 | + <script type="text/javascript" tal:content="string: |
184 | + LPS.use('event', 'bugs.bugtask_index', function(Y) { |
185 | + Y.on('load', function(e) { |
186 | + Y.bugs.setup_me_too( |
187 | + ${view/current_user_affected_js_status}, |
188 | + ${view/other_users_affected_count}); |
189 | + }, window); |
190 | + }); |
191 | + "> |
192 | + </script> |
193 | + |
194 | + </span> |
195 | + </div> |
196 | +</tal:affects-editable> |
197 | +<tal:affects-not-editable condition="not:context_menu/affectsmetoo/enabled"> |
198 | + <div class="actions" |
199 | + tal:define="statement view/anon_affected_statement" |
200 | + tal:condition="statement"> |
201 | + <span id="affectsmetoo" style="display: inline" tal:content="statement" /> |
202 | + </div> |
203 | +</tal:affects-not-editable> |
204 | |
205 | <script type="text/javascript"> |
206 | function toggleFormVisibility(row_id) { |
Gavin Panella (allenap) wrote : | # |
The cover letter needs some additional information:
lib/lp/
New property anon_affected_
users. Explains how many people are affected by the bug.
lib/lp/
Tests added for the template changes below.
lib/lp/
A simple statement about the number of affected users is now shown
to anonymous users.
Abel Deuring (adeuring) wrote : | # |
Wow, like Michael, I am really impressed by the effort you put into proper wording for the different cases if the current user is affected or not, iand how many other users are affected!
Thanks for the additional "normalisation", as dicussed on IRC:
(18:20:33) abel: allenap: shouldn't _getNewSourceNa
(18:21:41) allenap: abel: In that case it returns nothing. I'll add a comment to make that clear.
(18:23:13) abel: allenap: sure, is does nothing in this case. My question is: What will be displayed in this case?
(18:23:48) abel: allenap: ahm, now I got it...
(18:23:51) allenap: abel: The source_name values at around line 1552 in the same file.
(18:24:14) abel: allenap: thanks, sorry for being a bit confused today...
(18:24:25) allenap: abel: Perhaps I should change _getNewSourceNames to _getSourceNames and do it all from there?
(18:24:56) abel: allenap: right, I was thinking roughly in this direction too. sounds good
Gavin Panella (allenap) wrote : | # |
Thanks Abel!
I noticed (using LP_DEBUG_SQL=1) that the many references to current_
Gavin Panella (allenap) wrote : | # |
Abel reviewed the Javascript already.
Gavin Panella (allenap) wrote : | # |
Abel also approved revision 10039 on IRC, albeit in a version with white-space changes eliminated.
Preview Diff
1 | === modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js' |
2 | --- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-12-01 12:08:29 +0000 |
3 | +++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-12-11 14:04:24 +0000 |
4 | @@ -1495,7 +1495,7 @@ |
5 | * |
6 | * @method setup_me_too |
7 | */ |
8 | -bugs.setup_me_too = function(user_is_affected) { |
9 | +bugs.setup_me_too = function(user_is_affected, others_affected_count) { |
10 | // IE (7 & 8 tested) is stupid, stupid, stupid. |
11 | if (Y.UA.ie) { |
12 | return; |
13 | @@ -1504,7 +1504,8 @@ |
14 | var me_too_edit = new MeTooChoiceSource({ |
15 | contentBox: me_too_content, value: user_is_affected, |
16 | elementToFlash: me_too_content, |
17 | - editicon: ".dynamic img[src$=/@@/edit]" |
18 | + editicon: ".dynamic img[src$=/@@/edit]", |
19 | + others_affected_count: others_affected_count |
20 | }); |
21 | me_too_edit.render(); |
22 | }; |
23 | @@ -1547,12 +1548,10 @@ |
24 | */ |
25 | items: { |
26 | value: [ |
27 | - { name: 'Yes, it affects me', value: true, |
28 | - source_name: 'This bug affects me too', |
29 | - disabled: false }, |
30 | - { name: "No, it doesn't affect me", value: false, |
31 | - source_name: "This bug doesn't affect me", |
32 | - disabled: false } |
33 | + { name: 'Yes, it affects me', |
34 | + value: true, disabled: false }, |
35 | + { name: "No, it doesn't affect me", |
36 | + value: false, disabled: false } |
37 | ] |
38 | }, |
39 | |
40 | @@ -1572,6 +1571,16 @@ |
41 | set: function(v) { |
42 | return Y.one(v); |
43 | } |
44 | + }, |
45 | + |
46 | + /** |
47 | + * The number of other users currently affected by this bug. |
48 | + * |
49 | + * @attribute others_affected_count |
50 | + * @type Number |
51 | + */ |
52 | + others_affected_count: { |
53 | + value: null |
54 | } |
55 | }; |
56 | |
57 | @@ -1588,6 +1597,50 @@ |
58 | this.error_handler.showError = function(error_msg) { |
59 | widget.showError(error_msg); |
60 | }; |
61 | + // Set source_names. |
62 | + var others_affected_count = this.get('others_affected_count'); |
63 | + var source_names = this._getSourceNames(others_affected_count); |
64 | + Y.each(this.get('items'), function(item) { |
65 | + if (item.value in source_names) { |
66 | + item.source_name = source_names[item.value]; |
67 | + } |
68 | + }); |
69 | + }, |
70 | + |
71 | + /* |
72 | + * The results of _getSourceNames() should closely mirror the |
73 | + * results of BugTasksAndNominationsView.affected_statement and |
74 | + * anon_affected_statement. |
75 | + */ |
76 | + _getSourceNames: function(others_affected_count) { |
77 | + var source_names = {}; |
78 | + // What to say when the user is marked as affected. |
79 | + if (others_affected_count == 1) { |
80 | + source_names[true] = ( |
81 | + 'This bug affects you and 1 other person'); |
82 | + } |
83 | + else if (others_affected_count > 1) { |
84 | + source_names[true] = ( |
85 | + 'This bug affects you and ' + |
86 | + others_affected_count + ' other people'); |
87 | + } |
88 | + else { |
89 | + source_names[true] = 'This bug affects you'; |
90 | + } |
91 | + // What to say when the user is marked as not affected. |
92 | + if (others_affected_count == 1) { |
93 | + source_names[false] = ( |
94 | + 'This bug affects 1 person, but not you'); |
95 | + } |
96 | + else if (others_affected_count > 1) { |
97 | + source_names[false] = ( |
98 | + 'This bug affects ' + others_affected_count + |
99 | + ' people, but not you'); |
100 | + } |
101 | + else { |
102 | + source_names[false] = "This bug doesn't affect you"; |
103 | + } |
104 | + return source_names; |
105 | }, |
106 | |
107 | showError: function(err) { |
108 | @@ -1742,7 +1795,8 @@ |
109 | * @method setup_add_attachment |
110 | */ |
111 | function setup_add_attachment() { |
112 | - var attachment_link = Y.one('.menu-link-addcomment'); |
113 | + // Find zero or more links to modify. |
114 | + var attachment_link = Y.all('.menu-link-addcomment'); |
115 | attachment_link.on('click', function(e) { |
116 | var comment_input = Y.one('[id="field.comment"]'); |
117 | if (comment_input.get('value') !== '') { |
118 | |
119 | === modified file 'lib/canonical/launchpad/javascript/bugs/tests/test_me_too.js' |
120 | --- lib/canonical/launchpad/javascript/bugs/tests/test_me_too.js 2009-12-02 16:23:42 +0000 |
121 | +++ lib/canonical/launchpad/javascript/bugs/tests/test_me_too.js 2009-12-11 14:04:24 +0000 |
122 | @@ -72,7 +72,7 @@ |
123 | var me_too_content = Y.one('#affectsmetoo'); |
124 | this.config = { |
125 | contentBox: me_too_content, value: null, |
126 | - elementToFlash: me_too_content |
127 | + elementToFlash: me_too_content, others_affected_count: 5 |
128 | }; |
129 | this.choice_edit = new Y.bugs._MeTooChoiceSource(this.config); |
130 | this.choice_edit.render(); |
131 | @@ -205,6 +205,39 @@ |
132 | Assert.isNull( |
133 | edit_icon.get('src').match(/\/spinner$/), |
134 | "The edit icon is displaying a spinner once the choice has been made."); |
135 | + }, |
136 | + |
137 | + test__getSourceNames: function() { |
138 | + var names; |
139 | + // No other users affected. |
140 | + names = this.choice_edit._getSourceNames(0); |
141 | + Assert.areEqual( |
142 | + 'This bug affects you', names[true]); |
143 | + Assert.areEqual( |
144 | + "This bug doesn't affect you", names[false]); |
145 | + // 1 other user affected. |
146 | + names = this.choice_edit._getSourceNames(1); |
147 | + Assert.areEqual( |
148 | + 'This bug affects you and 1 other person', names[true]); |
149 | + Assert.areEqual( |
150 | + 'This bug affects 1 person, but not you', names[false]); |
151 | + // 2 other users affected. |
152 | + names = this.choice_edit._getSourceNames(2); |
153 | + Assert.areEqual( |
154 | + 'This bug affects you and 2 other people', names[true]); |
155 | + Assert.areEqual( |
156 | + 'This bug affects 2 people, but not you', names[false]); |
157 | + }, |
158 | + |
159 | + test_new_names_are_applied: function() { |
160 | + var names = {}; |
161 | + Y.each(this.choice_edit.get('items'), function(item) { |
162 | + names[item.value] = item.source_name; |
163 | + }); |
164 | + Assert.areEqual( |
165 | + 'This bug affects you and 5 other people', names[true]); |
166 | + Assert.areEqual( |
167 | + 'This bug affects 5 people, but not you', names[false]); |
168 | } |
169 | |
170 | })); |
171 | |
172 | === modified file 'lib/lp/bugs/browser/bugtask.py' |
173 | --- lib/lp/bugs/browser/bugtask.py 2009-11-28 08:11:20 +0000 |
174 | +++ lib/lp/bugs/browser/bugtask.py 2009-12-11 14:04:24 +0000 |
175 | @@ -3097,6 +3097,62 @@ |
176 | else: |
177 | return 'false' |
178 | |
179 | + @property |
180 | + def other_users_affected_count(self): |
181 | + """The number of other users affected by this bug.""" |
182 | + if self.current_user_affected_status: |
183 | + return self.context.users_affected_count - 1 |
184 | + else: |
185 | + return self.context.users_affected_count |
186 | + |
187 | + @property |
188 | + def affected_statement(self): |
189 | + """The default "this bug affects" statement to show. |
190 | + |
191 | + The outputs of this method should be mirrored in |
192 | + MeTooChoiceSource._getSourceNames() (Javascript). |
193 | + """ |
194 | + if self.other_users_affected_count == 1: |
195 | + if self.current_user_affected_status is None: |
196 | + return "This bug affects 1 person. Does this bug affect you?" |
197 | + elif self.current_user_affected_status: |
198 | + return "This bug affects you and 1 other person" |
199 | + else: |
200 | + return "This bug affects 1 person, but not you" |
201 | + elif self.other_users_affected_count > 1: |
202 | + if self.current_user_affected_status is None: |
203 | + return ( |
204 | + "This bug affects %d people. Does this bug " |
205 | + "affect you?" % (self.other_users_affected_count)) |
206 | + elif self.current_user_affected_status: |
207 | + return "This bug affects you and %d other people" % ( |
208 | + self.other_users_affected_count) |
209 | + else: |
210 | + return "This bug affects %d people, but not you" % ( |
211 | + self.other_users_affected_count) |
212 | + else: |
213 | + if self.current_user_affected_status is None: |
214 | + return "Does this bug affect you?" |
215 | + elif self.current_user_affected_status: |
216 | + return "This bug affects you" |
217 | + else: |
218 | + return "This bug doesn't affect you" |
219 | + |
220 | + @property |
221 | + def anon_affected_statement(self): |
222 | + """The "this bug affects" statement to show to anonymous users. |
223 | + |
224 | + The outputs of this method should be mirrored in |
225 | + MeTooChoiceSource._getSourceNames() (Javascript). |
226 | + """ |
227 | + if self.context.users_affected_count == 1: |
228 | + return "This bug affects 1 person" |
229 | + elif self.context.users_affected_count > 1: |
230 | + return "This bug affects %d people" % ( |
231 | + self.context.users_affected_count) |
232 | + else: |
233 | + return None |
234 | + |
235 | |
236 | class BugTaskTableRowView(LaunchpadView): |
237 | """Browser class for rendering a bugtask row on the bug page.""" |
238 | |
239 | === modified file 'lib/lp/bugs/browser/tests/test_bugtask.py' |
240 | --- lib/lp/bugs/browser/tests/test_bugtask.py 2009-09-18 20:23:46 +0000 |
241 | +++ lib/lp/bugs/browser/tests/test_bugtask.py 2009-12-11 14:04:24 +0000 |
242 | @@ -68,6 +68,142 @@ |
243 | self.bug.default_bugtask, False, False) |
244 | self.failUnless(row_view.many_bugtasks) |
245 | |
246 | + def test_other_users_affected_count(self): |
247 | + # The number of other users affected does not change when the |
248 | + # logged-in user marked him or herself as affected or not. |
249 | + self.failUnlessEqual( |
250 | + 1, self.view.other_users_affected_count) |
251 | + self.view.context.markUserAffected(self.view.user, True) |
252 | + self.failUnlessEqual( |
253 | + 1, self.view.other_users_affected_count) |
254 | + self.view.context.markUserAffected(self.view.user, False) |
255 | + self.failUnlessEqual( |
256 | + 1, self.view.other_users_affected_count) |
257 | + |
258 | + def test_other_users_affected_count_other_users(self): |
259 | + # The number of other users affected only changes when other |
260 | + # users mark themselves as affected. |
261 | + self.failUnlessEqual( |
262 | + 1, self.view.other_users_affected_count) |
263 | + other_user_1 = self.factory.makePerson() |
264 | + self.view.context.markUserAffected(other_user_1, True) |
265 | + self.failUnlessEqual( |
266 | + 2, self.view.other_users_affected_count) |
267 | + other_user_2 = self.factory.makePerson() |
268 | + self.view.context.markUserAffected(other_user_2, True) |
269 | + self.failUnlessEqual( |
270 | + 3, self.view.other_users_affected_count) |
271 | + self.view.context.markUserAffected(other_user_1, False) |
272 | + self.failUnlessEqual( |
273 | + 2, self.view.other_users_affected_count) |
274 | + self.view.context.markUserAffected(self.view.user, True) |
275 | + self.failUnlessEqual( |
276 | + 2, self.view.other_users_affected_count) |
277 | + |
278 | + def test_affected_statement_no_one_affected(self): |
279 | + self.bug.markUserAffected(self.bug.owner, False) |
280 | + self.failUnlessEqual( |
281 | + 0, self.view.other_users_affected_count) |
282 | + self.failUnlessEqual( |
283 | + "Does this bug affect you?", |
284 | + self.view.affected_statement) |
285 | + |
286 | + def test_affected_statement_only_you(self): |
287 | + self.view.context.markUserAffected(self.view.user, True) |
288 | + self.failUnless(self.bug.isUserAffected(self.view.user)) |
289 | + self.view.context.markUserAffected(self.bug.owner, False) |
290 | + self.failUnlessEqual( |
291 | + 0, self.view.other_users_affected_count) |
292 | + self.failUnlessEqual( |
293 | + "This bug affects you", |
294 | + self.view.affected_statement) |
295 | + |
296 | + def test_affected_statement_only_not_you(self): |
297 | + self.view.context.markUserAffected(self.view.user, False) |
298 | + self.failIf(self.bug.isUserAffected(self.view.user)) |
299 | + self.view.context.markUserAffected(self.bug.owner, False) |
300 | + self.failUnlessEqual( |
301 | + 0, self.view.other_users_affected_count) |
302 | + self.failUnlessEqual( |
303 | + "This bug doesn't affect you", |
304 | + self.view.affected_statement) |
305 | + |
306 | + def test_affected_statement_1_person_not_you(self): |
307 | + self.assertIs(None, self.bug.isUserAffected(self.view.user)) |
308 | + self.failUnlessEqual( |
309 | + 1, self.view.other_users_affected_count) |
310 | + self.failUnlessEqual( |
311 | + "This bug affects 1 person. Does this bug affect you?", |
312 | + self.view.affected_statement) |
313 | + |
314 | + def test_affected_statement_1_person_and_you(self): |
315 | + self.view.context.markUserAffected(self.view.user, True) |
316 | + self.failUnless(self.bug.isUserAffected(self.view.user)) |
317 | + self.failUnlessEqual( |
318 | + 1, self.view.other_users_affected_count) |
319 | + self.failUnlessEqual( |
320 | + "This bug affects you and 1 other person", |
321 | + self.view.affected_statement) |
322 | + |
323 | + def test_affected_statement_1_person_and_not_you(self): |
324 | + self.view.context.markUserAffected(self.view.user, False) |
325 | + self.failIf(self.bug.isUserAffected(self.view.user)) |
326 | + self.failUnlessEqual( |
327 | + 1, self.view.other_users_affected_count) |
328 | + self.failUnlessEqual( |
329 | + "This bug affects 1 person, but not you", |
330 | + self.view.affected_statement) |
331 | + |
332 | + def test_affected_statement_more_than_1_person_not_you(self): |
333 | + self.assertIs(None, self.bug.isUserAffected(self.view.user)) |
334 | + other_user = self.factory.makePerson() |
335 | + self.view.context.markUserAffected(other_user, True) |
336 | + self.failUnlessEqual( |
337 | + 2, self.view.other_users_affected_count) |
338 | + self.failUnlessEqual( |
339 | + "This bug affects 2 people. Does this bug affect you?", |
340 | + self.view.affected_statement) |
341 | + |
342 | + def test_affected_statement_more_than_1_person_and_you(self): |
343 | + self.view.context.markUserAffected(self.view.user, True) |
344 | + self.failUnless(self.bug.isUserAffected(self.view.user)) |
345 | + other_user = self.factory.makePerson() |
346 | + self.view.context.markUserAffected(other_user, True) |
347 | + self.failUnlessEqual( |
348 | + 2, self.view.other_users_affected_count) |
349 | + self.failUnlessEqual( |
350 | + "This bug affects you and 2 other people", |
351 | + self.view.affected_statement) |
352 | + |
353 | + def test_affected_statement_more_than_1_person_and_not_you(self): |
354 | + self.view.context.markUserAffected(self.view.user, False) |
355 | + self.failIf(self.bug.isUserAffected(self.view.user)) |
356 | + other_user = self.factory.makePerson() |
357 | + self.view.context.markUserAffected(other_user, True) |
358 | + self.failUnlessEqual( |
359 | + 2, self.view.other_users_affected_count) |
360 | + self.failUnlessEqual( |
361 | + "This bug affects 2 people, but not you", |
362 | + self.view.affected_statement) |
363 | + |
364 | + def test_anon_affected_statement_no_one_affected(self): |
365 | + self.bug.markUserAffected(self.bug.owner, False) |
366 | + self.failUnlessEqual(0, self.bug.users_affected_count) |
367 | + self.assertIs(None, self.view.anon_affected_statement) |
368 | + |
369 | + def test_anon_affected_statement_1_user_affected(self): |
370 | + self.failUnlessEqual(1, self.bug.users_affected_count) |
371 | + self.failUnlessEqual( |
372 | + "This bug affects 1 person", |
373 | + self.view.anon_affected_statement) |
374 | + |
375 | + def test_anon_affected_statement_2_users_affected(self): |
376 | + self.view.context.markUserAffected(self.view.user, True) |
377 | + self.failUnlessEqual(2, self.bug.users_affected_count) |
378 | + self.failUnlessEqual( |
379 | + "This bug affects 2 people", |
380 | + self.view.anon_affected_statement) |
381 | + |
382 | |
383 | def test_suite(): |
384 | suite = unittest.TestSuite() |
385 | |
386 | === modified file 'lib/lp/bugs/stories/bugs/xx-bug-affects-me-too.txt' |
387 | --- lib/lp/bugs/stories/bugs/xx-bug-affects-me-too.txt 2009-07-31 13:49:53 +0000 |
388 | +++ lib/lp/bugs/stories/bugs/xx-bug-affects-me-too.txt 2009-12-11 14:04:24 +0000 |
389 | @@ -10,13 +10,14 @@ |
390 | >>> logout() |
391 | |
392 | The user goes to the bug's index page, and finds a statement that the |
393 | -bug is not marked as affecting them. |
394 | +bug affects one other person (in this instance, the person who filed |
395 | +the bug). |
396 | |
397 | >>> user_browser.open(test_bug_url) |
398 | >>> print extract_text(find_tag_by_id( |
399 | ... user_browser.contents, 'affectsmetoo').find( |
400 | ... None, 'static')) |
401 | - This bug doesn't affect me |
402 | + This bug affects 1 person. Does this bug affect you? |
403 | |
404 | Next to the statement is a link containing an edit icon. |
405 | |
406 | @@ -45,7 +46,7 @@ |
407 | >>> print extract_text(find_tag_by_id( |
408 | ... user_browser.contents, 'affectsmetoo').find( |
409 | ... None, 'static')) |
410 | - This bug affects me too |
411 | + This bug affects you and 1 other person |
412 | |
413 | Next to it, we also see the 'hot bug' icon, to indicate that the user |
414 | has marked the bug as affecting them. |
415 | @@ -69,7 +70,28 @@ |
416 | >>> print extract_text(find_tag_by_id( |
417 | ... user_browser.contents, 'affectsmetoo').find( |
418 | ... None, 'static')) |
419 | - This bug doesn't affect me |
420 | + This bug affects 1 person, but not you |
421 | + |
422 | + |
423 | +== Anonymous users == |
424 | + |
425 | +Anonymous users just see the number of affected users. |
426 | + |
427 | + >>> anon_browser.open(test_bug_url) |
428 | + >>> print extract_text(find_tag_by_id( |
429 | + ... anon_browser.contents, 'affectsmetoo')) |
430 | + This bug affects 1 person |
431 | + |
432 | +If no one is marked as affected by the bug, the message does not |
433 | +appear at all to anonymous users. |
434 | + |
435 | + >>> login('test@canonical.com') |
436 | + >>> test_bug.markUserAffected(test_bug.owner, False) |
437 | + >>> logout() |
438 | + |
439 | + >>> anon_browser.open(test_bug_url) |
440 | + >>> print find_tag_by_id(anon_browser.contents, 'affectsmetoo') |
441 | + None |
442 | |
443 | |
444 | == Static and dynamic support == |
445 | |
446 | === modified file 'lib/lp/bugs/templates/bugtasks-and-nominations-table.pt' |
447 | --- lib/lp/bugs/templates/bugtasks-and-nominations-table.pt 2009-12-03 18:36:37 +0000 |
448 | +++ lib/lp/bugs/templates/bugtasks-and-nominations-table.pt 2009-12-11 14:04:24 +0000 |
449 | @@ -2,6 +2,63 @@ |
450 | xmlns:tal="http://xml.zope.org/namespaces/tal" |
451 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
452 | omit-tag=""> |
453 | + |
454 | +<tal:affects-me-too |
455 | + tal:condition="view/displayAlsoAffectsLinks"> |
456 | + <tal:editable |
457 | + condition="context/menu:context/affectsmetoo/enabled"> |
458 | + <div class="actions"> |
459 | + <span id="affectsmetoo" style="display: inline" |
460 | + tal:define="affected view/current_user_affected_status"> |
461 | + <tal:comment condition="nothing"> |
462 | + This .static section is shown in browsers with javascript |
463 | + enabled, and before setup_me_too is run. |
464 | + </tal:comment> |
465 | + <span class="static"> |
466 | + <tal:affected condition="affected"> |
467 | + <img width="14" height="14" src="/@@/flame-icon" alt="" /> |
468 | + <tal:statement replace="view/affected_statement" /> |
469 | + </tal:affected> |
470 | + <tal:not-affected condition="not:affected"> |
471 | + <tal:statement replace="view/affected_statement" /> |
472 | + </tal:not-affected> |
473 | + <a href="+affectsmetoo"> |
474 | + <img class="editicon" src="/@@/edit" alt="Edit" /> |
475 | + </a> |
476 | + </span> |
477 | + <tal:comment condition="nothing"> |
478 | + This .dynamic section is used by setup_me_too to display |
479 | + controls and information in the correct places. |
480 | + </tal:comment> |
481 | + <span class="dynamic unseen"> |
482 | + <img src="/@@/flame-icon" alt=""/> |
483 | + <a href="+affectsmetoo" class="js-action" |
484 | + ><span class="value" tal:content="view/affected_statement" /></a> |
485 | + <img class="editicon" src="/@@/edit" alt="Edit" /> |
486 | + </span> |
487 | + <script type="text/javascript" tal:content="string: |
488 | + LPS.use('event', 'bugs.bugtask_index', function(Y) { |
489 | + Y.on('load', function(e) { |
490 | + Y.bugs.setup_me_too( |
491 | + ${view/current_user_affected_js_status}, |
492 | + ${view/other_users_affected_count}); |
493 | + }, window); |
494 | + }); |
495 | + "> |
496 | + </script> |
497 | + </span> |
498 | + </div> |
499 | + </tal:editable> |
500 | + <tal:not-editable |
501 | + condition="not:context/menu:context/affectsmetoo/enabled"> |
502 | + <div class="actions" |
503 | + tal:define="statement view/anon_affected_statement" |
504 | + tal:condition="statement"> |
505 | + <span id="affectsmetoo" style="display: inline" tal:content="statement" /> |
506 | + </div> |
507 | + </tal:not-editable> |
508 | +</tal:affects-me-too> |
509 | + |
510 | <script type="text/javascript"> |
511 | function toggleFormVisibility(row_id) { |
512 | row = document.getElementById(row_id) |
513 | @@ -41,63 +98,21 @@ |
514 | <div class="actions" |
515 | tal:define="current_bugtask view/current_bugtask" |
516 | tal:condition="view/displayAlsoAffectsLinks"> |
517 | - <tal:also-affects-links define="context_menu context/menu:context"> |
518 | - <tal:addupstream |
519 | + <tal:also-affects-links |
520 | + define="context_menu context/menu:context"> |
521 | + <tal:addupstream |
522 | define="link context_menu/addupstream" |
523 | condition="link/enabled" |
524 | replace="structure link/render" /> |
525 | - <tal:adddistro |
526 | + <tal:adddistro |
527 | define="link context_menu/adddistro" |
528 | condition="link/enabled" |
529 | replace="structure link/render" /> |
530 | - <tal:nominate |
531 | + <tal:nominate |
532 | define="link context_menu/nominate" |
533 | condition="link/enabled" |
534 | replace="structure link/render" /> |
535 | - <span id="affectsmetoo" style="display: inline" |
536 | - tal:condition="link/enabled" |
537 | - tal:define="link context_menu/affectsmetoo; |
538 | - affected view/current_user_affected_status"> |
539 | - |
540 | - <tal:comment condition="nothing"> |
541 | - This .static section is shown in browsers with javascript |
542 | - enabled, and before setup_me_too is run. |
543 | - </tal:comment> |
544 | - <span class="static"> |
545 | - <tal:affected condition="affected"> |
546 | - <img width="14" height="14" src="/@@/flame-icon" alt="" /> |
547 | - This bug affects me too |
548 | - </tal:affected> |
549 | - <tal:not-affected condition="not:affected"> |
550 | - This bug doesn't affect me |
551 | - </tal:not-affected> |
552 | - <a href="+affectsmetoo"> |
553 | - <img class="editicon" src="/@@/edit" alt="Edit" /> |
554 | - </a> |
555 | - </span> |
556 | - |
557 | - <tal:comment condition="nothing"> |
558 | - This .dynamic section is used by setup_me_too to display |
559 | - controls and information in the correct places. |
560 | - </tal:comment> |
561 | - <span class="dynamic unseen"> |
562 | - <img src="/@@/flame-icon" alt=""/> |
563 | - <a href="+affectsmetoo" class="js-action" |
564 | - ><span class="value">Does this bug affect you?</span></a> |
565 | - <img class="editicon" src="/@@/edit" alt="Edit" /> |
566 | - </span> |
567 | - |
568 | - <script type="text/javascript" tal:content="string: |
569 | - LPS.use('event', 'bugs.bugtask_index', function(Y) { |
570 | - Y.on('load', function(e) { |
571 | - Y.bugs.setup_me_too(${view/current_user_affected_js_status}); |
572 | - }, window); |
573 | - }); |
574 | - "> |
575 | - </script> |
576 | - |
577 | - </span> |
578 | - </tal:also-affects-links> |
579 | + </tal:also-affects-links> |
580 | </div> |
581 | |
582 | </tal:root> |
583 | |
584 | === modified file 'lib/lp/bugs/windmill/tests/test_bug_me_too.py' |
585 | --- lib/lp/bugs/windmill/tests/test_bug_me_too.py 2009-11-04 14:06:04 +0000 |
586 | +++ lib/lp/bugs/windmill/tests/test_bug_me_too.py 2009-12-11 14:04:24 +0000 |
587 | @@ -111,7 +111,7 @@ |
588 | def check_for_save_not_affects(client): |
589 | client.asserts.assertText( |
590 | xpath=VALUE_LOCATION_XPATH, |
591 | - validator=u"This bug doesn't affect me") |
592 | + validator=u"This bug doesn't affect you") |
593 | |
594 | # Hah! But this bug does affect the logged-in user! The logged-in |
595 | # user made a mistake, oh noes. Better fix that. |
596 | @@ -125,7 +125,7 @@ |
597 | def check_for_save_does_affect(client): |
598 | client.asserts.assertText( |
599 | xpath=VALUE_LOCATION_XPATH, |
600 | - validator=u"This bug affects me too") |
601 | + validator=u"This bug affects you") |
602 | |
603 | # The flame icon is now visible. |
604 | client.asserts.assertElemJS( |
This branch does two things: shows the count of users affected by a
bug (previously this has not been shown), and moves the "Does this bug
affect you" line to above the bug task table.
Lint free.
To test:
bin/test -vvct 'test_bugtask| xx-bug- affects- me-too' launchpad/ javascript/ bugs/tests/ test_me_ too.html
bin/test -vvct test_bug_me_too.py --layer BugsWindmillLayer
firefox lib/canonical/
File by file:
lib/canonical/ launchpad/ javascript/ bugs/bugtask- index.js
Pass a new argument, others_ affected_ count, into the
MeTooChoiceSource constructor. Accept it too :)
New method, _getNewSourceNa mes(), that figures out the strings that
will be used in the page to represent the current state of the
choice source in the page. The results of this are used to override
the defaults in the choice source's "items" array.
lib/canonical/ launchpad/ javascript/ bugs/tests/ test_me_ too.js
Test the above.
lib/lp/ bugs/browser/ bugtask. py
New property BugTasksAndNomi nationsView. other_users_ affected_ count
which returns the number of users affected by the current bug
*excluding* the logged-in user, if he/she is affected.
New property BugTasksAndNomi nationsView. affected_ statement, which
returns a sentence or two to describe what the current state is. It
takes into consideration whether or not the current user has voted
either way or not, and also how many other users are affected.
lib/lp/ bugs/browser/ tests/test_ bugtask. py
Test the above.
lib/lp/ bugs/stories/ bugs/xx- bug-affects- me-too. txt
The affected_statement is used for both clients with and without
Javascript enabled, so this pagetest was updated.
lib/lp/ bugs/templates/ bugtasks- and-nominations -table. pt
Move the me-too widget to above the task table. Apart from that affected_ count is passed into setup_me_too().
there are very few changes. The affected_statement replaces the
static text in the static and dynamic parts of the supporting HTML,
and other_users_
lib/lp/ bugs/windmill/ tests/test_ bug_me_ too.py
Windmill test updated to reflect wording changes.