Bug 351888: Move comment creation out of post_bug.cgi and into Bugzilla::Bug

Bugzilla/Bug.pm

@@ -116,6 +116,8 @@ sub VALIDATORS {
         bug_file_loc   => \&_check_bug_file_loc,
         bug_severity   => \&_check_bug_severity,
         cc             => \&_check_cc,
+        comment        => \&_check_comment,
+        commentprivacy => \&_check_commentprivacy,
         deadline       => \&_check_deadline,
         estimated_time => \&_check_estimated_time,
         keywords       => \&_check_keywords,
@@ -253,6 +255,9 @@ sub create {
     delete $params->{dependson};
     my $blocked = $params->{blocked};
     delete $params->{blocked};
+    my ($comment, $privacy) = ($params->{comment}, $params->{commentprivacy});
+    delete $params->{comment};
+    delete $params->{commentprivacy};
 
     # Set up the keyword cache for bug creation.
     my $keywords = $params->{keywords};
@@ -264,6 +269,10 @@ sub create {
     my $timestamp = $params->{creation_ts}; 
     delete $params->{creation_ts};
 
+    $dbh->bz_lock_tables('bugs WRITE', 'bug_group_map WRITE', 
+        'longdescs WRITE', 'cc WRITE', 'keywords WRITE', 'dependencies WRITE',
+        'bugs_activity WRITE', 'fielddefs READ');
+
     my $bug = $class->insert_create_data($params);
 
     # Add the group restrictions
@@ -296,15 +305,23 @@ sub create {
         $sth_deps->execute($bug->bug_id, $depends_on_id);
         # Log the reverse action on the other bug.
         LogActivityEntry($depends_on_id, 'blocked', '', $bug->bug_id,
-                         $bug->reporter->id, $timestamp);
+                         $bug->{reporter_id}, $timestamp);
     }
     foreach my $blocked_id (@$blocked) {
         $sth_deps->execute($blocked_id, $bug->bug_id);
         # Log the reverse action on the other bug.
         LogActivityEntry($blocked_id, 'dependson', '', $bug->bug_id,
-                         $bug->reporter->id, $timestamp);
+                         $bug->{reporter_id}, $timestamp);
     }
 
+    # And insert the comment. We always insert a comment on bug creation,
+    # but sometimes it's blank.
+    $dbh->do('INSERT INTO longdescs (bug_id, who, bug_when, thetext, isprivate)
+                   VALUES (?, ?, ?, ?, ?)', undef, 
+             $bug->bug_id, $bug->{reporter_id}, $timestamp, $comment, $privacy);
+
+    $dbh->bz_unlock_tables();
+
     return $bug;
 }
 
