From 5d516a6ae2f021d2e276a842c24dff74d3448c45 Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Tue, 10 Nov 2009 01:36:02 +0000 Subject: Bug 472217: Create a Bugzilla::Comment object and eliminate GetComments Patch by Max Kanat-Alexander r=LpSolit, a=LpSolit --- Bugzilla/Bug.pm | 115 ++++++++++-------------------- Bugzilla/BugMail.pm | 39 +--------- Bugzilla/Comment.pm | 173 +++++++++++++++++++++++++++++++++++++++++++++ Bugzilla/Template.pm | 2 +- Bugzilla/User.pm | 2 +- Bugzilla/Util.pm | 83 +++++++++++++--------- Bugzilla/WebService/Bug.pm | 40 +++++------ 7 files changed, 282 insertions(+), 172 deletions(-) create mode 100644 Bugzilla/Comment.pm (limited to 'Bugzilla') diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 14fe20c02..2a248905d 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -46,6 +46,7 @@ use Bugzilla::Product; use Bugzilla::Component; use Bugzilla::Group; use Bugzilla::Status; +use Bugzilla::Comment; use List::Util qw(min); use Storable qw(dclone); @@ -1835,12 +1836,12 @@ sub set_cclist_accessible { $_[0]->set('cclist_accessible', $_[1]); } sub set_comment_is_private { my ($self, $comment_id, $isprivate) = @_; return unless Bugzilla->user->is_insider; - my ($comment) = grep($comment_id eq $_->{id}, @{$self->longdescs}); + my ($comment) = grep($comment_id == $_->id, @{ $self->comments }); ThrowUserError('comment_invalid_isprivate', { id => $comment_id }) if !$comment; $isprivate = $isprivate ? 1 : 0; - if ($isprivate != $comment->{isprivate}) { + if ($isprivate != $comment->is_private) { $self->{comment_isprivate} ||= {}; $self->{comment_isprivate}->{$comment_id} = $isprivate; } @@ -2665,12 +2666,38 @@ sub keyword_objects { return $self->{'keyword_objects'}; } -sub longdescs { - my ($self) = @_; - return $self->{'longdescs'} if exists $self->{'longdescs'}; +sub comments { + my ($self, $params) = @_; return [] if $self->{'error'}; - $self->{'longdescs'} = GetComments($self->{bug_id}); - return $self->{'longdescs'}; + $params ||= {}; + + if (!defined $self->{'comments'}) { + $self->{'comments'} = Bugzilla::Comment->match({ bug_id => $self->id }); + my $count = 0; + foreach my $comment (@{ $self->{'comments'} }) { + $comment->{count} = $count++; + } + } + my @comments = @{ $self->{'comments'} }; + + my $order = $params->{order} + || Bugzilla->user->settings->{'comment_sort_order'}->{'value'}; + if ($order ne 'oldest_to_newest') { + @comments = reverse @comments; + if ($order eq 'newest_to_oldest_desc_first') { + unshift(@comments, pop @comments); + } + } + + if ($params->{after}) { + my $from = datetime_from($params->{after}); + @comments = grep { datetime_from($_->creation_ts) > $from } @comments; + } + if ($params->{to}) { + my $to = datetime_from($params->{to}); + @comments = grep { datetime_from($_->creation_ts) <= $to } @comments; + } + return \@comments; } sub milestoneurl { @@ -2955,7 +2982,7 @@ sub update_comment { || ThrowCodeError('bad_arg', {argument => 'comment_id', function => 'update_comment'}); # The comment ID must belong to this bug. - my @current_comment_obj = grep {$_->{'id'} == $comment_id} @{$self->longdescs}; + my @current_comment_obj = grep {$_->id == $comment_id} @{$self->comments}; scalar(@current_comment_obj) || ThrowCodeError('bad_arg', {argument => 'comment_id', function => 'update_comment'}); @@ -2972,7 +2999,7 @@ sub update_comment { $self->_sync_fulltext(); # Update the comment object with this new text. - $current_comment_obj[0]->{'body'} = $new_comment; + $current_comment_obj[0]->{'thetext'} = $new_comment; } # Represents which fields from the bugs table are handled by process_bug.cgi. @@ -3032,74 +3059,6 @@ sub ValidateTime { } } -sub GetComments { - my ($id, $comment_sort_order, $start, $end, $raw) = @_; - my $dbh = Bugzilla->dbh; - - $comment_sort_order = $comment_sort_order || - Bugzilla->user->settings->{'comment_sort_order'}->{'value'}; - - my $sort_order = ($comment_sort_order eq "oldest_to_newest") ? 'asc' : 'desc'; - - my @comments; - my @args = ($id); - - my $query = 'SELECT longdescs.comment_id AS id, profiles.userid, ' . - $dbh->sql_date_format('longdescs.bug_when', '%Y.%m.%d %H:%i:%s') . - ' AS time, longdescs.thetext AS body, longdescs.work_time, - isprivate, already_wrapped, type, extra_data - FROM longdescs - INNER JOIN profiles - ON profiles.userid = longdescs.who - WHERE longdescs.bug_id = ?'; - - if ($start) { - $query .= ' AND longdescs.bug_when > ?'; - push(@args, $start); - } - if ($end) { - $query .= ' AND longdescs.bug_when <= ?'; - push(@args, $end); - } - - $query .= " ORDER BY longdescs.bug_when $sort_order"; - my $sth = $dbh->prepare($query); - $sth->execute(@args); - - # Cache the users we look up - my %users; - - while (my $comment_ref = $sth->fetchrow_hashref()) { - my %comment = %$comment_ref; - $users{$comment{'userid'}} ||= new Bugzilla::User($comment{'userid'}); - $comment{'author'} = $users{$comment{'userid'}}; - - # If raw data is requested, do not format 'special' comments. - $comment{'body'} = format_comment(\%comment) unless $raw; - - push (@comments, \%comment); - } - - if ($comment_sort_order eq "newest_to_oldest_desc_first") { - unshift(@comments, pop @comments); - } - - return \@comments; -} - -# Format language specific comments. -sub format_comment { - my $comment = shift; - my $template = Bugzilla->template_inner; - my $vars = {comment => $comment}; - my $body; - - $template->process("bug/format_comment.txt.tmpl", $vars, \$body) - || ThrowTemplateError($template->error()); - $body =~ s/^X//; - return $body; -} - # Get the activity of a bug, starting from $starttime (if given). # This routine assumes Bugzilla::Bug->check has been previously called. sub GetBugActivity { @@ -3644,7 +3603,7 @@ sub _validate_attribute { my @valid_attributes = ( # Miscellaneous properties and methods. qw(error groups product_id component_id - longdescs milestoneurl attachments + comments milestoneurl attachments isopened isunconfirmed flag_types num_attachment_flag_types show_attachment_flags any_flags_requesteeble), diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index d654a139b..2aff7daf4 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -119,6 +119,7 @@ sub Send { my $msg = ""; my $dbh = Bugzilla->dbh; + my $bug = new Bugzilla::Bug($id); # XXX - These variables below are useless. We could use field object # methods directly. But we first have to implement a cache in @@ -356,7 +357,7 @@ sub Send { } } - my $comments = get_comments_by_bug($id, $start, $end); + my $comments = $bug->comments({ after => $start, to => $end }); ########################################################################### # Start of email filtering code @@ -569,7 +570,7 @@ sub sendMail { } if (!$user->is_insider) { - @send_comments = grep { !$_->{isprivate} } @send_comments; + @send_comments = grep { !$_->is_private } @send_comments; } if ($difftext eq "" && !scalar(@send_comments) && !$isnew) { @@ -650,38 +651,4 @@ sub sendMail { return 1; } -# Get bug comments for the given period. -sub get_comments_by_bug { - my ($id, $start, $end) = @_; - my $dbh = Bugzilla->dbh; - - my $result = ""; - my $count = 0; - - # $start will be undef for new bugs, and defined for pre-existing bugs. - if ($start) { - # If $start is not NULL, obtain the count-index - # of this comment for the leading "Comment #xxx" line. - $count = $dbh->selectrow_array('SELECT COUNT(*) FROM longdescs - WHERE bug_id = ? AND bug_when <= ?', - undef, ($id, $start)); - } - - my $raw = 1; # Do not format comments which are not of type CMT_NORMAL. - my $comments = Bugzilla::Bug::GetComments($id, "oldest_to_newest", $start, $end, $raw); - my $attach_base = correct_urlbase() . 'attachment.cgi?id='; - - foreach my $comment (@$comments) { - $comment->{count} = $count++; - # If an attachment was created, then add an URL. (Note: the 'g'lobal - # replace should work with comments with multiple attachments.) - if ($comment->{body} =~ /Created an attachment \(/) { - $comment->{body} =~ s/(Created an attachment \(id=([0-9]+)\))/$1\n --> \($attach_base$2\)/g; - } - $comment->{body} = $comment->{'already_wrapped'} ? $comment->{body} : wrap_comment($comment->{body}); - } - - return $comments; -} - 1; diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm new file mode 100644 index 000000000..7072dbbee --- /dev/null +++ b/Bugzilla/Comment.pm @@ -0,0 +1,173 @@ +# -*- Mode: perl; indent-tabs-mode: nil -*- +# +# The contents of this file are subject to the Mozilla Public +# License Version 1.1 (the "License"); you may not use this file +# except in compliance with the License. You may obtain a copy of +# the License at http://www.mozilla.org/MPL/ +# +# Software distributed under the License is distributed on an "AS +# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or +# implied. See the License for the specific language governing +# rights and limitations under the License. +# +# The Original Code is the Bugzilla Bug Tracking System. +# +# The Initial Developer of the Original Code is James Robson. +# Portions created by James Robson are Copyright (c) 2009 James Robson. +# All rights reserved. +# +# Contributor(s): James Robson + +use strict; + +package Bugzilla::Comment; + +use base qw(Bugzilla::Object); + +use Bugzilla::Constants; +use Bugzilla::Error; +use Bugzilla::Util; + +############################### +#### Initialization #### +############################### + +use constant DB_COLUMNS => qw( + comment_id + bug_id + who + bug_when + work_time + thetext + isprivate + already_wrapped + type + extra_data +); + +use constant DB_TABLE => 'longdescs'; +use constant ID_FIELD => 'comment_id'; +use constant LIST_ORDER => 'bug_when'; + +############################### +#### Accessors ###### +############################### + +sub already_wrapped { return $_[0]->{'already_wrapped'}; } +sub body { return $_[0]->{'thetext'}; } +sub bug_id { return $_[0]->{'bug_id'}; } +sub creation_ts { return $_[0]->{'bug_when'}; } +sub is_private { return $_[0]->{'isprivate'}; } +sub work_time { return $_[0]->{'work_time'}; } + +sub author { + my $self = shift; + $self->{'author'} ||= new Bugzilla::User($self->{'who'}); + return $self->{'author'}; +} + +sub body_full { + my ($self, $params) = @_; + $params ||= {}; + my $template = Bugzilla->template_inner; + my $body; + $template->process("bug/format_comment.txt.tmpl", + { comment => $self, %$params }, \$body) + || ThrowTemplateError($template->error()); + $body =~ s/^X//; + if ($params->{wrap} and !$self->already_wrapped) { + $body = wrap_comment($body); + } + return $body; +} + +1; + +__END__ + +=head1 NAME + +Bugzilla::Comment - A Comment for a given bug + +=head1 SYNOPSIS + + use Bugzilla::Comment; + + my $comment = Bugzilla::Comment->new($comment_id); + my $comments = Bugzilla::Comment->new_from_list($comment_ids); + +=head1 DESCRIPTION + +Bugzilla::Comment represents a comment attached to a bug. + +This implements all standard C methods. See +L for more details. + +=head2 Accessors + +=over + +=item C + +C The ID of the bug to which the comment belongs. + +=item C + +C The comment creation timestamp. + +=item C + +C The body without any special additional text. + +=item C + +C Time spent as related to this comment. + +=item C + +C Comment is marked as private + +=item C + +If this comment is stored in the database word-wrapped, this will be C<1>. +C<0> otherwise. + +=item C + +L who created the comment. + +=item C + +=over + +=item B + +C Body of the comment, including any special text (such as +"this bug was marked as a duplicate of..."). + +=item B + +=over + +=item C + +C. C<1> if this comment should be formatted specifically for +bugmail. + +=item C + +C. C<1> if the comment should be returned word-wrapped. + +=back + +=item B + +A string, the full text of the comment as it would be displayed to an end-user. + +=back + + + +=back + +=cut diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index d5e371f64..fa7247243 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -170,7 +170,7 @@ sub quoteUrls { # If the comment is already wrapped, we should ignore newlines when # looking for matching regexps. Else we should take them into account. - my $s = ($comment && $comment->{already_wrapped}) + my $s = ($comment && $comment->already_wrapped) ? qr/\s/ : qr/[[:blank:]]/; # However, note that adding the title (for buglinks) can affect things diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 3843062fb..0fc2db336 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -1390,7 +1390,7 @@ sub wants_bug_mail { my $self = shift; my ($bug_id, $relationship, $fieldDiffs, $comments, $dependencyText, $changer, $bug_is_new) = @_; - my $comments_concatenated = join("\n", map { $_->{body} } (@$comments)); + my $comments_concatenated = join("\n", map { $_->body } @$comments); # Make a list of the events which have happened during this bug change, # from the point of view of this user. diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 21588417c..d1a52a5fc 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -40,7 +40,7 @@ use base qw(Exporter); diff_arrays trim wrap_hard wrap_comment find_wrap_point format_time format_time_decimal validate_date - validate_time + validate_time datetime_from file_mod_time is_7bit_clean bz_crypt generate_random_password validate_email_syntax clean_text @@ -396,7 +396,9 @@ sub format_time { # If $format is undefined, try to guess the correct date format. if (!defined($format)) { - if ($date =~ m/^(\d{4})[-\.](\d{2})[-\.](\d{2}) (\d{2}):(\d{2})(:(\d{2}))?$/) { + if (!ref $date + && $date =~ /^(\d{4})[-\.](\d{2})[-\.](\d{2}) (\d{2}):(\d{2})(:(\d{2}))?$/) + { my $sec = $7; if (defined $sec) { $format = "%Y-%m-%d %T %Z"; @@ -409,44 +411,49 @@ sub format_time { } } - # strptime($date) returns an empty array if $date has an invalid date format. + my $dt = ref $date ? $date : datetime_from($date, $timezone); + $date = defined $dt ? $dt->strftime($format) : ''; + return trim($date); +} + +sub datetime_from { + my ($date, $timezone) = @_; + + # strptime($date) returns an empty array if $date has an invalid + # date format. my @time = strptime($date); unless (scalar @time) { - # If an unknown timezone is passed (such as MSK, for Moskow), strptime() is - # unable to parse the date. We try again, but we first remove the timezone. + # If an unknown timezone is passed (such as MSK, for Moskow), + # strptime() is unable to parse the date. We try again, but we first + # remove the timezone. $date =~ s/\s+\S+$//; @time = strptime($date); } - if (scalar @time) { - # Fix a bug in strptime() where seconds can be undefined in some cases. - $time[0] ||= 0; - - # strptime() counts years from 1900, and months from 0 (January). - # We have to fix both values. - my $dt = DateTime->new({year => 1900 + $time[5], - month => ++$time[4], - day => $time[3], - hour => $time[2], - minute => $time[1], - # DateTime doesn't like fractional seconds. - second => int($time[0]), - # If importing, use the specified timezone, otherwise - # use the timezone specified by the server. - time_zone => Bugzilla->local_timezone->offset_as_string($time[6]) - || Bugzilla->local_timezone}); - - # Now display the date using the given timezone, - # or the user's timezone if none is given. - $dt->set_time_zone($timezone || Bugzilla->user->timezone); - $date = $dt->strftime($format); - } - else { - # Don't let invalid (time) strings to be passed to templates! - $date = ''; - } - return trim($date); + return undef if !@time; + + # strptime() counts years from 1900, and months from 0 (January). + # We have to fix both values. + my $dt = DateTime->new({ + year => $time[5] + 1900, + month => $time[4] + 1, + day => $time[3], + hour => $time[2], + minute => $time[1], + # DateTime doesn't like fractional seconds. + # Also, sometimes seconds are undef. + second => int($time[0] || 0), + # If a timezone was specified, use it. Otherwise, use the + # local timezone. + time_zone => Bugzilla->local_timezone->offset_as_string($time[6]) + || Bugzilla->local_timezone, + }); + + # Now display the date using the given timezone, + # or the user's timezone if none is given. + $dt->set_time_zone($timezone || Bugzilla->user->timezone); + return $dt; } sub format_time_decimal { @@ -641,6 +648,7 @@ Bugzilla::Util - Generic utility functions for bugzilla # Functions for formatting time format_time($time); + datetime_from($time, $timezone); # Functions for dealing with files $time = file_mod_time($filename); @@ -894,6 +902,15 @@ This routine is mainly called from templates to filter dates, see Returns a number with 2 digit precision, unless the last digit is a 0. Then it returns only 1 digit precision. +=item C + +Returns a DateTime object given a date string. If the string is not in some +valid date format that C understands, we return C. + +You can optionally specify a timezone for the returned date. If not +specified, defaults to the currently-logged-in user's timezone, or +the Bugzilla server's local timezone if there isn't a logged-in user. + =back diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index 6a3e93519..aea30cedb 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -23,6 +23,7 @@ package Bugzilla::WebService::Bug; use strict; use base qw(Bugzilla::WebService); +use Bugzilla::Comment; use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Field; @@ -95,12 +96,12 @@ sub comments { foreach my $bug_id (@$bug_ids) { my $bug = Bugzilla::Bug->check($bug_id); # We want the API to always return comments in the same order. - my $comments = Bugzilla::Bug::GetComments( - $bug->id, 'oldest_to_newest', $params->{new_since}); + + my $comments = $bug->comments({ order => 'oldest_to_newest', + after => $params->{new_since} }); my @result; foreach my $comment (@$comments) { - next if $comment->{isprivate} && !$user->is_insider; - $comment->{bug_id} = $bug->id; + next if $comment->is_private && !$user->is_insider; push(@result, $self->_translate_comment($comment, $params)); } $bugs{$bug->id}{'comments'} = \@result; @@ -109,15 +110,10 @@ sub comments { my %comments; if (scalar @$comment_ids) { my @ids = map { trim($_) } @$comment_ids; - my @sql_ids = map { $dbh->quote($_) } @ids; - my $comment_data = $dbh->selectall_arrayref( - 'SELECT comment_id AS id, bug_id, who, bug_when AS time, - isprivate, thetext AS body, type, extra_data - FROM longdescs WHERE ' . $dbh->sql_in('comment_id', \@sql_ids), - {Slice=>{}}); + my $comment_data = Bugzilla::Comment->new_from_list(\@ids); # See if we were passed any invalid comment ids. - my %got_ids = map { $_->{id} => 1 } @$comment_data; + my %got_ids = map { $_->id => 1 } @$comment_data; foreach my $comment_id (@ids) { if (!$got_ids{$comment_id}) { ThrowUserError('comment_id_invalid', { id => $comment_id }); @@ -125,16 +121,14 @@ sub comments { } # Now make sure that we can see all the associated bugs. - my %got_bug_ids = map { $_->{bug_id} => 1 } @$comment_data; + my %got_bug_ids = map { $_->bug_id => 1 } @$comment_data; Bugzilla::Bug->check($_) foreach (keys %got_bug_ids); foreach my $comment (@$comment_data) { - if ($comment->{isprivate} && !$user->is_insider) { - ThrowUserError('comment_is_private', { id => $comment->{id} }); + if ($comment->is_private && !$user->is_insider) { + ThrowUserError('comment_is_private', { id => $comment->id }); } - $comment->{author} = new Bugzilla::User($comment->{who}); - $comment->{body} = Bugzilla::Bug::format_comment($comment); - $comments{$comment->{id}} = + $comments{$comment->id} = $self->_translate_comment($comment, $params); } } @@ -146,12 +140,12 @@ sub comments { sub _translate_comment { my ($self, $comment, $filters) = @_; return filter $filters, { - id => $self->type('int', $comment->{id}), - bug_id => $self->type('int', $comment->{bug_id}), - author => $self->type('string', $comment->{author}->login), - time => $self->type('dateTime', $comment->{'time'}), - is_private => $self->type('boolean', $comment->{isprivate}), - text => $self->type('string', $comment->{body}), + id => $self->type('int', $comment->id), + bug_id => $self->type('int', $comment->bug_id), + author => $self->type('string', $comment->author->login), + time => $self->type('dateTime', $comment->creation_ts), + is_private => $self->type('boolean', $comment->is_private), + text => $self->type('string', $comment->body_full), }; } -- cgit v1.2.3-24-g4f1b