Code review comment for lp:~wallyworld/launchpad/unassign-private-bug

Revision history for this message
Ian Booth (wallyworld) wrote :

> This seems much more complex than needed.
>
> When unassigning, you know the current /user/ has permission
>
> All you need to do is unconditionally add that user to the cache. I couldn't
> really make heads or tails of your logic - and it looks to do db lookups that
> aren't needed.
>
> Sorry to be a wet blanket, but I think simpler is better here.

I've fixed the implementation as requested and written new tests to compliment the existing ones I found in lp.bugs.tests.test_bugchanges. I refactored the existing test_unassign_bugtask to become a helper _test_unassign_bugtask and created new test_unassign_bugtask and test_unassign_private_bugtask tests to call it.

« Back to merge proposal