The default bug view has changed. See this FAQ.

Set Flags on Bug Entry

RESOLVED FIXED in Bugzilla 3.0

Status

()

Bugzilla
Attachments & Requests
--
enhancement
RESOLVED FIXED
15 years ago
10 years ago

People

(Reporter: Jeff Hedlund, Assigned: Frédéric Buclin)

Tracking

({selenium})

unspecified
Bugzilla 3.0
selenium
Dependency tree / graph
Bug Flags:
approval +
documentation +
testcase +

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

15 years ago
Simiilar to bug 98103, but for bugs: I'd like to be able to set flags when
entering a bug instead of having to submit a bug, then immediately change it.
(Assignee)

Updated

12 years ago
Blocks: 315860

Updated

11 years ago
Severity: normal → enhancement
OS: Windows 2000 → All
Hardware: PC → All
(Assignee)

Comment 1

11 years ago
*** Bug 341932 has been marked as a duplicate of this bug. ***

Updated

11 years ago
QA Contact: mattyt-bugzilla → default-qa
(Assignee)

Updated

11 years ago
Assignee: myk → attach-and-request
Target Milestone: --- → Bugzilla 2.24
(Assignee)

Comment 2

11 years ago
Created attachment 228185 [details] [diff] [review]
patch, v1

Works with both bug flags and attachment flags. We should definitely put JS scripts out of the templates, but that's for another bug.
Assignee: attach-and-request → LpSolit
Status: NEW → ASSIGNED
Attachment #228185 - Flags: review?(myk)
(Assignee)

Updated

11 years ago
Attachment #228185 - Flags: review?(wurblzap)
(Assignee)

Comment 3

11 years ago
Created attachment 228187 [details] [diff] [review]
patch, v1.1

I forgot to include filterexceptions.pl in the patch.
Attachment #228185 - Attachment is obsolete: true
Attachment #228187 - Flags: review?(myk)
Attachment #228185 - Flags: review?(wurblzap)
Attachment #228185 - Flags: review?(myk)
(Assignee)

Comment 4

11 years ago
Created attachment 228190 [details] [diff] [review]
patch, v1.1.1

bah... ./runtests.pl 11 was complaining because I forgot to add a newline before =back in POD.
Attachment #228187 - Attachment is obsolete: true
Attachment #228190 - Flags: review?(myk)
Attachment #228187 - Flags: review?(myk)
(Assignee)

Updated

11 years ago
Attachment #228190 - Flags: review?(wurblzap)
Comment on attachment 228190 [details] [diff] [review]
patch, v1.1.1

>Index: Bugzilla/Product.pm
  ...
>+sub flag_types {
>+    my $self = shift;
>+
>+    if (!defined $self->{'flag_types'}) {
>+        $self->{'flag_types'} = {};
>+        foreach my $type ('bug', 'attachment') {
>+            my %flagtypes;
>+            foreach my $component (@{$self->components}) {
>+                foreach my $flagtype (@{$component->flag_types->{$type}}) {
>+                    $flagtypes{$flagtype->{'id'}} ||= $flagtype;
>+                }
>+            }
>+            $self->{'flag_types'}->{$type} = [sort { $a->{'sortkey'} <=> $b->{'sortkey'}
>+                                                    || $a->{'name'} cmp $b->{'name'} } values %flagtypes];
>+        }
>+    }
>+    return $self->{'flag_types'};
>+}

This makes product flag types be the union of component flag types, but flag types can apply to an entire product, in which case they don't belong to any specific component.  This code would need to take that into account.


>+        // We first disabled all flags. Then we will re-enable
>+        // those which are available for the selected component.

The comment describing what the set_assign_to function does should mention that it now also selectively enables flags.

Otherwise, the code looks good.  I haven't reviewed the UI, but if it's like the UI for flags that appears on the show bug page, it should be fine.
Attachment #228190 - Flags: review?(myk) → review-
(Assignee)

Comment 6

