Merge lp:~wallyworld/launchpad/delete-bugtask-ui-878909 into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Curtis Hovey |
Approved revision: | no longer in the source branch. |
Merged at revision: | 14255 |
Proposed branch: | lp:~wallyworld/launchpad/delete-bugtask-ui-878909 |
Merge into: | lp:launchpad |
Diff against target: |
345 lines (+199/-3) 8 files modified
lib/lp/app/javascript/activator/assets/skins/sam/activator-skin.css (+1/-0) lib/lp/bugs/browser/bugtask.py (+35/-1) lib/lp/bugs/browser/configure.zcml (+6/-0) lib/lp/bugs/browser/tests/test_bugtask.py (+118/-0) lib/lp/bugs/configure.zcml (+1/-0) lib/lp/bugs/templates/bugtask-delete.pt (+26/-0) lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt (+10/-0) lib/lp/testing/views.py (+2/-2) |
To merge this branch: | bzr merge lp:~wallyworld/launchpad/delete-bugtask-ui-878909 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email: mp+80179@code.launchpad.net |
Commit message
[r=sinzui][bug=878909] Update the ui to support bugtask deletion.
Description of the change
Update the ui to support bugtask deletion.
== Implementation ==
Add a new BugTaskDeleteView and render delete icons next to each bug task that can be deleted by the logged in user.
This mp performs the delete using html forms. A subsequent branch will provide ajax support.
The permission checking code was moved from the security adaptor to a method on BugTask so it could be reused by the view.
To recap: the delete capability is protected by a feature flag, is restricted to certain roles, and the last bug tasks for a bug cannot be deleted.
A change was made to the lazr-js activator css to ensure the edit icon lines up properly.
== Demo and QA ==
The delete icons are rendered as shown in the screenshot.
http://
== Tests ==
Add tests to browser/
TestBugTaskDele
(test that the delete links/icons are rendered as expected)
TestBugTaskDele
(test that the delete view itself works as expected)
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
Hi Ian.
I have a few concerns about the safety and maintainability of of two changes. I have some suggestions and questions too.
> === modified file 'lib/lp/ bugs/browser/ bugtask. py' bugs/browser/ bugtask. py 2011-10-19 14:54:07 +0000 bugs/browser/ bugtask. py 2011-10-25 08:17:26 +0000 extFromData( data) View(LaunchpadF ormView) : message( self): bug.title, target. bugtargetdispla yname))
> --- lib/lp/
> +++ lib/lp/
> @@ -1757,6 +1758,38 @@
> self.updateCont
>
>
> +class BugTaskDeletion
> + """Used to delete a bugtask."""
> +
> + schema = IBugTask
> + field_names = []
> +
> + label = 'Remove bug task'
> + page_title = label
> +
> + @property
> + def confirmation_
> + return ('<p>You are about to mark bug %s<br>'
> + 'as no longer affecting %s.</p>'
> + '<p>This operation will be permanent and cannot be '
> + 'undone.</p>'
> + % (self.context.
> + self.context.
This is dangerous. Both title and displayname may *always* contain
markup. We use structured() to build safe interpolated messages.
Would direct language be more effective at conveying the consequences of the
action:
Deletion is permanent, it cannot be undone.
...
> @@ -3552,6 +3585,8 @@ class=' highlight' if is_primary else None, link=canonical_ url(self. context. target) , link_title= self.target_ link_title, delete= self.user_ can_delete_ bugtask,
> row_css_
> target_
> target_
> + user_can_
> + delete_link=link + '/+delete',
While I am certain this link will work, but it is a crafted link we need to url(self. context. target, view_name= '+delete' )
maintain with extra testing.
canonical_
will raise an exception if the named view is unregistered.
> === modified file 'lib/lp/ bugs/browser/ tests/test_ bugtask. py' bugs/browser/ tests/test_ bugtask. py 2011-10-05 18:02:45 +0000 bugs/browser/ tests/test_ bugtask. py 2011-10-25 08:17:26 +0000 series. product. displayname, content) teLinks( TestCaseWithFac tory):
> --- lib/lp/
> +++ lib/lp/
> @@ -606,6 +614,99 @@
> self.assertIn(
>
>
> +class TestBugTaskDele
> + """ Test that the delete icons/links for each relevant bug task are
> + correctly rendered.
> +
> + Bug task deletion is protected by a feature flag.
> + """
The first line may not wrap according to PEP 256.
+ layer = DatabaseFunctio nalLayer delete_ only_bugtask( self): makeBug( ) bug.owner) initialized_ view( bugtasks- and-nominations -table' ) owView( bug.default_ bugtask, False, False) e(row_view. user_can_ delete_ bugtask) cache(row_ view).user_ can_delete_ bugtask DELETE_ BUGTASK_ ENABLED) : e(row_view. user_can_ delete_ bugtask) delete_ bugtask_ if_authorised( self): makeBug( )
+
+ def test_cannot_
+ # The last bugtask cannot be deleted.
+ bug = self.factory.
+ login_person(
+ view = create_
+ bug, name='+
+ row_view = view._getTableR
+ self.assertFals
+ del get_property_
+ with FeatureFixture(
+ self.assertFals
+
+ def test_can_
+ # The bugtask can be deleted if the user if authorised.
+ bug = self.factory.
+ bugtask = self.fac...