summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorlpsolit%gmail.com <>2008-09-08 23:21:24 +0200
committerlpsolit%gmail.com <>2008-09-08 23:21:24 +0200
commit4dd427ea99673391d923db9682836d344f178b54 (patch)
tree15cac366546a970dfbe8a0ed7163976f09dfbfb2
parent1d66aab134be7002268c4da177bbf4870ac90d6d (diff)
downloadbugzilla-4dd427ea99673391d923db9682836d344f178b54.tar.gz
bugzilla-4dd427ea99673391d923db9682836d344f178b54.tar.xz
Bug 453743: Decrease the number of calls to the DB about flags when viewing a bug - Patch by Frédéric Buclin <LpSolit@gmail.com> r/a=mkanat
-rw-r--r--Bugzilla/Attachment.pm60
-rw-r--r--Bugzilla/Bug.pm22
-rw-r--r--Bugzilla/Flag.pm38
-rwxr-xr-xattachment.cgi20
-rw-r--r--template/en/default/attachment/edit.html.tmpl8
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">