Hi Kristis,
I'm sharing my current cumulative patch on top of scmbug 0.26.17 which we're
using on our environment with Bugzilla 3.6 (final) and 3.4.6.
- Added support for Bugzilla 3.6 version number
- Same fix for status & resolution with Bugzilla 3.2+ (a bit reformatted
and simplified)
- Same fix with eval block around comment adding (a bit reformatted and a
bit simplified as per Max's comments)
- A bit extended revision of my old patch for Bugzilla e-mail notification
(http://bugzilla.mkgnu.net/show_bug.cgi?id=832). I've bee using that since
an year with no problems. Following daemon.conf -- bugtracker parameters
should be added to enable it:
notification_enabled => 1 # bugtracker e-mail notification enabled
changer_notification_enabled => 1 # notify bug changer too
Regards,
Yavor
On Thu, Apr 15, 2010 at 09:24, Yavor Nikolov <[email protected]>wrote:
> Bugzilla does throw exceptions (in a "die" way) on certain places: e.g.
> - on calling Bug::check(), when providing bad user
> - Invalid duplicate bug id
> ... etc.
>
> I.e. - more often Bugzilla throws exception when error is encountered (bad
> parameter, lack of permissions, etc.) than returning an error code. Actually
> - exception handling is in general a cleaner approach to manage error
> situations than dealing with return codes. Just the die/eval/$@/.. is some
> kind of limited workaround since older versions of Perl didn't have
> exceptions.
>
> So what happens if we don't catch these exceptions: scmbug dies, error
> message is logged in scmbug log. However end user gets a general purpose
> error which doesn't explain the root cause of the problem (i.e. user
> receives something like "scmbug died for unknown reason" than duplicate bug
> id is bad; or we don't have permissions to edit the bug).
>
> Regards,
> Yavor
>
>
> On Thu, Apr 15, 2010 at 02:18, Kristis Makris <[email protected]> wrote:
>
>>
>> > If there is a possibility for Bugzilla to raise exceptions
>> > inside the
>> > eval {} blocks, is there another way of eliminating that
>> > possibility ?
>> > I think If we use the WebService API error would be signalled in a bit
>> > different manner. Another approach would be to patch Bugzilla.
>>
>> I suppose my question is, why are we so fearful that something bad will
>> happen that will throw an exception ? Can't we simply check the return
>> values of function calls to see if the calls succeeded ?
>>
>>
>>
>
diff -Naur Scmbug.orig.0.26.17/Daemon/Bugtracker.pm Scmbug.patched.20100418/Daemon/Bugtracker.pm
--- Scmbug.orig.0.26.17/Daemon/Bugtracker.pm 2010-03-26 21:53:38.000000000 +0200
+++ Scmbug.patched.20100418/Daemon/Bugtracker.pm 2010-04-16 23:12:09.000000000 +0300
@@ -115,6 +115,8 @@
$self->username ( $data->{ database_username } );
$self->password ( $data->{ database_password } );
$self->bug_url_prefix ( $data->{ bug_url_prefix } );
+ $self->notification_enabled ( $data->{ notification_enabled } );
+ $self->changer_notification_enabled ( $data->{ changer_notification_enabled } );
}
@@ -294,6 +296,29 @@
}
+sub changer_notification_enabled {
+ my $self = shift;
+ my $data = shift;
+
+ if ( $data ) {
+ $self->{ changer_notification_enabled } = $data;
+ } else {
+ return $self->{ changer_notification_enabled };
+ }
+}
+
+
+sub notification_enabled {
+ my $self = shift;
+ my $data = shift;
+
+ if ( $data ) {
+ $self->{ notification_enabled } = $data;
+ } else {
+ return $self->{ notification_enabled };
+ }
+}
+
sub active_states_list {
my $self = shift;
@@ -633,6 +658,10 @@
return @comment_sets;
}
+#
+# Send notification
+sub integration_send_notification {
+}
1;
diff -Naur Scmbug.orig.0.26.17/Daemon/Bugzilla.pm Scmbug.patched.20100418/Daemon/Bugzilla.pm
--- Scmbug.orig.0.26.17/Daemon/Bugzilla.pm 2010-03-26 21:53:38.000000000 +0200
+++ Scmbug.patched.20100418/Daemon/Bugzilla.pm 2010-04-18 17:44:44.000000000 +0300
@@ -234,6 +234,7 @@
# Must load the Bugzilla package before any of its functions are used.
eval "use Bugzilla";
+ eval "use Bugzilla::BugMail";
}
# This is direct modification to bugzilla's internal variables
@@ -480,7 +481,7 @@
# Thus they function like Bugzilla 3.2
$self->{ version_type } = "up_to_3_1_2";
} elsif ( $major == 3 &&
- ($minor == 2 || $minor == 4)) {
+ ($minor == 2 || $minor == 4 || $minor == 6)) {
$self->{ version_type } = $BUGTRACKER_VERSION_LATEST;
} else {
$self->{ version_type } = $BUGTRACKER_VERSION_NOT_SUPPORTED;
@@ -565,18 +566,27 @@
}
} else {
# Since Bugzilla 3.1.3 AppendComment has been replaced by add_comment
- my $userid = Bugzilla::User::login_to_id( $username );
- if ( $userid > 0 ) {
- my $bug = Bugzilla::Bug->check($bugid);
- my $user = new Bugzilla::User($userid);
- Bugzilla->set_user($user);
- $bug->add_comment($comment, {isprivate => 0, work_time => 0, type => Bugzilla::Constants->CMT_NORMAL, extra_data => ""} );
+
+ my $dbh = Bugzilla->dbh;
+ my @ret_list = eval {
+ my $user = Bugzilla::User->check( $username );
+ Bugzilla->set_user( $user );
+ my $bug = Bugzilla::Bug->check( $bugid );
+ $dbh->bz_start_transaction();
+ $bug->add_comment( $comment );
$bug->update();
- return 0;
- } else {
- # This should never happen. Each user should have a
- # corresponding userid in the database schema
- return 1, "Login '$username' could not be converted to an id in Bugzilla. Is username mapping setup correctly in daemon.conf ?\n" ;
+ $dbh->bz_commit_transaction();
+
+ return 0;
+ }; # eval
+ if ( $@ ) {
+ my $err = $@;
+ log_daemon_warn( undef, $err );
+ $dbh->bz_rollback_transaction() if $dbh->bz_in_transaction();
+ return 1, "Error while adding bug comment: $err";
+ }
+ else {
+ return @ret_list;
}
}
}
@@ -606,7 +616,7 @@
$self->is_version_up_to_2_22() ) {
# Changing bug resolutions is not supported for earlier versions of Bugzilla
return ( 1, "Changing bug resolution is not implemented in this version of Bugzilla.\n");
- } elsif ( $self->is_version_up_to_3_1_2() || $self->is_version_latest() ) {
+ } elsif ( $self->is_version_up_to_3_1_2() ) {
my $privileges_are_required = 0; #Set within check_can_change_field, not used at the moment
my $bug = new Bugzilla::Bug($bugid);
@@ -784,9 +794,90 @@
} else {
return ( 1, "Changing bug resolution for bug '$bugid' wasn't successful. Nothing changed.\n");
}
+ } elsif ( $self->is_version_latest() ) {
+ # Bugzilla 3.2+
+ my $dbh = Bugzilla->dbh;
+
+ # Run in eval block to catch bugzilla exceptions
+ my @ret_list = eval {
+ my $user = Bugzilla::User->check( $username );
+ Bugzilla->set_user( $user );
+ my $bug = Bugzilla::Bug->check( $bugid );
+
+ my $has_changed = 0;
+
+ if( !defined $status ) {
+ $status = "none";
+ } else {
+ $status = lc $status;
+ }
+
+ my $new_status = uc $status; # Default status. May be changed later
+ my $resolution_params = {};
+
+ if ( $status =~ /^none$/ ) {
+ return ( 1, "Undefined status '$status' for bug '$bugid'.\n" );
+ } elsif ( $status =~ /^assigned$/ ) {
+ if ( defined( $resolution ) && ( '' ne $resolution ) ) {
+ # Reassign to given assignee
+ my $assignee_user = new Bugzilla::User( { name => $resolution } );
+ if( !defined( $assignee_user ) ) {
+ return ( 1, "Unknown assignee '$resolution'.\n" );
+ }
+
+ $bug->set_assigned_to( $assignee_user );
+ $new_status = "NEW";
+ } else {
+ # Accepting bug
+ if( $bug->assigned_to->id != $user->id ) {
+ return ( 1, "You cannot accept a bug if you aren't the assignee. Bug '$bugid' is assigned to '" . $bug->assigned_to->login . "'\n" );
+ }
+ }
+
+ $has_changed = 1;
+ } elsif ( $status =~ /^reopened$/ ) {
+ $has_changed = 1;
+ } elsif ( $status =~ /^resolved$/ ) {
+ $resolution = lc $resolution;
+
+ if( $resolution eq "moved" ) {
+ # Should never happen, anyway leave it here
+ return ( 1, "Movement of bugs isn't supported.\n" );
+ }
+ elsif ( $resolution eq "duplicate" ) {
+ $resolution_params->{ dupe_of } = $resolution_data;
+ }
+
+ $resolution_params->{ resolution } = uc $resolution;
+ $has_changed = 1;
+ } else {
+ return ( 1, "Unknown status '$status' for bug '$bugid'.\n" );
+ }
+
+ if( $has_changed == 1 ) {
+ $dbh->bz_start_transaction();
+ $bug->set_status( $new_status, $resolution_params );
+ $bug->update();
+ $dbh->bz_commit_transaction();
+
+ return 0;
+ } else {
+ return ( 1, "Changing bug resolution for bug '$bugid' wasn't successful. Nothing changed.\n" );
+ }
+ }; # eval
+ if ( $@ ) {
+ my $err = $@;
+ log_daemon_warn( undef, $err );
+ $dbh->bz_rollback_transaction() if $dbh->bz_in_transaction();
+ return 1, "Error while changing bug resolution: $err";
+ }
+ else {
+ return @ret_list;
+ }
+
} else {
# Changing bug resolutions is not supported for earlier versions of Bugzilla
- return ( 1, "Changing bug resolution is not yet implemented in this version of Bugzilla.\n");
+ return ( 1, "Changing bug resolution is not yet implemented in this version of Bugzilla.\n" );
}
}
@@ -1586,5 +1677,19 @@
return 1;
}
+#
+# Send notification
+sub integration_send_notification {
+ my $self = shift;
+ my ( $bugid, $vars ) = ( @_ );
+
+ # If vars changer is not defined then changer is also notified (from Bugzilla).
+ if ( $self->changer_notification_enabled ) {
+ $vars = {};
+ }
+
+ return Bugzilla::BugMail::Send( $bugid, $vars );
+}
+
1;
diff -Naur Scmbug.orig.0.26.17/Daemon/Integration.pm Scmbug.patched.20100418/Daemon/Integration.pm
--- Scmbug.orig.0.26.17/Daemon/Integration.pm 2010-03-26 21:53:38.000000000 +0200
+++ Scmbug.patched.20100418/Daemon/Integration.pm 2010-04-16 23:03:03.000000000 +0300
@@ -100,10 +100,30 @@
$self->bugtracker()->integration_disconnect_database();
+ # Trigger mailing.
+ if ( $self->bugtracker()->notification_enabled ) {
+ $self->send_bugtracker_notification_to_all();
+ }
+
return $retval;
}
+#
+# Send bugtracker notifications.
+#
+sub send_bugtracker_notification_to_all {
+ my $self = shift;
+
+ my $vars = {};
+ $vars->{ 'changer' } = $self->request()->{ translated_username };
+ my @bugs = $self->get_all_related_bug_ids();
+
+ foreach my $bugid ( @bugs ) {
+ my $ret_message = $self->bugtracker()->integration_send_notification( $bugid, $vars );
+ # could log result here
+ }
+}
#
# Processes a verify activity
@@ -647,6 +667,23 @@
return ( $matched_owner, $case_sensitive_username_verification_who );
}
+# Get union of all bug id-s and resolution ids
+sub get_all_related_bug_ids {
+ my $self = shift;
+ my @bug_ids = @{ $self->request()->{ ids } };
+ my @res_ids = keys %{ $self->request()->{ resolution_ids } };
+
+ my @res = ();
+ my %res = ();
+
+ foreach my $e ( @bug_ids, @res_ids ) {
+ $res{ $e }++;
+ }
+
+ @res = sort { $a <=> $b } keys %res;
+
+ return @res;
+}
1;
_______________________________________________
scmbug-users mailing list
[email protected]
http://lists.mkgnu.net/cgi-bin/mailman/listinfo/scmbug-users
