Bug 254498: Check for comment required for time validation came too late.

Patch by Tiago R. Mello <tiago@async.com.br>
r=kiko, a=justdave

diff --git a/process_bug.cgi b/process_bug.cgi
index 5869a0363d67168babac577fb8da5fb41e824b26..2eb6a41e3fccd0044dc6d596e4bd92e7cc9b26b3 100755 (executable)
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -101,39 +101,15 @@ foreach my $field ("estimated_time", "work_time", "remaining_time") {
     }
 }
 
-# do a match on the fields if applicable
-
-# The order of these function calls is important, as both Flag::validate
-# and FlagType::validate assume User::match_field has ensured that the values
-# in the requestee fields are legitimate user email addresses.
-&Bugzilla::User::match_field({
-    'qa_contact'                => { 'type' => 'single' },
-    'newcc'                     => { 'type' => 'multi'  },
-    'masscc'                    => { 'type' => 'multi'  },
-    'assigned_to'               => { 'type' => 'single' },
-    '^requestee(_type)?-(\d+)$' => { 'type' => 'single' },
-});
-# Validate flags, but only if the user is changing a single bug,
-# since the multi-change form doesn't include flag changes.
-if (defined $::FORM{'id'}) {
-    Bugzilla::Flag::validate(\%::FORM, $::FORM{'id'});
-    Bugzilla::FlagType::validate(\%::FORM, $::FORM{'id'});
-}
-
-# If we are duping bugs, let's also make sure that we can change 
-# the original.  This takes care of issue A on bug 96085.
-if (defined $::FORM{'dup_id'} && $::FORM{'knob'} eq "duplicate") {
-    ValidateBugID($::FORM{'dup_id'});
-
-    # Also, let's see if the reporter has authorization to see the bug
-    # to which we are duping.  If not we need to prompt.
-    DuplicateUserConfirm();
+if (UserInGroup(Param('timetrackinggroup'))) {
+    my $wk_time = $::FORM{'work_time'};
+    if ($::FORM{'comment'} =~ /^\s*$/ && $wk_time && $wk_time != 0) {
+        ThrowUserError('comment_required', undef, "abort");
+    }
 }
 
 ValidateComment($::FORM{'comment'});
 
-$::FORM{'dontchange'} = '' unless exists $::FORM{'dontchange'};
-
 # If the bug(s) being modified have dependencies, validate them
 # and rebuild the list with the validated values.  This is important
 # because there are situations where validation changes the value
@@ -152,6 +128,37 @@ foreach my $field ("dependson", "blocked") {
     }
 }
 
+# If we are duping bugs, let's also make sure that we can change 
+# the original.  This takes care of issue A on bug 96085.
+if (defined $::FORM{'dup_id'} && $::FORM{'knob'} eq "duplicate") {
+    ValidateBugID($::FORM{'dup_id'});
+
+    # Also, let's see if the reporter has authorization to see the bug
+    # to which we are duping.  If not we need to prompt.
+    DuplicateUserConfirm();
+}
+
+# do a match on the fields if applicable
+
+# The order of these function calls is important, as both Flag::validate
+# and FlagType::validate assume User::match_field has ensured that the values
+# in the requestee fields are legitimate user email addresses.
+&Bugzilla::User::match_field({
+    'qa_contact'                => { 'type' => 'single' },
+    'newcc'                     => { 'type' => 'multi'  },
+    'masscc'                    => { 'type' => 'multi'  },
+    'assigned_to'               => { 'type' => 'single' },
+    '^requestee(_type)?-(\d+)$' => { 'type' => 'single' },
+});
+# Validate flags, but only if the user is changing a single bug,
+# since the multi-change form doesn't include flag changes.
+if (defined $::FORM{'id'}) {
+    Bugzilla::Flag::validate(\%::FORM, $::FORM{'id'});
+    Bugzilla::FlagType::validate(\%::FORM, $::FORM{'id'});
+}
+
+$::FORM{'dontchange'} = '' unless exists $::FORM{'dontchange'};
+
 ######################################################################
 # End Data/Security Validation
 ######################################################################
@@ -1285,19 +1292,13 @@ foreach my $id (@idlist) {
     delete $::FORM{'work_time'} unless UserInGroup(Param('timetrackinggroup'));
 
     if ($::FORM{'comment'} || $::FORM{'work_time'}) {
-        if ($::FORM{'work_time'} && 
-            (!defined $::FORM{'comment'} || $::FORM{'comment'} =~ /^\s*$/)) {
-            SendSQL("UNLOCK TABLES");
-            ThrowUserError('comment_required');
-        } else {
-            AppendComment($id, $::COOKIE{'Bugzilla_login'}, $::FORM{'comment'},
-                          $::FORM{'commentprivacy'}, $timestamp, $::FORM{'work_time'});
-            if ($::FORM{'work_time'}) {
-                LogActivityEntry($id, "work_time", "", $::FORM{'work_time'},
-                                 $whoid, $timestamp);
-            }
-            $bug_changed = 1;
+        AppendComment($id, $::COOKIE{'Bugzilla_login'}, $::FORM{'comment'},
+                      $::FORM{'commentprivacy'}, $timestamp, $::FORM{'work_time'});
+        if ($::FORM{'work_time'}) {
+            LogActivityEntry($id, "work_time", "", $::FORM{'work_time'},
+                             $whoid, $timestamp);
         }
+        $bug_changed = 1;
     }
 
     if (@::legal_keywords) {

diff --git a/template/en/default/bug/show.html.tmpl b/template/en/default/bug/show.html.tmpl
index f98e05f0cbd87d4d5b62a5d07b0faaf8a33f52b7..8db59a9800444047e56d2f65e1af68e119fdffa3 100644 (file)
--- a/template/en/default/bug/show.html.tmpl
+++ b/template/en/default/bug/show.html.tmpl
@@ -28,17 +28,16 @@
 
 [% filtered_desc = bug.short_desc FILTER html %]
 [% filtered_timestamp = bug.delta_ts FILTER time %]
-[% bodyattrs = BLOCK %]class='bz_bug bz_status_[% bug.bug_status 
-  FILTER css_class_quote %] bz_component_[% bug.component 
-  FILTER css_class_quote %] bz_bug_[% bug.bug_id 
-  FILTER css_class_quote %]' 
-[% END %]
 [% PROCESS global/header.html.tmpl
   title = "$terms.Bug $bug.bug_id - $bug.short_desc"
   h1 = "$terms.Bugzilla $terms.Bug $bug.bug_id"
   h2 = filtered_desc
   h3 = "Last modified: $filtered_timestamp"
-  bodyattrs = bodyattrs
+  bodyclasses = ['bz_bug',
+                 "bz_status_$bug.bug_status",
+                 "bz_component_$bug.component",
+                 "bz_bug_$bug.bug_id"
+                ]
 %]
 
 [% PROCESS bug/navigate.html.tmpl %]

diff --git a/template/en/default/global/header.html.tmpl b/template/en/default/global/header.html.tmpl
index 5ee8c7f321acbf8a07818ee1dd0a2fe843b70210..ed0d801a613b3721b3714a92a4b7708bdb059446 100644 (file)
--- a/template/en/default/global/header.html.tmpl
+++ b/template/en/default/global/header.html.tmpl
@@ -28,6 +28,7 @@
   # h3: string. Right-aligned subheader.
   # bgcolor: string. the page's background color ("#rrggbb").
   # bodyattrs: any extra attributes for the <body> tag
+  # bodyclasses: array of extra CSS classes for the <body>
   # onload: string. JavaScript code to run when the page finishes loading.
   # javascript: string. Javascript to go in the header.
   # style: string. CSS style.