launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11347
[Merge] lp:~wallyworld/launchpad/branch-infotype-portlet-1040999 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/branch-infotype-portlet-1040999 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1040999 in Launchpad itself: "Cannot use branch information type portlet to set type"
https://bugs.launchpad.net/launchpad/+bug/1040999
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/branch-infotype-portlet-1040999/+merge/121404
== Implementation ==
This branch does 2 things:
1. add an edit link to the branch privacy portlet to allow information type to be edited
This branch does not wire up any javascript (that will be done next branch). So the user clicks on the edit link and is taken to a new edit page where they can edit the information type. ie the same non js infrastructure as used elsewhere for lifecycle status etc. The view used to process the information type change does have the ajax processing in place so the next branch will be only javascript.
2. change the algorithm used by getInformationTypesToShow() to determine the allowed information types
Previously, a branch could not be made private/public security unless there were such bugs linked. Now, a branch can always be marked as security. The hooks for checking linked bugs have been left in place though because even though not used right now, very soon user data branches will only be allowed when there are user data linked bugs.
== Tests ==
Add test to TestBranchEditView to check the XHR updating of information type work correctly.
Update tests in TestBranchEditViewInformationTypes to reflect new getInformationTypesToShow algorithm.
Update TestBranchPrivacyPortlet to check for edit link.
== Lint ==
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/code/browser/branch.py
lib/lp/code/browser/configure.zcml
lib/lp/code/browser/tests/test_branch.py
lib/lp/code/templates/branch-portlet-privacy.pt
--
https://code.launchpad.net/~wallyworld/launchpad/branch-infotype-portlet-1040999/+merge/121404
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/branch-infotype-portlet-1040999 into lp:launchpad.
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2012-08-16 05:55:48 +0000
+++ lib/lp/code/browser/branch.py 2012-08-27 23:07:30 +0000
@@ -308,13 +308,19 @@
'add_subscriber', 'browse_revisions', 'create_recipe', 'link_bug',
'link_blueprint', 'register_merge', 'source', 'subscription',
'edit_status', 'edit_import', 'upgrade_branch', 'view_recipes',
- 'create_queue']
+ 'create_queue', 'visibility']
@enabled_with_permission('launchpad.Edit')
def edit_status(self):
text = 'Change branch status'
return Link('+edit-status', text, icon='edit')
+ @enabled_with_permission('launchpad.Edit')
+ def visibility(self):
+ """Return the 'Set information type' Link."""
+ text = 'Change information type'
+ return Link('+edit-information-type', text)
+
def browse_revisions(self):
"""Return a link to the branch's revisions on codebrowse."""
text = 'All revisions'
@@ -430,8 +436,8 @@
return branch.url
-class BranchView(LaunchpadView, FeedsMixin, BranchMirrorMixin,
- InformationTypePortletMixin):
+class BranchView(InformationTypePortletMixin, FeedsMixin, BranchMirrorMixin,
+ LaunchpadView):
feed_types = (
BranchFeedLink,
@@ -731,23 +737,22 @@
# If we're stacked on a private branch, only show that
# information type.
if self.context.stacked_on and self.context.stacked_on.private:
- shown_types = set([self.context.stacked_on.information_type])
+ shown_types = {self.context.stacked_on.information_type}
else:
shown_types = (
InformationType.PUBLIC,
InformationType.USERDATA,
InformationType.PROPRIETARY,
InformationType.EMBARGOED,
- )
-
- # We only show Private Security and Public Security
- # if the branch is linked to a bug with one of those types,
- # as they're confusing and not generally useful otherwise.
- # Once Proprietary is fully deployed, it should be added here.
- hidden_types = (
InformationType.PUBLICSECURITY,
InformationType.PRIVATESECURITY,
)
+
+ # XXX Once Branch Visibility Policies are removed, we only want to
+ # show Private (USERDATA) if the branch is linked to such a bug.
+ hidden_types = (
+ # InformationType.USERDATA,
+ )
if set(allowed_types).intersection(hidden_types):
params = BugTaskSearchParams(
user=self.user, linked_branches=self.context.id,
@@ -800,7 +805,8 @@
"""See `LaunchpadFormView`"""
return {self.schema: self.context}
- @action('Change Branch', name='change')
+ @action('Change Branch', name='change',
+ failure=LaunchpadFormView.ajax_failure_handler)
def change_action(self, action, data):
# If the owner or product has changed, add an explicit notification.
# We take our own snapshot here to make sure that the snapshot records
@@ -849,9 +855,15 @@
# was in fact a change.
self.context.date_last_modified = UTC_NOW
+ if self.request.is_ajax:
+ return ''
+
@property
def next_url(self):
- return canonical_url(self.context)
+ """Return the next URL to call when this call completes."""
+ if not self.request.is_ajax:
+ return canonical_url(self.context)
+ return None
cancel_url = next_url
@@ -868,6 +880,12 @@
field_names = ['lifecycle_status']
+class BranchEditInformationTypeView(BranchEditFormView):
+ """A view for editing the information type only."""
+
+ field_names = ['information_type']
+
+
class BranchMirrorStatusView(LaunchpadFormView):
"""This view displays the mirror status of a branch.
@@ -1114,9 +1132,9 @@
def validate(self, data):
# Check that we're not moving a team branch to the +junk
# pseudo project.
- owner = data['owner']
if 'name' in data:
# Only validate if the name has changed or the owner has changed.
+ owner = data['owner']
if ((data['name'] != self.context.name) or
(owner != self.context.owner)):
# We only allow moving within the same branch target for now.
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2012-05-29 06:05:16 +0000
+++ lib/lp/code/browser/configure.zcml 2012-08-27 23:07:30 +0000
@@ -480,6 +480,13 @@
permission="launchpad.Edit"
template="../../app/templates/generic-edit.pt"/>
<browser:page
+ name="+edit-information-type"
+ for="lp.code.interfaces.branch.IBranch"
+ layer="lp.code.publisher.CodeLayer"
+ class="lp.code.browser.branch.BranchEditInformationTypeView"
+ permission="launchpad.Edit"
+ template="../../app/templates/generic-edit.pt"/>
+ <browser:page
name="+edit"
for="lp.code.interfaces.branch.IBranch"
layer="lp.code.publisher.CodeLayer"
=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py 2012-08-22 14:23:12 +0000
+++ lib/lp/code/browser/tests/test_branch.py 2012-08-27 23:07:30 +0000
@@ -71,7 +71,10 @@
setupBrowser,
setupBrowserForUser,
)
-from lp.testing.views import create_initialized_view
+from lp.testing.views import (
+ create_initialized_view,
+ create_view,
+ )
class TestBranchMirrorHidden(TestCaseWithFactory):
@@ -986,6 +989,29 @@
self.assertRaises(
LookupError, browser.getControl, "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()
+ branch = self.factory.makeProductBranch(owner=person)
+
+ extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
+ request = LaunchpadTestRequest(
+ method='POST', form={
+ 'field.actions.change': 'Change Branch',
+ 'field.information_type': 'PUBLICSECURITY'},
+ **extra)
+ with person_logged_in(person):
+ view = create_view(
+ branch, name='+edit-information-type',
+ request=request, principal=person)
+ request.traversed_objects = [person, branch.product, branch, view]
+ view.initialize()
+ result = view.render()
+ self.assertEqual('', result)
+ self.assertEqual(
+ branch.information_type, InformationType.PUBLICSECURITY)
+
class TestBranchEditViewInformationTypes(TestCaseWithFactory):
"""Tests for BranchEditView.getInformationTypesToShow."""
@@ -1000,28 +1026,14 @@
self.assertContentEqual(types, view.getInformationTypesToShow())
def test_public_branch(self):
- # A normal public branch on a public project can only be public.
- # We don't show information types like Public Security
- # unless there's a linked branch of that type, as they're not
- # useful or unconfusing otherwise.
+ # A normal public branch on a public project can only be a public
+ # information type.
# The model doesn't enforce this, so it's just a UI thing.
branch = self.factory.makeBranch(
information_type=InformationType.PUBLIC)
- self.assertShownTypes([InformationType.PUBLIC], branch)
-
- def test_public_branch_with_security_bug(self):
- # A public branch can be set to Public Security if it has a
- # linked Public Security bug. The project policy doesn't
- # allow private branches, so Private Security and Private
- # are unavailable.
- branch = self.factory.makeBranch(
- information_type=InformationType.PUBLIC)
- bug = self.factory.makeBug(
- information_type=InformationType.PUBLICSECURITY)
- with admin_logged_in():
- branch.linkBug(bug, branch.owner)
self.assertShownTypes(
- [InformationType.PUBLIC, InformationType.PUBLICSECURITY],
+ [InformationType.PUBLIC,
+ InformationType.PUBLICSECURITY],
branch)
def test_branch_with_disallowed_type(self):
@@ -1031,7 +1043,10 @@
branch = self.factory.makeBranch(
information_type=InformationType.PROPRIETARY)
self.assertShownTypes(
- [InformationType.PUBLIC, InformationType.PROPRIETARY], branch)
+ [InformationType.PUBLIC,
+ InformationType.PUBLICSECURITY,
+ InformationType.PROPRIETARY],
+ branch)
def test_stacked_on_private(self):
# A branch stacked on a private branch has its choices limited
@@ -1058,24 +1073,10 @@
branch.product.setBranchVisibilityTeamPolicy(
branch.owner, BranchVisibilityRule.PRIVATE)
self.assertShownTypes(
- [InformationType.PUBLIC, InformationType.USERDATA], branch)
-
- def test_private_branch_with_security_bug(self):
- # Branches on projects that allow private branches can use the
- # Private Security information type if they have a security
- # bug linked.
- branch = self.factory.makeBranch(
- information_type=InformationType.PUBLIC)
- with admin_logged_in():
- branch.product.setBranchVisibilityTeamPolicy(
- branch.owner, BranchVisibilityRule.PRIVATE)
- bug = self.factory.makeBug(
- information_type=InformationType.PUBLICSECURITY)
- with admin_logged_in():
- branch.linkBug(bug, branch.owner)
- self.assertShownTypes(
- [InformationType.PUBLIC, InformationType.PUBLICSECURITY,
- InformationType.PRIVATESECURITY, InformationType.USERDATA],
+ [InformationType.PUBLIC,
+ InformationType.PUBLICSECURITY,
+ InformationType.PRIVATESECURITY,
+ InformationType.USERDATA],
branch)
def test_branch_for_project_with_embargoed_and_proprietary(self):
@@ -1138,6 +1139,7 @@
owner=owner, information_type=InformationType.USERDATA)
with person_logged_in(owner):
view = create_initialized_view(branch, '+portlet-privacy')
+ edit_url = '/' + branch.unique_name + '/+edit-information-type'
soup = BeautifulSoup(view.render())
information_type = soup.find('strong')
description = soup.find('div', id='information-type-description')
@@ -1145,3 +1147,5 @@
InformationType.USERDATA.title, information_type.renderContents())
self.assertTextMatchesExpressionIgnoreWhitespace(
InformationType.USERDATA.description, description.renderContents())
+ self.assertIsNotNone(
+ soup.find('a', id='privacy-link', attrs={'href': edit_url}))
=== modified file 'lib/lp/code/templates/branch-portlet-privacy.pt'
--- lib/lp/code/templates/branch-portlet-privacy.pt 2012-06-20 05:25:44 +0000
+++ lib/lp/code/templates/branch-portlet-privacy.pt 2012-08-27 23:07:30 +0000
@@ -6,9 +6,15 @@
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 branch
- contains <strong id="information-type" tal:content="view/information_type"></strong> information</span>
- <div id="information-type-description" style="padding-top: 5px" tal:content="view/information_type_description"></div>
+ 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>
Follow ups
