Code review comment for lp:~arthur-she/lava-scheduler/fix-for-bug-1172724

Revision history for this message
Antonio Terceiro (terceiro) wrote :

I realize this has already been merged, but I need to comment. :-)

> === modified file 'lava_scheduler_app/models.py'
> --- lava_scheduler_app/models.py 2013-02-10 21:26:06 +0000
> +++ lava_scheduler_app/models.py 2013-06-24 09:24:26 +0000
> @@ -477,7 +477,10 @@
> """ used to check for things like if the user can cancel or annotate
> a job failure
> """
> - return user.is_superuser or user == self.submitter
> + ci_admin_groups = ['builds', 'baselines']
> + return (user.is_superuser or user == self.submitter or
> + ('ciadmin' == self.submitter.username and
> + user.groups.filter(name__in=ci_admin_groups).exists()))

Maybe we could instead add an attribute to groups that says whether a
group can manage jobs and then check for that being True here, instead
of hardcoding group names in the code like this.

--
Antonio Terceiro
Software Engineer - Linaro
http://www.linaro.org

« Back to merge proposal