Code review comment for lp:~deryck/launchpad/less-restrictive-assign-someone-else-603281

Revision history for this message
Brad Crittenden (bac) wrote :

Hi Deryck,

Thanks for making this fix rolling back some restrictions.

In userCanSetAnyAssignee,I think the test would read better if you factored out the test for 'user is None' and return False immediately. So I'm suggesting (I hope formatting is preserved)

    def userCanSetAnyAssignee(self, user):
        """See `IBugTask`."""
        celebrities = getUtility(ILaunchpadCelebrities)
        if user is None:
            return False
        elif self.pillar.bug_supervisor is None:
            return True
        else:
            return (
                user.inTeam(self.pillar.bug_supervisor) or
                user.inTeam(self.pillar.owner) or
                user.inTeam(self.pillar.driver) or
                (self.distroseries is not None and
                 user.inTeam(self.distroseries.driver)) or
                (self.productseries is not None and
                 user.inTeam(self.productseries.driver)) or
                user.inTeam(celebrities.admin)
                or user == celebrities.bug_importer)

The technique you provided in setBugSupervisor of having the base class provide two alternatives for the subclasses to choose is very interesting. I've never seen that before.

review: Approve

« Back to merge proposal