Merge branch '5.0/make-config-history-more-readable2' into 5.0-trunk

zhendeh · Apr 7, 2023 · d4029a4 · d4029a4
2 parents 352c6c4 + 89b886d
commit d4029a4
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 7 deletions.
diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
@@ -1417,6 +1417,9 @@ sub _CanonicalizeRoleName {
         my $self = shift;
         my ($new_value, $old_value);
 
+        my $meta = $RT::Config::META{$self->Field} || {};
+        my $show_details = ( $meta->{Widget} // '' ) =~ m{/(?:Boolean|Integer|String|Select)$} ? 0 : 1;
+
         # pull in new value from reference if exists
         if ( $self->NewReference ) {
             my $newobj = RT::Configuration->new($self->CurrentUser);
@@ -1429,10 +1432,19 @@ sub _CanonicalizeRoleName {
             my $oldobj = RT::Configuration->new($self->CurrentUser);
             $oldobj->Load($self->OldReference);
             $old_value = $oldobj->Content;
-            return ('[_1] changed from "[_2]" to "[_3]"', $self->Field, $old_value // '', $new_value // ''); #loc()
+            if ( !$show_details ) {
+                return ( '[_1] changed from "[_2]" to "[_3]"', $self->Field, $old_value // '', $new_value // '' ); #loc()
+            }
+        }
+
+        if ( !$show_details ) {
+            return ( '[_1] changed to "[_2]"', $self->Field, $new_value // '' );    #loc()
+        }
+        elsif ( !defined($old_value) || ( $old_value eq '' ) ) {
+            return ( "[_1] added", $self->Field );                                  #loc()
         }
         else {
-            return ('[_1] changed to "[_2]"', $self->Field, $new_value // ''); #loc()
+            return ( "[_1] changed", $self->Field );                                #loc()
         }
     },
     DeleteConfig => sub  {

diff --git a/share/html/Admin/Tools/ConfigHistory.html b/share/html/Admin/Tools/ConfigHistory.html
@@ -77,3 +77,10 @@
   </div>
  </&>
 </div>
+<script type="text/javascript">
+jQuery(function() {
+    jQuery('.transaction .toggle-txn-details').click(function () {
+        return toggleTransactionDetails.apply(this);
+    });
+});
+</script>
diff --git a/share/html/Elements/ShowTransaction b/share/html/Elements/ShowTransaction
@@ -83,7 +83,7 @@ $m->comp(
 ) if $ShowBody;
 </%PERL>
   </div>
-% if ($Transaction->Type eq 'CustomField' && $Transaction->Field ) {
+% if ( ( $Transaction->Type eq 'CustomField' && $Transaction->Field ) or ( $Transaction->Type eq 'SetConfig' ) ) {
 %     my ($old, $new);
 %     my $cf = RT::CustomField->new( $session{CurrentUser} );
 %     $cf->SetContextObject( $Transaction->Object );
@@ -94,14 +94,32 @@ $m->comp(
 %        $old = $old_ref ? $m->scomp($comp, Object => $old_ref) : loc('(no value)');
 %        my $new_ref = $Transaction->NewReferenceObject;
 %        $new = $new_ref ? $m->scomp($comp, Object => $new_ref) : loc('(no value)');
+%     }
+%     elsif ( $show_config_diff ) {
+%         # pull in new value from reference if exists
+%         if ( my $newobj = $Transaction->NewReferenceObject ) {
+%             $new = $newobj->Content;
+%         }
+%         else {
+%             $new = loc('(no value)');
+%         }
+%         # pull in old value from reference if exists
+%         if ( my $oldobj = $Transaction->OldReferenceObject ) {
+%             $old = $oldobj->Content // loc('(no value)');
+%         }
+%         else {
+%             $old = loc('(no value)');
+%         }
+%     }
+%     if ( $old || $new ) {
 <div class="details hidden" id="txn-<% $Transaction->Id %>-details">
-% if ( $old eq loc('(no value)')  ) {
+% if ( $old && $old eq loc('(no value)')  ) {
   <div class="form-row">
     <div class="label col-2"><% loc('Added') %>:</div>
     <div class="value col-10"><% $new |n %></div>
   </div>
 % }
-% elsif ( $new eq loc('(no value)')  ) {
+% elsif ( $new && $new eq loc('(no value)')  ) {
   <div class="form-row">
     <div class="label col-2"><% loc('Deleted') %>:</div>
     <div class="value col-10"><% $old |n %></div>
@@ -169,7 +187,7 @@ my @classes = (
 );
 
 my $desc = $Transaction->BriefDescriptionAsHTML;
-if ( $Object->id != $Transaction->ObjectId ) {
+if ( $Object->id && $Object->id != $Transaction->ObjectId ) {
     # merged objects
     $desc = join " - ",
         $m->interp->apply_escapes(
@@ -192,6 +210,8 @@ if ( $ShowBody && !$Attachments ) {
 }
 
 my $show_cf_diff = 0; # Show/hide colorized diff panel in transaction display
+my $show_config_diff = 0;
+
 my @actions = ();
 my $txn_type = $Transaction->Type;
 if ( $txn_type =~ /EmailRecord$/ ) {
@@ -229,6 +249,13 @@ elsif ($txn_type eq 'CustomField' && $Transaction->Field) {
         push @actions, { class => 'toggle-txn-details', title => loc('Show Details'), path => '#' };
     }
 }
+elsif ($txn_type eq 'SetConfig' && $Transaction->Field) {
+    my $meta = $RT::Config::META{$Transaction->Field} || {};
+    $show_config_diff = ( $meta->{Widget} // '' ) =~ m{/(?:Boolean|Integer|String|Select)$} ? 0 : 1;
+    if ( $show_config_diff ) {
+        push @actions, { class => 'toggle-txn-details', title => loc('Show Details'), path => '#' };
+    }
+}
 # If the transaction has anything attached to it at all
 elsif ( %$Attachments && $ShowActions ) {
     my %has_right = map {

diff --git a/t/web/admin_tools_editconfig.t b/t/web/admin_tools_editconfig.t
@@ -137,7 +137,7 @@ sub check_history_page_item {
     } else {
         $m->text_contains($change->{new_value});
     }
-    $m->text_contains("$change->{setting} changed", 'fetched tx has changed field');
+    $m->text_like(qr/$change->{setting} (?:added|changed)/, 'fetched tx has changed field');
 }
 
 sub compactify {