launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/shareetable-first-sharee-insert into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #966803 in Launchpad itself: ""This project is not shared..." message is not removed when the first sharee is added"
  https://bugs.launchpad.net/launchpad/+bug/966803

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/shareetable-first-sharee-insert/+merge/99661

== Implementation ==

Simple fix to ensure the "This project is not shared...." message is removed from the sharee table when the first sharee is added.

== Tests ==

Add a new test to test_shareetable:
- test_first_sharee_added

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/javascript/sharing/shareetable.js
  lib/lp/registry/javascript/sharing/tests/test_shareelisting_navigator.html
  lib/lp/registry/javascript/sharing/tests/test_shareetable.html
  lib/lp/registry/javascript/sharing/tests/test_shareetable.js

-- 
https://code.launchpad.net/~wallyworld/launchpad/shareetable-first-sharee-insert/+merge/99661
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/shareetable-first-sharee-insert into lp:launchpad.

=== modified file 'lib/lp/registry/javascript/sharing/shareetable.js'
--- lib/lp/registry/javascript/sharing/shareetable.js	2012-03-26 01:11:56 +0000
+++ lib/lp/registry/javascript/sharing/shareetable.js	2012-03-28 03:58:18 +0000
@@ -182,7 +182,8 @@
 
     _sharee_table_empty_row: function() {
         return [
-            '<tr><td colspan="5" style="padding-left: 0.25em">',
+            '<tr id="sharee-table-not-shared">',
+            '<td colspan="5" style="padding-left: 0.25em">',
             'This project\'s private information is not shared with ',
             'anyone.',
             '</td></tr>'].join('');
@@ -454,6 +455,12 @@
             if (Y.Lang.isValue(row_node)) {
                 row_node.replace(new_table_row);
             } else {
+                // Remove the "No sharees..." row if it's there.
+                var not_shared_row = sharee_table.one(
+                    'tr#sharee-table-not-shared');
+                if (Y.Lang.isValue(not_shared_row)) {
+                    not_shared_row.remove(true);
+                }
                 var first_row = sharee_table.one('tbody>:first-child');
                 if (Y.Lang.isValue(first_row)) {
                     first_row.insertBefore(new_table_row, first_row);
@@ -495,7 +502,7 @@
             rows_to_delete.remove(true);
             if (all_rows_deleted === true) {
                 sharee_table.one('tbody')
-                    .appendChild('<tr id="sharee-table-loading"></tr>')
+                    .appendChild('<tr id="sharee-table-not-shared"></tr>')
                     .appendChild('<td></td>')
                     .setContent("This project's private information " +
                                 "is not shared with anyone.");

=== modified file 'lib/lp/registry/javascript/sharing/tests/test_shareelisting_navigator.html'
--- lib/lp/registry/javascript/sharing/tests/test_shareelisting_navigator.html	2012-03-20 06:11:07 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_shareelisting_navigator.html	2012-03-28 03:58:18 +0000
@@ -53,7 +53,7 @@
         <div id="fixture"></div>
         <script type="text/x-template" id="sharee-table-template">
             <table id='sharee-table'>
-                <tr id='sharee-table-loading'><td>
+                <tr id='sharee-table-not-shared'><td>
                     Loading...
                 </td></tr>
             </table>

=== modified file 'lib/lp/registry/javascript/sharing/tests/test_shareetable.html'
--- lib/lp/registry/javascript/sharing/tests/test_shareetable.html	2012-03-21 05:31:39 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_shareetable.html	2012-03-28 03:58:18 +0000
@@ -76,7 +76,7 @@
         <div id="fixture"></div>
         <script type="text/x-template" id="sharee-table-template">
             <table id='sharee-table'>
-                <tr id='sharee-table-loading'><td>
+                <tr id='sharee-table-not-shared'><td>
                     Loading...
                 </td></tr>
             </table>

=== modified file 'lib/lp/registry/javascript/sharing/tests/test_shareetable.js'
--- lib/lp/registry/javascript/sharing/tests/test_shareetable.js	2012-03-26 01:11:56 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_shareetable.js	2012-03-28 03:58:18 +0000
@@ -105,6 +105,22 @@
                 Y.one('#sharee-table tr td').getContent());
         },
 
+        // When the first sharee is added, the "No sharees" row is removed.
+        test_first_sharee_added: function() {
+            this.sharee_table = this._create_Widget({
+                sharees: []
+            });
+            this.sharee_table.render();
+            Y.Assert.isNotNull(Y.one('tr#sharee-table-not-shared'));
+            var new_sharee = {
+                'name': 'joe', 'display_name': 'Joe Smith',
+                'role': '(Maintainer)', web_link: '~joe',
+                'self_link': '~joe',
+                'permissions': {'P1': 's2'}};
+            this.sharee_table.update_sharees([new_sharee]);
+            Y.Assert.isNull(Y.one('tr#sharee-table-not-shared'));
+        },
+
         // The given sharee is correctly rendered.
         _test_sharee_rendered: function(sharee) {
             // The sharee row
@@ -315,7 +331,7 @@
             Y.Assert.areEqual(
                 "This project's private information is not shared " +
                 "with anyone.",
-                Y.one('#sharee-table tr#sharee-table-loading td')
+                Y.one('#sharee-table tr#sharee-table-not-shared td')
                     .getContent());
         },