@@ -504,9 +521,7 @@ sub _check_cc {
 sub _check_comment {
     my ($invocant, $comment) = @_;
 
-    if (!defined $comment) {
-        ThrowCodeError('undefined_field', { field => 'comment' });
-    }
+    $comment = '' unless defined $comment;
 
     # Remove any trailing whitespace. Leading whitespace could be
     # a valid part of the comment.
@@ -519,9 +534,20 @@ sub _check_comment {
         ThrowUserError("description_required");
     }
 
+    # On creation only, there must be a single-space comment, or
+    # email will be supressed.
+    $comment = ' ' if $comment eq '' && !ref($invocant);
+
     return $comment;
 }
 
+sub _check_commentprivacy {
+    my ($invocant, $comment_privacy) = @_;
+    my $insider_group = Bugzilla->params->{"insidergroup"};
+    return ($insider_group && Bugzilla->user->in_group($insider_group) 
+            && $comment_privacy) ? 1 : 0;
+}
+
 sub _check_component {
     my ($invocant, $product, $name) = @_;
     $name = trim($name);

Bugzilla/DB/Schema.pm

@@ -270,7 +270,7 @@ use constant ABSTRACT_SCHEMA => {
             bug_when        => {TYPE => 'DATETIME', NOTNULL => 1},
             work_time       => {TYPE => 'decimal(5,2)', NOTNULL => 1,
                                 DEFAULT => '0'},
-            thetext         => {TYPE => 'MEDIUMTEXT'},
+            thetext         => {TYPE => 'MEDIUMTEXT', NOTNULL => 1},
             isprivate       => {TYPE => 'BOOLEAN', NOTNULL => 1,
                                 DEFAULT => 'FALSE'},
             already_wrapped => {TYPE => 'BOOLEAN', NOTNULL => 1,

Bugzilla/Install/DB.pm

@@ -493,6 +493,9 @@ sub update_table_definitions {
 
     $dbh->bz_add_column('setting', 'subclass', {TYPE => 'varchar(32)'});
 
+    $dbh->bz_alter_column('longdescs', 'thetext', 
+        { TYPE => 'MEDIUMTEXT', NOTNULL => 1 }, '');
+
     ################################################################
     # New --TABLE-- changes should go *** A B O V E *** this point #
     ################################################################

post_bug.cgi

@@ -88,29 +88,6 @@ if ($token) {
     '^requestee_type-(\d+)$' => { 'type' => 'multi' },
 });
 
-# The format of the initial comment can be structured by adding fields to the
-# enter_bug template and then referencing them in the comment template.
-my $comment;
-
-my $format = $template->get_format("bug/create/comment",
-                                   scalar($cgi->param('format')), "txt");
-
-$template->process($format->{'template'}, $vars, \$comment)
-  || ThrowTemplateError($template->error());
-
-# Check that the product exists and that the user
-# is allowed to enter bugs into this product.
-my $product = Bugzilla::Bug->_check_product($cgi->param('product'));
-
-# Set cookies
-if (defined $cgi->param('product')) {
-    if (defined $cgi->param('version')) {
-        $cgi->send_cookie(-name => "VERSION-" . $product->name,
-                          -value => $cgi->param('version'),
-                          -expires => "Fri, 01-Jan-2038 00:00:00 GMT");
-    }
-}
-
 if (defined $cgi->param('maketemplate')) {
     $vars->{'url'} = $cgi->query_string();
     $vars->{'short_desc'} = $cgi->param('short_desc');
@@ -123,13 +100,6 @@ if (defined $cgi->param('maketemplate')) {
 
 umask 0;
 
-# This has to go somewhere after 'maketemplate' 
-# or it breaks bookmarks with no comments. 
-$comment = Bugzilla::Bug->_check_comment($cgi->param('comment'));
-# If comment is all whitespace, it'll be null at this point. That's
-# OK except for the fact that it causes e-mail to be suppressed.
-$comment = $comment ? $comment : " ";
-
 # get current time
 my $timestamp = $dbh->selectrow_array(q{SELECT NOW()});
 
@@ -140,6 +110,14 @@ foreach my $group (grep(/^bit-\d+$/, $cgi->param())) {
     push(@selected_groups, $1);
 }
 
+# The format of the initial comment can be structured by adding fields to the
+# enter_bug template and then referencing them in the comment template.
+my $comment;
+my $format = $template->get_format("bug/create/comment",
+                                   scalar($cgi->param('format')), "txt");
+$template->process($format->{'template'}, $vars, \$comment)
+    || ThrowTemplateError($template->error());
+
 # Include custom fields editable on bug creation.
 my @custom_bug_fields = Bugzilla->get_fields(
     { custom => 1, obsolete => 0, enter_bug => 1 });
@@ -159,6 +137,7 @@ push(@bug_fields, qw(
 
     alias
     blocked
+    commentprivacy
     bug_file_loc
     bug_severity
     bug_status
@@ -182,43 +161,21 @@ foreach my $field (@bug_fields) {
 $bug_params{'creation_ts'} = $timestamp;
 $bug_params{'cc'}          = [$cgi->param('cc')];
 $bug_params{'groups'}      = \@selected_groups;
-
-# Add the bug report to the DB.
-$dbh->bz_lock_tables('bugs WRITE', 'bug_group_map WRITE', 'longdescs WRITE',
-                     'cc WRITE', 'keywords WRITE', 'dependencies WRITE',
-                     'bugs_activity WRITE', 'groups READ',
-                     'user_group_map READ', 'group_group_map READ',
-                     'keyworddefs READ', 'fielddefs READ',
-                     'products READ', 'versions READ', 'milestones READ',
-                     'components READ', 'profiles READ', 'bug_severity READ',
-                     'op_sys READ', 'priority READ', 'rep_platform READ',
-                     'group_control_map READ', @custom_tables);
+$bug_params{'comment'}     = $comment;
 
 my $bug = Bugzilla::Bug->create(\%bug_params);
 
 # Get the bug ID back.
 my $id = $bug->bug_id;
 
-# Add the initial comment, allowing for the fact that it may be private
-my $privacy = 0;
-if (Bugzilla->params->{"insidergroup"} 
-    && Bugzilla->user->in_group(Bugzilla->params->{"insidergroup"})) 
-{
-    $privacy = $cgi->param('commentprivacy') ? 1 : 0;
+# Set Version cookie, but only if the user actually selected
+# a version on the page.
+if (defined $cgi->param('version')) {
+    $cgi->send_cookie(-name => "VERSION-" . $bug->product,
+                      -value => $bug->version,
+                      -expires => "Fri, 01-Jan-2038 00:00:00 GMT");
 }
 
-trick_taint($comment);
-$dbh->do(q{INSERT INTO longdescs (bug_id, who, bug_when, thetext,isprivate)
-           VALUES (?, ?, ?, ?, ?)}, undef, ($id, $user->id, $timestamp,
-                                            $comment, $privacy));
-
-# All fields related to the newly created bug are set.
-# The bug can now be made accessible.
-$dbh->do("UPDATE bugs SET creation_ts = ? WHERE bug_id = ?",
-          undef, ($timestamp, $id));
-
-$dbh->bz_unlock_tables();
-
 # We don't have to check if the user can see the bug, because a user filing
 # a bug can always see it. You can't change reporter_accessible until
 # after the bug is filed.