230732 - Display an "additional review" for multiplicable flags

Status: ASSIGNED

(Bugzilla :: Attachments & Requests, enhancement)

Description

[Asa Dotzler [:asa]]

This is a request for a change to multiplicable flags. The current behavior to
show an "additional review" flag (for the case of a flag named "review") only
after the first flag has been set. The requested change is that we always show
one  unset "additional review" flag. The initial condition for a multiplicable
flag in an attachment would be two flags, a "review" flag and an "additional
review" flag. We would always have an unset additional review so if one of those
initial flags were set, the other would remain. If both were set, a third flag
would be added to the patch.

Comment 1

[Dave Miller [:justdave]]

Sounds good to me.  There's often been times when I've wished to request review
from two people at once... :)

Comment 2

[Max Kanat-Alexander]

This is a useful change, and I bet it'd be easy to do.

Just add a checkbox to the flags-creation page, called "always show additional
<something>" (I'm not quite sure how to phrase it) that would make there always
be two flags shown.

Even better would be "always have at least X blank flags."

Comment 3

[Myk Melez [:myk] [@mykmelez]]

I think we can make this even easier on ourselves and still better for our users
by implementing this as the standard behavior without making it configurable,
since the new behavior is not only more usable, it's also more consistent with
the state of the UI once a flag has been set.  Consider what we're doing now:

foo: [   ]            but            foo:       [ + ]
                                     addl. foo: [   ]

This bug would have us do:

foo:       [   ]      and            foo:       [ + ]
addl. foo: [   ]                     addl. foo: [   ]

The new behavior doesn't make things more confusing or take up additional space
that won't be taken up anyway once a flag is set (and thus needs to be
contingently allocated anyway), and it surprises the user less (because UI
doesn't magically appear after a state change).  We might consider disabling the
"addl. foo" control until the "foo" control gets set, but that's icing on the
cake.  I think we can implement this as the standard behavior to everyone's
mutual benefit.

Comment 4

[Asa Dotzler [:asa]]

Myk, I agree. This should be the standard behavior for multipicable flags. 

Comment 5

[Reed Loden [:reed]]

Attachment: Just allow multiplicable flags to always be set - v1

This just removes the restriction that the flag must already exist. Basically, just always display "addl. <flag>".

Comment 6

[Frédéric Buclin]

Comment on attachment 241024 [details] [diff] [review]
Just allow multiplicable flags to always be set - v1

>-    [% NEXT UNLESS type.flags && type.flags.size > 0 && type.is_multiplicable && type.is_active %]
>+    [% NEXT UNLESS type.is_multiplicable && type.is_active %]

Doing so, you now have duplicated IDs within form elements (two fields have the same flag_type-nnn ID), and so the page doesn't pass HTML 4.01 validation anymore. Moreover, Bugzilla::Flag::validate() doesn't expect to get several fields with the same name, and so ignore the "addl. Foo" field.