Code review comment for lp:~wgrant/launchpad/export-bug-nominations

Revision history for this message
William Grant (wgrant) wrote :

= Summary =
Bug nominations want to be exported through the API.

== Proposed fix ==
Export IBugNomination and relevant methods from IBug, moving access control code from views to model classes.

== Implementation details ==
All of the relevant IBug and IBugNomination methods already took the user as an argument, so no signature changes were required.

The main non-trivial bits are the refactoring of IBug.canBeNominatedFor and IBug.addNomination to properly forbid attempts to nominate a non-series. Rather than introduce an ISeriesBugTarget as suggested in the bug, I opted to just use IBugTarget. Further type checking was going to be required anyway when dealing with an ISeriesBugTarget, so it's probably cleaner this way.

There are four significant model behaviour changes:
 - IBug.addNomination will no longer automatically approve nominations if possible. The view does this explicitly instead.
 - BugNomination.approve is now a no-op if it is already approved, as IBugNomination says. Previously it would crash or create a duplicate task.
 - BugNomination.decline now fails if it is already approved, as IBugNomination says. Previously it would successfully decline the task.
 - Bug.addNomination will refuse to create a nomination for a series on a pillar that does not already have a task. This has always been enforced in the view.

== Tests ==
Direct tests were added for IBug.canBeNominatedFor and the webservice, and a test to verify that nominations are automatically approved was dropped. Other tests required trivial changes to comply with the new model restrictions.

 $ bin/test -t test_bugnomination -t bugtask-edit-views.txt -t bug-nomination.txt -t bug-set-status.txt -t bugtask.txt -t xx-bug.txt -t test_bugchanges

« Back to merge proposal