diff options
| -rw-r--r-- | Bugzilla/Attachment.pm | 60 | ||||
| -rw-r--r-- | Bugzilla/Bug.pm | 22 | ||||
| -rw-r--r-- | Bugzilla/Flag.pm | 38 | ||||
| -rwxr-xr-x | attachment.cgi | 20 | ||||
| -rw-r--r-- | template/en/default/attachment/edit.html.tmpl | 8 |
5 files changed, 107 insertions, 41 deletions
diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index 314227c87..fcaf38c9f 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -139,8 +139,6 @@ the ID of the bug to which the attachment is attached =cut -# XXX Once Bug.pm slims down sufficiently this should become a reference -# to a bug object. sub bug_id { my $self = shift; return $self->{bug_id}; @@ -148,6 +146,24 @@ sub bug_id { =over +=item C<bug> + +the bug object to which the attachment is attached + +=back + +=cut + +sub bug { + my $self = shift; + + require Bugzilla::Bug; + $self->{bug} = Bugzilla::Bug->new($self->bug_id); + return $self->{bug}; +} + +=over + =item C<description> user-provided text describing the attachment @@ -430,6 +446,30 @@ sub flags { return $self->{flags}; } +=over + +=item C<flag_types> + +Return all flag types available for this attachment as well as flags +already set, grouped by flag type. + +=back + +=cut + +sub flag_types { + my $self = shift; + return $self->{flag_types} if exists $self->{flag_types}; + + my $vars = { target_type => 'attachment', + product_id => $self->bug->product_id, + component_id => $self->bug->component_id, + attach_id => $self->id }; + + $self->{flag_types} = Bugzilla::Flag::_flag_types($vars); + return $self->{flag_types}; +} + # Instance methods; no POD documentation here yet because the only ones so far # are private. @@ -538,7 +578,7 @@ Returns: a reference to an array of attachment objects. =cut sub get_attachments_by_bug { - my ($class, $bug_id) = @_; + my ($class, $bug_id, $vars) = @_; my $user = Bugzilla->user; my $dbh = Bugzilla->dbh; @@ -556,6 +596,20 @@ sub get_attachments_by_bug { WHERE bug_id = ? $and_restriction", undef, @values); my $attachments = Bugzilla::Attachment->get_list($attach_ids); + + # To avoid $attachment->flags to run SQL queries itself for each + # attachment listed here, we collect all the data at once and + # populate $attachment->{flags} ourselves. + if ($vars->{preload}) { + $_->{flags} = [] foreach @$attachments; + my %att = map { $_->id => $_ } @$attachments; + + my $flags = Bugzilla::Flag->match({ bug_id => $bug_id, + target_type => 'attachment' }); + + push(@{$att{$_->attach_id}->{flags}}, $_) foreach @$flags; + $attachments = [sort {$a->id <=> $b->id} values %att]; + } return $attachments; } diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index d625953a9..14f781c42 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -2314,7 +2314,7 @@ sub attachments { return [] if $self->{'error'}; $self->{'attachments'} = - Bugzilla::Attachment->get_attachments_by_bug($self->bug_id); + Bugzilla::Attachment->get_attachments_by_bug($self->bug_id, {preload => 1}); return $self->{'attachments'}; } @@ -2421,22 +2421,12 @@ sub flag_types { return $self->{'flag_types'} if exists $self->{'flag_types'}; return [] if $self->{'error'}; - # The types of flags that can be set on this bug. - # If none, no UI for setting flags will be displayed. - my $flag_types = Bugzilla::FlagType::match( - {'target_type' => 'bug', - 'product_id' => $self->{'product_id'}, - 'component_id' => $self->{'component_id'} }); - - foreach my $flag_type (@$flag_types) { - $flag_type->{'flags'} = Bugzilla::Flag->match( - { 'bug_id' => $self->bug_id, - 'type_id' => $flag_type->{'id'}, - 'target_type' => 'bug' }); - } - - $self->{'flag_types'} = $flag_types; + my $vars = { target_type => 'bug', + product_id => $self->{product_id}, + component_id => $self->{component_id}, + bug_id => $self->bug_id }; + $self->{'flag_types'} = Bugzilla::Flag::_flag_types($vars); return $self->{'flag_types'}; } diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index 8201a907d..ea3e940d8 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -1158,6 +1158,44 @@ sub CancelRequests { \@old_summaries, \@new_summaries); } +# This is an internal function used by $bug->flag_types +# and $attachment->flag_types to collect data about available +# flag types and existing flags set on them. You should never +# call this function directly. +sub _flag_types { + my $vars = shift; + + my $target_type = $vars->{target_type}; + my $flags; + + # Retrieve all existing flags for this bug/attachment. + if ($target_type eq 'bug') { + my $bug_id = delete $vars->{bug_id}; + $flags = Bugzilla::Flag->match({target_type => 'bug', bug_id => $bug_id}); + } + elsif ($target_type eq 'attachment') { + my $attach_id = delete $vars->{attach_id}; + $flags = Bugzilla::Flag->match({attach_id => $attach_id}); + } + else { + ThrowCodeError('bad_arg', {argument => 'target_type', + function => 'Bugzilla::Flag::_flag_types'}); + } + + # Get all available flag types for the given product and component. + my $flag_types = Bugzilla::FlagType::match($vars); + + $_->{flags} = [] foreach @$flag_types; + my %flagtypes = map { $_->id => $_ } @$flag_types; + + # Group existing flags per type. + # Call the internal 'type_id' variable instead of the method + # to not create a flagtype object. + push(@{$flagtypes{$_->{type_id}}->{flags}}, $_) foreach @$flags; + + return [sort {$a->sortkey <=> $b->sortkey || $a->name cmp $b->name} values %flagtypes]; +} + =head1 SEE ALSO =over diff --git a/attachment.cgi b/attachment.cgi index c28a300a0..4f3dabd55 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -435,32 +435,14 @@ sub insert { # Validations are done later when the user submits changes. sub edit { my $attachment = validateID(); - my $dbh = Bugzilla->dbh; - # Retrieve a list of attachments for this bug as well as a summary of the bug - # to use in a navigation bar across the top of the screen. my $bugattachments = Bugzilla::Attachment->get_attachments_by_bug($attachment->bug_id); # We only want attachment IDs. @$bugattachments = map { $_->id } @$bugattachments; - my ($bugsummary, $product_id, $component_id) = - $dbh->selectrow_array('SELECT short_desc, product_id, component_id - FROM bugs - WHERE bug_id = ?', undef, $attachment->bug_id); - - # Get a list of flag types that can be set for this attachment. - my $flag_types = Bugzilla::FlagType::match({ 'target_type' => 'attachment' , - 'product_id' => $product_id , - 'component_id' => $component_id }); - foreach my $flag_type (@$flag_types) { - $flag_type->{'flags'} = Bugzilla::Flag->match({ 'type_id' => $flag_type->id, - 'attach_id' => $attachment->id }); - } - $vars->{'flag_types'} = $flag_types; - $vars->{'any_flags_requesteeble'} = grep($_->is_requesteeble, @$flag_types); + $vars->{'any_flags_requesteeble'} = grep($_->is_requesteeble, @{$attachment->flag_types}); $vars->{'attachment'} = $attachment; - $vars->{'bugsummary'} = $bugsummary; $vars->{'attachments'} = $bugattachments; print $cgi->header(); diff --git a/template/en/default/attachment/edit.html.tmpl b/template/en/default/attachment/edit.html.tmpl index 49c28aa64..8d0422065 100644 --- a/template/en/default/attachment/edit.html.tmpl +++ b/template/en/default/attachment/edit.html.tmpl @@ -29,7 +29,7 @@ Attachment [% attachment.id %] Details for [%+ "$terms.Bug ${attachment.bug_id}" FILTER bug_link(attachment.bug_id) FILTER none %] [% END %] -[% subheader = BLOCK %][% bugsummary FILTER html %][% END %] +[% subheader = BLOCK %][% attachment.bug.short_desc FILTER html %][% END %] [% PROCESS global/header.html.tmpl title = title @@ -250,9 +250,11 @@ <br> </small> - [% IF flag_types.size > 0 %] + [% IF attachment.flag_types.size > 0 %] [% PROCESS "flag/list.html.tmpl" bug_id = attachment.bug_id - attach_id = attachment.id %]<br> + attach_id = attachment.id + flag_types = attachment.flag_types + %]<br> [% END %] <div id="smallCommentFrame"> |