launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18703
[Merge] lp:~cjwatson/launchpad/git-repository-ui-edit-basics into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/git-repository-ui-edit-basics into lp:launchpad.
Commit message:
Add some basic edit views (name, information_type, and reviewer) to GitRepository.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-repository-ui-edit-basics/+merge/261117
Add some basic edit views (name, information_type, and reviewer) to GitRepository, in line with Branch. Changing the owner, target, and the various default fields is more complicated, and will come in subsequent branches.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-repository-ui-edit-basics into lp:launchpad.
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2015-06-02 08:57:02 +0000
+++ lib/lp/code/browser/configure.zcml 2015-06-04 17:01:40 +0000
@@ -762,6 +762,9 @@
name="+index"
template="../templates/gitrepository-index.pt"/>
<browser:page
+ name="+portlet-privacy"
+ template="../templates/gitrepository-portlet-privacy.pt"/>
+ <browser:page
name="++repository-information"
template="../templates/gitrepository-information.pt"/>
<browser:page
@@ -781,6 +784,24 @@
template="../templates/gitrepository-portlet-subscribers-content.pt"/>
<browser:page
for="lp.code.interfaces.gitrepository.IGitRepository"
+ class="lp.code.browser.gitrepository.GitRepositoryEditInformationTypeView"
+ permission="launchpad.Edit"
+ name="+edit-information-type"
+ template="../../app/templates/generic-edit.pt"/>
+ <browser:page
+ for="lp.code.interfaces.gitrepository.IGitRepository"
+ class="lp.code.browser.gitrepository.GitRepositoryEditReviewerView"
+ permission="launchpad.Edit"
+ name="+reviewer"
+ template="../../app/templates/generic-edit.pt"/>
+ <browser:page
+ for="lp.code.interfaces.gitrepository.IGitRepository"
+ class="lp.code.browser.gitrepository.GitRepositoryEditView"
+ permission="launchpad.Edit"
+ name="+edit"
+ template="../../app/templates/generic-edit.pt"/>
+ <browser:page
+ for="lp.code.interfaces.gitrepository.IGitRepository"
class="lp.code.browser.gitrepository.GitRepositoryDeletionView"
permission="launchpad.Edit"
name="+delete"
=== modified file 'lib/lp/code/browser/gitrepository.py'
--- lib/lp/code/browser/gitrepository.py 2015-05-26 12:18:12 +0000
+++ lib/lp/code/browser/gitrepository.py 2015-06-04 17:01:40 +0000
@@ -10,24 +10,41 @@
'GitRepositoryBreadcrumb',
'GitRepositoryContextMenu',
'GitRepositoryDeletionView',
+ 'GitRepositoryEditInformationTypeView',
'GitRepositoryEditMenu',
+ 'GitRepositoryEditReviewerView',
+ 'GitRepositoryEditView',
'GitRepositoryNavigation',
'GitRepositoryURL',
'GitRepositoryView',
]
+from lazr.lifecycle.event import ObjectModifiedEvent
+from lazr.lifecycle.snapshot import Snapshot
+from lazr.restful.interface import copy_field
from storm.expr import Desc
-from zope.interface import implements
+from zope.event import notify
+from zope.interface import (
+ implements,
+ Interface,
+ providedBy,
+ )
from lp.app.browser.informationtype import InformationTypePortletMixin
from lp.app.browser.launchpadform import (
action,
+ custom_widget,
+ LaunchpadEditFormView,
LaunchpadFormView,
)
from lp.app.errors import NotFoundError
+from lp.app.vocabularies import InformationTypeVocabulary
+from lp.app.widgets.itemswidgets import LaunchpadRadioWidgetWithDescription
+from lp.code.errors import GitRepositoryExists
from lp.code.interfaces.gitref import IGitRefBatchNavigator
from lp.code.interfaces.gitrepository import IGitRepository
from lp.services.config import config
+from lp.services.database.constants import UTC_NOW
from lp.services.propertycache import cachedproperty
from lp.services.webapp import (
canonical_url,
@@ -46,6 +63,7 @@
)
from lp.services.webapp.batching import TableBatchNavigator
from lp.services.webapp.breadcrumb import NameBreadcrumb
+from lp.services.webapp.escaping import structured
from lp.services.webapp.interfaces import ICanonicalUrlData
@@ -106,7 +124,17 @@
usedfor = IGitRepository
facet = "branches"
title = "Edit Git repository"
- links = ["delete"]
+ links = ["edit", "reviewer", "delete"]
+
+ @enabled_with_permission("launchpad.Edit")
+ def edit(self):
+ text = "Change repository details"
+ return Link("+edit", text, icon="edit")
+
+ @enabled_with_permission("launchpad.Edit")
+ def reviewer(self):
+ text = "Set repository reviewer"
+ return Link("+reviewer", text, icon="edit")
@enabled_with_permission("launchpad.Edit")
def delete(self):
@@ -119,7 +147,7 @@
usedfor = IGitRepository
facet = "branches"
- links = ["add_subscriber", "source", "subscription"]
+ links = ["add_subscriber", "source", "subscription", "visibility"]
@enabled_with_permission("launchpad.AnyPerson")
def subscription(self):
@@ -144,6 +172,12 @@
url = self.context.getCodebrowseUrl()
return Link(url, text, icon="info")
+ @enabled_with_permission("launchpad.Edit")
+ def visibility(self):
+ """Return the "Change information type" Link."""
+ text = "Change information type"
+ return Link("+edit-information-type", text)
+
class GitRefBatchNavigator(TableBatchNavigator):
"""Batch up the branch listings."""
@@ -201,6 +235,160 @@
return GitRefBatchNavigator(self, self.context)
+class GitRepositoryEditFormView(LaunchpadEditFormView):
+ """Base class for forms that edit a Git repository."""
+
+ field_names = None
+
+ def getInformationTypesToShow(self):
+ """Get the information types to display on the edit form.
+
+ We display a customised set of information types: anything allowed
+ by the repository's model, plus the current type.
+ """
+ allowed_types = set(self.context.getAllowedInformationTypes(self.user))
+ allowed_types.add(self.context.information_type)
+ return allowed_types
+
+ @cachedproperty
+ def schema(self):
+ info_types = self.getInformationTypesToShow()
+
+ class GitRepositoryEditSchema(Interface):
+ """Defines the fields for the edit form.
+
+ This is necessary to make various fields editable that are not
+ normally editable through the interface.
+ """
+ information_type = copy_field(
+ IGitRepository["information_type"], readonly=False,
+ vocabulary=InformationTypeVocabulary(types=info_types))
+ name = copy_field(IGitRepository["name"], readonly=False)
+ reviewer = copy_field(IGitRepository["reviewer"], required=True)
+
+ return GitRepositoryEditSchema
+
+ @property
+ def page_title(self):
+ return "Edit %s" % self.context.display_name
+
+ @property
+ def label(self):
+ return self.page_title
+
+ @property
+ def adapters(self):
+ """See `LaunchpadFormView`."""
+ return {self.schema: self.context}
+
+ @action("Change Git Repository", name="change",
+ failure=LaunchpadFormView.ajax_failure_handler)
+ def change_action(self, action, data):
+ # If the owner has changed, add an explicit notification. We take
+ # our own snapshot here to make sure that the snapshot records
+ # changes to the owner, and we notify the listeners explicitly below
+ # rather than the notification that would normally be sent in
+ # updateContextFromData.
+ changed = False
+ repository_before_modification = Snapshot(
+ self.context, providing=providedBy(self.context))
+ if "name" in data:
+ name = data.pop("name")
+ if name != self.context.name:
+ self.context.setName(name, self.user)
+ changed = True
+ if "information_type" in data:
+ information_type = data.pop("information_type")
+ self.context.transitionToInformationType(
+ information_type, self.user)
+ if "reviewer" in data:
+ reviewer = data.pop("reviewer")
+ if reviewer != self.context.code_reviewer:
+ if reviewer == self.context.owner:
+ # Clear the reviewer if set to the same as the owner.
+ self.context.reviewer = None
+ else:
+ self.context.reviewer = reviewer
+ changed = True
+
+ if self.updateContextFromData(data, notify_modified=False):
+ changed = True
+
+ if changed:
+ # Notify that the object has changed with the snapshot that was
+ # taken earlier.
+ field_names = [
+ form_field.__name__ for form_field in self.form_fields]
+ notify(ObjectModifiedEvent(
+ self.context, repository_before_modification, field_names))
+ # Only specify that the context was modified if there
+ # was in fact a change.
+ self.context.date_last_modified = UTC_NOW
+
+ if self.request.is_ajax:
+ return ""
+
+ @property
+ def next_url(self):
+ """Return the next URL to call when this call completes."""
+ if not self.request.is_ajax and not self.errors:
+ return self.cancel_url
+ return None
+
+ @property
+ def cancel_url(self):
+ return canonical_url(self.context)
+
+
+class GitRepositoryEditInformationTypeView(GitRepositoryEditFormView):
+ """A view to set the information type."""
+
+ field_names = ["information_type"]
+
+
+class GitRepositoryEditReviewerView(GitRepositoryEditFormView):
+ """A view to set the review team."""
+
+ field_names = ["reviewer"]
+
+ @property
+ def initial_values(self):
+ return {"reviewer": self.context.code_reviewer}
+
+
+class GitRepositoryEditView(GitRepositoryEditFormView):
+ """The main view for editing repository attributes."""
+
+ field_names = [
+ "name",
+ "information_type",
+ ]
+
+ custom_widget("information_type", LaunchpadRadioWidgetWithDescription)
+
+ def _setRepositoryExists(self, existing_repository, field_name="name"):
+ owner = existing_repository.owner
+ if owner == self.user:
+ prefix = "You already have"
+ else:
+ prefix = "%s already has" % owner.displayname
+ message = structured(
+ "%s a repository for <em>%s</em> called <em>%s</em>.",
+ prefix, existing_repository.target.displayname,
+ existing_repository.name)
+ self.setFieldError(field_name, message)
+
+ def validate(self, data):
+ if "name" in data:
+ name = data["name"]
+ if name != self.context.name:
+ namespace = self.context.namespace
+ try:
+ namespace.validateMove(self.context, self.user, name=name)
+ except GitRepositoryExists as e:
+ self._setRepositoryExists(e.existing_repository)
+
+
class GitRepositoryDeletionView(LaunchpadFormView):
schema = IGitRepository
=== modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
--- lib/lp/code/browser/tests/test_gitrepository.py 2015-05-19 11:31:59 +0000
+++ lib/lp/code/browser/tests/test_gitrepository.py 2015-06-04 17:01:40 +0000
@@ -20,10 +20,14 @@
from zope.security.proxy import removeSecurityProxy
from lp.app.enums import InformationType
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.app.interfaces.services import IService
from lp.code.interfaces.revision import IRevisionSet
+from lp.registry.enums import BranchSharingPolicy
from lp.registry.interfaces.person import PersonVisibility
+from lp.services.database.constants import UTC_NOW
from lp.services.webapp.publisher import canonical_url
+from lp.services.webapp.servers import LaunchpadTestRequest
from lp.testing import (
admin_logged_in,
BrowserTestCase,
@@ -41,7 +45,10 @@
setupBrowserForUser,
)
from lp.testing.publication import test_traverse
-from lp.testing.views import create_initialized_view
+from lp.testing.views import (
+ create_initialized_view,
+ create_view,
+ )
class TestGitRepositoryNavigation(TestCaseWithFactory):
@@ -186,6 +193,185 @@
self.assertThat(recorder2, HasQueryCount(Equals(recorder1.count)))
+class TestGitRepositoryEditReviewerView(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_initial_reviewer_not_set(self):
+ # If the reviewer is not set, the field is populated with the owner
+ # of the repository.
+ repository = self.factory.makeGitRepository()
+ self.assertIsNone(repository.reviewer)
+ view = create_view(repository, "+reviewer")
+ self.assertEqual(repository.owner, view.initial_values["reviewer"])
+
+ def test_initial_reviewer_set(self):
+ # If the reviewer has been set, it is shown as the initial value.
+ repository = self.factory.makeGitRepository()
+ login_person(repository.owner)
+ repository.reviewer = self.factory.makePerson()
+ view = create_view(repository, "+reviewer")
+ self.assertEqual(repository.reviewer, view.initial_values["reviewer"])
+
+ def test_set_reviewer(self):
+ # Test setting the reviewer.
+ repository = self.factory.makeGitRepository()
+ reviewer = self.factory.makePerson()
+ login_person(repository.owner)
+ view = create_initialized_view(repository, "+reviewer")
+ view.change_action.success({"reviewer": reviewer})
+ self.assertEqual(reviewer, repository.reviewer)
+ # Last modified has been updated.
+ self.assertSqlAttributeEqualsDate(
+ repository, "date_last_modified", UTC_NOW)
+
+ def test_set_reviewer_as_owner_clears_reviewer(self):
+ # If the reviewer is set to be the repository owner, the review
+ # field is cleared in the database.
+ repository = self.factory.makeGitRepository()
+ login_person(repository.owner)
+ repository.reviewer = self.factory.makePerson()
+ view = create_initialized_view(repository, "+reviewer")
+ view.change_action.success({"reviewer": repository.owner})
+ self.assertIsNone(repository.reviewer)
+ # Last modified has been updated.
+ self.assertSqlAttributeEqualsDate(
+ repository, "date_last_modified", UTC_NOW)
+
+ def test_set_reviewer_to_same_does_not_update_last_modified(self):
+ # If the user has set the reviewer to be same and clicked on save,
+ # then the underlying object hasn't really been changed, so the last
+ # modified is not updated.
+ modified_date = datetime(2007, 1, 1, tzinfo=pytz.UTC)
+ repository = self.factory.makeGitRepository(date_created=modified_date)
+ view = create_initialized_view(repository, "+reviewer")
+ view.change_action.success({"reviewer": repository.owner})
+ self.assertIsNone(repository.reviewer)
+ # Last modified has not been updated.
+ self.assertEqual(modified_date, repository.date_last_modified)
+
+
+class TestGitRepositoryEditView(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_rename(self):
+ # The name of a repository can be changed via the UI by an
+ # authorised user.
+ person = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=person, name=u"foo")
+ browser = self.getUserBrowser(
+ canonical_url(repository) + "/+edit", user=person)
+ browser.getControl(name="field.name").value = u"bar"
+ browser.getControl("Change Git Repository").click()
+ with person_logged_in(person):
+ self.assertEqual(u"bar", repository.name)
+
+ def test_information_type_in_ui(self):
+ # The information_type of a repository can be changed via the UI by
+ # an authorised user.
+ person = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=person)
+ admin = getUtility(ILaunchpadCelebrities).admin.teamowner
+ browser = self.getUserBrowser(
+ canonical_url(repository) + "/+edit", user=admin)
+ browser.getControl("Private", index=1).click()
+ browser.getControl("Change Git Repository").click()
+ with person_logged_in(person):
+ self.assertEqual(
+ InformationType.USERDATA, repository.information_type)
+
+ def test_edit_view_ajax_render(self):
+ # An information type change request is processed as expected when
+ # an XHR request is made to the view.
+ person = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=person)
+
+ extra = {"HTTP_X_REQUESTED_WITH": "XMLHttpRequest"}
+ request = LaunchpadTestRequest(
+ method="POST", form={
+ "field.actions.change": "Change Git Repository",
+ "field.information_type": "PUBLICSECURITY"},
+ **extra)
+ with person_logged_in(person):
+ view = create_initialized_view(
+ repository, name="+edit-information-type",
+ request=request, principal=person)
+ request.traversed_objects = [
+ person, repository.target, repository, view]
+ result = view.render()
+ self.assertEqual("", result)
+ self.assertEqual(
+ repository.information_type, InformationType.PUBLICSECURITY)
+
+
+class TestGitRepositoryEditViewInformationTypes(TestCaseWithFactory):
+ """Tests for GitRepositoryEditView.getInformationTypesToShow."""
+
+ layer = DatabaseFunctionalLayer
+
+ def assertShownTypes(self, types, repository, user=None):
+ if user is None:
+ user = removeSecurityProxy(repository).owner
+ with person_logged_in(user):
+ view = create_initialized_view(repository, "+edit", user=user)
+ self.assertContentEqual(types, view.getInformationTypesToShow())
+
+ def test_public_repository(self):
+ # A normal public repository on a public project can be any
+ # information type except embargoed and proprietary.
+ # The model doesn't enforce this, so it's just a UI thing.
+ repository = self.factory.makeGitRepository(
+ information_type=InformationType.PUBLIC)
+ self.assertShownTypes(
+ [InformationType.PUBLIC, InformationType.PUBLICSECURITY,
+ InformationType.PRIVATESECURITY, InformationType.USERDATA],
+ repository)
+
+ def test_repository_with_disallowed_type(self):
+ # We don't force repositories with a disallowed type (e.g.
+ # Proprietary on a non-commercial project) to change, so the current
+ # type is shown.
+ project = self.factory.makeProduct()
+ self.factory.makeAccessPolicy(pillar=project)
+ repository = self.factory.makeGitRepository(
+ target=project, information_type=InformationType.PROPRIETARY)
+ self.assertShownTypes(
+ [InformationType.PUBLIC, InformationType.PUBLICSECURITY,
+ InformationType.PRIVATESECURITY, InformationType.USERDATA,
+ InformationType.PROPRIETARY], repository)
+
+ def test_repository_for_project_with_embargoed_and_proprietary(self):
+ # Repositories for commercial projects which have a policy of
+ # embargoed or proprietary allow only embargoed and proprietary
+ # types.
+ owner = self.factory.makePerson()
+ project = self.factory.makeProduct(owner=owner)
+ self.factory.makeCommercialSubscription(product=project)
+ with person_logged_in(owner):
+ project.setBranchSharingPolicy(
+ BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY)
+ repository = self.factory.makeGitRepository(
+ owner=owner, target=project,
+ information_type=InformationType.PROPRIETARY)
+ self.assertShownTypes(
+ [InformationType.EMBARGOED, InformationType.PROPRIETARY],
+ repository)
+
+ def test_repository_for_project_with_proprietary(self):
+ # Repositories for commercial projects which have a policy of
+ # proprietary allow only the proprietary type.
+ owner = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=owner)
+ self.factory.makeCommercialSubscription(product=product)
+ with person_logged_in(owner):
+ product.setBranchSharingPolicy(BranchSharingPolicy.PROPRIETARY)
+ repository = self.factory.makeGitRepository(
+ owner=owner, target=product,
+ information_type=InformationType.PROPRIETARY)
+ self.assertShownTypes([InformationType.PROPRIETARY], repository)
+
+
class TestGitRepositoryDeletionView(BrowserTestCase):
layer = DatabaseFunctionalLayer
=== modified file 'lib/lp/code/javascript/tests/test_information_type_choice.js'
--- lib/lp/code/javascript/tests/test_information_type_choice.js 2013-03-20 03:41:40 +0000
+++ lib/lp/code/javascript/tests/test_information_type_choice.js 2015-06-04 17:01:40 +0000
@@ -60,6 +60,7 @@
makeWidget: function() {
this.widget = new ns.BranchInformationTypeWidget({
io_provider: this.mockio,
+ object_type: 'Branch',
use_animation: false
});
this.widget.render();
=== modified file 'lib/lp/code/templates/branch-portlet-privacy.pt'
--- lib/lp/code/templates/branch-portlet-privacy.pt 2012-08-28 01:52:34 +0000
+++ lib/lp/code/templates/branch-portlet-privacy.pt 2015-06-04 17:01:40 +0000
@@ -24,8 +24,11 @@
LPJS.use('lp.code.branch.information_type_choice', function(Y) {
Y.on('domready',
function(e) {
- var widget = new Y.lp.code.branch.information_type_choice.BranchInformationTypeWidget();
- widget.render();
+ var config = {
+ object_type: 'Branch'
+ };
+ var widget = new Y.lp.code.branch.information_type_choice.BranchInformationTypeWidget(config);
+ widget.render();
},
window);
});
=== modified file 'lib/lp/code/templates/gitrepository-index.pt'
--- lib/lp/code/templates/gitrepository-index.pt 2015-04-21 09:31:58 +0000
+++ lib/lp/code/templates/gitrepository-index.pt 2015-06-04 17:01:40 +0000
@@ -18,6 +18,7 @@
<body>
<metal:side fill-slot="side">
+ <div tal:replace="structure context/@@+portlet-privacy" />
<div tal:replace="structure context/@@+global-actions" />
<tal:subscribers replace="structure context/@@+portlet-subscribers" />
</metal:side>
=== added file 'lib/lp/code/templates/gitrepository-portlet-privacy.pt'
--- lib/lp/code/templates/gitrepository-portlet-privacy.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/gitrepository-portlet-privacy.pt 2015-06-04 17:01:40 +0000
@@ -0,0 +1,37 @@
+<div
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ id="privacy"
+ tal:attributes="
+ class python: path('context/private') and 'portlet private' or 'portlet public'
+ "
+ tal:define="link context/menu:context/visibility"
+>
+ <span id="information-type-summary"
+ tal:attributes="class view/information_type_css;">This repository
+ contains
+ <strong id="information-type" tal:content="view/information_type"></strong>
+ information</span> <a class="sprite edit action-icon" id="privacy-link"
+ tal:attributes="href link/path" tal:condition="link/enabled"
+ >Edit</a>
+ <div id="information-type-description" style="padding-top: 5px"
+ tal:content="view/information_type_description"></div>
+</div>
+
+<tal:script>
+ <script type="text/javascript">
+ LPJS.use('lp.code.branch.information_type_choice', function(Y) {
+ Y.on('domready',
+ function(e) {
+ var config = {
+ object_type: 'Git Repository'
+ };
+ var widget = new Y.lp.code.branch.information_type_choice.BranchInformationTypeWidget(config);
+ widget.render();
+ },
+ window);
+ });
+ </script>
+</tal:script>
+
Follow ups
