← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wallyworld/launchpad/sharing-details-delete-966641 into lp:launchpad

 

Ian Booth has proposed merging lp:~wallyworld/launchpad/sharing-details-delete-966641 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #966641 in Launchpad itself: "Delete button on sharing details page does nothing"
  https://bugs.launchpad.net/launchpad/+bug/966641

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/sharing-details-delete-966641/+merge/100551

== Implementation ==

This is the first branch in a series to add the ability to revoke an artifact access grant from the sharing details page. It implements the user interface and make the server calls to do the work but the server side isn't implemented yet. That will be done next.

The PillarPersonSharingView was enhanced to place additional required data in the json request cache for the view.

A new yui class was implemented - SharingDetailsView. The implementation is similar to that used for the sharing information page. A view/controller class grabs data from the json request cache, instantiates and renders the required widgets, wires up listeners to the various links/buttons, makes any server side calls required, renders the results. The existing SharingDetailsTable is used as a widget to render the main portion of the view.

== Tests ==

A fair bit of clean up was done to the existing test_sharingdetails yui test to make it align with the current test templates and consistent with the other sharing yui tests. New tests were added to test the delete button etc.

A new yui test test_sharingdetailsview was added for the new yui sharing details view class.

On the server side of things, the PillarSharingDetails tests were enhanced to better check the view data model and test for new things added to the model.

bin/test -vvct test_pillar_sharing -t test_sharingdetailsview -t test_sharingdetails

== 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/sharingdetailsview.js
  lib/lp/registry/javascript/sharing/tests/test_sharingdetails.html
  lib/lp/registry/javascript/sharing/tests/test_sharingdetails.js
  lib/lp/registry/javascript/sharing/tests/test_sharingdetailsview.html
  lib/lp/registry/javascript/sharing/tests/test_sharingdetailsview.js
  lib/lp/registry/templates/pillar-sharing-details.pt
-- 
https://code.launchpad.net/~wallyworld/launchpad/sharing-details-delete-966641/+merge/100551
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/sharing-details-delete-966641 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py	2012-03-31 11:32:15 +0000
+++ lib/lp/registry/browser/pillar.py	2012-04-03 05:05:25 +0000
@@ -17,6 +17,7 @@
 
 from lazr.restful import ResourceJSONEncoder
 from lazr.restful.interfaces import IJSONRequestCache
