launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18483
Re: [Merge] lp:~cjwatson/launchpad/git-sharing-ui into lp:launchpad
Review: Approve code
Could to with a s/branch/Bazaar branch/ throughout, and there's some debugging left over, but otherwise looks good.
Diff comments:
> === modified file 'lib/lp/registry/browser/pillar.py'
> --- lib/lp/registry/browser/pillar.py 2015-02-16 13:01:34 +0000
> +++ lib/lp/registry/browser/pillar.py 2015-04-29 10:34:20 +0000
> @@ -425,6 +425,8 @@
> cache = IJSONRequestCache(self.request)
> request = get_current_web_service_request()
> branch_data = self._build_branch_template_data(self.branches, request)
> + gitrepository_data = self._build_gitrepository_template_data(
> + self.gitrepositories, request)
> bug_data = self._build_bug_template_data(self.bugtasks, request)
> spec_data = self._build_specification_template_data(
> self.specifications, request)
> @@ -439,6 +441,7 @@
> cache.objects['pillar'] = pillar_data
> cache.objects['bugs'] = bug_data
> cache.objects['branches'] = branch_data
> + cache.objects['gitrepositories'] = gitrepository_data
> cache.objects['specifications'] = spec_data
>
> def _loadSharedArtifacts(self):
> @@ -476,6 +479,17 @@
> information_type=branch.information_type.title))
> return branch_data
>
> + def _build_gitrepository_template_data(self, repositories, request):
> + repository_data = []
> + for repository in repositories:
> + repository_data.append(dict(
> + self_link=absoluteURL(repository, request),
> + web_link=canonical_url(repository, path_only_if_possible=True),
> + repository_name=repository.unique_name,
> + repository_id=repository.id,
> + information_type=repository.information_type.title))
> + return repository_data
> +
> def _build_bug_template_data(self, bugtasks, request):
> bug_data = []
> for bugtask in bugtasks:
>
> === modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
> --- lib/lp/registry/browser/tests/test_pillar_sharing.py 2013-06-20 05:50:00 +0000
> +++ lib/lp/registry/browser/tests/test_pillar_sharing.py 2015-04-29 10:34:20 +0000
> @@ -1,4 +1,4 @@
> -# Copyright 2012-2013 Canonical Ltd. This software is licensed under the
> +# Copyright 2012-2015 Canonical Ltd. This software is licensed under the
> # GNU Affero General Public License version 3 (see the file LICENSE).
>
> """Test views that manage sharing."""
> @@ -21,6 +21,7 @@
>
> from lp.app.enums import InformationType
> from lp.app.interfaces.services import IService
> +from lp.code.interfaces.gitrepository import GIT_FEATURE_FLAG
> from lp.registry.enums import (
> BranchSharingPolicy,
> BugSharingPolicy,
> @@ -29,11 +30,13 @@
> from lp.registry.model.pillar import PillarPerson
> from lp.services.config import config
> from lp.services.database.interfaces import IStore
> +from lp.services.features.testing import FeatureFixture
> from lp.services.webapp.interfaces import StormRangeFactoryError
> from lp.services.webapp.publisher import canonical_url
> from lp.testing import (
> login_person,
> logout,
> + normalize_whitespace,
> person_logged_in,
> StormStatementRecorder,
> TestCaseWithFactory,
> @@ -55,6 +58,7 @@
>
> def setUp(self):
> super(SharingBaseTestCase, self).setUp()
> + self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
> self.driver = self.factory.makePerson()
> self.owner = self.factory.makePerson()
> if self.pillar_type == 'distribution':
> @@ -75,11 +79,13 @@
> return grantee
>
> def makeArtifactGrantee(self, grantee=None, with_bug=True,
> - with_branch=False, security=False):
> + with_branch=False, with_gitrepository=True,
> + security=False):
> if grantee is None:
> grantee = self.factory.makePerson()
>
> branch = None
> + gitrepository = None
> bug = None
> artifacts = []
>
> @@ -90,6 +96,13 @@
> artifacts.append(
> self.factory.makeAccessArtifact(concrete=branch))
>
> + if with_gitrepository and self.pillar_type == 'product':
> + gitrepository = self.factory.makeGitRepository(
> + target=self.pillar, owner=self.pillar.owner,
> + information_type=InformationType.PRIVATESECURITY)
> + artifacts.append(
> + self.factory.makeAccessArtifact(concrete=gitrepository))
> +
> if with_bug:
> if security:
> owner = self.factory.makePerson()
> @@ -120,7 +133,7 @@
> """Test the pillar sharing details view."""
>
> def getPillarPerson(self, person=None, security=False):
> - person = self.makeArtifactGrantee(person, True, True, security)
> + person = self.makeArtifactGrantee(person, True, True, True, security)
> return PillarPerson(self.pillar, person)
>
> def test_view_filters_security_wisely(self):
> @@ -160,8 +173,8 @@
> pillarperson.pillar.name, pillarperson.person.name)
> browser = self.getUserBrowser(user=self.owner, url=url)
> self.assertIn(
> - 'There are no shared bugs, branches, or blueprints.',
> - browser.contents)
> + 'There are no shared bugs, branches, Git repositories, or '
> + 'blueprints.', normalize_whitespace(browser.contents))
>
> def test_init_works(self):
> # The view works with a feature flag.
> @@ -203,19 +216,28 @@
> 'web_link': canonical_url(branch, path_only_if_possible=True),
> 'self_link': absoluteURL(branch, request),
> }, cache.objects.get('branches')[0])
> + gitrepository = list(view.gitrepositories)[0]
> + self.assertEqual({
> + 'repository_id': gitrepository.id,
> + 'repository_name': gitrepository.unique_name,
> + 'information_type': gitrepository.information_type.title,
> + 'web_link': canonical_url(
> + gitrepository, path_only_if_possible=True),
> + 'self_link': absoluteURL(gitrepository, request),
> + }, cache.objects.get('gitrepositories')[0])
>
> def test_view_query_count(self):
> # Test that the view bulk loads artifacts.
> person = self.factory.makePerson()
> for x in range(0, 15):
> - self.makeArtifactGrantee(person, True, True, False)
> + self.makeArtifactGrantee(person, True, True, True, False)
> pillarperson = PillarPerson(self.pillar, person)
>
> # Invalidate the Storm cache and check the query count.
> IStore(self.pillar).invalidate()
> with StormStatementRecorder() as recorder:
> create_initialized_view(pillarperson, '+index')
> - self.assertThat(recorder, HasQueryCount(LessThan(13)))
> + self.assertThat(recorder, HasQueryCount(LessThan(14)))
>
>
> class TestProductSharingDetailsView(
>
> === modified file 'lib/lp/registry/enums.py'
> --- lib/lp/registry/enums.py 2014-04-28 00:18:58 +0000
> +++ lib/lp/registry/enums.py 2015-04-29 10:34:20 +0000
> @@ -1,4 +1,4 @@
> -# Copyright 2010-2013 Canonical Ltd. This software is licensed under the
> +# Copyright 2010-2015 Canonical Ltd. This software is licensed under the
> # GNU Affero General Public License version 3 (see the file LICENSE).
>
> """Enums for the Registry app."""
> @@ -56,44 +56,46 @@
> PUBLIC = DBItem(1, """
> Public
>
> - Branches are public unless they contain sensitive security
> - information.
> + Branches and Git repositories are public unless they contain
"Bazaar branches"?
> + sensitive security information.
> """)
>
> PUBLIC_OR_PROPRIETARY = DBItem(2, """
> Public, can be proprietary
>
> - New branches are public, but can be made proprietary later.
> + New branches and Git repositories are public, but can be made
> + proprietary later.
> """)
>
> PROPRIETARY_OR_PUBLIC = DBItem(3, """
> Proprietary, can be public
>
> - New branches are proprietary, but can be made public later. Only
> - people who can see the project's proprietary information can create
> - new branches.
> + New branches and Git repositories are proprietary, but can be made
> + public later. Only people who can see the project's proprietary
> + information can create new branches or Git repositories.
> """)
>
> PROPRIETARY = DBItem(4, """
> Proprietary
>
> - Branches are always proprietary. Only people who can see the
> - project's proprietary information can create new branches.
> + Branches and Git repositories are always proprietary. Only people
> + who can see the project's proprietary information can create new
> + branches or Git repositories.
> """)
>
> EMBARGOED_OR_PROPRIETARY = DBItem(5, """
> Embargoed, can be proprietary
>
> - New branches are embargoed, but can be made proprietary later. Only
> - people who can see the project's proprietary information can create
> - new branches.
> + New branches and Git repositories are embargoed, but can be made
> + proprietary later. Only people who can see the project's proprietary
> + information can create new branches or Git repositories.
> """)
>
> FORBIDDEN = DBItem(6, """
> Forbidden
>
> - No new branches may be created, but existing branches may still be
> - updated.
> + No new branches or Git repositories may be created, but existing
> + branches and Git repositories may still be updated.
> """)
>
>
>
> === modified file 'lib/lp/registry/javascript/sharing/sharingdetails.js'
> --- lib/lp/registry/javascript/sharing/sharingdetails.js 2013-04-09 05:05:39 +0000
> +++ lib/lp/registry/javascript/sharing/sharingdetails.js 2015-04-29 10:34:20 +0000
> @@ -1,4 +1,4 @@
> -/* Copyright 2012 Canonical Ltd. This software is licensed under the
> +/* Copyright 2012-2015 Canonical Ltd. This software is licensed under the
> * GNU Affero General Public License version 3 (see the file LICENSE).
> *
> * Sharing details widget
> @@ -74,6 +74,34 @@
> ].join(' ');
> },
>
> + _gitrepository_details_row_template: function() {
> + return [
> + '<tr id="shared-gitrepository-{{ repository_id }}">',
> + ' <td>',
> + ' <span class="sortkey">sorttable_branchsortkey</span>',
> + ' <a href="{{web_link}}">',
> + ' {{repository_name}}',
> + ' </a>',
> + ' </td>',
> + ' <td class="action-icons nowrap">',
> + ' <span id="remove-gitrepository-{{repository_id}}">',
> + ' <a class="sprite remove action-icon" href="#"',
> + ' title="Unshare Git repository {{repository_name}} with ' +
> + '{{displayname}}"',
> + ' data-self_link="{{self_link}}" ' +
> + 'data-name="{{repository_name}}"',
> + ' data-type="gitrepository">Remove</a>',
> + ' </span>',
> + ' </td>',
> + ' <td>',
> + ' <span class="information_type">',
> + ' {{information_type}}',
> + ' </span>',
> + ' </td>',
> + '</tr>'
> + ].join(' ');
> + },
> +
> /**
> * If the artifact with id exists in the model, return it.
> * @param artifact_id
> @@ -129,6 +157,9 @@
> '{{#branches}}',
> '{{> branch}}',
> '{{/branches}}',
> + '{{#gitrepositories}}',
> + '{{> gitrepository}}',
> + '{{/gitrepositories}}',
> '{{#specifications}}',
> '{{> spec}}',
> '{{/specifications}}',
> @@ -151,7 +182,7 @@
> },
>
> // Delete the specified grantees from the table.
> - delete_artifacts: function(bugs, branches, specifications,
> + delete_artifacts: function(bugs, branches, gitrepositories, specifications,
> all_rows_deleted) {
> var deleted_row_selectors = [];
> var details_table_body = this.get('details_table_body');
> @@ -169,6 +200,14 @@
> deleted_row_selectors.push(selector);
> }
> });
> + Y.Array.each(gitrepositories, function(gitrepository) {
> + var selector = 'tr[id=shared-gitrepository-' +
> + gitrepository.repository_id + ']';
> + var table_row = details_table_body.one(selector);
> + if (Y.Lang.isValue(table_row)) {
> + deleted_row_selectors.push(selector);
> + }
> + });
> Y.Array.each(specifications, function(spec) {
> var selector = 'tr[id=shared-spec-' + spec.id + ']';
> var table_row = details_table_body.one(selector);
> @@ -191,7 +230,9 @@
> details_table_body
> .appendChild('<tr></tr>')
> .appendChild('<td colspan="3"></td>')
> - .setContent("There are no shared bugs, branches, or blueprints.");
> + .setContent(
> + "There are no shared bugs, branches, " +
> + "Git repositories, or blueprints.");
> }
> };
> var anim_duration = this.get('anim_duration');
> @@ -228,6 +269,10 @@
> this._branch_details_row_template());
>
> this.set(
> + 'gitrepository_details_row_template',
> + this._gitrepository_details_row_template());
> +
> + this.set(
> 'spec_details_row_template',
> this._spec_details_row_template());
>
> @@ -240,15 +285,17 @@
> renderUI: function() {
> // Load the data
> var branches = this.get('branches');
> + var gitrepositories = this.get('gitrepositories');
> var bugs = this.get('bugs');
> var specs = this.get('specifications');
>
> if (bugs.length === 0 && branches.length === 0 &&
> - specs.length === 0 ) {
> + gitrepositories.length === 0 && specs.length === 0 ) {
> return;
> }
> var partials = {
> branch: this.get('branch_details_row_template'),
> + gitrepository: this.get('gitrepository_details_row_template'),
> bug: this.get('bug_details_row_template'),
> spec: this.get('spec_details_row_template')
> };
> @@ -257,11 +304,13 @@
> template,
> {
> branches: branches,
> + gitrepositories: gitrepositories,
> bugs: bugs,
> specifications: specs,
> displayname: this.get('person_name')
> },
> partials);
> + Y.log("cjw html: " + html);
Can probably be dropped now.
>
> var details_table_body = this.get('details_table_body');
> var table_body_node = Y.Node.create(html);
> @@ -294,12 +343,15 @@
> // been removed.
> var existing_bugs = this.get('bugs');
> var existing_branches = this.get('branches');
> + var existing_gitrepositories = this.get('gitrepositories');
> var existing_specifications = this.get('specifications');
> var model_bugs = LP.cache.bugs;
> var model_branches = LP.cache.branches;
> + var model_gitrepositories = LP.cache.gitrepositories;
> var model_specifications = LP.cache.specifications;
> var deleted_bugs = [];
> var deleted_branches = [];
> + var deleted_gitrepositories = [];
> var deleted_specifications = [];
>
> var self = this;
> @@ -319,6 +371,15 @@
> deleted_branches.push(branch);
> }
> });
> + Y.Array.each(existing_gitrepositories, function(gitrepository) {
> + var model_gitrepository =
> + self._get_artifact_from_model(
> + gitrepository.repository_id, 'repository_id',
> + model_gitrepositories);
> + if (!Y.Lang.isValue(model_gitrepository)) {
> + deleted_gitrepositories.push(gitrepository);
> + }
> + });
>
> Y.Array.each(existing_specifications, function(spec) {
> var model_specification = self._get_artifact_from_model(
> @@ -329,14 +390,19 @@
> });
>
> if (deleted_bugs.length > 0 || deleted_branches.length > 0 ||
> + deleted_gitrepositories.length > 0 ||
> deleted_specifications.length > 0) {
> this.delete_artifacts(
> - deleted_bugs, deleted_branches, deleted_specifications,
> - model_bugs.length === 0 && model_branches.length === 0 && deleted_specifications.length === 0);
> + deleted_bugs, deleted_branches, deleted_gitrepositories,
> + deleted_specifications,
> + model_bugs.length === 0 && model_branches.length === 0 &&
> + model_gitrepositories.length === 0 &&
> + deleted_specifications.length === 0);
> }
>
> this.set('bugs', model_bugs);
> this.set('branches', model_branches);
> + this.set('gitrepositories', model_gitrepositories);
> this.set('specifications', model_specifications);
>
> Y.lp.app.sorttable.SortTable.registerSortKeyFunction(
> @@ -360,6 +426,10 @@
> value: null
> },
>
> + gitrepository_details_row_template: {
> + value: null
> + },
> +
> spec_details_row_template: {
> value: null
> },
> @@ -371,6 +441,13 @@
> setter: clone_data
> },
>
> + gitrepositories: {
> + value: [],
> + // We clone the data passed in so external modifications do not
> + // interfere.
> + setter: clone_data
> + },
> +
> bugs: {
> value: [],
> // We clone the data passed in so external modifications do not
>
> === modified file 'lib/lp/registry/javascript/sharing/sharingdetailsview.js'
> --- lib/lp/registry/javascript/sharing/sharingdetailsview.js 2012-12-02 03:01:23 +0000
> +++ lib/lp/registry/javascript/sharing/sharingdetailsview.js 2015-04-29 10:34:20 +0000
> @@ -1,4 +1,4 @@
> -/* Copyright 2012 Canonical Ltd. This software is licensed under the
> +/* Copyright 2012-2015 Canonical Ltd. This software is licensed under the
> * GNU Affero General Public License version 3 (see the file LICENSE).
> *
> * Disclosure infrastructure.
> @@ -31,6 +31,7 @@
> var details_table = new ns.SharingDetailsTable({
> bugs: LP.cache.bugs,
> branches: LP.cache.branches,
> + gitrepositories: LP.cache.gitrepositories,
> person_name: LP.cache.grantee.displayname,
> specifications: LP.cache.specifications,
> write_enabled: true
> @@ -75,6 +76,15 @@
> }
> });
> break;
> + case 'gitrepository':
> + Y.Array.some(LP.cache.gitrepositories,
> + function(gitrepository) {
> + if (gitrepository.self_link === artifact_uri) {
> + artifact_id = gitrepository.repository_id;
> + return true;
> + }
> + });
> + break;
> case 'spec':
> Y.Array.some(LP.cache.specifications, function(spec) {
> if (spec.self_link === artifact_uri) {
> @@ -181,6 +191,14 @@
> return true;
> }
> });
> + var gitrepository_data = LP.cache.gitrepositories;
> + Y.Array.some(gitrepository_data, function(gitrepository, index) {
> + if (gitrepository.self_link === artifact_uri) {
> + gitrepository_data.splice(index, 1);
> + self.syncUI();
> + return true;
> + }
> + });
> var spec_data = LP.cache.specifications;
> Y.Array.some(spec_data, function(spec, index) {
> if (spec.self_link === artifact_uri) {
> @@ -203,6 +221,7 @@
> var error_handler = this._error_handler(artifact_uri, artifact_type);
> var bugs = [];
> var branches = [];
> + var gitrepositories = [];
> var specifications = [];
> switch (artifact_type) {
> case 'bug':
> @@ -211,6 +230,9 @@
> case 'branch':
> branches = [artifact_uri];
> break;
> + case 'gitrepository':
> + gitrepositories = [artifact_uri];
> + break;
> case 'spec':
> specifications = [artifact_uri];
> break;
> @@ -233,6 +255,7 @@
> grantee: LP.cache.grantee.self_link,
> bugs: bugs,
> branches: branches,
> + gitrepositories: gitrepositories,
> specifications: specifications
> }
> };
>
> === modified file 'lib/lp/registry/javascript/sharing/tests/test_sharingdetails.js'
> --- lib/lp/registry/javascript/sharing/tests/test_sharingdetails.js 2013-03-20 03:41:40 +0000
> +++ lib/lp/registry/javascript/sharing/tests/test_sharingdetails.js 2015-04-29 10:34:20 +0000
> @@ -1,4 +1,4 @@
> -/* Copyright 2012 Canonical Ltd. This software is licensed under the
> +/* Copyright 2012-2015 Canonical Ltd. This software is licensed under the
> * GNU Affero General Public License version 3 (see the file LICENSE). */
> YUI.add('lp.registry.sharing.sharingdetails.test', function(Y) {
>
> @@ -35,6 +35,15 @@
> branch_name:'lp:~someone/+junk/somebranch',
> information_type: 'Private'
> }
> + ],
> + gitrepositories: [
> + {
> + self_link: 'api/devel/~someone/+git/somerepo',
> + web_link: '/~someone/+git/somerepo',
> + repository_id: '2',
> + repository_name: '~someone/+git/somerepo',
> + information_type: 'Private'
> + }
> ]
> }
> };
> @@ -61,6 +70,7 @@
> person_name: 'Fred',
> bugs: window.LP.cache.bugs,
> branches: window.LP.cache.branches,
> + gitrepositories: window.LP.cache.gitrepositories,
> write_enabled: true
> }, overrides);
> window.LP.cache.grantee_data = config.grantees;
> @@ -124,6 +134,24 @@
> "sorttable_branchsortkey", this._strip(sortkey));
> },
>
> + // Test that Git repositories are correctly rendered.
> + test_render_gitrepositories: function () {
> + this.details_widget = this._create_Widget();
> + this.details_widget.render();
> + var row = Y.one(
> + '#sharing-table-body tr#shared-gitrepository-2');
> + var web_link = row.one('a');
> + var expected = "~someone/+git/somerepo";
> + var actual_text = web_link.get('text');
> + Assert.areEqual(expected, this._strip(actual_text));
> + var info_type = row.one('.information_type');
> + var info_text = info_type.get('text');
> + Assert.areEqual('Private', this._strip(info_text));
> + var sortkey = row.one('.sortkey').get('text');
> + Assert.areEqual(
> + "sorttable_branchsortkey", this._strip(sortkey));
> + },
> +
> // Test that bugs are correctly rendered.
> test_render_bugs: function () {
> this.details_widget = this._create_Widget();
> @@ -189,6 +217,9 @@
> var branch_row_selector =
> '#sharing-table-body tr[id=shared-branch-2]';
> Y.Assert.isNotNull(Y.one(branch_row_selector));
> + var gitrepository_row_selector =
> + '#sharing-table-body tr[id=shared-gitrepository-2]';
> + Y.Assert.isNotNull(Y.one(gitrepository_row_selector));
> // The sorting data is initialised.
> Y.Assert.isTrue(sorttable_init_called);
> sorttable_ns.SortTable.init = orig_init;
> @@ -223,6 +254,34 @@
> Y.Assert.isTrue(event_fired);
> },
>
> + // When the Git repository revoke link is clicked, the correct event
> + // is published.
> + test_gitrepository_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];
> + var artifact_type = e.details[3];
> + Y.Assert.areEqual(
> + 'api/devel/~someone/+git/somerepo',
> + artifact_uri);
> + Y.Assert.areEqual('~someone/+git/somerepo', artifact_name);
> + Y.Assert.areEqual('gitrepository', artifact_type);
> + 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-gitrepository-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();
> @@ -230,7 +289,7 @@
> 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);
> + [window.LP.cache.bugs[0]], [], [], false);
> Y.Assert.isNull(Y.one(row_selector));
> },
>
> @@ -242,7 +301,20 @@
> 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);
> + [], [window.LP.cache.branches[0]], [], false);
> + Y.Assert.isNull(Y.one(row_selector));
> + },
> +
> + // The delete_artifacts call removes the specified Git repositories
> + // from the table.
> + test_delete_gitrepositories: function() {
> + this.details_widget = this._create_Widget();
> + this.details_widget.render();
> + var row_selector =
> + '#sharing-table-body tr[id=shared-gitrepository-2]';
> + Y.Assert.isNotNull(Y.one(row_selector));
> + this.details_widget.delete_artifacts(
> + [], [], [window.LP.cache.gitrepositories[0]], false);
> Y.Assert.isNull(Y.one(row_selector));
> },
>
> @@ -252,9 +324,12 @@
> this.details_widget.render();
> this.details_widget.delete_artifacts(
> [window.LP.cache.bugs[0]],
> - [window.LP.cache.branches[0]], true);
> + [window.LP.cache.branches[0]],
> + [window.LP.cache.gitrepositories[0]],
> + true);
> Y.Assert.areEqual(
> - 'There are no shared bugs, branches, or blueprints.',
> + 'There are no shared bugs, branches, Git repositories, or ' +
> + 'blueprints.',
> Y.one('#sharing-table-body tr').get('text'));
> },
>
> @@ -286,6 +361,23 @@
> };
> this.details_widget.display_error(2, 'branch', 'An error occurred');
> Y.Assert.isTrue(success);
> + },
> +
> + test_display_error_gitrepository: function() {
> + // A Git repository operation error is displayed correctly.
> + this.details_widget = this._create_Widget();
> + this.details_widget.render();
> + var row_gitrepository = Y.one(
> + '#sharing-table-body tr[id=shared-gitrepository-2]');
> + var success = false;
> + Y.lp.app.errors.display_error = function(flash_node, msg) {
> + Y.Assert.areEqual(row_gitrepository, flash_node);
> + Y.Assert.areEqual('An error occurred', msg);
> + success = true;
> + };
> + this.details_widget.display_error(
> + 2, 'gitrepository', 'An error occurred');
> + Y.Assert.isTrue(success);
> }
> }));
>
>
> === modified file 'lib/lp/registry/javascript/sharing/tests/test_sharingdetailsview.js'
> --- lib/lp/registry/javascript/sharing/tests/test_sharingdetailsview.js 2013-03-20 03:41:40 +0000
> +++ lib/lp/registry/javascript/sharing/tests/test_sharingdetailsview.js 2015-04-29 10:34:20 +0000
> @@ -1,4 +1,4 @@
> -/* Copyright 2012 Canonical Ltd. This software is licensed under the
> +/* Copyright 2012-2015 Canonical Ltd. This software is licensed under the
> * GNU Affero General Public License version 3 (see the file LICENSE). */
>
> YUI.add('lp.registry.sharing.sharingdetailsview.test', function (Y) {
> @@ -31,12 +31,21 @@
> branch_name:'lp:~someone/+junk/somebranch'
> }
> ],
> + gitrepositories: [
> + {
> + self_link: 'api/devel/~someone/+git/somerepo',
> + web_link: '/~someone/+git/somerepo',
> + repository_id: '2',
> + repository_name: '~someone/+git/somerepo'
> + }
> + ],
> specifications: [
> {
> id: 2,
> information_type: "Proprietary",
> name: "big-project",
> - self_link: "api/devel/obsolete-junk/+spec/big-project",
> + self_link:
> + "api/devel/obsolete-junk/+spec/big-project",
> web_link: "/obsolete-junk/+spec/big-project"
> }
> ],
> @@ -109,7 +118,7 @@
> Y.Assert.isTrue(confirmRemove_called);
> },
>
> - // Clicking a bug remove link calls the confirm_grant_removal
> + // Clicking a branch remove link calls the confirm_grant_removal
> // method with the correct parameters.
> test_remove_branch_grant_click: function() {
> this.view = this._create_Widget();
> @@ -132,6 +141,28 @@
> Y.Assert.isTrue(confirmRemove_called);
> },
>
> + // Clicking a Git repository remove link calls the
> + // confirm_grant_removal method with the correct parameters.
> + test_remove_gitrepository_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, artifact_type) {
> + Y.Assert.areEqual(
> + 'api/devel/~someone/+git/somerepo', artifact_uri);
> + Y.Assert.areEqual('~someone/+git/somerepo', artifact_name);
> + Y.Assert.areEqual('gitrepository', artifact_type);
> + 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-gitrepository-2] a');
> + delete_link_to_click.simulate('click');
> + Y.Assert.isTrue(confirmRemove_called);
> + },
> +
> // Clicking a spec remove link calls the confirm_grant_removal
> // method with the correct parameters.
> test_remove_spec_grant_click: function() {
> @@ -273,6 +304,47 @@
> Y.Assert.isTrue(remove_grant_success_called);
> },
>
> + // The perform_remove_grant method makes the expected XHR calls when a
> + // Git repository grant remove link is clicked.
> + test_perform_remove_gitrepository_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/~someone/+git/somerepo', artifact_uri);
> + remove_grant_success_called = true;
> + };
> + var delete_link =
> + Y.one('#sharing-table-body span[id=remove-gitrepository-2] a');
> + this.view.perform_remove_grant(
> + delete_link, 'api/devel/~someone/+git/somerepo',
> + 'gitrepository');
> + 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, 'grantee', '~fred');
> + expected_qs = Y.lp.client.append_qs(
> + expected_qs, 'gitrepositories',
> + 'api/devel/~someone/+git/somerepo');
> + 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});
> @@ -306,6 +378,23 @@
> });
> },
>
> + // The remove Git repository grant callback updates the model and
> + // syncs the UI.
> + test_remove_gitrepository_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/+git/somerepo');
> + Y.Assert.isTrue(syncUI_called);
> + Y.Array.each(window.LP.cache.gitrepositories,
> + function(gitrepository) {
> + Y.Assert.areNotEqual(2, gitrepository.repository_id);
> + });
> + },
> +
> // The remove specification grant callback updates the model and syncs
> // the UI.
> test_remove_spec_grant_success: function() {
> @@ -326,7 +415,7 @@
>
> // XHR calls display errors correctly.
> _assert_error_displayed_on_failure: function(
> - bug_or_branch, invoke_operation) {
> + artifact, invoke_operation) {
> var mockio = new Y.lp.testing.mockio.MockIo();
> var lp_client = new Y.lp.client.Launchpad({
> io_provider: mockio
> @@ -340,7 +429,7 @@
> grantee_table.display_error = function(
> artifact_id, artifact_type, error_msg) {
> Y.Assert.areEqual(2, artifact_id);
> - Y.Assert.areEqual(bug_or_branch, artifact_type);
> + Y.Assert.areEqual(artifact, artifact_type);
> Y.Assert.areEqual(
> 'Server error, please contact an administrator.',
> error_msg);
> @@ -378,6 +467,20 @@
> this._assert_error_displayed_on_failure('branch', invoke_remove);
> },
>
> + // The perform_remove_grant method handles errors correctly with Git
> + // repositories.
> + test_perform_remove_gitrepository_error: function() {
> + var invoke_remove = function(view) {
> + var delete_link =
> + Y.one('#sharing-table-body span[id=remove-gitrepository-2] a');
> + view.perform_remove_grant(
> + delete_link, 'api/devel/~someone/+git/somerepo',
> + 'gitrepository');
> + };
> + this._assert_error_displayed_on_failure(
> + 'gitrepository', invoke_remove);
> + },
> +
> // The perform_remove_grant method handles errors correctly with
> // specifications.
> test_perform_remove_spec_error: function() {
>
> === modified file 'lib/lp/registry/templates/pillar-sharing-details.pt'
> --- lib/lp/registry/templates/pillar-sharing-details.pt 2012-12-03 12:49:35 +0000
> +++ lib/lp/registry/templates/pillar-sharing-details.pt 2015-04-29 10:34:20 +0000
> @@ -29,14 +29,15 @@
> <p>
> <tal:bugs replace="view/shared_bugs_count">0</tal:bugs> bugs,
> <tal:branches replace="view/shared_branches_count">0</tal:branches> branches,
> + <tal:gitrepositories replace="view/shared_gitrepositories_count">0</tal:gitrepositories> Git repositories,
> and <tal:specifications
> replace="view/shared_specifications_count">0</tal:specifications>
> blueprints shared with <tal:name replace="view/person/displayname">
> grantee</tal:name>.<br />
>
> <tal:is-team condition="view/person/is_team">
> - <tal:members>3</tal:members> team members can view these bugs and
> - branches.
> + <tal:members>3</tal:members> team members can view these bugs,
> + branches, Git repositories, and blueprints.
> </tal:is-team>
> </p>
> </div>
> @@ -48,7 +49,7 @@
> <thead>
> <tr>
> <th colspan="2" width="">
> - Subscribed Bug Report, Branch, or Blueprint
> + Subscribed Bug Report, Branch, Git Repository, or Blueprint
> </th>
> <th>
> Information Type
> @@ -58,7 +59,8 @@
> <tbody id="sharing-table-body">
> <tr>
> <td colspan="3">
> - There are no shared bugs, branches, or blueprints.
> + There are no shared bugs, branches, Git repositories, or
> + blueprints.
> </td>
> </tr>
> </tbody>
>
> === modified file 'lib/lp/registry/templates/pillar-sharing.pt'
> --- lib/lp/registry/templates/pillar-sharing.pt 2012-12-03 18:42:09 +0000
> +++ lib/lp/registry/templates/pillar-sharing.pt 2015-04-29 10:34:20 +0000
> @@ -102,7 +102,8 @@
> <p tal:condition="python: info_type == 'Public'"
> tal:content="string:
> Everyone can see ${context/displayname}'s public information. You can
> - choose who can see the private bugs, branches and blueprints.">
> + choose who can see the private bugs, branches, Git repositories, and
> + blueprints.">
> </p>
> <p tal:condition="python: info_type != 'Public'"
> tal:content="string: This project has no public information.">
>
--
https://code.launchpad.net/~cjwatson/launchpad/git-sharing-ui/+merge/257735
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
References
