Code review comment for lp:~wallyworld/launchpad/delete-bugtask-ui-878909

Hi

I've addressed the issues raised. Reverting the security check code to
how it was makes the diff much shorter.

>> @@ -3552,6 +3585,8 @@
>> row_css_class='highlight' if is_primary else None,
>> target_link=canonical_url(self.context.target),
>> target_link_title=self.target_link_title,
>> + user_can_delete=self.user_can_delete_bugtask,
>> + delete_link=link + '/+delete',
>
> While I am certain this link will work, but it is a crafted link we need to
> maintain with extra testing.
> canonical_url(self.context.target, view_name='+delete')
> will raise an exception if the named view is unregistered.
>

I copied what was done in the test for the +editstatus link. I've fixed
that one too.

>
> The TestBrowser is an indirect means to test rendering. The view does that.
> you can render any view (that had the principal argument passed) by calling
> it; view(). Launchpad views add striping rules so __call__ calls render().
> You can render a view's templates using view() or view.render(). It is much
> faster than TestBrowser.
>
> def test_bugtask_delete_icon(self):
> # The bugtask delete icon is rendered correctly for those tasks the
> # user is allowed to delete.
> bug = self.factory.makeBug()
> bugtask = self.factory.makeBugTask(bug=bug)
> with FeatureFixture(DELETE_BUGTASK_ENABLED):
> login_person(bugtask.owner)
> view = create_initialized_view(
> bug, name='+bugtasks-and-nominations-table'
> principal=bugtask.owner)
> content = view.render()
> # bugtask can be deleted because the user owns it.
> delete_icon = find_tag_by_id(
> content, 'bugtask-delete-task%d' % bugtask.id)
> self.assertEqual(url + '/+delete', delete_icon['href'])
> # default_bugtask cannot be deleted.
> delete_icon = find_tag_by_id(
> content, 'bugtask-delete-task%d' % bug.default_bugtask.id)
> self.assertIsNone(delete_icon)
>

I double checked my original work. I tried again using
 > view = create_initialized_view(
 > bug, name='+bugtasks-and-nominations-table'
 > principal=bugtask.ow

and it wouldn't render. I also tried other variations. So I think
TestBrowser is required for this test sadly.