+from lazr.restful.utils import get_current_web_service_request
 import simplejson
 from zope.component import getUtility
 from zope.interface import (
@@ -26,6 +27,7 @@
 from zope.schema.interfaces import IVocabulary
 from zope.schema.vocabulary import getVocabularyRegistry
 from zope.security.interfaces import Unauthorized
+from zope.traversing.browser.absoluteurl import absoluteURL
 
 from lp.app.browser.launchpad import iter_view_registrations
 from lp.app.browser.tales import MenuAPI
@@ -376,10 +378,25 @@
         self._loadSharedArtifacts()
 
         cache = IJSONRequestCache(self.request)
-        branch_data = self._build_branch_template_data(self.branches)
-        bug_data = self._build_bug_template_data(self.bugs)
+        request = get_current_web_service_request()
+        branch_data = self._build_branch_template_data(self.branches, request)
+        bug_data = self._build_bug_template_data(self.bugs, request)
+        sharee_data = {
+            'displayname': self.person.displayname,
+            'self_link': absoluteURL(self.person, request)
+        }
+        pillar_data = {
+            'self_link': absoluteURL(self.pillar, request)
+        }
+        cache.objects['sharee'] = sharee_data
+        cache.objects['pillar'] = pillar_data
         cache.objects['bugs'] = bug_data
         cache.objects['branches'] = branch_data
+        enabled_writable_flag = (
+            'disclosure.enhanced_sharing.writable')
+        write_flag_enabled = bool(getFeatureFlag(enabled_writable_flag))
+        cache.objects['sharing_write_enabled'] = (write_flag_enabled
+            and check_permission('launchpad.Edit', self.pillar))
 
     def _loadSharedArtifacts(self):
         bugs = []
@@ -397,31 +414,35 @@
         self.shared_bugs_count = len(bugs)
         self.shared_branches_count = len(branches)
 
-    def _build_branch_template_data(self, branches):
+    def _build_branch_template_data(self, branches, request):
         branch_data = []
         for branch in branches:
             branch_data.append(dict(
-                branch_link=canonical_url(branch),
+                self_link=absoluteURL(branch, request),
+                web_link=canonical_url(branch, path_only_if_possible=True),
                 branch_name=branch.unique_name,
                 branch_id=branch.id))
         return branch_data
 
-    def _build_bug_template_data(self, bugs):
+    def _build_bug_template_data(self, bugs, request):
         bug_data = []
         for bug in bugs:
             [bugtask] = [task for task in bug.bugtasks if
                             task.target == self.pillar]
             if bugtask is not None:
-                url = canonical_url(bugtask, path_only_if_possible=True)
+                web_link = canonical_url(bugtask, path_only_if_possible=True)
+                self_link = absoluteURL(bugtask, request)
                 importance = bugtask.importance.title.lower()
             else:
                 # This shouldn't ever happen, but if it does there's no reason
                 # to crash.
-                url = canonical_url(bug, path_only_if_possible=True)
+                web_link = canonical_url(bug, path_only_if_possible=True)
+                self_link = absoluteURL(bug, request)
                 importance = bug.default_bugtask.importance.title.lower()
 
             bug_data.append(dict(
-                bug_link=url,
+                self_link=self_link,
+                web_link=web_link,
                 bug_summary=bug.title,
                 bug_id=bug.id,
                 bug_importance=importance))

=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-03-31 11:32:15 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-04-03 05:05:25 +0000
@@ -7,6 +7,7 @@
 
 from BeautifulSoup import BeautifulSoup
 from lazr.restful.interfaces import IJSONRequestCache
+from lazr.restful.utils import get_current_web_service_request
 import simplejson
 from testtools.matchers import (
     LessThan,
@@ -17,6 +18,7 @@
 from zope.component import getUtility
 from zope.publisher.interfaces import NotFound
 from zope.security.interfaces import Unauthorized
+from zope.traversing.browser.absoluteurl import absoluteURL
 
 from lp.app.interfaces.services import IService
 from lp.registry.enums import InformationType
@@ -41,6 +43,9 @@
 
 
 DETAILS_ENABLED_FLAG = {'disclosure.enhanced_sharing_details.enabled': 'true'}
+DETAILS_WRITE_FLAG = {
+    'disclosure.enhanced_sharing_details.enabled': 'true',
+    'disclosure.enhanced_sharing.writable': 'true'}
 ENABLED_FLAG = {'disclosure.enhanced_sharing.enabled': 'true'}
 WRITE_FLAG = {'disclosure.enhanced_sharing.writable': 'true'}
 
@@ -55,21 +60,34 @@
             person = self.factory.makePerson()
         if with_sharing:
             if self.pillar_type == 'product':
-                bug = self.factory.makeBug(
+                self.bug = self.factory.makeBug(
+                    product=self.pillar,
+                    owner=self.pillar.owner,
+                    private=True)
+                self.branch = self.factory.makeBranch(
                     product=self.pillar,
                     owner=self.pillar.owner,
                     private=True)
             elif self.pillar_type == 'distribution':
-                bug = self.factory.makeBug(
+                self.branch = None
+                self.bug = self.factory.makeBug(
                     distribution=self.pillar,
                     owner=self.pillar.owner,
                     private=True)
-            artifact = self.factory.makeAccessArtifact(concrete=bug)
+            artifact = self.factory.makeAccessArtifact(concrete=self.bug)
             policy = self.factory.makeAccessPolicy(pillar=self.pillar)
             self.factory.makeAccessPolicyArtifact(
                 artifact=artifact, policy=policy)
             self.factory.makeAccessArtifactGrant(
                 artifact=artifact, grantee=person, grantor=self.pillar.owner)
+            if self.branch:
+                artifact = self.factory.makeAccessArtifact(
+                    concrete=self.branch)
+                self.factory.makeAccessPolicyArtifact(
+                    artifact=artifact, policy=policy)
+                self.factory.makeAccessArtifactGrant(
+                    artifact=artifact, grantee=person,
+                    grantor=self.pillar.owner)
 
         return PillarPerson(self.pillar, person)
 
@@ -110,6 +128,54 @@
             view = create_initialized_view(pillarperson, '+index')
             self.assertEqual(pillarperson.person.displayname, view.page_title)
 
+    def test_view_data_model(self):
+        # Test that the json request cache contains the view data model.
+        with FeatureFixture(DETAILS_ENABLED_FLAG):
+            pillarperson = self.getPillarPerson()
+            view = create_initialized_view(pillarperson, '+index')
+            cache = IJSONRequestCache(view.request)
+            request = get_current_web_service_request()
+            self.assertEqual({
+                'self_link': absoluteURL(pillarperson.person, request),
+                'displayname': pillarperson.person.displayname
+            }, cache.objects.get('sharee'))
+            self.assertEqual({
+                'self_link': absoluteURL(pillarperson.pillar, request),
+            }, cache.objects.get('pillar'))
+            bugtask = self.bug.default_bugtask
+            self.assertEqual({
+                'bug_id': self.bug.id,
+                'bug_summary': self.bug.title,
+                'bug_importance': bugtask.importance.title.lower(),
+                'web_link': canonical_url(
+                    bugtask, path_only_if_possible=True),
+                'self_link': absoluteURL(bugtask, request),
+            }, cache.objects.get('bugs')[0])
+            if self.pillar_type == 'product':
+                self.assertEqual({
+                    'branch_id': self.branch.id,
+                    'branch_name': self.branch.unique_name,
+                    'web_link': canonical_url(
+                        self.branch, path_only_if_possible=True),
+                    'self_link': absoluteURL(self.branch, request),
+                }, cache.objects.get('branches')[0])
+
+    def test_view_write_enabled_without_feature_flag(self):
+        # Test that sharing_write_enabled is not set without the feature flag.
+        with FeatureFixture(DETAILS_ENABLED_FLAG):
+            pillarperson = self.getPillarPerson()
+            view = create_initialized_view(pillarperson, '+index')
+            cache = IJSONRequestCache(view.request)
+            self.assertFalse(cache.objects.get('sharing_write_enabled'))
+
+    def test_view_write_enabled_with_feature_flag(self):
+        # Test that sharing_write_enabled is set when required.
+        with FeatureFixture(DETAILS_WRITE_FLAG):
+            pillarperson = self.getPillarPerson()
+            view = create_initialized_view(pillarperson, '+index')
+            cache = IJSONRequestCache(view.request)
+            self.assertTrue(cache.objects.get('sharing_write_enabled'))
+
 
 class TestProductSharingDetailsView(
     TestCaseWithFactory, PillarSharingDetailsMixin):

=== modified file 'lib/lp/registry/javascript/sharing/sharingdetails.js'
--- lib/lp/registry/javascript/sharing/sharingdetails.js	2012-03-28 22:21:37 +0000
+++ lib/lp/registry/javascript/sharing/sharingdetails.js	2012-04-03 05:05:25 +0000
@@ -3,12 +3,18 @@
  *
  * Sharing details widget
  *
- * @module lp.registry.sharing.details
+ * @module lp.registry.sharing.sharingdetails
  */
 
-YUI.add('lp.registry.sharing.details', function(Y) {
-
-var namespace = Y.namespace('lp.registry.sharing.details');
+YUI.add('lp.registry.sharing.sharingdetails', function(Y) {
+
+var namespace = Y.namespace('lp.registry.sharing.sharingdetails');
+
+var
+    NAME = "sharingDetailsTable",
+    // Events
+    REMOVE_GRANT = 'removeGrant';
+
 /*
  * Sharing details table widget.
  * This widget displays the details of a specific person's shared artifacts.
@@ -18,7 +24,12 @@
 }
 
 SharingDetailsTable.ATTRS = {
-
+    // The node holding the details table.
+    details_table_body: {
+        getter: function() {
+            return Y.one('#sharing-table-body');
+        }
+    },
     table_body_template: {
         value: null
     },
@@ -37,20 +48,20 @@
 
     branches: {
         value: []
+    },
+
+    write_enabled: {
+        value: false
+    },
+
+    person_name: {
+        value: null
     }
 };
 
 Y.extend(SharingDetailsTable, Y.Widget, {
 
     initializer: function(config) {
-        if (Y.Lang.isValue(config.branches)) {
-            this.set('branches', config.branches);
-        }
-
-        if (Y.Lang.isValue(config.bugs)) {
-            this.set('bugs', config.bugs);
-        }
-
         this.set(
             'bug_details_row_template',
             this._bug_details_row_template());
@@ -62,6 +73,7 @@
         this.set(
             'table_body_template',
             this._table_body_template());
+        this.publish(REMOVE_GRANT);
     },
 
     renderUI: function() {
@@ -74,37 +86,72 @@
         var template = this.get('table_body_template');
         var html = Y.lp.mustache.to_html(
             template,
-            {branches: branch_data, bugs: bug_data},
+            {branches: branch_data, bugs: bug_data,
+            displayname: this.get('person_name')},
             partials);
-        var table = Y.one('#sharing-table-body');
-        table.set('innerHTML', html);
-    },
+
+        var details_table_body = this.get('details_table_body');
+        var table_body_node = Y.Node.create(html);
+        details_table_body.replace(table_body_node);
+        this._update_editable_status();
+    },
+
+    _update_editable_status: function() {
+        var details_table_body = this.get('details_table_body');
+        if (!this.get('write_enabled')) {
+            details_table_body.all('.sprite.remove').each(function(node) {
+                node.addClass('unseen');
+            });
+        }
+    },
+
+     bindUI: function() {
+        // Bind the delete links.
+        if (!this.get('write_enabled')) {
+            return;
+        }
+        var details_table_body = this.get('details_table_body');
+        var self = this;
+        details_table_body.delegate('click', function(e) {
+            e.halt();
+            var delete_link = e.currentTarget;
+            var artifact_uri = delete_link.getAttribute('data-self_link');
+            var artifact_name = delete_link.getAttribute('data-name');
+            self.fire(
+                REMOVE_GRANT, delete_link, artifact_uri, artifact_name);
+        }, 'span[id^=remove-] a');
+     },
 
     _table_body_template: function() {
         return [
+        '<tbody id="sharing-table-body">',
         '{{#branches}}',
         '{{> branch}}',
         '{{/branches}}',
         '{{#bugs}}',
         '{{> bug}}',
-        '{{/bugs}}'
+        '{{/bugs}}',
+        '</tbody>'
         ].join(' ');
     },
 
     _bug_details_row_template: function() {
         return [
-        '<tr>',
+        '<tr id="shared-bug-{{ bug_id }}">',
         '    <td class="icon right">',
-        '        <span class="sprite bug-{{ bug_importance }}"></span>',
+        '        <span class="sprite bug-{{bug_importance}}"></span>',
         '    </td>',
-        '    <td class="amount">{{ bug_id }}</td>',
+        '    <td class="amount">{{bug_id}}</td>',
         '    <td>',
-        '        <a href="{{ bug_link }}">{{ bug_summary }}</a>',
+        '        <a href="{{web_link}}">{{bug_summary}}</a>',
         '    </td>',
-        '    <td>&mdash;</td>',
-        '    <td class="actions" id="remove-bug-{{ bug_id }}">',
-        '        <a class="sprite remove" href="#"',
-        '            title="Unshare this with the user">&nbsp;</a>',
+        '    <td class="action-icons nowrap">',
+        '    <span id="remove-bug-{{ bug_id }}">',
+        '    <a class="sprite remove" href="#"',
+        '        title="Unshare bug {{bug_id}} with {{displayname}}"',
+        '        data-self_link="{{self_link}}" data-name="Bug {{bug_id}}">',
+        '    &nbsp;</a>',
+        '    </span>',
         '    </td>',
         '</tr>'
         ].join(' ');
@@ -112,27 +159,30 @@
 
     _branch_details_row_template: function() {
         return [
-        '<tr>',
+        '<tr id="shared-branch-{{ branch_id }}">',
         '    <td colspan="3">',
-        '        <a class="sprite branch" href="{{ branch_link }}">',
-        '            {{ branch_name }}',
+        '        <a class="sprite branch" href="{{web_link}}">',
+        '            {{branch_name}}',
         '        </a>',
         '    </td>',
-        '    <td>&mdash;</td>',
-        '    <td class="actions" id="remove-branch-{{ branch_id }}">',
-        '        <a class="sprite remove" href="#"',
-        '            title="Unshare this with the user">&nbsp;</a>',
+        '    <td class="action-icons nowrap">',
+        '    <span id="remove-branch-{{branch_id}}">',
+        '    <a class="sprite remove" href="#"',
+        '        title="Unshare branch {{branch_name}} with {{displayname}}"',
+        '        data-self_link="{{self_link}}" data-name="{{branch_name}}">',
+        '    &nbsp;</a>',
+        '    </span>',
         '    </td>',
         '</tr>'
         ].join(' ');
     }
 });
 
-SharingDetailsTable.NAME = 'sharingDetailsTable';
+SharingDetailsTable.NAME = NAME;
+SharingDetailsTable.REMOVE_GRANT = REMOVE_GRANT;
 
 namespace.SharingDetailsTable = SharingDetailsTable;
 
 }, "0.1", { "requires": [
-    'node',
-    'lp.mustache'
+    'node', 'event', 'lp.mustache'
 ] });

=== added file 'lib/lp/registry/javascript/sharing/sharingdetailsview.js'
--- lib/lp/registry/javascript/sharing/sharingdetailsview.js	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/javascript/sharing/sharingdetailsview.js	2012-04-03 05:05:25 +0000
@@ -0,0 +1,188 @@
+/* Copyright 2012 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Disclosure infrastructure.
+ *
+ * @module lp.registry.sharing
+ */
+
+YUI.add('lp.registry.sharing.sharingdetailsview', function(Y) {
+
+var namespace = Y.namespace('lp.registry.sharing.sharingdetailsview');
+
+function SharingDetailsView(config) {
+    SharingDetailsView.superclass.constructor.apply(this, arguments);
+}
+
+SharingDetailsView.ATTRS = {
+    lp_client: {
+        value: new Y.lp.client.Launchpad()
+    },
+
+    write_enabled: {
+        value: false
+    },
+
+    sharing_details_table: {
+        value: null
+    }
+};
+
+Y.extend(SharingDetailsView, Y.Widget, {
+
+    initializer: function(config) {
+        if (LP.cache.sharing_write_enabled !== true) {
+            return;
+        }
+        this.set('write_enabled', true);
+    },
+
+    renderUI: function() {
+        var ns = Y.lp.registry.sharing.sharingdetails;
+        var details_table = new ns.SharingDetailsTable({
+            bugs: LP.cache.bugs,
+            branches: LP.cache.branches,
+            person_name: LP.cache.sharee.displayname,
+            write_enabled: this.get('write_enabled')
+        });
+        this.set('sharing_details_table', details_table);
+        details_table.render();
+    },
+
+    bindUI: function() {
+        if (!this.get('write_enabled')) {
+            return;
+        }
+        var self = this;
+        var sharing_details_table = this.get('sharing_details_table');
+        var ns = Y.lp.registry.sharing.sharingdetails;
+        sharing_details_table.subscribe(
+            ns.SharingDetailsTable.REMOVE_GRANT, function(e) {
+                self.confirm_grant_removal(
+                    e.details[0], e.details[1], e.details[2]);
+        });
+    },
+
+    syncUI: function() {
+        var sharing_details_table = this.get('sharing_details_table');
+        sharing_details_table.syncUI();
+    },
+
+    /**
+     * Show a spinner next to the delete icon.
+     *
+     * @method _show_delete_spinner
+     */
+    _show_delete_spinner: function(delete_link) {
+        var spinner_node = Y.Node.create(
+        '<img class="spinner" src="/@@/spinner" alt="Removing..." />');
+        delete_link.insertBefore(spinner_node, delete_link);
+        delete_link.addClass('unseen');
+    },
+
+    /**
+     * Hide the delete spinner.
+     *
+     * @method _hide_delete_spinner
+     */
+    _hide_delete_spinner: function(delete_link) {
+        delete_link.removeClass('unseen');
+        var spinner = delete_link.get('parentNode').one('.spinner');
+        if (Y.Lang.isValue(spinner)) {
+            spinner.remove();
+        }
+    },
+
+    /**
+     * Prompt the user to confirm the removal of access to the selected
+     * artifact.
+     *
+     * @method confirm_grant_removal
+     */
+    confirm_grant_removal: function(delete_link, artifact_uri,
+                                    artifact_name) {
+        var confirm_text_template = [
+            '<p class="large-warning" style="padding:2px 2px 0 36px;">',
+            '    Do you really want to stop sharing',
+            '    "{artifact}" with {person_name}?<br><br>',
+            '</p>'
+            ].join('');
+        var person_name = LP.cache.sharee.displayname;
+        var confirm_text = Y.Lang.sub(confirm_text_template,
+                {artifact: artifact_name,
+                 person_name: person_name});
+        var self = this;
+        var co = new Y.lp.app.confirmationoverlay.ConfirmationOverlay({
+            submit_fn: function() {
+                self.perform_remove_grant(delete_link, artifact_uri);
+            },
+            form_content: confirm_text,
+            headerContent: '<h2>Stop sharing</h2>'
+        });
+        co.show();
+    },
+
+    /**
+     * The server call to remove the specified sharee has succeeded.
+     * Update the model and view.
+     * @method remove_grant_success
+     * @param artifact_uri
+     */
+    remove_grant_success: function(artifact_uri) {
+        var bugs_data = LP.cache.bugs;
+        var self = this;
+        Y.Array.some(bugs_data, function(bug, index) {
+            if (bug.self_link === artifact_uri) {
+                bugs_data.splice(index, 1);
+                self.syncUI();
+                return true;
+            }
+        });
+        var branch_data = LP.cache.branches;
+        Y.Array.some(branch_data, function(branch, index) {
+            if (branch.self_link === artifact_uri) {
+                branch_data.splice(index, 1);
+                self.syncUI();
+                return true;
+            }
+        });
+    },
+
+    /**
+     * Make a server call to remove access to the specified artifact.
+     * @method perform_remove_sharee
+     * @param delete_link
+     * @param artifact_uri
+     */
+    perform_remove_grant: function(delete_link, artifact_uri) {
+        var error_handler = new Y.lp.client.ErrorHandler();
+        var self = this;
+        var y_config =  {
+            on: {
+                start: Y.bind(
+                    self._show_delete_spinner, namespace, delete_link),
+                end: Y.bind(self._hide_delete_spinner, namespace, delete_link),
+                success: function() {
+                    self.remove_grant_success(artifact_uri);
+                },
+                failure: error_handler.getFailureHandler()
+            },
+            parameters: {
+                pillar: LP.cache.pillar.self_link,
+                sharee: LP.cache.sharee.self_link,
+                artifacts: [artifact_uri]
+            }
+        };
+        this.get('lp_client').named_post(
+            '/+services/sharing', 'revokeAccessGrants', y_config);
+    }
+});
+
+SharingDetailsView.NAME = 'sharingDetailsView';
+namespace.SharingDetailsView = SharingDetailsView;
+
+}, "0.1", { "requires": [
+    'node', 'selector-css3', 'lp.client', 'lp.mustache',
+    'lp.registry.sharing.sharingdetails', 'lp.app.confirmationoverlay'
+    ]});
+

=== renamed file 'lib/lp/registry/javascript/sharing/tests/test_sharing_details.html' => 'lib/lp/registry/javascript/sharing/tests/test_sharingdetails.html'
--- lib/lp/registry/javascript/sharing/tests/test_sharing_details.html	2012-03-27 15:10:21 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_sharingdetails.html	2012-04-03 05:05:25 +0000
@@ -34,18 +34,20 @@
       <script type="text/javascript" src="../sharingdetails.js"></script>
 
       <!-- The test suite. -->
-      <script type="text/javascript" src="test_sharing_details.js"></script>
+      <script type="text/javascript" src="test_sharingdetails.js"></script>
 
     </head>
     <body class="yui3-skin-sam">
         <!-- The example markup required by the script to run -->
         <ul id="suites">
-            <li>lp.registry.sharing.details.test</li>
+            <li>lp.registry.sharing.sharingdetails.test</li>
         </ul>
-        <table>
-          <tbody id="sharing-table-body">
-          </tbody>
-        </table>
-
+        <div id="fixture"></div>
+        <script type="text/x-template" id="sharing-table-template">
+            <table>
+              <tbody id="sharing-table-body">
+              </tbody>
+            </table>
+        </script>
     </body>
 </html>

=== renamed file 'lib/lp/registry/javascript/sharing/tests/test_sharing_details.js' => 'lib/lp/registry/javascript/sharing/tests/test_sharingdetails.js'
--- lib/lp/registry/javascript/sharing/tests/test_sharing_details.js	2012-03-27 20:45:36 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_sharingdetails.js	2012-04-03 05:05:25 +0000
@@ -1,70 +1,169 @@
 /* Copyright (c) 2012, Canonical Ltd. All rights reserved. */
-YUI.add('lp.registry.sharing.details.test', function(Y) {
+YUI.add('lp.registry.sharing.sharingdetails.test', function(Y) {
 
 // Local aliases
-    var Assert = Y.Assert,
-        ArrayAssert = Y.ArrayAssert;
-    var sharing_details = Y.lp.registry.sharing.details;
+    var Assert = Y.Assert;
+    var sharing_details = Y.lp.registry.sharing.sharingdetails;
 
-    var tests = Y.namespace('lp.registry.sharing.details.test');
+    var tests = Y.namespace('lp.registry.sharing.sharingdetails.test');
     tests.suite = new Y.Test.Suite(
-        "lp.registry.sharing.details Tests");
+        "lp.registry.sharing.sharingdetails Tests");
 
     tests.suite.add(new Y.Test.Case({
         name: 'Sharing Details',
 
-        setUp: function() {
+        setUp: function () {
             window.LP = {
                 links: {},
-                cache: {}
+                cache: {
+                    bugs: [
+                        {
+                            self_link: 'api/devel/bugs/2',
+                            web_link:'/bugs/2',
+                            bug_id: '2',
+                            bug_importance: 'critical',
+                            bug_summary:'Everything is broken.'
+                        }
+                    ],
+                    branches: [
+                        {
+                            self_link: 'api/devel/~someone/+junk/somebranch',
+                            web_link:'/~someone/+junk/somebranch',
+                            branch_id: '2',
+                            branch_name:'lp:~someone/+junk/somebranch'
+                        }
+                    ]
+                }
             };
-        },
-
-        tearDown: function() {
-        },
-
+            this.fixture = Y.one('#fixture');
+            var sharing_table = Y.Node.create(
+                    Y.one('#sharing-table-template').getContent());
+            this.fixture.appendChild(sharing_table);
+
+        },
+
+        tearDown: function () {
+            if (this.fixture !== null) {
+                this.fixture.empty(true);
+            }
+            delete this.fixture;
+            delete window.LP;
+        },
+
+        _create_Widget: function(overrides) {
+            if (!Y.Lang.isValue(overrides)) {
+                overrides = {};
+            }
+            var config = Y.merge({
+                person_name: 'Fred',
+                bugs: window.LP.cache.bugs,
+                branches: window.LP.cache.branches,
+                write_enabled: true
+            }, overrides);
+            window.LP.cache.sharee_data = config.sharees;
+            return new sharing_details.SharingDetailsTable(config);
+        },
+
+        test_library_exists: function () {
+            Y.Assert.isObject(Y.lp.registry.sharing.sharingdetails,
+                "We should be able to locate the " +
+                "lp.registry.sharing.sharingdetails module");
+        },
+
+        test_widget_can_be_instantiated: function() {
+            this.details_widget = this._create_Widget();
+            Y.Assert.isInstanceOf(
+                Y.lp.registry.sharing.sharingdetails.SharingDetailsTable,
+                this.details_widget,
+                "Sharing details table failed to be instantiated");
+        },
+
+        // Read only mode disables the correct things.
+        test_readonly: function() {
+            this.details_widget = this._create_Widget({
+                write_enabled: false
+            });
+            this.details_widget.render();
+            Y.all('#sharing-table-body .sprite.remove a')
+                .each(function(link) {
+                    Y.Assert.isTrue(link.hasClass('unseen'));
+            });
+        },
+
+        // Test that branches are correctly rendered.
         test_render_branches: function () {
-            var config = {
-                branches: [
-                    {
-                        branch_link:'/~someone/+junk/somebranch',
-                        branch_id: '2',
-                        branch_name:'lp:~someone/+junk/somebranch'
-                    }
-                ]
-            };
-            details_module = Y.lp.registry.sharing.details;
-            table_constructor = details_module.SharingDetailsTable;
-            var details_widget = new table_constructor(config);
-            details_widget.render();
+            this.details_widget = this._create_Widget();
+            this.details_widget.render();
             var expected = "lp:~someone/+junk/somebranch";
-            var branch_link = Y.one('#sharing-table-body').one('a');
-            var actual_text = branch_link.get('text').replace(/\s+/g, '');
+            var web_link = Y.one(
+                '#sharing-table-body tr#shared-branch-2').one('a');
+            var actual_text = web_link.get('text').replace(/\s+/g, '');
             Assert.areEqual(expected, actual_text);
         },
 
+        // Test that bugs are correctly rendered.
         test_render_bugs: function () {
-            var config = {
-                bugs: [
-                    {
-                        bug_link:'/bugs/2',
-                        bug_id: '2',
-                        bug_importance: 'critical',
-                        bug_summary:'Everything is broken.'
-                    }
-                ]
-            };
-            details_module = Y.lp.registry.sharing.details;
-            table_constructor = details_module.SharingDetailsTable;
-            var details_widget = new table_constructor(config);
-            details_widget.render();
+            this.details_widget = this._create_Widget();
+            this.details_widget.render();
             var expected = "Everythingisbroken.";
-            var bug_link = Y.one('#sharing-table-body').one('a');
-            var actual_text = bug_link.get('text').replace(/\s+/g, '');
+            var web_link = Y.one(
+                '#sharing-table-body tr#shared-bug-2').one('a');
+            var actual_text = web_link.get('text').replace(/\s+/g, '');
             Assert.areEqual(expected, actual_text);
+        },
+
+        // When the bug revoke link is clicked, the correct event is published.
+        test_bug_revoke_click: function() {
+            this.details_widget = this._create_Widget();
+            this.details_widget.render();
+            var event_fired = false;
+            this.details_widget.subscribe(
+                sharing_details.SharingDetailsTable.REMOVE_GRANT,
+                function(e) {
+                    var delete_link = e.details[0];
+                    var artifact_uri = e.details[1];
+                    var artifact_name = e.details[2];
+                    Y.Assert.areEqual('api/devel/bugs/2', artifact_uri);
+                    Y.Assert.areEqual('Bug 2', artifact_name);
+                    Y.Assert.areEqual(delete_link_to_click, delete_link);
+                    event_fired = true;
+                }
+            );
+            var delete_link_to_click =
+                Y.one('#sharing-table-body span[id=remove-bug-2] a');
+            delete_link_to_click.simulate('click');
+            Y.Assert.isTrue(event_fired);
+        },
+
+        // When the branch revoke link is clicked, the correct event is
+        // published.
+        test_branch_revoke_click: function() {
+            this.details_widget = this._create_Widget();
+            this.details_widget.render();
+            var event_fired = false;
+            this.details_widget.subscribe(
+                sharing_details.SharingDetailsTable.REMOVE_GRANT,
+                function(e) {
+                    var delete_link = e.details[0];
+                    var artifact_uri = e.details[1];
+                    var artifact_name = e.details[2];
+                    Y.Assert.areEqual(
+                        'api/devel/~someone/+junk/somebranch',
+                        artifact_uri);
+                    Y.Assert.areEqual(
+                        'lp:~someone/+junk/somebranch', artifact_name);
+                    Y.Assert.areEqual(delete_link_to_click, delete_link);
+                    event_fired = true;
+                }
+            );
+            var delete_link_to_click =
+                Y.one('#sharing-table-body span[id=remove-branch-2] a');
+            delete_link_to_click.simulate('click');
+            Y.Assert.isTrue(event_fired);
         }
     }));
 
 
-}, '0.1', { 'requires': [ 'test', 'console', 'event',
-                          'lp.registry.sharing.details']});
+}, '0.1', { 'requires':
+    [ 'test', 'console', 'event', 'node-event-simulate',
+        'lp.registry.sharing.sharingdetails']});

=== added file 'lib/lp/registry/javascript/sharing/tests/test_sharingdetailsview.html'
--- lib/lp/registry/javascript/sharing/tests/test_sharingdetailsview.html	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_sharingdetailsview.html	2012-04-03 05:05:25 +0000
@@ -0,0 +1,66 @@
+<!DOCTYPE html>
+<!--
+Copyright 2012 Canonical Ltd.  This software is licensed under the
+GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<html>
+  <head>
+      <title>Sharing Details View 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" />
+
+      <!-- Dependencies -->
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/testing/mockio.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/client.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/confirmationoverlay/confirmationoverlay.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/mustache.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/formoverlay/formoverlay.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/overlay/overlay.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/registry/sharing/sharingdetails.js"></script>
+
+      <!-- The module under test. -->
+      <script type="text/javascript" src="../sharingdetailsview.js"></script>
+
+      <!-- The test suite -->
+      <script type="text/javascript" src="test_sharingdetailsview.js"></script>
+
+      <script id="test-fixture" type="text/x-template">
+          <table id='sharing-details-table'></table>
+      </script>
+    </head>
+    <body class="yui3-skin-sam">
+        <ul id="suites">
+            <li>lp.registry.sharing.sharingdetailsview.test</li>
+        </ul>
+        <div id='fixture'>
+        </div>
+        <script type="text/x-template" id="sharing-table-template">
+            <table>
+              <tbody id="sharing-table-body">
+              </tbody>
+            </table>
+        </script>
+    </body>
+</html>

=== added file 'lib/lp/registry/javascript/sharing/tests/test_sharingdetailsview.js'
--- lib/lp/registry/javascript/sharing/tests/test_sharingdetailsview.js	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_sharingdetailsview.js	2012-04-03 05:05:25 +0000
@@ -0,0 +1,261 @@
+/* Copyright (c) 2012, Canonical Ltd. All rights reserved. */
+
+YUI.add('lp.registry.sharing.sharingdetailsview.test', function (Y) {
+
+    var tests = Y.namespace('lp.registry.sharing.sharingdetailsview.test');
+    tests.suite = new Y.Test.Suite(
+        'lp.registry.sharing.sharingdetailsview Tests');
+
+    tests.suite.add(new Y.Test.Case({
+        name: 'lp.registry.sharing.sharingdetailsview_tests',
+
+        setUp: function () {
+            window.LP = {
+                links: {},
+                cache: {
+                    bugs: [
+                        {
+                            self_link: 'api/devel/bugs/2',
+                            web_link:'/bugs/2',
+                            bug_id: '2',
+                            bug_importance: 'critical',
+                            bug_summary:'Everything is broken.'
+                        }
+                    ],
+                    branches: [
+                        {
+                            self_link: 'api/devel/~someone/+junk/somebranch',
+                            web_link:'/~someone/+junk/somebranch',
+                            branch_id: '2',
+                            branch_name:'lp:~someone/+junk/somebranch'
+                        }
+                    ],
+                    sharee: {
+                        displayname: 'Fred Bloggs',
+                        self_link: '~fred'
+                    },
+                    pillar: {
+                        self_link: '/pillar'
+                    },
+                    sharing_write_enabled: true
+                }
+            };
+            this.fixture = Y.one('#fixture');
+            var sharee_table = Y.Node.create(
+                    Y.one('#sharing-table-template').getContent());
+            this.fixture.appendChild(sharee_table);
+        },
+
+        tearDown: function () {
+            Y.one('#fixture').empty(true);
+            delete window.LP;
+        },
+
+        _create_Widget: function(cfg) {
+            var ns = Y.lp.registry.sharing.sharingdetailsview;
+            return new ns.SharingDetailsView(cfg);
+        },
+
+        test_library_exists: function () {
+            Y.Assert.isObject(Y.lp.registry.sharing.sharingdetailsview,
+                "We should be able to locate the " +
+                "lp.registry.sharing.sharingdetailsview module");
+        },
+
+        test_widget_can_be_instantiated: function() {
+            this.view = this._create_Widget();
+            Y.Assert.isInstanceOf(
+                Y.lp.registry.sharing.sharingdetailsview.SharingDetailsView,
+                this.view,
+                "Sharing details view failed to be instantiated");
+        },
+
+        // The view is correctly rendered.
+        test_render: function() {
+            this.view = this._create_Widget();
+            this.view.render();
+            // The sharing details table - we'll just check one row
+            Y.Assert.isNotNull(
+                Y.one('#sharing-table-body tr[id=shared-bug-2]'));
+        },
+
+        // Read only mode disables the correct things.
+        test_readonly: function() {
+            window.LP.cache.sharing_write_enabled = false;
+            this.view = this._create_Widget();
+            this.view.render();
+            Y.Assert.isFalse(
+                this.view.get('sharing_details_table')
+                    .get('write_enabled'));
+        },
+
+        // Clicking a bug remove link calls the confirm_grant_removal
+        // method with the correct parameters.
+        test_remove_bug_grant_click: function() {
+            this.view = this._create_Widget();
+            this.view.render();
+            var confirmRemove_called = false;
+            this.view.confirm_grant_removal = function(
+                    delete_link, artifact_uri, artifact_name) {
+                Y.Assert.areEqual('api/devel/bugs/2', artifact_uri);
+                Y.Assert.areEqual('Bug 2', artifact_name);
+                Y.Assert.areEqual(delete_link_to_click, delete_link);
+                confirmRemove_called = true;
+
+            };
+            var delete_link_to_click =
+                Y.one('#sharing-table-body span[id=remove-bug-2] a');
+            delete_link_to_click.simulate('click');
+            Y.Assert.isTrue(confirmRemove_called);
+        },
+
+        // Clicking a bug remove link calls the confirm_grant_removal
+        // method with the correct parameters.
+        test_remove_branch_grant_click: function() {
+            this.view = this._create_Widget();
+            this.view.render();
+            var confirmRemove_called = false;
+            this.view.confirm_grant_removal = function(
+                    delete_link, artifact_uri, artifact_name) {
+                Y.Assert.areEqual(
+                    'api/devel/~someone/+junk/somebranch', artifact_uri);
+                Y.Assert.areEqual(
+                    'lp:~someone/+junk/somebranch', artifact_name);
+                Y.Assert.areEqual(delete_link_to_click, delete_link);
+                confirmRemove_called = true;
+
+            };
+            var delete_link_to_click =
+                Y.one('#sharing-table-body span[id=remove-branch-2] a');
+            delete_link_to_click.simulate('click');
+            Y.Assert.isTrue(confirmRemove_called);
+        },
+
+        //Test the behaviour of the removal confirmation dialog.
+        _test_confirm_grant_removal: function(click_ok) {
+            this.view = this._create_Widget();
+            this.view.render();
+            var performRemove_called = false;
+            this.view.perform_remove_grant = function(
+                delete_link, artifact_uri) {
+                Y.Assert.areEqual('api/devel/bugs/2', artifact_uri);
+                Y.Assert.areEqual(artifact_delete_link, delete_link);
+                performRemove_called = true;
+
+            };
+            var artifact_delete_link =
+                Y.one('#sharing-table-body td[id=remove-bug-2] a');
+            this.view.confirm_grant_removal(
+                artifact_delete_link, 'api/devel/bugs/2', 'Bug 2');
+            var co = Y.one('.yui3-overlay.yui3-lp-app-confirmationoverlay');
+            var actions = co.one('.yui3-lazr-formoverlay-actions');
+            var btn_style;
+            if (click_ok) {
+                btn_style = '.ok-btn';
+            } else {
+                btn_style = '.cancel-btn';
+            }
+            var button = actions.one(btn_style);
+            button.simulate('click');
+            Y.Assert.areEqual(click_ok, performRemove_called);
+            Y.Assert.isTrue(
+                    co.hasClass('yui3-lp-app-confirmationoverlay-hidden'));
+        },
+
+        //Test the remove confirmation dialog when the user clicks Ok.
+        test_confirm_sharee_removal_ok: function() {
+            this._test_confirm_grant_removal(true);
+        },
+
+        //Test the remove confirmation dialog when the user clicks Cancel.
+        test_confirm_sharee_removal_cancel: function() {
+            this._test_confirm_grant_removal(false);
+        },
+
+        // The perform_remove_grant method makes the expected XHR calls.
+        test_perform_remove_grant: function() {
+            var mockio = new Y.lp.testing.mockio.MockIo();
+            var lp_client = new Y.lp.client.Launchpad({
+                io_provider: mockio
+            });
+            this.view = this._create_Widget({
+                lp_client: lp_client
+            });
+            this.view.render();
+            var remove_grant_success_called = false;
+            var self = this;
+            this.view.remove_grant_success = function(artifact_uri) {
+                Y.Assert.areEqual('api/devel/bugs/2', artifact_uri);
+                remove_grant_success_called = true;
+            };
+            var delete_link =
+                Y.one('#sharing-table-body span[id=remove-bug-2] a');
+            this.view.perform_remove_grant(delete_link, 'api/devel/bugs/2');
+            Y.Assert.areEqual(
+                '/api/devel/+services/sharing',
+                mockio.last_request.url);
+            var expected_qs = '';
+            expected_qs = Y.lp.client.append_qs(
+                expected_qs, 'ws.op', 'revokeAccessGrants');
+            expected_qs = Y.lp.client.append_qs(
+                expected_qs, 'pillar', '/pillar');
+            expected_qs = Y.lp.client.append_qs(
+                expected_qs, 'sharee', '~fred');
+            expected_qs = Y.lp.client.append_qs(
+                expected_qs, 'artifacts', 'api/devel/bugs/2');
+            Y.Assert.areEqual(expected_qs, mockio.last_request.config.data);
+            mockio.last_request.successJSON({});
+            Y.Assert.isTrue(remove_grant_success_called);
+        },
+
+        // The remove bug grant callback updates the model and syncs the UI.
+        test_remove_bug_grant_success: function() {
+            this.view = this._create_Widget({anim_duration: 0});
+            this.view.render();
+            var syncUI_called = false;
+            this.view.syncUI = function() {
+                syncUI_called = true;
+            };
+            this.view.remove_grant_success('api/devel/bugs/2');
+            Y.Assert.isTrue(syncUI_called);
+            Y.Array.each(window.LP.cache.bugs,
+                function(bug) {
+                    Y.Assert.areNotEqual(2, bug.bug_id);
+            });
+        },
+
+        // The remove branch grant callback updates the model and syncs the UI.
+        test_remove_branch_grant_success: function() {
+            this.view = this._create_Widget({anim_duration: 0});
+            this.view.render();
+            var syncUI_called = false;
+            this.view.syncUI = function() {
+                syncUI_called = true;
+            };
+            this.view.remove_grant_success(
+                'api/devel/~someone/+junk/somebranch');
+            Y.Assert.isTrue(syncUI_called);
+            Y.Array.each(window.LP.cache.branches,
+                function(branch) {
+                    Y.Assert.areNotEqual(2, branch.branch_id);
+            });
+        },
+
+        // Test that syncUI works as expected.
+        test_syncUI: function() {
+            this.view = this._create_Widget();
+            this.view.render();
+            var sharee_table = this.view.get('sharing_details_table');
+            var table_syncUI_called = false;
+            sharee_table.syncUI = function() {
+                table_syncUI_called = true;
+            };
+            this.view.syncUI();
+            Y.Assert.isTrue(table_syncUI_called);
+        }
+    }));
+
+}, '0.1', {'requires': ['test', 'console', 'event', 'node-event-simulate',
+        'lp.testing.mockio',
+        'lp.registry.sharing.sharingdetails',
+        'lp.registry.sharing.sharingdetailsview']});

=== modified file 'lib/lp/registry/templates/pillar-sharing-details.pt'
--- lib/lp/registry/templates/pillar-sharing-details.pt	2012-03-27 21:12:27 +0000
+++ lib/lp/registry/templates/pillar-sharing-details.pt	2012-04-03 05:05:25 +0000
@@ -10,17 +10,12 @@
 <head>
     <metal:block fill-slot="head_epilogue">
     <script>
-            LPJS.use('base', 'node', 'event', 'lp.registry.sharing.details',
+            LPJS.use('base', 'node', 'event', 'lp.registry.sharing.sharingdetailsview',
                 function(Y) {
             Y.on('domready', function() {
-                var config = {
-                    branches: LP.cache.branches,
-                    bugs: LP.cache.bugs
-                };
-                var details_module  = Y.lp.registry.sharing.details;
-                var details_widget = new details_module.SharingDetailsTable(
-                                                          config);
-                details_widget.render();
+                var details_module  = Y.lp.registry.sharing.sharingdetailsview;
+                var details_view = new details_module.SharingDetailsView();
+                details_view.render();
             });
           });
     </script>
@@ -47,16 +42,12 @@
       <col width="20px"/>
       <col width="auto"/>
       <col width="auto"/>
-      <col width="30%"/>
       <col width="auto"/>
       <thead>
         <tr>
-          <th colspan="3" width="">
+          <th colspan="4" width="">
             <a href="#" class="sortheader">Bug Report or Branch</a>
           </th>
-          <th colspan="2" width="">
-            <a href="#" class="sortheader">Via</a>
-          </th>
         </tr>
       </thead>
       <tbody id="sharing-table-body"></tbody>