> 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.
> 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.