830330 - flags should honour check_can_change_field()

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[:glob ✱]

if an extension uses the bug_check_can_change_field hook to disable editing of fields, flags are still displayed as editable, and can be set/cleared/etc.

eg.
> sub bug_check_can_change_field {
>     push(@$priv_results, PRIVILEGES_REQUIRED_EMPOWERED);
> }

Comment 1

[Simon Green]

Attachment: v1 patch

Turns out we didn't have checks in Bugzilla::Flag either, so I've added this is part of the patch.

Regards,
Hugo

Comment 2

[:glob ✱]

Comment on attachment 703288 [details] [diff] [review]
v1 patch

r=glob

this patch doesn't allow for extensions to control flag editability on a per-flag basis, however if that's required it can be added at a later date.

Comment 3

[Frédéric Buclin]

Comment on attachment 703288 [details] [diff] [review]
v1 patch

A powerless user who is not the reporter can no longer edit/request flags, even for flags which have no group restrictions set on them.

Comment 4

[Frédéric Buclin]

I would tend to wontfix this bug, because flags are already controlled by groups. And uneditable bugs are already controlled by the CANEDIT bit set at the product level.

Comment 5

[:glob ✱]

(In reply to Frédéric Buclin from comment #4)
> I would tend to wontfix this bug, because flags are already controlled by
> groups. And uneditable bugs are already controlled by the CANEDIT bit set at
> the product level.

currently extensions can use the bug_check_can_change_field hook to selectively allow/block editing of each field except for flags.  while groups and canedit exists, neither are suitable for hooking via extensions, and i don't see why flags should be treated differently from any other field.

Comment 6

[:glob ✱]

i don't think this should be WONTFIX'ed, reopening.

hugo, i'm happy to address frederic's points in comment 4 if you don't want to continue with this bug.

Comment 7

[:glob ✱]

Attachment: patch v2

minor change to allow anyone to set flags.

Comment 8

[David Lawrence [:dkl]]

Comment on attachment 710085 [details] [diff] [review]
patch v2

r=dkl

Comment 9

[:glob ✱]

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified Bugzilla/Bug.pm
modified Bugzilla/Flag.pm
modified template/en/default/flag/list.html.tmpl
Committed revision 8516.

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Bug.pm
modified Bugzilla/Flag.pm
modified template/en/default/flag/list.html.tmpl
Committed revision 8575.

Comment 10

[Frédéric Buclin]

Comment on attachment 710085 [details] [diff] [review]
patch v2

>=== modified file 'template/en/default/flag/list.html.tmpl'

>-[% IF user.id AND !read_only_flags %]
>+[% IF user.id && !read_only_flags && bug.check_can_change_field('flagtypes.name', 0, 1) %]

When creating a new bug, the bug object doesn't exist yet and so bug.check_can_change_field() is always false, meaning that you can no longer set/request flags on bug creation.

Comment 11

[Frédéric Buclin]

Attachment: fix bustage

I'm fixing bustage using this patch, but this means that check_can_change_field() isn't called on bug creation anymore. Approving this patch myself as module owner.

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified template/en/default/flag/list.html.tmpl
Committed revision 8581.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified template/en/default/flag/list.html.tmpl
Committed revision 8521.

Comment 12

[Frédéric Buclin]

Added to relnotes for 4.4rc2.

File a separate bug if you want a hook to control flags on bug creation.