Merge lp:~wallyworld/launchpad/sharing-details-delete3-966641 into lp:launchpad

[Ian Booth]

Wire up the sharing details ui so that when a pillar artifact is deleted, the ui is updated.

== Implementation ==

Complete the work to allow grants on bugs/branches to be revoked for a user. This branch wires up the ui so that when a successful XHR call is made to revoke access, the sharing details table is updated.

A change was made to the original sharing detail implementation. A NotFound error was raised if the Sharing Details page was requested but there were no shared artifacts. This would only have been applicable if the user url hacked since the link to the details page is not rendered on the Sharing Information page if there are no shared items. However, consider the case where a user goes to the Sharing Details page and there are shared artifacts. They delete all of them. This branch includes javascript code to display a suitable message if the XHR call to revoke removes the last shared artifact - "There are no shared bugs or branches.". If the user hits refresh, they would now get an error and this doesn't seem the best thing to do. Instead now, this branch ensures that they simply see the Sharing Details page with the above message.

== Tests ==

Add yui tests for the new sharingdetails widget functionality.
Modify the PillarSharingDetailsMixin to test for the new behaviour of showing a user friendly message if there are no shared items.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/pillar.py
  lib/lp/registry/browser/tests/test_pillar_sharing.py
  lib/lp/registry/javascript/sharing/sharingdetails.js
  lib/lp/registry/javascript/sharing/tests/test_sharingdetails.html
  lib/lp/registry/javascript/sharing/tests/test_sharingdetails.js
  lib/lp/registry/templates/pillar-sharing-details.pt

[Curtis Hovey (sinzui)]

Thank you.

review: Approve (code)

[Richard Harding (rharding)]

review: Abstain

=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py	2012-04-05 03:52:27 +0000
+++ lib/lp/registry/browser/pillar.py	2012-04-05 03:52:28 +0000
@@ -43,10 +43,6 @@
     )
 from lp.bugs.interfaces.bug import IBug
 from lp.code.interfaces.branch import IBranch
-from lp.registry.interfaces.accesspolicy import (
-    IAccessPolicyGrantFlatSource,
-    IAccessPolicySource,
-    )
 from lp.registry.interfaces.distributionsourcepackage import (
     IDistributionSourcePackage,
     )
@@ -85,11 +81,6 @@
         person = getUtility(IPersonSet).getByName(name)
         if person is None:
             return None
-        policies = getUtility(IAccessPolicySource).findByPillar([self.context])
-        source = getUtility(IAccessPolicyGrantFlatSource)
-        artifacts = source.findArtifactsByGrantee(person, policies)
-        if artifacts.is_empty():
-            return None
         return PillarPerson.create(self.context, person)
 
 

=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-04-05 03:52:27 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-04-05 03:52:28 +0000
@@ -16,7 +16,6 @@
     Raises,
     )
 from zope.component import getUtility
-from zope.publisher.interfaces import NotFound
 from zope.security.interfaces import Unauthorized
 from zope.traversing.browser.absoluteurl import absoluteURL
 
@@ -103,17 +102,19 @@
             browser = self.getUserBrowser(user=self.owner, url=url)
             self.assertEqual(expected, browser.title)
 
-    def test_not_found_without_sharing(self):
-        # If there is no sharing between pillar and person, NotFound is the
-        # result.
+    def test_no_sharing_message(self):
+        # If there is no sharing between pillar and person, a suitable message
+        # is displayed.
         with FeatureFixture(DETAILS_ENABLED_FLAG):
             # We have to do some fun url hacking to force the traversal a user
             # encounters.
             pillarperson = self.getPillarPerson(with_sharing=False)
             url = 'http://launchpad.dev/%s/+sharingdetails/%s' % (
                 pillarperson.pillar.name, pillarperson.person.name)
-            browser = self.getUserBrowser(user=self.owner)
-            self.assertRaises(NotFound, browser.open, url)
+            browser = self.getUserBrowser(user=self.owner, url=url)
+            self.assertIn(
+                'There are no shared bugs or branches.',
+                browser.contents)
 
     def test_init_without_feature_flag(self):
         # We need a feature flag to enable the view.

=== modified file 'lib/lp/registry/javascript/sharing/sharingdetails.js'
--- lib/lp/registry/javascript/sharing/sharingdetails.js	2012-04-05 03:52:27 +0000
+++ lib/lp/registry/javascript/sharing/sharingdetails.js	2012-04-05 03:52:28 +0000
@@ -23,7 +23,18 @@
     SharingDetailsTable.superclass.constructor.apply(this, arguments);
 }
 
