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
diff --git a/lib/lp/registry/javascript/team.js b/lib/lp/registry/javascript/team.js
index 65b64bb..517cc6b 100644
--- a/lib/lp/registry/javascript/team.js
+++ b/lib/lp/registry/javascript/team.js
@@ -54,7 +54,8 @@ var _add_member = function(selected_person) {
54 if (did_status_change === false) {54 if (did_status_change === false) {
55 disable_spinner();55 disable_spinner();
56 Y.lp.app.errors.display_info(56 Y.lp.app.errors.display_info(
57 selected_person.title + ' is already ' +57 selected_person.value +
58 ' is already ' +
58 current_status.toLowerCase() +59 current_status.toLowerCase() +
59 ' as a member of the team.');60 ' as a member of the team.');
60 return;61 return;
diff --git a/lib/lp/registry/javascript/tests/test_team.html b/lib/lp/registry/javascript/tests/test_team.html
index bf98b3e..9a5704d 100644
--- a/lib/lp/registry/javascript/tests/test_team.html
+++ b/lib/lp/registry/javascript/tests/test_team.html
@@ -27,6 +27,30 @@ GNU Affero General Public License version 3 (see the file LICENSE).
2727
28 <!-- The test suite -->28 <!-- The test suite -->
29 <script type="text/javascript" src="test_team.js"></script>29 <script type="text/javascript" src="test_team.js"></script>
30 <script type="text/javascript"
31 src="../../../../../build/js/lp/app/client.js"></script>
32<script type="text/javascript"
33 src="../../../../../build/js/lp/app/errors.js"></script>
34<script type="text/javascript"
35 src="../../../../../build/js/lp/app/lp-names.js"></script>
36<script type="text/javascript"
37 src="../../../../../build/js/lp/app/anim/anim.js"></script>
38<script type="text/javascript"
39 src="../../../../../build/js/lp/app/activator/activator.js"></script>
40<script type="text/javascript"
41 src="../../../../../build/js/lp/app/extras/extras.js"></script>
42<script type="text/javascript"
43 src="../../../../../build/js/lp/app/formoverlay/formoverlay.js"></script>
44<script type="text/javascript"
45 src="../../../../../build/js/lp/app/overlay/overlay.js"></script>
46<script type="text/javascript"
47 src="../../../../../build/js/lp/app/ui/ui.js"></script>
48<script type="text/javascript"
49 src="../../../../../build/js/lp/app/picker/picker.js"></script>
50<script type="text/javascript"
51 src="../../../../../build/js/lp/app/picker/person_picker.js"></script>
52<script type="text/javascript"
53 src="../../../../../build/js/lp/app/picker/picker_patcher.js"></script>
3054
31</head>55</head>
32<body class="yui3-skin-sam">56<body class="yui3-skin-sam">
@@ -90,6 +114,12 @@ GNU Affero General Public License version 3 (see the file LICENSE).
90 </div>114 </div>
91 <p class="formHelp">Help text</p>115 <p class="formHelp">Help text</p>
92 </div>116 </div>
117 <li id="membership">
118 <span id="add-member-spinner" class="hidden update-in-progress-message">
119 Saving...
120 </span>
121 <a class="menu-link-add_member sprite add js-action">Add member</a>
122 </li>
93 </script>123 </script>
94</body>124 </body>
95</html>125</html>
diff --git a/lib/lp/registry/javascript/tests/test_team.js b/lib/lp/registry/javascript/tests/test_team.js
index 987ae9b..e4e08aa 100644
--- a/lib/lp/registry/javascript/tests/test_team.js
+++ b/lib/lp/registry/javascript/tests/test_team.js
@@ -16,9 +16,23 @@ YUI.add('lp.registry.team.test', function (Y) {
16 var visibility_widget = Y.Node.create(template.getContent());16 var visibility_widget = Y.Node.create(template.getContent());
17 this.placeholder = Y.one('#placeholder');17 this.placeholder = Y.one('#placeholder');
18 this.placeholder.appendChild(visibility_widget);18 this.placeholder.appendChild(visibility_widget);
19 Y.lp.client.Launchpad.prototype.named_post =
20 function(url, func, config) {
21 config.on.success([false, 'administrator']);
22 };
23 window.LP = {
24 "cache": {
25 "related_features": {},
26 "context": {}
27 },
28 "links": {
29 "me": "/~user1"
30 }
31 };
19 },32 },
20 tearDown: function () {33 tearDown: function () {
21 this.placeholder.empty();34 this.placeholder.empty();
35 window.LP = {};
22 },36 },
2337
24 test_library_exists: function() {38 test_library_exists: function() {
@@ -40,6 +54,24 @@ YUI.add('lp.registry.team.test', function (Y) {
40 namespace.visibility_changed = orig_visibility_changed;54 namespace.visibility_changed = orig_visibility_changed;
41 },55 },
4256
57 // When a user is already a member of a team and it uses
58 // "Pick me" button an error message is displayed
59 test_pick_me_error_message: function() {
60 namespace.setup_add_member_handler('Add member handler');
61 var add_member_link = Y.one(".menu-link-add_member");
62 add_member_link.simulate('click');
63 var assign_me_button = Y.one(".yui-picker-assign-me-button");
64 assign_me_button.simulate('click');
65 var childNodes = Y.one('.yui3-widget-bd')._node.childNodes;
66 Y.Assert.isTrue(childNodes.length > 0);
67 var widget_box_text = childNodes[0].innerText;
68 Y.Assert.areEqual(
69 "user1 is already administrator as a member of the team.\nOK",
70 widget_box_text
71 );
72 },
73
74
43 // When the visibility field changes value, the visibility_changed75 // When the visibility field changes value, the visibility_changed
44 // callback is invoked with the new value.76 // callback is invoked with the new value.
45 test_visibility_change_trigger: function() {77 test_visibility_change_trigger: function() {

Subscribers

People subscribed via source and target branches

to status/vote changes: