From 2904ac3261ff9bb59e29b74d55d4ada294986ffe Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Tue, 25 Jul 2006 06:22:53 +0000 Subject: Bug 174039: Set flags on bug entry - Patch by Frédéric Buclin r=wurblzap r=myk a=myk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Bugzilla/Attachment.pm | 124 ++++++++++++++++++++++-- Bugzilla/Component.pm | 32 +++++- Bugzilla/Error.pm | 2 +- Bugzilla/Product.pm | 28 ++++++ attachment.cgi | 90 ++--------------- post_bug.cgi | 25 ++++- template/en/default/bug/create/create.html.tmpl | 116 +++++++++++++++------- template/en/default/filterexceptions.pl | 4 +- template/en/default/flag/list.html.tmpl | 8 +- template/en/default/global/code-error.html.tmpl | 7 ++ template/en/default/global/messages.html.tmpl | 5 + template/en/default/global/user-error.html.tmpl | 10 +- 12 files changed, 312 insertions(+), 139 deletions(-) diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index f012c3f2e..90ec68974 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -51,7 +51,8 @@ use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Flag; use Bugzilla::User; -use Bugzilla::Util qw(trick_taint); +use Bugzilla::Util; +use Bugzilla::Field; sub get { my $invocant = shift; @@ -594,12 +595,89 @@ sub validate_content_type { =pod -=item C +=item C + +Description: validates if the user is allowed to edit the attachment. + Only the submitter or someone with editbugs privs can edit it. + +Returns: 1 on success. Else an error is thrown. + +=cut + +sub validate_can_edit { + my ($class, $attach_id) = @_; + my $dbh = Bugzilla->dbh; + my $user = Bugzilla->user; + + # People in editbugs can edit all attachments + return if $user->in_group('editbugs'); + + # Bug 97729 - the submitter can edit their attachments + my ($ref) = $dbh->selectrow_array('SELECT attach_id FROM attachments + WHERE attach_id = ? AND submitter_id = ?', + undef, ($attach_id, $user->id)); + + $ref || ThrowUserError('illegal_attachment_edit', { attach_id => $attach_id }); +} + +=item C + +Description: validates if attachments the user wants to mark as obsolete + really belong to the given bug and are not already obsolete. + +Params: $bug_id - The bug ID obsolete attachments should belong to. + +Returns: 1 on success. Else an error is thrown. + +=cut + +sub validate_obsolete { + my ($class, $bug_id) = @_; + my $cgi = Bugzilla->cgi; + + # Make sure the attachment id is valid and the user has permissions to view + # the bug to which it is attached. + my @obsolete_attachments; + foreach my $attachid ($cgi->param('obsolete')) { + my $vars = {}; + $vars->{'attach_id'} = $attachid; + + detaint_natural($attachid) + || ThrowCodeError('invalid_attach_id_to_obsolete', $vars); + + my $attachment = Bugzilla::Attachment->get($attachid); + + # Make sure the attachment exists in the database. + ThrowUserError('invalid_attach_id', $vars) unless $attachment; + + $vars->{'description'} = $attachment->description; + + if ($attachment->bug_id != $bug_id) { + $vars->{'my_bug_id'} = $bug_id; + $vars->{'attach_bug_id'} = $attachment->bug_id; + ThrowCodeError('mismatched_bug_ids_on_obsolete', $vars); + } + + if ($attachment->isobsolete) { + ThrowCodeError('attachment_already_obsolete', $vars); + } + + # Check that the user can modify this attachment. + $class->validate_can_edit($attachid); + push(@obsolete_attachments, $attachment); + } + return @obsolete_attachments; +} + + +=pod + +=item C Description: inserts an attachment from CGI input for the given bug. -Params: C<$bug_id> - integer - the ID of the bug for which - to insert the attachment. +Params: C<$bug> - Bugzilla::Bug object - the bug for which to insert + the attachment. C<$user> - Bugzilla::User object - the user we're inserting an attachment for. C<$timestamp> - scalar - timestamp of the insert as returned @@ -614,7 +692,7 @@ Returns: the ID of the new attachment. =cut sub insert_attachment_for_bug { - my ($class, $throw_error, $bug_id, $user, $timestamp, $hr_vars) = @_; + my ($class, $throw_error, $bug, $user, $timestamp, $hr_vars) = @_; my $cgi = Bugzilla->cgi; my $dbh = Bugzilla->dbh; @@ -671,8 +749,8 @@ sub insert_attachment_for_bug { # Setting the third param to -1 will force this function to check this # point. # XXX needs $throw_error treatment - Bugzilla::Flag::validate($cgi, $bug_id, -1); - Bugzilla::FlagType::validate($cgi, $bug_id, -1); + Bugzilla::Flag::validate($cgi, $bug->bug_id, -1); + Bugzilla::FlagType::validate($cgi, $bug->bug_id, -1); # Escape characters in strings that will be used in SQL statements. my $description = $cgi->param('description'); @@ -684,7 +762,7 @@ sub insert_attachment_for_bug { "INSERT INTO attachments (bug_id, creation_ts, filename, description, mimetype, ispatch, isurl, isprivate, submitter_id) - VALUES (?,?,?,?,?,?,?,?,?)", undef, ($bug_id, $timestamp, $filename, + VALUES (?,?,?,?,?,?,?,?,?)", undef, ($bug->bug_id, $timestamp, $filename, $description, $contenttype, $cgi->param('ispatch'), $isurl, $isprivate, $user->id)); # Retrieve the ID of the newly created attachment record. @@ -724,6 +802,36 @@ sub insert_attachment_for_bug { close AH; close $fh; } + + # Now handle flags. + my @obsolete_attachments; + if ($cgi->param('obsolete')) { + @obsolete_attachments = $class->validate_obsolete($bug->bug_id); + } + + # Make existing attachments obsolete. + my $fieldid = get_field_id('attachments.isobsolete'); + + foreach my $obsolete_attachment (@obsolete_attachments) { + # If the obsolete attachment has request flags, cancel them. + # This call must be done before updating the 'attachments' table. + Bugzilla::Flag::CancelRequests($bug, $obsolete_attachment, $timestamp); + + $dbh->do('UPDATE attachments SET isobsolete = 1 WHERE attach_id = ?', + undef, $obsolete_attachment->id); + + $dbh->do('INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, + fieldid, removed, added) + VALUES (?,?,?,?,?,?,?)', + undef, ($bug->bug_id, $obsolete_attachment->id, $user->id, + $timestamp, $fieldid, 0, 1)); + } + + # Create flags. + my $attachment = Bugzilla::Attachment->get($attachid); + Bugzilla::Flag::process($bug, $attachment, $timestamp, $cgi); + + # Return the ID of the new attachment. return $attachid; } diff --git a/Bugzilla/Component.pm b/Bugzilla/Component.pm index 827be789d..abd3711f5 100644 --- a/Bugzilla/Component.pm +++ b/Bugzilla/Component.pm @@ -13,7 +13,7 @@ # The Original Code is the Bugzilla Bug Tracking System. # # Contributor(s): Tiago R. Mello -# +# Frédéric Buclin use strict; @@ -22,6 +22,7 @@ package Bugzilla::Component; use Bugzilla::Util; use Bugzilla::Error; use Bugzilla::User; +use Bugzilla::FlagType; ############################### #### Initialization #### @@ -135,6 +136,24 @@ sub default_qa_contact { return $self->{'default_qa_contact'}; } +sub flag_types { + my $self = shift; + + if (!defined $self->{'flag_types'}) { + $self->{'flag_types'} = {}; + $self->{'flag_types'}->{'bug'} = + Bugzilla::FlagType::match({ 'target_type' => 'bug', + 'product_id' => $self->product_id, + 'component_id' => $self->id }); + + $self->{'flag_types'}->{'attachment'} = + Bugzilla::FlagType::match({ 'target_type' => 'attachment', + 'product_id' => $self->product_id, + 'component_id' => $self->id }); + } + return $self->{'flag_types'}; +} + ############################### #### Accessors #### ############################### @@ -193,6 +212,8 @@ Bugzilla::Component - Bugzilla product component class. my $product_id = $component->product_id; my $default_assignee = $component->default_assignee; my $default_qa_contact = $component->default_qa_contact; + my $bug_flag_types = $component->flag_types->{'bug'}; + my $attach_flag_types = $component->flag_types->{'attachment'}; my $component = Bugzilla::Component::check_component($product, 'AcmeComp'); @@ -252,6 +273,15 @@ Component.pm represents a Product Component object. Returns: A Bugzilla::User object. +=item C + + Description: Returns all bug and attachment flagtypes available for + the component. + + Params: none. + + Returns: Two references to an array of flagtype objects. + =back =head1 SUBROUTINES diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm index 5498f7670..b88c4eeb8 100644 --- a/Bugzilla/Error.pm +++ b/Bugzilla/Error.pm @@ -78,7 +78,7 @@ sub _throw_error { my $message; $template->process($name, $vars, \$message) || ThrowTemplateError($template->error()); - die("$message"); + die("$message\n"); } else { print Bugzilla->cgi->header(); $template->process($name, $vars) diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm index 995369130..b025cd7cb 100644 --- a/Bugzilla/Product.pm +++ b/Bugzilla/Product.pm @@ -177,6 +177,24 @@ sub user_has_access { undef, $self->id); } +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'}; +} ############################### #### Accessors ###### @@ -231,6 +249,7 @@ Bugzilla::Product - Bugzilla product class. my $bugcount = $product->bug_count(); my $bug_ids = $product->bug_ids(); my $has_access = $product->user_has_access($user); + my $flag_types = $product->flag_types(); my $id = $product->id; my $name = $product->name; @@ -320,6 +339,15 @@ below. Returns C<1> If this user's groups allow him C access to this Product, C<0> otherwise. +=item C + + Description: Returns flag types available for at least one of + its components. + + Params: none. + + Returns: Two references to an array of flagtype objects. + =back =head1 SUBROUTINES diff --git a/attachment.cgi b/attachment.cgi index 94f4fbe5c..7f66c9984 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -206,24 +206,6 @@ sub validateContext return $context; } -sub validateCanEdit -{ - my ($attach_id) = (@_); - my $dbh = Bugzilla->dbh; - - # People in editbugs can edit all attachments - return if UserInGroup("editbugs"); - - # Bug 97729 - the submitter can edit their attachments - my ($ref) = $dbh->selectrow_array("SELECT attach_id FROM attachments - WHERE attach_id = ? - AND submitter_id = ?", - undef, ($attach_id, Bugzilla->user->id)); - - - $ref || ThrowUserError("illegal_attachment_edit",{ attach_id => $attach_id }); -} - sub validateCanChangeAttachment { my ($attachid) = @_; @@ -270,41 +252,6 @@ sub validatePrivate $cgi->param('isprivate', $cgi->param('isprivate') ? 1 : 0); } -sub validateObsolete { - # Make sure the attachment id is valid and the user has permissions to view - # the bug to which it is attached. - my @obsolete_attachments; - foreach my $attachid ($cgi->param('obsolete')) { - my $vars = {}; - $vars->{'attach_id'} = $attachid; - - detaint_natural($attachid) - || ThrowCodeError("invalid_attach_id_to_obsolete", $vars); - - my $attachment = Bugzilla::Attachment->get($attachid); - - # Make sure the attachment exists in the database. - ThrowUserError("invalid_attach_id", $vars) unless $attachment; - - $vars->{'description'} = $attachment->description; - - if ($attachment->bug_id != $cgi->param('bugid')) { - $vars->{'my_bug_id'} = $cgi->param('bugid'); - $vars->{'attach_bug_id'} = $attachment->bug_id; - ThrowCodeError("mismatched_bug_ids_on_obsolete", $vars); - } - - if ($attachment->isobsolete) { - ThrowCodeError("attachment_already_obsolete", $vars); - } - - # Check that the user can modify this attachment - validateCanEdit($attachid); - push(@obsolete_attachments, $attachment); - } - return @obsolete_attachments; -} - # Returns 1 if the parameter is a content-type viewable in this browser # Note that we don't use $cgi->Accept()'s ability to check if a content-type # matches, because this will return a value even if it's matched by the generic @@ -789,38 +736,19 @@ sub insert ValidateComment(scalar $cgi->param('comment')); my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()"); + my $bug = new Bugzilla::Bug($bugid, $user->id); my $attachid = - Bugzilla::Attachment->insert_attachment_for_bug(THROW_ERROR, - $bugid, $user, + Bugzilla::Attachment->insert_attachment_for_bug(THROW_ERROR, $bug, $user, $timestamp, \$vars); - my $isprivate = $cgi->param('isprivate') ? 1 : 0; - my @obsolete_attachments; - @obsolete_attachments = validateObsolete() if $cgi->param('obsolete'); # Insert a comment about the new attachment into the database. my $comment = "Created an attachment (id=$attachid)\n" . $cgi->param('description') . "\n"; $comment .= ("\n" . $cgi->param('comment')) if defined $cgi->param('comment'); + my $isprivate = $cgi->param('isprivate') ? 1 : 0; AppendComment($bugid, $user->id, $comment, $isprivate, $timestamp); - # Make existing attachments obsolete. - my $fieldid = get_field_id('attachments.isobsolete'); - my $bug = new Bugzilla::Bug($bugid, $user->id); - - foreach my $obsolete_attachment (@obsolete_attachments) { - # If the obsolete attachment has request flags, cancel them. - # This call must be done before updating the 'attachments' table. - Bugzilla::Flag::CancelRequests($bug, $obsolete_attachment, $timestamp); - - $dbh->do("UPDATE attachments SET isobsolete = 1 " . - "WHERE attach_id = ?", undef, $obsolete_attachment->id); - $dbh->do("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, - fieldid, removed, added) - VALUES (?,?,?,?,?,?,?)", undef, - $bugid, $obsolete_attachment->id, $user->id, $timestamp, $fieldid, 0, 1); - } - # Assign the bug to the user, if they are allowed to take it my $owner = ""; @@ -862,13 +790,7 @@ sub insert } } } - - # Create flags. - # Update the bug object with updated data. - $bug = new Bugzilla::Bug($bugid, $user->id); - my $attachment = Bugzilla::Attachment->get($attachid); - Bugzilla::Flag::process($bug, $attachment, $timestamp, $cgi); - + # Define the variables and functions that will be passed to the UI template. $vars->{'mailrecipients'} = { 'changer' => $user->login, 'owner' => $owner }; @@ -947,7 +869,7 @@ sub update # Retrieve and validate parameters ValidateComment(scalar $cgi->param('comment')); my ($attach_id, $bugid) = validateID(); - validateCanEdit($attach_id); + Bugzilla::Attachment->validate_can_edit($attach_id); validateCanChangeAttachment($attach_id); Bugzilla::Attachment->validate_description(THROW_ERROR); Bugzilla::Attachment->validate_is_patch(THROW_ERROR); @@ -1113,7 +1035,7 @@ sub delete_attachment { # Make sure the administrator is allowed to edit this attachment. my ($attach_id, $bug_id) = validateID(); - validateCanEdit($attach_id); + Bugzilla::Attachment->validate_can_edit($attach_id); validateCanChangeAttachment($attach_id); my $attachment = Bugzilla::Attachment->get($attach_id); diff --git a/post_bug.cgi b/post_bug.cgi index 083f577ac..6fb054c9f 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -536,11 +536,13 @@ $dbh->do("UPDATE bugs SET creation_ts = ? WHERE bug_id = ?", $dbh->bz_unlock_tables(); +my $bug = new Bugzilla::Bug($id, $user->id); + # Add an attachment if requested. if (defined($cgi->upload('data')) || $cgi->param('attachurl')) { $cgi->param('isprivate', $cgi->param('commentprivacy')); Bugzilla::Attachment->insert_attachment_for_bug(!THROW_ERROR, - $id, $user, $timestamp, + $bug, $user, $timestamp, \$vars) || ($vars->{'message'} = 'attachment_creation_failed'); @@ -551,11 +553,30 @@ if (defined($cgi->upload('data')) || $cgi->param('attachurl')) { }; } +# Add flags, if any. To avoid dying if something goes wrong +# while processing flags, we will eval() flag validation. +# This requires to be in batch mode. +# XXX: this can go away as soon as flag validation is able to +# fail without dying. +Bugzilla->batch(1); +eval { + # Make sure no flags have already been set for this bug. + # Impossible? - Well, depends if you hack the URL or not. + # Passing a bug ID of 0 will make it complain if it finds one. + Bugzilla::Flag::validate($cgi, 0); + Bugzilla::FlagType::validate($cgi, $id); + Bugzilla::Flag::process($bug, undef, $timestamp, $cgi); +}; +Bugzilla->batch(0); +if ($@) { + $vars->{'message'} = 'flag_creation_failed'; + $vars->{'flag_creation_error'} = $@; +} + # Email everyone the details of the new bug $vars->{'mailrecipients'} = {'changer' => $user->login}; $vars->{'id'} = $id; -my $bug = new Bugzilla::Bug($id, $user->id); $vars->{'bug'} = $bug; ThrowCodeError("bug_error", { bug => $bug }) if $bug->error; diff --git a/template/en/default/bug/create/create.html.tmpl b/template/en/default/bug/create/create.html.tmpl index 8b2bcb997..bb4936861 100644 --- a/template/en/default/bug/create/create.html.tmpl +++ b/template/en/default/bug/create/create.html.tmpl @@ -37,6 +37,7 @@ var initialowners = new Array([% product.components.size %]); var last_initialowner; var components = new Array([% product.components.size %]); +var flags = new Array([% product.components.size %]); [% IF Param("useqacontact") %] var initialqacontacts = new Array([% product.components.size %]); var last_initialqacontact; @@ -45,6 +46,17 @@ var components = new Array([% product.components.size %]); [%- FOREACH c = product.components %] components[[% count %]] = "[% c.name FILTER js %]"; initialowners[[% count %]] = "[% c.default_assignee.login FILTER js %]"; + var flag_list = new Array([% c.flag_types.bug.size + c.flag_types.attachment.size %]); + [% flag_count = 0 %] + [% FOREACH f = c.flag_types.bug %] + flag_list[[% flag_count %]] = "[% f.id %]"; + [% flag_count = flag_count + 1 %] + [% END %] + [% FOREACH f = c.flag_types.attachment %] + flag_list[[% flag_count %]] = "[% f.id %]"; + [% flag_count = flag_count + 1 %] + [% END %] + flags[[% count %]] = flag_list; [% IF Param("useqacontact") %] initialqacontacts[[% count %]] = "[% c.default_qa_contact.login FILTER js %]"; [% END %] @@ -53,8 +65,8 @@ var components = new Array([% product.components.size %]); function set_assign_to() { // Based on the selected component, fill the "Assign To:" field - // with the default component owner, and the the "QA Contact:" field - // with the default QA Contact. + // with the default component owner, and the "QA Contact:" field + // with the default QA Contact. It also selectively enables flags. var form = document.Create; var assigned_to = form.assigned_to.value; @@ -87,6 +99,31 @@ function set_assign_to() { last_initialqacontact = contact; } [% END %] + + // First, we disable all flags. Then we re-enable those + // which are available for the selected component. + var inputElements = document.getElementsByTagName("select"); + var inputElement, flagField; + for ( var i=0 ; i -
- - - - +   [%# Migration note: The following file corresponds to the old Param # 'entryheaderhtml' @@ -127,7 +160,8 @@ function handleWantsAttachment(wants_attachment) { -
+   +   @@ -173,7 +207,7 @@ function handleWantsAttachment(wants_attachment) {   - +   @@ -185,29 +219,30 @@ function handleWantsAttachment(wants_attachment) { - [% IF Param('letsubmitterchoosepriority') %] - [% sel = { description => 'Priority', name => 'priority' } %] - [% INCLUDE select %] - [% ELSE %] - - [% END %] + [% IF Param('letsubmitterchoosepriority') %] + [% sel = { description => 'Priority', name => 'priority' } %] + [% INCLUDE select %] + [% ELSE %] + + + + [% END %] - [% sel = { description => 'Severity', name => 'bug_severity' } %] - [% INCLUDE select %] + [% sel = { description => 'Severity', name => 'bug_severity' } %] + [% INCLUDE select %] [% IF Param('usetargetmilestone') && Param('letsubmitterchoosemilestone') %] - [% sel = { description => 'Target Milestone', name => 'target_milestone' } %] - [% INCLUDE select %] - + [% sel = { description => 'Target Milestone', name => 'target_milestone' } %] + [% INCLUDE select %] +   [% END %]   - +   @@ -220,7 +255,19 @@ function handleWantsAttachment(wants_attachment) { [% sel = { description => 'Initial State', name => 'bug_status' } %] [% INCLUDE select %] [% END %] - +   + [%# Calculate the number of rows we can use for flags %] + [% num_rows = 6 + (Param("useqacontact") ? 1 : 0) + + (UserInGroup(Param('timetrackinggroup')) ? 3 : 0) + + (Param("usebugaliases") ? 1 : 0) + %] + + [% IF product.flag_types.bug.size > 0 %] + [% PROCESS "flag/list.html.tmpl" flag_types = product.flag_types.bug + any_flags_requesteeble = 1 + %] + [% END %] + @@ -229,7 +276,7 @@ function handleWantsAttachment(wants_attachment) { Assign To: - + [% INCLUDE global/userselect.html.tmpl name => "assigned_to" value => assigned_to @@ -244,7 +291,7 @@ function handleWantsAttachment(wants_attachment) { [% IF Param("useqacontact") %] QA Contact: - + [% INCLUDE global/userselect.html.tmpl name => "qa_contact" value => qa_contact @@ -259,7 +306,7 @@ function handleWantsAttachment(wants_attachment) { Cc: - + [% INCLUDE global/userselect.html.tmpl name => "cc" value => cc @@ -272,19 +319,19 @@ function handleWantsAttachment(wants_attachment) {   - + [% IF UserInGroup(Param('timetrackinggroup')) %] Estimated Hours: - + Deadline: - + (YYYY-MM-DD) @@ -292,14 +339,14 @@ function handleWantsAttachment(wants_attachment) {   - + [% END %] [% IF Param("usebugaliases") %] Alias: - + @@ -307,7 +354,7 @@ function handleWantsAttachment(wants_attachment) { URL: - + @@ -324,7 +371,7 @@ function handleWantsAttachment(wants_attachment) { Summary: - + @@ -389,7 +436,8 @@ function handleWantsAttachment(wants_attachment) {
Add an attachment - [% PROCESS attachment/createformcontents.html.tmpl %] + [% PROCESS attachment/createformcontents.html.tmpl + flag_types = product.flag_types.attachment %]