← Back to team overview

launchpad-reviewers team mailing list archive

[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")