launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06390
lp:~wallyworld/launchpad/new-private-team-subscription-policy-933159 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/new-private-team-subscription-policy-933159 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #933159 in Launchpad itself: "+newteam form does not state that restricted is the only valid subscription policy for private teams"
https://bugs.launchpad.net/launchpad/+bug/933159
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/new-private-team-subscription-policy-933159/+merge/93339
== Implementation ==
Add some javascript to do the work. If a team is private, widget the extra help text with the (i) icon is set to inform the user private teams must be restricted and all other choices are hidden. If the user makes the team public, the subscription policy choices come back and any extra help text is restored.
== Demo and QA ==
Create or edit a private team where you have permission to set the visibility policy. Toggle the policy between private and public and watch the action with the subscription policy choices.
== Tests ==
Add some new yui tests - pretty self explanatory.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/javascript/team.js
lib/lp/registry/javascript/tests/test_team.html
lib/lp/registry/javascript/tests/test_team.js
lib/lp/registry/templates/people-newteam.pt
lib/lp/registry/templates/person-macros.pt
lib/lp/registry/templates/team-edit.pt
--
https://code.launchpad.net/~wallyworld/launchpad/new-private-team-subscription-policy-933159/+merge/93339
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/new-private-team-subscription-policy-933159 into lp:launchpad.
=== modified file 'lib/lp/registry/javascript/team.js'
--- lib/lp/registry/javascript/team.js 2011-08-09 14:18:02 +0000
+++ lib/lp/registry/javascript/team.js 2012-02-16 07:06:22 +0000
@@ -62,15 +62,15 @@
return;
}
- if (current_status == 'Invited') {
+ if (current_status === 'Invited') {
members_section = box.one('#recently-invited');
members_ul = box.one('#recently-invited-ul');
count_elem = box.one('#invited-member-count');
- } else if (current_status == 'Proposed') {
+ } else if (current_status === 'Proposed') {
members_section = box.one('#recently-proposed');
members_ul = box.one('#recently-proposed-ul');
count_elem = box.one('#proposed-member-count');
- } else if (current_status == 'Approved') {
+ } else if (current_status === 'Approved') {
members_section = box.one('#recently-approved');
members_ul = box.one('#recently-approved-ul');
count_elem = box.one('#approved-member-count');
@@ -83,7 +83,7 @@
var first_node = members_ul.get('firstChild');
var xhtml_person_handler = function(person_html) {
- if (count_elem === null && current_status == 'Invited') {
+ if (count_elem === null && current_status === 'Invited') {
count_elem = Y.Node.create(
'<strong id="invited-member-count">' +
'1</strong>');
@@ -133,6 +133,101 @@
LP.cache.context.self_link, 'addMember', addmember_config);
};
+/**
+ * Update the widget's subscription policy extra help message according to the
+ * team's, visibility. If PRIVATE, display a fixed message explaining the
+ * subscription policy must be private. If PUBLIC, display whatever message
+ * was previously displayed (if any).
+ * @param widget
+ * @param visibility
+ */
+module.show_subscription_policy_extra_help = function(widget, visibility) {
+ var extra_help_widget = widget.ancestor('div').one('.info');
+
+ // The current extra help text as displayed.
+ var current_extra_help = null;
+ if (Y.Lang.isObject(extra_help_widget)) {
+ current_extra_help = extra_help_widget.get('text');
+ }
+ // The previously saved extra help text from when the team was public.
+ var saved_extra_help = widget.getData('public_extra_help');
+
+ var extra_help = 'Private teams must have a restricted ' +
+ 'subscription policy.';
+ if (visibility === 'PRIVATE') {
+ // Save the last public extra help text.
+ if (Y.Lang.isValue(current_extra_help)) {
+ widget.setData('public_extra_help', current_extra_help);
+ }
+ } else {
+ extra_help = saved_extra_help;
+ }
+ // extra_help contains the text to display. If none, destroy the extra
+ // help widget.
+ if (Y.Lang.isValue(extra_help)) {
+ //Create the extra help widget if necessary.
+ if (!Y.Lang.isObject(extra_help_widget)) {
+ extra_help_widget = Y.Node.create('<div></div>')
+ .addClass('sprite')
+ .addClass('info');
+ widget.insert(extra_help_widget, 'before');
+ }
+ extra_help_widget.set('text', extra_help);
+ } else {
+ if (Y.Lang.isObject(extra_help_widget)) {
+ extra_help_widget.remove(true);
+ }
+ }
+};
+
+/**
+ * The team's visibility has changed so we need to show or hide subscription
+ * policy choices as appropriate.
+ * @param visibility
+ */
+module.visibility_changed = function(visibility) {
+ var widget_label = Y.one("[for=field.subscriptionpolicy]");
+ if (!Y.Lang.isValue(widget_label)) {
+ return;
+ }
+ var widget = widget_label.ancestor('div').one('.radio-button-widget');
+ widget.all("td input[name=field.subscriptionpolicy]")
+ .each(function(choice_node) {
+ var table_row = choice_node.ancestor('tr');
+ var policy_value = choice_node.get('value');
+ // PRIVATE teams can only have RESTRICTED subscription policy.
+ if (visibility === 'PRIVATE') {
+ if (policy_value === 'RESTRICTED') {
+ choice_node.set('checked', 'checked');
+ } else {
+ choice_node.set('checked', '');
+ table_row.addClass('unseen');
+ table_row.next().addClass('unseen');
+ }
+ } else {
+ table_row.removeClass('unseen');
+ table_row.next().removeClass('unseen');
+ }
+ });
+ module.show_subscription_policy_extra_help(widget, visibility);
+};
+
+/**
+ * Setup javascript for team adding/editing.
+ */
+module.initialise_team_edit = function() {
+ // If no visibility widget, then bale.
+ var visibility_widget = Y.one("[name='field.visibility']");
+ if (!Y.Lang.isValue(visibility_widget)) {
+ return;
+ }
+ var visibility = visibility_widget.get('value');
+ module.visibility_changed(visibility);
+ visibility_widget.on('change', function(e) {
+ module.visibility_changed(visibility_widget.get('value'));
+ });
+};
+
}, '0.1', {requires: ['node',
'lp.anim',
'lp.app.errors',
=== added file 'lib/lp/registry/javascript/tests/test_team.html'
--- lib/lp/registry/javascript/tests/test_team.html 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/javascript/tests/test_team.html 2012-02-16 07:06:22 +0000
@@ -0,0 +1,92 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
+ "http://www.w3.org/TR/html4/strict.dtd";>
+<!--
+Copyright 2012 Canonical Ltd. This software is licensed under the
+GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<html>
+<head>
+ <title>Team Tests</title>
+
+ <!-- YUI and test setup -->
+ <script type="text/javascript"
+ src="../../../../../build/js/yui/yui/yui.js">
+ </script>
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/console/assets/console-core.css" />
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/console/assets/skins/sam/console.css" />
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/test/assets/skins/sam/test.css" />
+
+ <script type="text/javascript"
+ src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
+
+ <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
+
+ <!-- The module under test. -->
+ <script type="text/javascript" src="../team.js"></script>
+
+ <!-- The test suite -->
+ <script type="text/javascript" src="test_team.js"></script>
+
+</head>
+<body class="yui3-skin-sam">
+ <ul id="suites">
+ <li>lp.registry.team.test</li>
+ </ul>
+
+ <div id='placeholder'></div>
+ <script type="text/x-template" id="visibility_setup">
+ <div>
+ <label for="field.subscriptionpolicy">Subscription policy:</label>
+ <div>
+ <table class="radio-button-widget"><tbody>
+ <tr>
+ <td rowspan="2"><input type="radio" value="OPEN"
+ name="field.subscriptionpolicy"
+ id="field.subscriptionpolicy.0" class="radioType"></td>
+ <td><label for="field.subscriptionpolicy.0">Open Team</label></td>
+ </tr>
+ <tr>
+ <td class="formHelp">Open Help</td>
+ </tr>
+ <tr>
+ <td rowspan="2"><input type="radio" value="DELEGATED"
+ name="field.subscriptionpolicy"
+ id="field.subscriptionpolicy.1" class="radioType"></td>
+ <td><label for="field.subscriptionpolicy.1">Delegated Team</label></td>
+ </tr>
+ <tr>
+ <td class="formHelp">Delegated Help</td>
+ </tr>
+ <tr>
+ <td rowspan="2"><input type="radio" value="MODERATED"
+ name="field.subscriptionpolicy"
+ id="field.subscriptionpolicy.2" class="radioType"></td>
+ <td><label for="field.subscriptionpolicy.2">Moderated Team</label></td>
+ </tr>
+ <tr>
+ <td class="formHelp">Closed Help</td>
+ </tr>
+ <tr>
+ <td rowspan="2"><input type="radio" value="RESTRICTED"
+ name="field.subscriptionpolicy"
+ id="field.subscriptionpolicy.3" checked="checked" class="radioType"></td>
+ <td><label for="field.subscriptionpolicy.3">Restricted Team</label></td>
+ </tr>
+ <tr>
+ <td class="formHelp">Restricted Help</td>
+ </tr>
+ </tbody></table>
+ </div>
+ </div>
+
+ <select size="1" name="field.visibility" id="field.visibility">
+ <option value="PUBLIC" selected="selected">Public</option>
+ <option value="PRIVATE">Private</option>
+ </select>
+ </script>
+</body>
+</html>
=== added file 'lib/lp/registry/javascript/tests/test_team.js'
--- lib/lp/registry/javascript/tests/test_team.js 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/javascript/tests/test_team.js 2012-02-16 07:06:22 +0000
@@ -0,0 +1,133 @@
+/* Copyright (c) 2012, Canonical Ltd. All rights reserved. */
+
+YUI.add('lp.registry.team.test', function (Y) {
+
+ var namespace = Y.lp.registry.team;
+
+ var tests = Y.namespace('lp.registry.team.test');
+ tests.suite = new Y.Test.Suite('Team Tests');
+
+ tests.suite.add(new Y.Test.Case({
+ name: 'team_tests',
+
+ setUp: function () {
+ var template = Y.one('#visibility_setup');
+ var visibility_widget = Y.Node.create(template.getContent());
+ this.placeholder = Y.one('#placeholder');
+ this.placeholder.appendChild(visibility_widget);
+ },
+ tearDown: function () {
+ this.placeholder.empty();
+ },
+
+ test_library_exists: function() {
+ Y.Assert.isObject(Y.lp.registry.team,
+ "We should be able to locate the lp.registry.team module");
+ },
+
+ // The initialise_team_edit() method invokes the visibility_changed()
+ // callback with the initial value of the visibility field.
+ test_initialise_team_edit: function() {
+ var orig_visibility_changed = namespace.visibility_changed;
+ var visibility_changed_called = false;
+ namespace.visibility_changed = function(visibility) {
+ Y.Assert.areEqual('PUBLIC', visibility);
+ visibility_changed_called = true;
+ };
+ namespace.initialise_team_edit();
+ Y.Assert.isTrue(visibility_changed_called);
+ namespace.visibility_changed = orig_visibility_changed;
+ },
+
+ // When the visibility field changes value, the visibility_changed
+ // callback is invoked with the new value.
+ test_visibility_change_trigger: function() {
+ namespace.initialise_team_edit();
+ var orig_visibility_changed = namespace.visibility_changed;
+ var visibility_changed_called = false;
+ namespace.visibility_changed = function(visibility) {
+ Y.Assert.areEqual('PRIVATE', visibility);
+ visibility_changed_called = true;
+ };
+ Y.Assert.isFalse(visibility_changed_called);
+ var visibility_field = Y.one('[name=field.visibility]');
+ visibility_field.set('value', 'PRIVATE');
+ visibility_field.simulate('change');
+ Y.Assert.isTrue(visibility_changed_called);
+ namespace.visibility_changed = orig_visibility_changed;
+ },
+
+ // When the visibility becomes private, only the restricted
+ // subscription policy is shown and the extra help message informs
+ // the user of this fact.
+ test_visibility_change_private: function() {
+ namespace.visibility_changed('PRIVATE');
+ var nr_radio_buttons = 0;
+ Y.all('input[type=radio]').each(function(radio_button) {
+ if (radio_button.ancestor('tr').hasClass('unseen')) {
+ return;
+ }
+ nr_radio_buttons++;
+ Y.Assert.areEqual(
+ 'RESTRICTED', radio_button.get('value'));
+ });
+ Y.Assert.isTrue(nr_radio_buttons === 1);
+ var extra_help = Y.one('[for=field.subscriptionpolicy]')
+ .ancestor('div').one('.info');
+ Y.Assert.areEqual(
+ 'Private teams must have a restricted subscription '+
+ 'policy.',
+ extra_help.get('text'));
+ },
+
+ // When the visibility field becomes public, all subscription policies
+ // are visible again.
+ test_visibility_change_public: function() {
+ namespace.visibility_changed('PRIVATE');
+ var extra_help = Y.one('[for=field.subscriptionpolicy]')
+ .ancestor('div').one('.info');
+ Y.Assert.areEqual(
+ 'Private teams must have a restricted subscription '+
+ 'policy.',
+ extra_help.get('text'));
+
+ namespace.visibility_changed('PUBLIC');
+ var nr_radio_buttons = 0;
+ Y.all('input[type=radio]').each(function(radio_button) {
+ Y.Assert.isFalse(
+ radio_button.ancestor('tr').hasClass('unseen'));
+ nr_radio_buttons++;
+ });
+ Y.Assert.isTrue(nr_radio_buttons === 4);
+ extra_help = Y.one('[for=field.subscriptionpolicy]')
+ .ancestor('div').one('.info');
+ Y.Assert.isNull(extra_help);
+ },
+
+ // When the subscription policy changes to private and back to public,
+ // any original extra help text that was there is restored.
+ test_visibility_change_public_restores_extra_help: function() {
+ var widget = Y.one('[for=field.subscriptionpolicy]')
+ .ancestor('div').one('.radio-button-widget');
+ var extra_help = Y.Node.create('<div>Help Me</div>')
+ .addClass('sprite')
+ .addClass('info');
+ widget.insert(extra_help, 'before');
+
+ namespace.visibility_changed('PRIVATE');
+ extra_help = Y.one('[for=field.subscriptionpolicy]')
+ .ancestor('div').one('.info');
+ Y.Assert.areEqual(
+ 'Private teams must have a restricted subscription '+
+ 'policy.',
+ extra_help.get('text'));
+
+ namespace.visibility_changed('PUBLIC');
+ extra_help = Y.one('[for=field.subscriptionpolicy]')
+ .ancestor('div').one('.info');
+ Y.Assert.areEqual('Help Me', extra_help.get('text'));
+ }
+ }));
+
+}, '0.1', {'requires': ['test', 'console', 'node', 'event',
+ 'node-event-simulate', 'lp.registry.team']});
=== modified file 'lib/lp/registry/templates/people-newteam.pt'
--- lib/lp/registry/templates/people-newteam.pt 2009-08-31 17:46:33 +0000
+++ lib/lp/registry/templates/people-newteam.pt 2012-02-16 07:06:22 +0000
@@ -16,7 +16,7 @@
language.
</p>
</div>
- <metal:js use-macro="context/@@+person-macros/private-team-js" />
+ <metal:js use-macro="context/@@+person-macros/team-js" />
</div>
</body>
</html>
=== modified file 'lib/lp/registry/templates/person-macros.pt'
--- lib/lp/registry/templates/person-macros.pt 2012-02-01 15:31:32 +0000
+++ lib/lp/registry/templates/person-macros.pt 2012-02-16 07:06:22 +0000
@@ -243,11 +243,10 @@
</metal:macro>
-<metal:macro define-macro="private-team-js">
+<metal:macro define-macro="team-js">
<tal:comment replace="nothing">
- This macro inserts the javascript necessary to automatically insert the
- private team prefix into a name field. It is here since it is shared in
- multiple templates.
+ This macro inserts the javascript used on the team add/edit pages.
+ It is here since it is shared in multiple templates.
</tal:comment>
<tal:script define="private_prefix view/private_prefix|nothing"
condition="private_prefix">
@@ -276,6 +275,14 @@
});">
</script>
</tal:script>
+ <script type="text/javascript"
+ tal:content="string:
+ LPJS.use('node', 'event','lp.registry.team', function(Y) {
+ Y.on('domready', function(e) {
+ Y.lp.registry.team.initialise_team_edit();
+ });
+ });">
+ </script>
</metal:macro>
</tal:root>
=== modified file 'lib/lp/registry/templates/team-edit.pt'
--- lib/lp/registry/templates/team-edit.pt 2009-08-17 02:34:16 +0000
+++ lib/lp/registry/templates/team-edit.pt 2012-02-16 07:06:22 +0000
@@ -15,7 +15,7 @@
<div metal:use-macro="context/@@launchpad_form/form" />
- <metal:js use-macro="context/@@+person-macros/private-team-js" />
+ <metal:js use-macro="context/@@+person-macros/team-js" />
<tal:menu replace="structure view/@@+related-pages" />