+var clone_data = function(value) {
+    if (!Y.Lang.isArray(value)) {
+        return value;
+    }
+    return Y.JSON.parse(Y.JSON.stringify(value));
+};
+
 SharingDetailsTable.ATTRS = {
+    // The duration for various animations eg row deletion.
+    anim_duration: {
+        value: 1
+    },
     // The node holding the details table.
     details_table_body: {
         getter: function() {
@@ -43,11 +54,17 @@
     },
 
     bugs: {
-        value: []
+        value: [],
+        // We clone the data passed in so external modifications do not
+        // interfere.
+        setter: clone_data
     },
 
     branches: {
-        value: []
+        value: [],
+        // We clone the data passed in so external modifications do not
+        // interfere.
+        setter: clone_data
     },
 
     write_enabled: {
@@ -79,6 +96,9 @@
     renderUI: function() {
         var branch_data = this.get('branches');
         var bug_data = this.get('bugs');
+        if (bug_data.length === 0 && branch_data.length === 0) {
+            return;
+        }
         var partials = {
             branch: this.get('branch_details_row_template'),
             bug: this.get('bug_details_row_template')
@@ -124,6 +144,106 @@
         }, 'span[id^=remove-] a');
      },
 
+    /**
+     * If the artifact with id exists in the model, return it.
+     * @param artifact_id
+     * @param id_property
+     * @param model
+     * @return {*}
+     * @private
+     */
+    _get_artifact_from_model: function(artifact_id, id_property, model) {
+        var artifact = null;
+        Y.Array.some(model, function(item) {
+            if (item[id_property] === artifact_id) {
+                artifact = item;
+                return true;
+            }
+            return false;
+        });
+        return artifact;
+    },
+
+    syncUI: function() {
+        // Examine the widget's data model and delete any artifacts which have
+        // been removed.
+        var existing_bugs = this.get('bugs');
+        var existing_branches = this.get('branches');
+        var model_bugs = LP.cache.bugs;
+        var model_branches = LP.cache.branches;
+        var deleted_bugs = [];
+        var deleted_branches = [];
+        var self = this;
+        Y.Array.each(existing_bugs, function(bug) {
+            var model_bug =
+                self._get_artifact_from_model(
+                    bug.bug_id, 'bug_id', model_bugs);
+            if (!Y.Lang.isValue(model_bug)) {
+                deleted_bugs.push(bug);
+            }
+        });
+        Y.Array.each(existing_branches, function(branch) {
+            var model_branch =
+                self._get_artifact_from_model(
+                    branch.branch_id, 'branch_id', model_branches);
+            if (!Y.Lang.isValue(model_branch)) {
+                deleted_branches.push(branch);
+            }
+        });
+        if (deleted_bugs.length > 0 || deleted_branches.length > 0) {
+            this.delete_artifacts(
+                deleted_bugs, deleted_branches,
+                model_bugs.length === 0 && model_branches.length === 0);
+        }
+        this.set('bugs', model_bugs);
+        this.set('branches', model_branches);
+    },
+
+    // Delete the specified sharees from the table.
+    delete_artifacts: function(bugs, branches, all_rows_deleted) {
+        var deleted_row_selectors = [];
+        var details_table_body = this.get('details_table_body');
+        Y.Array.each(bugs, function(bug) {
+            var selector = 'tr[id=shared-bug-' + bug.bug_id + ']';
+            var table_row = details_table_body.one(selector);
+            if (Y.Lang.isValue(table_row)) {
+                deleted_row_selectors.push(selector);
+            }
+        });
+        Y.Array.each(branches, function(branch) {
+            var selector = 'tr[id=shared-branch-' + branch.branch_id + ']';
+            var table_row = details_table_body.one(selector);
+            if (Y.Lang.isValue(table_row)) {
+                deleted_row_selectors.push(selector);
+            }
+        });
+        if (deleted_row_selectors.length === 0) {
+            return;
+        }
+        var rows_to_delete = details_table_body.all(
+            deleted_row_selectors.join(','));
+        var delete_rows = function() {
+            rows_to_delete.remove(true);
+            if (all_rows_deleted === true) {
+                details_table_body
+                    .appendChild('<tr></tr>')
+                    .appendChild('<td colspan="4"></td>')
+                    .setContent("There are no shared bugs or branches.");
+            }
+        };
+        var anim_duration = this.get('anim_duration');
+        if (anim_duration === 0 ) {
+            delete_rows();
+            return;
+        }
+        var anim = Y.lp.anim.green_flash(
+            {node: rows_to_delete, duration:anim_duration});
+        anim.on('end', function() {
+            delete_rows();
+        });
+        anim.run();
+    },
+
     _table_body_template: function() {
         return [
         '<tbody id="sharing-table-body">',
@@ -188,5 +308,5 @@
 namespace.SharingDetailsTable = SharingDetailsTable;
 
 }, "0.1", { "requires": [
-    'node', 'event', 'lp.mustache'
+    'node', 'event', 'json', 'lp.mustache', 'lp.anim'
 ] });

=== modified file 'lib/lp/registry/javascript/sharing/tests/test_sharingdetails.html'
--- lib/lp/registry/javascript/sharing/tests/test_sharingdetails.html	2012-04-05 03:52:27 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_sharingdetails.html	2012-04-05 03:52:28 +0000
@@ -29,6 +29,10 @@
           src="../../../../../../build/js/lp/app/lp.js"></script>
       <script type="text/javascript"
           src="../../../../../../build/js/lp/app/mustache.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/anim/anim.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/extras/extras.js"></script>
 
       <!-- The module under test. -->
       <script type="text/javascript" src="../sharingdetails.js"></script>

=== modified file 'lib/lp/registry/javascript/sharing/tests/test_sharingdetails.js'
--- lib/lp/registry/javascript/sharing/tests/test_sharingdetails.js	2012-04-05 03:52:27 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_sharingdetails.js	2012-04-05 03:52:28 +0000
@@ -55,6 +55,7 @@
                 overrides = {};
             }
             var config = Y.merge({
+                anim_duration: 0,
                 person_name: 'Fred',
                 bugs: window.LP.cache.bugs,
                 branches: window.LP.cache.branches,
@@ -137,6 +138,23 @@
             Y.Assert.isTrue(event_fired);
         },
 
+        // Model changes are rendered correctly when syncUI() is called.
+        test_syncUI: function() {
+            this.details_widget = this._create_Widget();
+            this.details_widget.render();
+            // We manipulate the cached model data - delete a bug.
+            var bugs = window.LP.cache.bugs;
+            // Delete the first bug.
+            bugs.splice(0, 1);
+            this.details_widget.syncUI();
+            // Check the results.
+            var bug_row_selector = '#sharing-table-body tr[id=shared-bug-2]';
+            Y.Assert.isNull(Y.one(bug_row_selector));
+            var branch_row_selector =
+                '#sharing-table-body tr[id=shared-branch-2]';
+            Y.Assert.isNotNull(Y.one(branch_row_selector));
+        },
+
         // When the branch revoke link is clicked, the correct event is
         // published.
         test_branch_revoke_click: function() {
@@ -164,6 +182,41 @@
                 Y.one('#sharing-table-body span[id=remove-branch-2] a');
             delete_link_to_click.simulate('click');
             Y.Assert.isTrue(event_fired);
+        },
+
+        // The delete_artifacts call removes the specified bugs from the table.
+        test_delete_bugs: function() {
+            this.details_widget = this._create_Widget();
+            this.details_widget.render();
+            var row_selector = '#sharing-table-body tr[id=shared-bug-2]';
+            Y.Assert.isNotNull(Y.one(row_selector));
+            this.details_widget.delete_artifacts(
+                [window.LP.cache.bugs[0]], [], false);
+            Y.Assert.isNull(Y.one(row_selector));
+        },
+
+        // The delete_artifacts call removes the specified branches from the
+        // table.
+        test_delete_branches: function() {
+            this.details_widget = this._create_Widget();
+            this.details_widget.render();
+            var row_selector = '#sharing-table-body tr[id=shared-branch-2]';
+            Y.Assert.isNotNull(Y.one(row_selector));
+            this.details_widget.delete_artifacts(
+                [], [window.LP.cache.branches[0]], false);
+            Y.Assert.isNull(Y.one(row_selector));
+        },
+
+        // When all artifacts are deleted, a suitable message is displayed.
+        test_delete_all_artifacts: function() {
+            this.details_widget = this._create_Widget();
+            this.details_widget.render();
+            this.details_widget.delete_artifacts(
+                [window.LP.cache.bugs[0]],
+                [window.LP.cache.branches[0]], true);
+            Y.Assert.areEqual(
+                'There are no shared bugs or branches.',
+                Y.one('#sharing-table-body tr').get('text'));
         }
     }));
 

=== modified file 'lib/lp/registry/templates/pillar-sharing-details.pt'
--- lib/lp/registry/templates/pillar-sharing-details.pt	2012-04-05 03:52:27 +0000
+++ lib/lp/registry/templates/pillar-sharing-details.pt	2012-04-05 03:52:28 +0000
@@ -50,7 +50,13 @@
           </th>
         </tr>
       </thead>
-      <tbody id="sharing-table-body"></tbody>
+      <tbody id="sharing-table-body">
+          <tr>
+              <td colspan="4">
+                  There are no shared bugs or branches.
+              </td>
+          </tr>
+      </tbody>
     </table>
 
   </div>