Patch by freitag@suse.de (Klaas Freitag) -- add options to let Bugzilla require comments on certain operations.

3 files changed, 90 insertions, 11 deletions

diff --git a/CGI.pl b/CGI.pl
index a5a4b3b1e..1e22afc52 100644
--- a/CGI.pl
+++ b/CGI.pl
@@ -631,8 +631,16 @@ sub DumpBugActivity {
 }
 
 
-
-
+#
+# Prints a warnbanner incl. image with given message
+# 
+sub warnBanner( $ )
+{
+  my ($msg) = (@_);
+  print Param("warnbannerhtml");
+  print $msg;
+  print Param("warnfooterhtml");
+}
 
 
 ############# Live code below here (that is, not subroutine defs) #############
diff --git a/defparams.pl b/defparams.pl
index f035b5761..41841eb19 100644
--- a/defparams.pl
+++ b/defparams.pl
@@ -176,6 +176,17 @@ information about what Bugzilla is and what it can do, see
 
 
     
+DefParam("warnbannerhtml",
+         "HTML to prepend to warning messages.",
+         "l",
+         "<font size=+1>");
+
+DefParam("warnfooterhtml",
+         "HTML to append to warning messages.",
+         "l",
+         "</font>");
+
+
 DefParam("changedmail",
 q{The email that gets sent to people when a bug changes.  Within this
 text, %to% gets replaced by the assigned-to and reported-by people,
@@ -378,5 +389,36 @@ DefParam("browserbugmessage",
          "If strictvaluechecks is on, and the bugzilla gets unexpected data from the browser, in addition to displaying the cause of the problem, it will output this HTML as well.",
          "l",
          "this may indicate a bug in your browser.\n");
+
+#
+# Parameters to force users to comment their changes for different actions.
+DefParam("commentonaccept", 
+         "If this option is on, the user needs to enter a short comment if he accepts the bug",
+         "b", 0 );
+DefParam("commentonclearresolution", 
+         "If this option is on, the user needs to enter a short comment if the bugs resolution is cleared",
+         "b", 0 );
+DefParam("commentonresolve", 
+         "If this option is on, the user needs to enter a short comment if the bug is resolved",
+         "b", 0 );
+DefParam("commentonreassign", 
+         "If this option is on, the user needs to enter a short comment if the bug is reassigned",
+         "b", 0 );
+DefParam("commentonreassignbycomponent", 
+         "If this option is on, the user needs to enter a short comment if the bug is reassigned by component",
+         "b", 0 );
+DefParam("commentonreopen", 
+         "If this option is on, the user needs to enter a short comment if the bug is reopened",
+         "b", 0 );
+DefParam("commentonverify", 
+         "If this option is on, the user needs to enter a short comment if the bug is verified",
+         "b", 0 );
+DefParam("commentonclose", 
+         "If this option is on, the user needs to enter a short comment if the bug is closed",
+         "b", 0 );
+DefParam("commentonduplicate",
+         "If this option is on, the user needs to enter a short comment if the bug is marked as duplicate",
+         "b", 0 );
+
 1;
 
diff --git a/process_bug.cgi b/process_bug.cgi
index 63e1147fa..858f04319 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -169,6 +169,35 @@ sub ChangeResolution {
     }
 }
 
+#
+# This function checks if there is a comment required for a specific
+# function and tests, if the comment was given.
+# If comments are required for functions  is defined by params.
+#
+sub CheckonComment( $ ) {
+    my ($function) = (@_);
+    
+    # Param is 1 if comment should be added !
+    my $ret = Param( "commenton" . $function );
+
+    # Allow without comment in case of undefined Params.
+    $ret = 0 unless ( defined( $ret ));
+
+    if( $ret ) {
+        if (!defined $::FORM{'comment'} || $::FORM{'comment'} =~ /^\s*$/) {
+            # No commet - sorry, action not allowed !
+            warnBanner("You have to specify a <b>comment</b> on this change." .
+                       "<p>" .
+                       "Please press <b>Back</b> and give some words " .
+                       "on the reason of the your change.\n" );
+            exit( 0 );
+        } else {
+            $ret = 0;
+        }
+    }
+    return( ! $ret ); # Return val has to be inverted
+}
+
 
 my $foundbit = 0;
 foreach my $b (grep(/^bit-\d*$/, keys %::FORM)) {
@@ -220,20 +249,20 @@ SWITCH: for ($::FORM{'knob'}) {
     /^none$/ && do {
         last SWITCH;
     };
-    /^accept$/ && do {
+    /^accept$/ && CheckonComment( "accept" ) && do {
         ChangeStatus('ASSIGNED');
         last SWITCH;
     };
-    /^clearresolution$/ && do {
+    /^clearresolution$/ && CheckonComment( "clearresolution" ) &&do {
         ChangeResolution('');
         last SWITCH;
     };
-    /^resolve$/ && do {
+    /^resolve$/ && CheckonComment( "resolve" ) && do {
         ChangeStatus('RESOLVED');
         ChangeResolution($::FORM{'resolution'});
         last SWITCH;
     };
-    /^reassign$/ && do {
+    /^reassign$/ && CheckonComment( "reassign" ) && do {
         ChangeStatus('NEW');
         DoComma();
         if ( Param("strictvaluechecks") ) {
@@ -248,7 +277,7 @@ SWITCH: for ($::FORM{'knob'}) {
         $::query .= "assigned_to = $newid";
         last SWITCH;
     };
-    /^reassignbycomponent$/ && do {
+    /^reassignbycomponent$/  && CheckonComment( "reassignbycomponent" ) && do {
         if ($::FORM{'product'} eq $::dontchange) {
             print "You must specify a product to help determine the new\n";
             print "owner of these bugs.\n";
@@ -269,19 +298,19 @@ SWITCH: for ($::FORM{'knob'}) {
         $::query .= "assigned_to = $newid";
         last SWITCH;
     };   
-    /^reopen$/ && do {
+    /^reopen$/  && CheckonComment( "reopen" ) && do {
         ChangeStatus('REOPENED');
         last SWITCH;
     };
-    /^verify$/ && do {
+    /^verify$/ && CheckonComment( "verify" ) && do {
         ChangeStatus('VERIFIED');
         last SWITCH;
     };
-    /^close$/ && do {
+    /^close$/ && CheckonComment( "close" ) && do {
         ChangeStatus('CLOSED');
         last SWITCH;
     };
-    /^duplicate$/ && do {
+    /^duplicate$/ && CheckonComment( "duplicate" ) && do {
         ChangeStatus('RESOLVED');
         ChangeResolution('DUPLICATE');
         if ( Param('strictvaluechecks') ) {