11 years ago
(In reply to comment #5)

> This makes product flag types be the union of component flag types, but flag
> types can apply to an entire product, in which case they don't belong to any
> specific component.  This code would need to take that into account.

Myk, I think it does, see Bugzilla::FlagType::sqlify_criteria():

  push(@criteria, "(i.component_id = $component_id OR i.component_id IS NULL)");
Comment on attachment 228190 [details] [diff] [review]
patch, v1.1.1

> Myk, I think it does, see Bugzilla::FlagType::sqlify_criteria():

Ah, indeed.  Ok, r=myk
Attachment #228190 - Flags: review- → review+
Comment on attachment 228190 [details] [diff] [review]
patch, v1.1.1

Nice feature, and well-working patch!

The UI offers me to set/request flags I'm not permitted to set/request by flag type grant group or request group (case "setting the approval flag without being an approver"). If I try to do this, the next page has a title of "Flag Creation Failure", then there is a Bugzilla message box containing screen-filling HTML, starting with "An error occured while validating flags: <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 [...]". There is the "Only a sufficiently empowered user can make this change" message buried in there, so I assume I'm seeing the code of a ThrowUserError page here.
Below the fold (for my resolution/textsize/...), I find "Bug [...] has been added to the database". It correctly doesn't have the flag set.
I think either the UI should not offer to set/request flags I'm not allowed to set/request, or the error should be human-readable.

Nit: if I have JavaScript switched off, the UI offers me to set/request a flag I'm not permitted to set/request by flag type inclusion or exclusion settings. If I try to do this, the bug is correctly created without the flag. There is no message about this, though. I see three options here: 1) Show a message along the lines of "setting/requesting flag xy has been turned down because you're not allowed to set/request this flag", 2) turn off flag addition on bug creation if JavaScript is switched off, 3) leave it as it is and assume non-JavaScript users to be clever enough to understand what's going on.

Index: post_bug.cgi
@@ -538,11 +547,30 @@
+# This requires to be in batch mode.

Uh... This is no reason for r-, but it's unfortunate that we need to resort to stuff like this. So as food for thought: we might want to start thinking of a more general way to handle stuff like this. Attachment addition on bug creation comes to mind as a candidate to use this, and there are probably other places, too. Can we do Java-like exception throwing in perl, so that our ThrowXXXError functions don't take side-exits?

>Index: template/en/default/bug/create/create.html.tmpl
>@@ -87,6 +99,31 @@
>                   last_initialqacontact = contact;
>             }
>         [% END %]
>+
>+        // We first disabled all flags. Then we will re-enable

Nit: "First, we disable all flags. Then we re-enable..." seems to work better for me.

+      <td colspan="2">&nbsp;</td>

Why are you adding &nbsp; all over the place? If it's not necessary, I think you shouldn't do this.
Attachment #228190 - Flags: review?(wurblzap) → review-
(Assignee)

Comment 9

