Launchpad itself

Merge ~pelpsi/launchpad:mispelled-error-message-adding-yourself-team into launchpad:master

  1. Git
  2. lp:~pelpsi/launchpad
  3. mispelled-error-message-adding-yourself-team
  4. Merge into master
Proposed by Simone Pelosi
Status: Merged
Approved by: Simone Pelosi
Approved revision: 521399bea9e10082bf541032e462bebb67a8eb84
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pelpsi/launchpad:mispelled-error-message-adding-yourself-team
Merge into: launchpad:master
Diff against target: 116 lines (+65/-2)
3 files modified
lib/lp/registry/javascript/team.js (+2/-1)
lib/lp/registry/javascript/tests/test_team.html (+31/-1)
lib/lp/registry/javascript/tests/test_team.js (+32/-0)
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+439041@code.launchpad.net

Commit message

Misspelled error message

Add yourself to a team by hitting "Add member" and then "Pick me", the error message if you're already a member was:
"Me is already a member of this team"
now is:
"<username> is already a member of this team"

LP: #1056790

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

Thanks for your first contribution! :party_hat:

If you're selecting another user, I think this will change behaviour from saying "Colin Watson is already a member of this team" to saying "cjwatson is already a member of this team". That's probably fine, but it's worth noting. For comparison, the non-JS version of this error message is in `lib/lp/registry/browser/team.py` and would say "Colin Watson (cjwatson) is already a member of <team display name>".

I don't think this code is currently covered by any tests. You don't have to spend too long on it, and this is optional because I suspect it may be a bit difficult, but can you see if there's a reasonable way to add some test coverage? It would go in `lib/lp/registry/javascript/tests/test_team.js`, possibly needing some added supporting HTML in `lib/lp/registry/javascript/tests/test_team.html`. The files listed by `git grep -Fl lp.client.Launchpad '**/tests/*'` may be useful in figuring out how to do this sort of thing where you need to simulate responses returned from the webservice API to JS code - perhaps `lib/lp/app/javascript/tests/test_information_type.js`.

review: Approve
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/registry/javascript/team.js b/lib/lp/registry/javascript/team.js
2index 65b64bb..517cc6b 100644
3--- a/lib/lp/registry/javascript/team.js
4+++ b/lib/lp/registry/javascript/team.js
5@@ -54,7 +54,8 @@ var _add_member = function(selected_person) {
6 if (did_status_change === false) {
7 disable_spinner();
8 Y.lp.app.errors.display_info(
9- selected_person.title + ' is already ' +
10+ selected_person.value +
11+ ' is already ' +
12 current_status.toLowerCase() +
13 ' as a member of the team.');
14 return;
15diff --git a/lib/lp/registry/javascript/tests/test_team.html b/lib/lp/registry/javascript/tests/test_team.html
16index bf98b3e..9a5704d 100644
17--- a/lib/lp/registry/javascript/tests/test_team.html
18+++ b/lib/lp/registry/javascript/tests/test_team.html
19@@ -27,6 +27,30 @@ GNU Affero General Public License version 3 (see the file LICENSE).
20
21 <!-- The test suite -->
22 <script type="text/javascript" src="test_team.js"></script>
23+ <script type="text/javascript"
24+ src="../../../../../build/js/lp/app/client.js"></script>
25+<script type="text/javascript"
26+ src="../../../../../build/js/lp/app/errors.js"></script>
27+<script type="text/javascript"
28+ src="../../../../../build/js/lp/app/lp-names.js"></script>
29+<script type="text/javascript"
30+ src="../../../../../build/js/lp/app/anim/anim.js"></script>
31+<script type="text/javascript"
32+ src="../../../../../build/js/lp/app/activator/activator.js"></script>
33+<script type="text/javascript"
34+ src="../../../../../build/js/lp/app/extras/extras.js"></script>
35+<script type="text/javascript"
36+ src="../../../../../build/js/lp/app/formoverlay/formoverlay.js"></script>
37+<script type="text/javascript"
38+ src="../../../../../build/js/lp/app/overlay/overlay.js"></script>
39+<script type="text/javascript"
40+ src="../../../../../build/js/lp/app/ui/ui.js"></script>
41+<script type="text/javascript"
42+ src="../../../../../build/js/lp/app/picker/picker.js"></script>
43+<script type="text/javascript"
44+ src="../../../../../build/js/lp/app/picker/person_picker.js"></script>
45+<script type="text/javascript"
46+ src="../../../../../build/js/lp/app/picker/picker_patcher.js"></script>
47
48 </head>
49 <body class="yui3-skin-sam">
50@@ -90,6 +114,12 @@ GNU Affero General Public License version 3 (see the file LICENSE).
51 </div>
52 <p class="formHelp">Help text</p>
53 </div>
54+ <li id="membership">
55+ <span id="add-member-spinner" class="hidden update-in-progress-message">
56+ Saving...
57+ </span>
58+ <a class="menu-link-add_member sprite add js-action">Add member</a>
59+ </li>
60 </script>
61-</body>
62+ </body>
63 </html>
64diff --git a/lib/lp/registry/javascript/tests/test_team.js b/lib/lp/registry/javascript/tests/test_team.js
65index 987ae9b..e4e08aa 100644
66--- a/lib/lp/registry/javascript/tests/test_team.js
67+++ b/lib/lp/registry/javascript/tests/test_team.js
68@@ -16,9 +16,23 @@ YUI.add('lp.registry.team.test', function (Y) {
69 var visibility_widget = Y.Node.create(template.getContent());
70 this.placeholder = Y.one('#placeholder');
71 this.placeholder.appendChild(visibility_widget);
72+ Y.lp.client.Launchpad.prototype.named_post =
73+ function(url, func, config) {
74+ config.on.success([false, 'administrator']);
75+ };
76+ window.LP = {
77+ "cache": {
78+ "related_features": {},
79+ "context": {}
80+ },
81+ "links": {
82+ "me": "/~user1"
83+ }
84+ };
85 },
86 tearDown: function () {
87 this.placeholder.empty();
88+ window.LP = {};
89 },
90
91 test_library_exists: function() {
92@@ -40,6 +54,24 @@ YUI.add('lp.registry.team.test', function (Y) {
93 namespace.visibility_changed = orig_visibility_changed;
94 },
95
96+ // When a user is already a member of a team and it uses
97+ // "Pick me" button an error message is displayed
98+ test_pick_me_error_message: function() {
99+ namespace.setup_add_member_handler('Add member handler');
100+ var add_member_link = Y.one(".menu-link-add_member");
101+ add_member_link.simulate('click');
102+ var assign_me_button = Y.one(".yui-picker-assign-me-button");
103+ assign_me_button.simulate('click');
104+ var childNodes = Y.one('.yui3-widget-bd')._node.childNodes;
105+ Y.Assert.isTrue(childNodes.length > 0);
106+ var widget_box_text = childNodes[0].innerText;
107+ Y.Assert.areEqual(
108+ "user1 is already administrator as a member of the team.\nOK",
109+ widget_box_text
110+ );
111+ },
112+
113+
114 // When the visibility field changes value, the visibility_changed
115 // callback is invoked with the new value.
116 test_visibility_change_trigger: function() {

Subscribers

People subscribed via source and target branches

to status/vote changes: