From bb023d74f9631d81cb136b16e5d01a0637402ddc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= Date: Thu, 2 Oct 2025 22:51:30 -0300 Subject: [PATCH] Improve feature coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Cohen Arazi --- lib/GitBz/Commands/Attach.pm | 142 +++++++++++++++++++++++--- lib/GitBz/Commands/Edit.pm | 188 +++++++++++++++++++++++++++-------- lib/GitBz/Git.pm | 12 ++- lib/GitBz/RestClient.pm | 77 +++++++++++++- lib/GitBz/StatusWorkflow.pm | 50 ++++++++++ 5 files changed, 407 insertions(+), 62 deletions(-) create mode 100644 lib/GitBz/StatusWorkflow.pm diff --git a/lib/GitBz/Commands/Attach.pm b/lib/GitBz/Commands/Attach.pm index bc97c8e..fdac4f3 100644 --- a/lib/GitBz/Commands/Attach.pm +++ b/lib/GitBz/Commands/Attach.pm @@ -16,10 +16,15 @@ package GitBz::Commands::Attach; # along with git-bz; if not, see . use Modern::Perl; + +use utf8; + use Getopt::Long qw(GetOptionsFromArray); use Try::Tiny qw(catch try); +use File::Temp; use GitBz::Git; use GitBz::Exception; +use GitBz::StatusWorkflow; sub new { my ( $class, $commands ) = @_; @@ -29,17 +34,11 @@ sub new { sub execute { my ( $self, @args ) = @_; - my %opts = ( - 'add-url' => undef, - ); + my %opts; GetOptionsFromArray( \@args, - 'edit|e' => \$opts{edit}, - 'mail|m' => \$opts{mail}, - 'add-url|u' => \$opts{'add-url'}, - 'no-add-url|n' => sub { $opts{'add-url'} = 0 }, - 'bugzilla|b=s' => \$opts{bugzilla}, + 'edit|e' => \$opts{edit}, ) or GitBz::Exception->throw("Invalid options"); return try { @@ -91,21 +90,140 @@ sub attach_patches { my ( $self, $bug_ref, $commits, $opts ) = @_; my $client = $self->{commands}->{client}; - + my $bug = $client->get_bug($bug_ref); + + my $is_first = 1; for my $commit (@$commits) { my $patch = GitBz::Git->format_patch( $commit->{id} . '^..' . $commit->{id} ); my $filename = sprintf( "%s.patch", substr( $commit->{id}, 0, 7 ) ); + my $description = $commit->{subject}; + my $body = GitBz::Git->run( 'log', '--format=%b', '-1', $commit->{id} ); + my $comment = $body || "Patch from commit " . substr( $commit->{id}, 0, 7 ); + my @obsoletes; + + if ( $opts->{edit} && $is_first ) { + ( $description, $comment, my $obsoletes_ref ) = $self->edit_attachment_comment( $bug, $commit ); + @obsoletes = @$obsoletes_ref if $obsoletes_ref; + } $client->add_attachment( $bug_ref, $patch, $filename, - $commit->{subject}, - comment => "Patch from commit " . substr( $commit->{id}, 0, 7 ) + $description, + comment => $comment, + obsoletes => \@obsoletes ); - print "Attached: $commit->{subject}\n"; + print "Attached: $description\n"; + $is_first = 0; + } +} + +sub edit_attachment_comment { + my ( $self, $bug, $commit ) = @_; + + my $template = ""; + $template .= "# Attachment to Bug $bug->{id} - $bug->{summary}\n\n"; + $template .= $commit->{subject} . "\n\n"; + + # Add commit body as initial comment + my $body = GitBz::Git->run( 'log', '--format=%b', '-1', $commit->{id} ); + $template .= $body . "\n\n" if $body; + + # Show existing patches for obsoleting + if ( $bug->{attachments} && @{ $bug->{attachments} } ) { + for my $patch ( @{ $bug->{attachments} } ) { + next unless $patch->{is_patch}; + my $obsoleted = ( $commit->{subject} eq $patch->{summary} ) ? "" : "#"; + $template .= "${obsoleted}Obsoletes: $patch->{id} - $patch->{summary}\n"; + } + $template .= "\n"; + } + + # Add status options + my $client = $self->{commands}->{client}; + my $workflow = GitBz::StatusWorkflow->new($client); + $template .= "# Current status: $bug->{status}\n"; + my $status_values = $workflow->get_next_status_values($bug->{status}); + for my $status (@$status_values) { + $template .= "# Status: $status\n"; + } + $template .= "\n"; + + # Add patch complexity options + my $complexity = $bug->{cf_patch_complexity} || ""; + $template .= "# Current patch-complexity: $complexity\n"; + my $complexity_values = $client->get_field_values('cf_patch_complexity'); + if ($complexity_values) { + for my $comp (@$complexity_values) { + $template .= "# Patch-complexity: $comp\n"; + } + } + $template .= "\n"; + + # Add depends options + my $depends = $bug->{depends_on} || []; + my $depends_str = ref($depends) eq 'ARRAY' ? join(' ', @$depends) : $depends; + $template .= "# Current depends: $depends_str\n"; + $template .= "# Depends: bug xxxx\n"; + $template .= "# Depends: bug yyyy\n"; + $template .= "\n"; + + $template .= "# Please edit the description (first line) and comment (other lines).\n"; + $template .= "# Lines starting with '#' will be ignored. Delete everything to abort.\n"; + $template .= "# To obsolete existing patches, uncomment the appropriate lines.\n"; + + my $edited = $self->edit_template($template); + return $self->parse_edited_content($edited); +} + +sub edit_template { + my ( $self, $template ) = @_; + + my $temp = File::Temp->new( SUFFIX => '.txt' ); + binmode $temp, ':utf8'; + print $temp $template; + close $temp; + + my $editor = $ENV{EDITOR} || $ENV{GIT_EDITOR} || 'vi'; + system( $editor, $temp->filename ); + + open my $fh, '<:utf8', $temp->filename or die "Cannot read temp file: $!"; + my $content = do { local $/; <$fh> }; + close $fh; + + return $content; +} + +sub parse_edited_content { + my ( $self, $content ) = @_; + + my @lines = split /\n/, $content; + my @non_comment_lines = grep { !/^#/ && /\S/ } @lines; # Also filter empty lines + + my $description = shift @non_comment_lines || ""; + $description =~ s/^\s+|\s+$//g; + + + + my @obsoletes; + my @comment_lines; + + for my $line (@non_comment_lines) { + if ( $line =~ /^\s*Obsoletes\s*:\s*(\d+)/ ) { + push @obsoletes, $1; + } else { + push @comment_lines, $line; + } } + + my $comment = join( "\n", @comment_lines ); + $comment =~ s/^\s+|\s+$//g; + + GitBz::Exception->throw("Empty description, aborting\n") unless $description; + + return ( $description, $comment, \@obsoletes ); } 1; diff --git a/lib/GitBz/Commands/Edit.pm b/lib/GitBz/Commands/Edit.pm index 4b36f52..3d1031a 100644 --- a/lib/GitBz/Commands/Edit.pm +++ b/lib/GitBz/Commands/Edit.pm @@ -16,11 +16,15 @@ package GitBz::Commands::Edit; # along with git-bz; if not, see . use Modern::Perl; + +use utf8; + use Getopt::Long qw(GetOptionsFromArray); use Try::Tiny qw(catch try); use File::Temp; use GitBz::Git; use GitBz::Exception; +use GitBz::StatusWorkflow; sub new { my ( $class, $commands ) = @_; @@ -50,17 +54,19 @@ sub execute { if ( $arg =~ /^\d+$/ ) { # Bug reference - $self->edit_bug( $arg, \%opts ); + my $changed = $self->edit_bug( $arg, \%opts ); + print "No changes made\n" unless $changed; } else { # Commit or revision range my @commits = GitBz::Git->get_commits($arg); GitBz::Exception->throw("No commits found") unless @commits; - $self->edit_commits( \@commits, \%opts ); + my $changed = $self->edit_commits( \@commits, \%opts ); + print "No changes made\n" unless $changed; } - print "Edit completed successfully\n"; + # Success message is handled in individual methods } catch { GitBz::Exception->throw("Edit failed: $_"); }; @@ -76,7 +82,7 @@ sub edit_bug { my $template = $self->create_bug_template( $bug, $opts ); my $edited = $self->edit_template($template); - $self->update_bug( $client, $bug_ref, $edited, $opts ); + return $self->update_bug( $client, $bug_ref, $edited, $opts ); } sub edit_commits { @@ -94,10 +100,14 @@ sub edit_commits { GitBz::Exception->throw("No bug references found in commits") unless %bugs; # Edit each bug + my $any_changed = 0; for my $bug_ref ( keys %bugs ) { print "Editing bug $bug_ref...\n"; - $self->edit_bug_with_commits( $bug_ref, $bugs{$bug_ref}, $opts ); + my $changed = $self->edit_bug_with_commits( $bug_ref, $bugs{$bug_ref}, $opts ); + $any_changed ||= $changed; } + + return $any_changed; } sub edit_bug_with_commits { @@ -110,27 +120,56 @@ sub edit_bug_with_commits { my $template = $self->create_bug_template_with_commits( $bug, $commits, $opts ); my $edited = $self->edit_template($template); - $self->update_bug( $client, $bug_ref, $edited, $opts ); + return $self->update_bug( $client, $bug_ref, $edited, $opts ); } sub create_bug_template { my ( $self, $bug, $opts ) = @_; + my $client = $self->{commands}->{client}; my $template = ""; - - if ( $opts->{fix} ) { - $template .= "# Closing bug as FIXED\n"; - $template .= "Status: RESOLVED\n"; - $template .= "Resolution: FIXED\n\n"; + $template .= "# Bug $bug->{id} - $bug->{summary}\n\n"; + + # Show existing patches for obsoleting + my $attachments = $client->get_attachments($bug->{id}); + if ( $attachments && @$attachments ) { + for my $patch ( @$attachments ) { + next unless $patch->{is_patch} && !$patch->{is_obsolete}; + $template .= "#Obsoletes: $patch->{id} - $patch->{summary}\n"; + } + $template .= "\n"; } - - $template .= "# Bug $bug->{id}: $bug->{summary}\n"; - $template .= "# Product: $bug->{product}\n"; - $template .= "# Component: $bug->{component}\n"; - $template .= "# Status: $bug->{status}\n\n"; - - $template .= "Comment:\n"; - $template .= "# Enter your comment above. Lines starting with # are ignored.\n"; + + # Add status options + my $workflow = GitBz::StatusWorkflow->new($client); + $template .= "# Current status: $bug->{status}\n"; + my $status_values = $workflow->get_next_status_values($bug->{status}); + for my $status (@$status_values) { + $template .= "# Status: $status\n"; + } + $template .= "\n"; + + # Add patch complexity options + my $complexity = $bug->{cf_patch_complexity} || ""; + $template .= "# Current patch-complexity: $complexity\n"; + my $complexity_values = $client->get_field_values('cf_patch_complexity'); + if ($complexity_values) { + for my $comp (@$complexity_values) { + $template .= "# Patch-complexity: $comp\n"; + } + } + $template .= "\n"; + + # Add depends options + my $depends = $bug->{depends_on} || []; + my $depends_str = ref($depends) eq 'ARRAY' ? join(' ', @$depends) : $depends; + $template .= "# Current depends: $depends_str\n"; + $template .= "# Depends: bug xxxx\n"; + $template .= "# Depends: bug yyyy\n"; + $template .= "\n"; + + $template .= "# Enter comment below. Lines starting with '#' will be ignored.\n"; + $template .= "# To obsolete patches, uncomment the appropriate Obsoletes lines.\n"; return $template; } @@ -159,51 +198,114 @@ sub edit_template { my ( $self, $template ) = @_; my $temp = File::Temp->new( SUFFIX => '.txt' ); + binmode $temp, ':utf8'; print $temp $template; close $temp; - my $editor = $ENV{EDITOR} || 'vi'; + my $editor = $ENV{EDITOR} || $ENV{GIT_EDITOR} || 'vi'; system( $editor, $temp->filename ); - open my $fh, '<', $temp->filename or die "Cannot read temp file: $!"; + open my $fh, '<:utf8', $temp->filename or die "Cannot read temp file: $!"; my $content = do { local $/; <$fh> }; close $fh; - # Remove comment lines - my @lines = grep { !/^#/ } split /\n/, $content; - return join( "\n", @lines ); + return $content; } sub update_bug { my ( $self, $client, $bug_ref, $edited, $opts ) = @_; + my @lines = split /\n/, $edited; + my @non_comment_lines = grep { !/^#/ && /\S/ } @lines; + + # Early return if no non-comment lines - no API calls needed + return 0 unless @non_comment_lines; + + my @obsoletes; + my @comment_lines; my %update_params; - - # Parse edited content - my @lines = split /\n/, $edited; - my $comment = ''; - my $in_comment = 0; - - for my $line (@lines) { - if ( $line =~ /^Status:\s*(.+)/ ) { + + for my $line (@non_comment_lines) { + if ( $line =~ /^\s*Status\s*:\s*(.+)/ ) { $update_params{status} = $1; - } elsif ( $line =~ /^Resolution:\s*(.+)/ ) { + } elsif ( $line =~ /^\s*Resolution\s*:\s*(.+)/ ) { $update_params{resolution} = $1; - } elsif ( $line =~ /^Comment:/ ) { - $in_comment = 1; - } elsif ( $in_comment && $line =~ /\S/ ) { - $comment .= $line . "\n"; + } elsif ( $line =~ /^\s*Patch-complexity\s*:\s*(.+)/ ) { + $update_params{cf_patch_complexity} = $1; + } elsif ( $line =~ /^\s*Depends\s*:\s*([Bb][Uu][Gg])?\s*(\d+)/ ) { + push @{ $update_params{depends_on} }, $2; + } elsif ( $line =~ /^\s*Obsoletes\s*:\s*(\d+)/ ) { + push @obsoletes, $1; + } else { + push @comment_lines, $line; } } - + + my $comment = join( "\n", @comment_lines ); + $comment =~ s/^\s+|\s+$//g; + + # Early return if no changes + return 0 unless %update_params || $comment || @obsoletes; + + # Only fetch bug data if we have changes to process + my $bug = $client->get_bug($bug_ref); + my $changed = 0; + + # Check if there are actual changes before making API call + my %actual_changes; + if ($update_params{status} && $update_params{status} ne $bug->{status}) { + $actual_changes{status} = $update_params{status}; + } + if ($update_params{resolution} && $update_params{resolution} ne ($bug->{resolution} || '')) { + $actual_changes{resolution} = $update_params{resolution}; + } + if ($update_params{cf_patch_complexity} && $update_params{cf_patch_complexity} ne ($bug->{cf_patch_complexity} || '')) { + $actual_changes{cf_patch_complexity} = $update_params{cf_patch_complexity}; + } + if ($update_params{depends_on}) { + $actual_changes{depends_on} = $update_params{depends_on}; + } if ($comment) { - $update_params{comment} = { body => $comment }; + $actual_changes{comment} = { body => $comment }; } - - if (%update_params) { - $client->update_bug( $bug_ref, %update_params ); - print "Updated bug $bug_ref\n"; + + if (%actual_changes) { + print "Updating bug $bug_ref\n"; + + # Show field changes + if ($actual_changes{status}) { + print "Status changed: $bug->{status} → $actual_changes{status}\n"; + } + if ($actual_changes{resolution}) { + my $old = $bug->{resolution} || 'none'; + print "Resolution changed: $old → $actual_changes{resolution}\n"; + } + if ($actual_changes{cf_patch_complexity}) { + my $old = $bug->{cf_patch_complexity} || 'none'; + print "Patch-complexity changed: $old → $actual_changes{cf_patch_complexity}\n"; + } + if ($actual_changes{comment}) { + print "Added comment\n"; + } + + $client->update_bug( $bug_ref, %actual_changes ); + $changed = 1; + } + + # Handle obsoleting attachments + if (@obsoletes) { + my $attachments = $client->get_attachments($bug_ref); + my %attach_lookup = map { $_->{id} => $_->{summary} } @$attachments; + + for my $attach_id (@obsoletes) { + $client->obsolete_attachment($attach_id); + my $summary = $attach_lookup{$attach_id} || "Unknown"; + print "Obsoleted attachment $attach_id - $summary\n"; + $changed = 1; + } } + + return $changed; } sub extract_bug_ref { diff --git a/lib/GitBz/Git.pm b/lib/GitBz/Git.pm index a8cc865..490cb7d 100644 --- a/lib/GitBz/Git.pm +++ b/lib/GitBz/Git.pm @@ -63,12 +63,18 @@ sub format_patch { sub get_commits { my ( $class, $range ) = @_; - # Try as single commit first - my $rev = try { $class->run( 'rev-parse', $range, '--verify' ) } catch { undef }; + # Try as single commit first - exactly like original git-bz + my $rev = try { + $class->run( 'rev-parse', $range, '--verify' ); + } catch { + undef + }; if ($rev) { - return $class->rev_list( $rev, '--max-count=1' ); + # Single commit - return just that one commit + return $class->rev_list( '--max-count=1', $rev ); } else { + # Not a single commit, treat as range return $class->rev_list($range); } } diff --git a/lib/GitBz/RestClient.pm b/lib/GitBz/RestClient.pm index 83217e5..537dc46 100644 --- a/lib/GitBz/RestClient.pm +++ b/lib/GitBz/RestClient.pm @@ -16,6 +16,7 @@ package GitBz::RestClient; # along with git-bz; if not, see . use Modern::Perl; + use LWP::UserAgent; use JSON; use MIME::Base64; @@ -39,6 +40,8 @@ sub new { ua => $ua, base_url => $base_url, token => undef, + username => $args{username}, + password => $args{password}, %args }, $class; } @@ -46,11 +49,14 @@ sub new { sub login { my ( $self, $username, $password ) = @_; + $self->{username} = $username if $username; + $self->{password} = $password if $password; + my $response = $self->{ua}->get( sprintf( "%s/login?login=%s&password=%s", - $self->{base_url}, $username, - $password + $self->{base_url}, $self->{username}, + $self->{password} ) ); @@ -63,6 +69,19 @@ sub login { GitBz::Exception::Bugzilla->throw( "Login failed: " . $response->status_line ); } +sub get_token { + my ($self) = @_; + + return $self->{token} if $self->{token}; + + if ($self->{username} && $self->{password}) { + $self->login(); + return $self->{token}; + } + + return undef; +} + sub get_bug { my ( $self, $bug_id ) = @_; @@ -106,7 +125,8 @@ sub add_attachment { }; $payload->{comment} = $opts{comment} if $opts{comment}; - $payload->{token} = $self->{token} if $self->{token}; + my $token = $self->get_token(); + $payload->{token} = $token if $token; my $response = $self->{ua}->post( $url, @@ -131,7 +151,8 @@ sub update_bug { %params }; - $payload->{token} = $self->{token} if $self->{token}; + my $token = $self->get_token(); + $payload->{token} = $token if $token; my $response = $self->{ua}->put( $url, @@ -146,4 +167,52 @@ sub update_bug { return decode_json( $response->content ); } +sub get_field_values { + my ( $self, $field_name ) = @_; + + my $url = sprintf( "%s/field/bug", $self->{base_url} ); + my $response = $self->{ua}->get($url); + + if ( !$response->is_success ) { + return []; + } + + my $data = decode_json( $response->content ); + + # Find the field in the fields array + for my $field ( @{ $data->{fields} || [] } ) { + if ( $field->{name} eq $field_name && $field->{values} ) { + return [ map { $_->{name} } @{ $field->{values} } ]; + } + } + + return []; +} + +sub obsolete_attachment { + my ( $self, $attachment_id ) = @_; + + my $url = sprintf( "%s/bug/attachment/%s", $self->{base_url}, $attachment_id ); + + my $payload = { + ids => [$attachment_id], + is_obsolete => JSON::true, + }; + + my $token = $self->get_token(); + $payload->{token} = $token if $token; + + my $response = $self->{ua}->put( + $url, + Content_Type => 'application/json', + Content => encode_json($payload) + ); + + if ( !$response->is_success ) { + GitBz::Exception::Bugzilla->throw( "Failed to obsolete attachment: " . $response->status_line ); + } + + return decode_json( $response->content ); +} + 1; diff --git a/lib/GitBz/StatusWorkflow.pm b/lib/GitBz/StatusWorkflow.pm new file mode 100644 index 0000000..df2be64 --- /dev/null +++ b/lib/GitBz/StatusWorkflow.pm @@ -0,0 +1,50 @@ +package GitBz::StatusWorkflow; + +# This file is part of git-bz. +# +# git-bz is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# git-bz is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with git-bz; if not, see . + +use Modern::Perl; + +sub new { + my ( $class, $client ) = @_; + bless { client => $client }, $class; +} + +sub get_next_status_values { + my ( $self, $current_status ) = @_; + + # Koha Bugzilla workflow transitions + my %transitions = ( + 'NEW' => ['ASSIGNED', 'Needs Signoff', 'RESOLVED'], + 'ASSIGNED' => ['Needs Signoff', 'RESOLVED'], + 'Needs Signoff' => ['Signed Off', 'Failed QA', 'RESOLVED'], + 'Signed Off' => ['Passed QA', 'Failed QA', 'RESOLVED'], + 'Failed QA' => ['Needs Signoff', 'RESOLVED'], + 'Passed QA' => ['Pushed to Master', 'Pushed to Stable', 'RESOLVED'], + 'Pushed to Master' => ['RESOLVED'], + 'Pushed to Stable' => ['RESOLVED'], + 'RESOLVED' => ['REOPENED'], + 'REOPENED' => ['ASSIGNED', 'Needs Signoff', 'RESOLVED'], + ); + + return $transitions{$current_status} || []; +} + +sub get_all_status_values { + my ( $self ) = @_; + return $self->{client}->get_field_values('bug_status'); +} + +1; \ No newline at end of file -- 2.51.0