11 years ago
(In reply to comment #8)
> Why are you adding &nbsp; all over the place? If it's not necessary, I think
> you shouldn't do this.

Because I seem to recall that some web browsers display columns incorrectly (their width not being the one expected) when the content of some cells is completely empty.
(Assignee)

Comment 10

11 years ago
(In reply to comment #8)
> The UI offers me to set/request flags I'm not permitted to set/request by flag
> type grant group or request group (case "setting the approval flag without
> being an approver").

You have the same "problem" when viewing an existing bug. That's not something I plan to fix now, at least not in this bug (minor issue).
(Assignee)

Comment 11

11 years ago
Created attachment 229469 [details] [diff] [review]
patch, v2

In batch mode, Error.pm die(), but the error message which is displayed is HTML-formatted, which explains the mess Marc got when reviewing my patch. To fix that, we have to return from {code|user}-error.html.tmpl before displaying the HTML header (and footer).

I also added a "\n" at the end of the die() message in Error.pm to avoid the text "in Bugzilla/Error.pm at line 81" from being appended to the error message.

I don't plan to do anything about browsers not using JS. FlagType::validate() will silently ignore invalid flags, per design.
Attachment #228190 - Attachment is obsolete: true
Attachment #229469 - Flags: review?(wurblzap)
(Assignee)

Comment 12

11 years ago
Comment on attachment 229469 [details] [diff] [review]
patch, v2

myk, do you want to look at my patch again? See my previous comment for changes I made in this patch. Else Marc's review is enough.
Attachment #229469 - Flags: review?(myk)
(Assignee)

Updated

11 years ago
Blocks: 343810
Comment on attachment 229469 [details] [diff] [review]
patch, v2

Great.
And a good way to fix it, too, I think.
Attachment #229469 - Flags: review?(wurblzap) → review+
(Assignee)

Comment 14

11 years ago
Comment on attachment 229469 [details] [diff] [review]
patch, v2

myk already granted review for my previous patch.
Attachment #229469 - Flags: review?(myk)
(Assignee)

Updated

11 years ago
Flags: approval?
Flags: approval? → approval+
(Assignee)

Comment 15

11 years ago
Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.117; previous revision: 1.116
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.158; previous revision: 1.157
done
Checking in Bugzilla/Attachment.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Attachment.pm,v  <--  Attachment.pm
new revision: 1.38; previous revision: 1.37
done
Checking in Bugzilla/Component.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Component.pm,v  <--  Component.pm
new revision: 1.11; previous revision: 1.10
done
Checking in Bugzilla/Error.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Error.pm,v  <--  Error.pm
new revision: 1.17; previous revision: 1.16
done
Checking in Bugzilla/Product.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Product.pm,v  <--  Product.pm
new revision: 1.19; previous revision: 1.18
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v  <--  filterexceptions.pl
new revision: 1.74; previous revision: 1.73
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.63; previous revision: 1.62
done
Checking in template/en/default/flag/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/flag/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.21; previous revision: 1.20
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v  <--  code-error.html.tmpl
new revision: 1.72; previous revision: 1.71
done
Checking in template/en/default/global/messages.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl,v  <--  messages.html.tmpl
new revision: 1.36; previous revision: 1.35
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.178; previous revision: 1.177
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: relnote
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Blocks: 345929
(Assignee)

Updated

11 years ago
Flags: documentation?
(Assignee)

Updated

11 years ago
Flags: testcase?
(Assignee)

Comment 16

11 years ago
Created attachment 246282 [details] [diff] [review]
documentation patch, v1

I had to rewrite a part of the documentation about bug creation as a lot of stuff was missing (and pretty unhelpful).
Attachment #246282 - Flags: review?(documentation)
(Assignee)

Comment 17

11 years ago
Created attachment 246286 [details] [diff] [review]
documentation patch, v2

Updated based on timeless "Live" review.
Attachment #246282 - Attachment is obsolete: true
Attachment #246286 - Flags: review?(documentation)
Attachment #246282 - Flags: review?(documentation)
(Assignee)

Comment 18

11 years ago
Created attachment 246288 [details] [diff] [review]
documentation patch, v2.1

A few more things timeless found.
Attachment #246286 - Attachment is obsolete: true
Attachment #246288 - Flags: review?(documentation)
Attachment #246286 - Flags: review?(documentation)

Comment 19

11 years ago
Comment on attachment 246288 [details] [diff] [review]
documentation patch, v2.1

good enough.

although "queries on" => "queries for" :)
Attachment #246288 - Flags: review?(documentation) → review+
(Assignee)

Comment 20

11 years ago
Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.51; previous revision: 1.50
done
Flags: documentation? → documentation+

Comment 21

10 years ago
Added to the release notes as part of bug 349423.
Keywords: relnote
(Assignee)

Updated

10 years ago
Flags: testcase? → testcase+
(Assignee)

Updated

10 years ago
Keywords: selenium
You need to log in before you can comment on or make changes to this bug.