Got the different issues split to separate patches - here are the ones more
relevant to the one discussed in this thread.
Regards,
Yavor
On Sun, Apr 18, 2010 at 20:24, Yavor Nikolov <[email protected]>wrote:
> Hi,
>
> On Sun, Apr 18, 2010 at 19:32, Kristis Makris <[email protected]> wrote:
>
>> Hi Yavor.
>>
>> On Sun, 2010-04-18 at 19:02 +0300, Yavor Nikolov wrote:
>> > 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)
>>
>> It doesn't seem simplified. For example, the eval{} block still includes
>> most of the source, instead of just the call to check().
>>
> I meant I removed the default parameters of comment add and removed a
> redundant check which is anyway performed by Bugzilla. Single eval still
> seems simpler to me than multiple ones. Not only check() calls may throw
> error: set_status, set_assigned_to, set_..., update (and maybe others), can
> die too. Transaction blocks stay since as I've explained - that seems safer
> to me; otherwise we may lose a fully-valid bug updates in certain
> circumstances.
>
>
>>
>> > - 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
>>
>> Hold it right there...
>>
>> You are mixing multiple things in this patch. Bugzilla e-mail
>> notifications are a separate issue from status resolution. They
>> shouldn't be part of this patch.
>>
> You're right - sorry for that! I'm aware that problems should be targeted
> separately. (Changes around commend add; and 3.6 version type support are
> also diversions from original issue...). But I'm just living with that
> e-mail patch applied; I tweaked the other changes on top of it; and I've
> been too lazy to split the changes apart for separate diffs. I just wanted
> to share the current state of all necessary for us modifications which make
> scmbug compatible enough with Bugzilla.
> (I just realized the e-mail patch is anyway a bit incomplete since SendMail
> may die with exception too.../even though not very likely/)
>
> Regards,
> Yavor
>
>
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
@@ -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;
}
}
}
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
@@ -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" );
}
}
_______________________________________________
scmbug-users mailing list
[email protected]
http://lists.mkgnu.net/cgi-bin/mailman/listinfo/scmbug-users
