launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02202
[Merge] lp:~sinzui/launchpad/this-and-that into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/this-and-that into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#1782 doc/security.txt is out of date
https://bugs.launchpad.net/bugs/1782
#246671 xx-presenting-private-bugs.txt claims privacy provenance is shown, but it isn't
https://bugs.launchpad.net/bugs/246671
#250617 Purpose of link tests in xx-person-packages is unclear
https://bugs.launchpad.net/bugs/250617
#276881 Remove bug nicknames from the UI
https://bugs.launchpad.net/bugs/276881
Fixes for trivial issues in tests.
Launchpad bug:
https://bugs.launchpad.net/bugs/250617
https://bugs.launchpad.net/bugs/1782
https://bugs.launchpad.net/bugs/246671
https://bugs.launchpad.net/bugs/276881
Pre-implementation: no one
Test command: ./bin/test -vv \
-t xx-person-packages \
-t xx-presenting-private-bugs \
-t bug-views.txt -t xx-bug-edit.txt
Bug #250617 [Purpose of link tests in xx-person-packages is unclear]
In xx-person-packages test, the purpose of testing some of the page links
is unclear
Bug #1782 [doc/security.txt is out of date]
Security.txt referenced non-existent
Bug #246671 [xx-presenting-private-bugs.txt claims privacy provenance]
xx-presenting-private-bugs.txt says "The date and time at which the bug
report was marked private and the person who did it is also shown (as a
title/tooltip)". But the test doesn't test this, and nor is it implemented
in the page.
Bug #276881 [Remove bug nicknames from the UI]
Bug nicknames should be removed from the UI, and just kept as a reference
when importing from other bug trackers.
--------------------------------------------------------------------
RULES
Bug #250617 [Purpose of link tests in xx-person-packages is unclear]
* Revise the text to read like a story.
Bug #1782 [doc/security.txt is out of date]
* Used current examples of how to define and register a security adapter.
* Move the doc to /doc where new developers can find it.
Bug #276881 [Remove bug nicknames from the UI]
* Remove the nickname from the bug edit form, the only place it is
shown
Bug #246671 [xx-presenting-private-bugs.txt claims privacy provenance]
* Remove the "who and when" from the story narrative.
QA
Bug #250617 [Purpose of link tests in xx-person-packages is unclear]
None.
Bug #1782 [doc/security.txt is out of date]
None
Bug #276881 [Remove bug nicknames from the UI]
* Visit https://bugs.launchpad.net/launchpad/+bug/276881/+edit
* Verify the nickname field is not shown.
Bug #246671 [xx-presenting-private-bugs.txt claims privacy provenance]
* None
LINT
doc/security.txt
lib/lp/bugs/browser/bug.py
lib/lp/bugs/browser/tests/bug-views.txt
lib/lp/bugs/stories/bug-privacy/xx-presenting-private-bugs.txt
lib/lp/bugs/stories/bugs/xx-bug-edit.txt
lib/lp/soyuz/stories/soyuz/xx-person-packages.txt
^ Lint hates some of these files. I can fix these before I land.
IMPLEMENTATION
Revised the text of documentation
doc/security.txt
lib/lp/bugs/stories/bug-privacy/xx-presenting-private-bugs.txt
lib/lp/soyuz/stories/soyuz/xx-person-packages.txt
Bug #276881 [Remove bug nicknames from the UI]
Removed the field and updated the tests to not verify it.
lib/lp/bugs/browser/bug.py
lib/lp/bugs/browser/tests/bug-views.txt
lib/lp/bugs/stories/bugs/xx-bug-edit.txt
--
https://code.launchpad.net/~sinzui/launchpad/this-and-that/+merge/44131
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/this-and-that into lp:launchpad.
=== renamed file 'lib/canonical/launchpad/doc/security.txt' => 'doc/security.txt'
--- lib/canonical/launchpad/doc/security.txt 2010-11-08 13:45:59 +0000
+++ doc/security.txt 2010-12-18 15:02:55 +0000
@@ -11,27 +11,28 @@
This document is about security policy in Launchpad.
Defining Permissions in Launchpad
-=================================
+---------------------------------
-********************************************************************************
+**************************************************************************
NOTE: A new permission should only be defined if absolutely necessary, and
it should be considered thoroughly in a code review.
-********************************************************************************
+**************************************************************************
Occassionally, you'll find yourself in a situation where the existing
-permissions in Launchpad aren't enough for what you want. For example, as I was
-writing this document I needed a permission I could attach to things to provide
-policy for who can view a thing. That is, I wanted a permission called
+permissions in Launchpad aren't enough for what you want. For example, as I
+was writing this document I needed a permission I could attach to things to
+provide policy for who can view a thing. That is, I wanted a permission called
launchpad.View.
-
A new permission (see the NOTE above) is defined in Launchpad in the file
lib/canonical/launchpad/permissions.zcml. So, to define the permission
launchpad.View, we'd add a line like this to that file:
- <permission id="launchpad.View" title="Viewing something" />
+ <permission id="launchpad.View" title="Viewing something"
+ access_level="read" />
+
Defining Authorization Policies for Permissions
-===============================================
+-----------------------------------------------
Once you've defined a permission, you'll probably want to define some logic
somewhere to express the authorization policy for that permission on a certain
@@ -56,18 +57,22 @@
Read the IAuthorization interface to ensure that you've defined the adapter
appropriately.
-2. Attach the security adapter to a permission on a given interface, in
-lib/canonical/launchpad/security.zcml. So, for the above adapter, here's how
-it's hooked up to IProduct, where IProduct is protected with the launchpad.Edit
-permission:
+2. Declare the permission on a given interface in a zcml file. So, for the
+above adapter, here's how it's hooked up to IProduct, where IProduct is
+protected with the launchpad.Edit permission:
- <adapter
- provides="canonical.launchpad.webapp.interfaces.IAuthorization"
- for="lp.registry.interfaces.product.IProduct"
- name="launchpad.Edit"
- factory="canonical.launchpad.security.EditByOwner"
- />
+ <class
+ class="lp.registry.model.product.Product">
+ <allow
+ interface="lp.registry.interfaces.product.IProductPublic"/>
+ <require
+ permission="launchpad.Edit"
+ interface="lp.registry.interfaces.product.IProductEditRestricted"/>
+ <require
+ permission="launchpad.Edit"
+ set_attributes="commercial_subscription description"/>
+ </class>
In this example, the EditByOwner adapter's checkAuthenticated method will be
-called to determine if the currently authenticated user is authorized to access
-whatever is protected by launchpad.Edit on an IProduct.
+called to determine if the currently authenticated user is authorized to
+access whatever is protected by launchpad.Edit on an IProduct.
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py 2010-11-23 23:22:27 +0000
+++ lib/lp/bugs/browser/bug.py 2010-12-18 15:02:55 +0000
@@ -634,7 +634,7 @@
class BugEditView(BugEditViewBase):
"""The view for the edit bug page."""
- field_names = ['title', 'description', 'tags', 'name']
+ field_names = ['title', 'description', 'tags']
custom_widget('title', TextWidget, displayWidth=30)
custom_widget('tags', BugTagsWidget)
next_url = None
=== modified file 'lib/lp/bugs/browser/tests/bug-views.txt'
--- lib/lp/bugs/browser/tests/bug-views.txt 2010-10-21 01:42:14 +0000
+++ lib/lp/bugs/browser/tests/bug-views.txt 2010-12-18 15:02:55 +0000
@@ -589,7 +589,7 @@
Bug Edit Page
=============
-The bug edit page is used to edit the summary, description, nick name,
+The bug edit page is used to edit the summary, description,
and bug tags. If the user try to add a tag that hasn't been used in the
current context, we display a confirmation button, which shouldn't be
automatically rendered by the form template. In order to show how it
@@ -611,6 +611,8 @@
>>> bug_edit.view.render()
EDIT BUG
+ >>> bug.field_names
+ ['title', 'description', 'tags']
>>> [action.label for action in bug_edit.view.actions]
['Change']
@@ -621,7 +623,6 @@
>>> edit_values = {
... 'field.title': u'New title',
... 'field.description': u'New description.',
- ... 'field.name': u'new-nickname',
... 'field.tags': u'doc'}
>>> bug_edit.submit('change', edit_values)
@@ -634,8 +635,6 @@
u'New title'
>>> bug_one.description
u'New description.'
- >>> bug_one.name
- u'new-nickname'
>>> bug_one.tags
[u'doc']
=== modified file 'lib/lp/bugs/stories/bug-privacy/xx-presenting-private-bugs.txt'
--- lib/lp/bugs/stories/bug-privacy/xx-presenting-private-bugs.txt 2009-06-12 16:36:02 +0000
+++ lib/lp/bugs/stories/bug-privacy/xx-presenting-private-bugs.txt 2010-12-18 15:02:55 +0000
@@ -1,4 +1,5 @@
-= Presenting private bug reports =
+Presenting private bug reports
+==============================
When a bug report is public, it says so.
@@ -8,21 +9,17 @@
This report is public
But when marked private, it gains the standard Launchpad presentation
-for private things. The date and time at which the bug report was
-marked private and the person who did it is also shown (as a
-title/tooltip):
+for private things.
>>> browser.open("http://bugs.launchpad.dev/firefox/+bug/4/+secrecy")
- >>> browser.getControl("This bug report should be private").selected = True
+ >>> browser.getControl(
+ ... "This bug report should be private").selected = True
>>> browser.getControl("Change").click()
>>> print browser.url
http://bugs.launchpad.dev/firefox/+bug/4
>>> print extract_text(find_tag_by_id(browser.contents, 'privacy'))
This report is private
-XXX 20080708 mpt: The date and person are not actually shown anywhere
-(bug 246671).
-
Bugs created before we started recording the date and time and who
marked the bug private show only a simple message:
=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-edit.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-edit.txt 2010-01-19 17:04:20 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-edit.txt 2010-12-18 15:02:55 +0000
@@ -1,17 +1,5 @@
-Sample person tries to change the bug's nickname to a existing one and gets a nice error message.
-
- >>> user_browser.open(
- ... 'http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/3')
- >>> user_browser.getLink(url="+edit").click()
- >>> user_browser.url
- 'http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/3/+edit'
- >>> user_browser.getControl('Nickname').value = 'blackhole'
- >>> user_browser.getControl('Change').click()
- >>> print user_browser.contents
- <...
- ...There is 1 error...
- ...
- ...blackhole is already in use by another bug...
+Bug editing
+===========
The user should be able to look at the page title or heading and
see the bug number being edited.
@@ -33,15 +21,15 @@
>>> browser.url
'http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/3/+edit'
>>> browser.getControl('Summary').value = 'New Summary'
- >>> browser.getControl('Nickname').value = 'testbug'
>>> browser.getControl('Description').value = 'New description.'
>>> browser.getControl('Change').click()
>>> browser.url
'http://.../debian/+source/mozilla-firefox/+bug/3'
- >>> print browser.contents
- <...testbug...
- ...New Summary...
- ...New description...
+ >>> content = find_main_content(browser.contents)
+ >>> print extract_text(content.h1)
+ New Summary
+ >>> print extract_text(find_tag_by_id(content, 'maincontentsub'))
+ Bug Description New description. ...
== Viewing the original description ==
=== modified file 'lib/lp/soyuz/stories/soyuz/xx-person-packages.txt'
--- lib/lp/soyuz/stories/soyuz/xx-person-packages.txt 2010-10-18 22:24:59 +0000
+++ lib/lp/soyuz/stories/soyuz/xx-person-packages.txt 2010-12-18 15:02:55 +0000
@@ -50,9 +50,8 @@
that link to a distributionsourcepackage, distroseriessourcepackage and
distrosourcepackagerelease respectively.
-Following a link on any item in the name column will take us to a
-distribution source package page. We'll use "cnews" on the third
-line here as it has working links in the sample data:
+The package name column links to the distribution source package page.
+The user chooses to see the distribution information for cnews:
>>> browser.open("http://launchpad.dev/~name16/+related-software")
>>> link = browser.getLink("cnews")
@@ -62,8 +61,8 @@
>>> browser.title
'...cnews... package : Ubuntu'
-Same process for the second column should take us to a distribution series
-source package page:
+The second column links to the distribution series source package page. The
+user follows the "Ubuntu Hoary" link next to cnews:
>>> browser.open("http://launchpad.dev/~name16/+related-software")
>>> link = browser.getLink(url="/ubuntu/hoary/+source/cnews")
@@ -73,8 +72,8 @@
>>> browser.title
'...cnews... package : Hoary (5.04) : Ubuntu'
-Same again for the third column should take us to a distribution source
-package release page:
+The third column links to the distribution source package release page. The
+user follows the cnews version link to see the page.
>>> browser.open("http://launchpad.dev/~name16/+related-software")
>>> link = browser.getLink(url="/ubuntu/+source/cnews/cr.g7-37")