[Rt-commit] rt branch 5.0/clean-up-bulk-update-messages created. rt-5.0.4-142-gcae931dd03

BPS Git Server git at git.bestpractical.com
Wed Aug 30 18:57:15 UTC 2023


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".

The branch, 5.0/clean-up-bulk-update-messages has been created
        at  cae931dd03fa8ee6408e2bb8c3f207520e83a135 (commit)

- Log -----------------------------------------------------------------
commit cae931dd03fa8ee6408e2bb8c3f207520e83a135
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Wed Aug 30 14:55:54 2023 -0400

    Document changes to some update messages
    
    REST 2 responses could be changed, so document the change
    in case some users have automated systems that check
    response messages.

diff --git a/docs/UPGRADING-5.0 b/docs/UPGRADING-5.0
index f7248f5022..49f7704222 100644
--- a/docs/UPGRADING-5.0
+++ b/docs/UPGRADING-5.0
@@ -611,4 +611,27 @@ you also need to update the module L<DBIx::SearchBuilder>.
 
 =back
 
+=head1 UPGRADING FROM 5.0.4 AND EARLIER
+
+=over 4
+
+=item Update Messages Changed for Consistency
+
+Some parts of RT, notably Bulk Update, previously could show update messages
+like:
+
+    Ticket 123: Ticket 123: Status changed from 'new' to 'open'
+
+This is fixed in RT 5.0.5. As part of this fix, some JSON responses sent for
+REST 2 operations are also updated. For example, an Article update message
+is changed:
+
+    Old: Article More updates using REST: Name changed from...
+    New: Article 123: Name changed from...
+
+If you have any automation using REST 2 that checks these JSON response
+messages, you may need to update your system to match the new format.
+
+=back
+
 =cut

commit 4a2a882e18f2292c1be1e9b1cf1a81a53974526e
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Wed Aug 30 14:41:14 2023 -0400

    Update version note for MySQL 8 support
    
    MySQL 8 support was released in version 5.0.4, but we missed
    the commit to add it to the UPGRADING-5.0 doc. Update to the
    correct version even though we're adding the note in 5.0.5.

diff --git a/docs/UPGRADING-5.0 b/docs/UPGRADING-5.0
index 1dd721a98e..f7248f5022 100644
--- a/docs/UPGRADING-5.0
+++ b/docs/UPGRADING-5.0
@@ -604,15 +604,9 @@ fields containing C<HTML>, you can update them and switch the type to C<HTML>
 to use CKEditor for editing. This is specifically useful for the Content
 custom field in articles.
 
-=back
-
-=head1 UPGRADING FROM 5.0.4 AND EARLIER
-
-=over 4
-
 =item * MySQL 8 now supported
 
-Starting with RT 5.0.5, RT now supports MySQL 8. Note that as part of this upgrade
+Starting with RT 5.0.4, RT now supports MySQL 8. Note that as part of this upgrade
 you also need to update the module L<DBIx::SearchBuilder>.
 
 =back

commit 3abcfb30fcaa658fcad19a27ec34b7a42c3c4800
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu May 26 15:38:40 2022 +0800

    Test bulk update messages to make sure the duplicated prefix issue is gone

diff --git a/t/assets/web.t b/t/assets/web.t
index 2cc7d5eeef..7f3e2f8d52 100644
--- a/t/assets/web.t
+++ b/t/assets/web.t
@@ -111,7 +111,15 @@ diag "Create with CFs in other groups";
 diag "Bulk update";
 {
     $m->follow_link_ok( { id => 'assets-simple_search' }, "Asset search page" );
-    $m->submit_form_ok( { form_id => 'AssetSearch' }, "Search assets" );
+    $m->submit_form_ok(
+        {
+            form_id => 'AssetSearch',
+            fields  => { Catalog => $catalog->Id },
+            button  => 'SearchAssets'
+        },
+        "Search assets"
+    );
+
     $m->follow_link_ok( { text => 'Bulk Update' }, "Asset bulk update page" );
 
     my $form = $m->form_id('BulkUpdate');
@@ -121,6 +129,19 @@ diag "Bulk update";
         [ '', 'allocated', 'deleted', 'in-use', 'new', 'recycled', 'stolen' ],
         'Status options'
     );
+
+    $m->submit_form_ok(
+        {
+            fields => {
+                UpdateStatus => 'allocated',
+            },
+            button => 'Update',
+        },
+        'Submit form BulkUpdate'
+    );
+    $m->text_like( qr{Asset \d+: Status changed from 'new' to 'allocated'}, 'Bulk update messages' );
+    $m->text_unlike( qr{Asset \d+: Asset \d+:'}, 'Bulk update messages do not have duplicated prefix' );
+
     # TODO: test more bulk update actions
 }
 
diff --git a/t/web/search_bulk_update.t b/t/web/search_bulk_update.t
index c272345d3f..f47c33e5ac 100644
--- a/t/web/search_bulk_update.t
+++ b/t/web/search_bulk_update.t
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 
-use RT::Test tests => 46;
+use RT::Test tests => undef;
 my ( $url, $m ) = RT::Test->started_ok;
 ok( $m->login, 'logged in' );
 
@@ -154,3 +154,13 @@ $m->submit_form(
 );
 $m->content_lacks( 'DeleteLink--', 'links are all deleted' );
 
+$m->submit_form(
+    form_name => 'BulkUpdate',
+    fields      => {
+        Status => 'open',
+    },
+);
+$m->text_like( qr{Ticket \d+: Status changed from 'new' to 'open'}, 'Bulk update messages' );
+$m->text_unlike( qr{Ticket \d+: Ticket \d+:'}, 'Bulk update messages do not have duplicated prefix' );
+
+done_testing;

commit 11e13a31a0d506a9dc8079ee4ba2d4bd48359380
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu May 26 15:45:42 2022 +0800

    Rename search_bulk_update_links.t to test more general things there

diff --git a/t/web/search_bulk_update_links.t b/t/web/search_bulk_update.t
similarity index 100%
rename from t/web/search_bulk_update_links.t
rename to t/web/search_bulk_update.t

commit 0401f6e4c01d7185e5beaa9deec9c84025dc404b
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu May 26 16:03:34 2022 +0800

    Update tests for the prefix change of core field update messages
    
    See also 2e48e824e7

diff --git a/t/rest2/article-customfields.t b/t/rest2/article-customfields.t
index ed25f2aa77..7e6f8e5dda 100644
--- a/t/rest2/article-customfields.t
+++ b/t/rest2/article-customfields.t
@@ -172,7 +172,7 @@ TODO: {
     }
     is_deeply(
         $mech->json_response,
-        [   'Article Article creation using REST: Permission Denied',
+        [   qq{Article $article_id: Permission Denied},
             "Could not add a new value to custom field 'Content': Permission Denied"
         ]
     );
@@ -183,7 +183,7 @@ TODO: {
     is( $res->code, 200 );
     is_deeply(
         $mech->json_response,
-        [   'Article Article update using REST: Name changed from "Article creation using REST" to "Article update using REST"',
+        [   qq{Article $article_id: Name changed from "Article creation using REST" to "Article update using REST"},
             "Could not add a new value to custom field 'Content': Permission Denied"
         ]
     );
@@ -207,7 +207,7 @@ TODO: {
     is( $res->code, 200 );
     is_deeply(
         $mech->json_response,
-        [   'Article More updates using REST: Name changed from "Article update using REST" to "More updates using REST"',
+        [   qq{Article $article_id: Name changed from "Article update using REST" to "More updates using REST"},
             'Content Modified CF added'
         ]
     );
@@ -252,7 +252,7 @@ TODO: {
     $res = $mech->put_json( $article_url, $payload, 'Authorization' => $auth, );
     is( $res->code, 200 );
     is_deeply( $mech->json_response,
-        ['Article No CF change: Name changed from "More updates using REST" to "No CF change"'] );
+        [qq{Article $article_id: Name changed from "More updates using REST" to "No CF change"}] );
 
     $res = $mech->get( $article_url, 'Authorization' => $auth, );
     is( $res->code, 200 );
diff --git a/t/rest2/articles.t b/t/rest2/articles.t
index db6ccd8538..462a407dd7 100644
--- a/t/rest2/articles.t
+++ b/t/rest2/articles.t
@@ -130,7 +130,7 @@ TODO: {
         local $TODO = "RT ->Update isn't introspectable";
         is( $res->code, 403 );
     }
-    is_deeply( $mech->json_response, ['Article Article creation using REST: Permission Denied'] );
+    is_deeply( $mech->json_response, ['Article 1: Permission Denied'] );
 
     $user->PrincipalObj->GrantRight( Right => 'ModifyArticle' );
 
@@ -138,7 +138,7 @@ TODO: {
     is( $res->code, 200 );
     is_deeply(
         $mech->json_response,
-        [   'Article Article update using REST: Name changed from "Article creation using REST" to "Article update using REST"'
+        [   'Article 1: Name changed from "Article creation using REST" to "Article update using REST"'
         ]
     );
 
diff --git a/t/rest2/asset-customfields.t b/t/rest2/asset-customfields.t
index 0cb8da6fb6..f625c38405 100644
--- a/t/rest2/asset-customfields.t
+++ b/t/rest2/asset-customfields.t
@@ -188,7 +188,7 @@ my $no_asset_cf_values = bag(
         = RT::Handle::cmp_version( $RT::VERSION, '4.4.6' ) >= 0
         ? "Could not add a new value to custom field 'Single': Permission Denied"
         : 'Could not add new custom field value: Permission Denied';
-    is_deeply($mech->json_response, ['Asset Asset creation using REST: Permission Denied', 'Asset Asset creation using REST: Permission Denied', $cf_error]);
+    is_deeply($mech->json_response, [qq{Asset $asset_id: Permission Denied}, qq{Asset $asset_id: Permission Denied}, $cf_error]);
 
     $user->PrincipalObj->GrantRight( Right => 'ModifyAsset' );
 
@@ -197,7 +197,7 @@ my $no_asset_cf_values = bag(
         'Authorization' => $auth,
     );
     is($res->code, 200);
-    is_deeply($mech->json_response, ["Asset Asset update using REST: Name changed from 'Asset creation using REST' to 'Asset update using REST'", "Asset Asset update using REST: Status changed from 'new' to 'allocated'", $cf_error]);
+    is_deeply($mech->json_response, ["Asset $asset_id: Name changed from 'Asset creation using REST' to 'Asset update using REST'", "Asset $asset_id: Status changed from 'new' to 'allocated'", $cf_error]);
 
     $res = $mech->get($asset_url,
         'Authorization' => $auth,
@@ -225,7 +225,7 @@ my $no_asset_cf_values = bag(
         'Authorization' => $auth,
     );
     is($res->code, 200);
-    is_deeply($mech->json_response, ["Asset More updates using REST: Name changed from 'Asset update using REST' to 'More updates using REST'", "Asset More updates using REST: Status changed from 'allocated' to 'in-use'", 'Single Modified CF added']);
+    is_deeply($mech->json_response, ["Asset $asset_id: Name changed from 'Asset update using REST' to 'More updates using REST'", "Asset $asset_id: Status changed from 'allocated' to 'in-use'", 'Single Modified CF added']);
 
     $res = $mech->get($asset_url,
         'Authorization' => $auth,
@@ -272,7 +272,7 @@ my $no_asset_cf_values = bag(
         'Authorization' => $auth,
     );
     is($res->code, 200);
-    is_deeply($mech->json_response, ["Asset No CF change: Name changed from 'More updates using REST' to 'No CF change'"]);
+    is_deeply($mech->json_response, ["Asset $asset_id: Name changed from 'More updates using REST' to 'No CF change'"]);
 
     $res = $mech->get($asset_url,
         'Authorization' => $auth,
diff --git a/t/rest2/assets.t b/t/rest2/assets.t
index 4c1984f339..2685b2dff2 100644
--- a/t/rest2/assets.t
+++ b/t/rest2/assets.t
@@ -172,7 +172,7 @@ my ($asset_url, $asset_id);
         local $TODO = "RT ->Update isn't introspectable";
         is($res->code, 403);
     };
-    is_deeply($mech->json_response, ['Asset Asset creation using REST: Permission Denied', 'Asset Asset creation using REST: Permission Denied']);
+    is_deeply($mech->json_response, ['Asset 1: Permission Denied', 'Asset 1: Permission Denied']);
 
     $user->PrincipalObj->GrantRight( Right => 'ModifyAsset' );
 
@@ -181,7 +181,7 @@ my ($asset_url, $asset_id);
         'Authorization' => $auth,
     );
     is($res->code, 200);
-    is_deeply($mech->json_response, ["Asset Asset update using REST: Name changed from 'Asset creation using REST' to 'Asset update using REST'", "Asset Asset update using REST: Status changed from 'new' to 'allocated'"]);
+    is_deeply($mech->json_response, ["Asset 1: Name changed from 'Asset creation using REST' to 'Asset update using REST'", "Asset 1: Status changed from 'new' to 'allocated'"]);
 
     $res = $mech->get($asset_url,
         'Authorization' => $auth,
diff --git a/t/rest2/catalogs.t b/t/rest2/catalogs.t
index 95afc1f67d..b65a454d36 100644
--- a/t/rest2/catalogs.t
+++ b/t/rest2/catalogs.t
@@ -88,7 +88,7 @@ my $catalog_url;
         'Authorization' => $auth,
     );
     is($res->code, 200);
-    is_deeply($mech->json_response, ["Catalog General assets: Description changed from 'The default catalog' to 'gotta serve em all'", "Catalog Servers: Name changed from 'General assets' to 'Servers'"]);
+    is_deeply($mech->json_response, ["Catalog 1: Description changed from 'The default catalog' to 'gotta serve em all'", "Catalog 1: Name changed from 'General assets' to 'Servers'"]);
 
     $res = $mech->get($catalog_url,
         'Authorization' => $auth,
diff --git a/t/rest2/classes.t b/t/rest2/classes.t
index 56f4b9ffc2..f084d8ecfa 100644
--- a/t/rest2/classes.t
+++ b/t/rest2/classes.t
@@ -81,8 +81,8 @@ my $class_url;
     is( $res->code, 200 );
     is_deeply(
         $mech->json_response,
-        [   'Class General: Description changed from "The default class" to "gotta serve em all"',
-            'Class Servers: Name changed from "General" to "Servers"'
+        [   'Class 1: Description changed from "The default class" to "gotta serve em all"',
+            'Class 1: Name changed from "General" to "Servers"'
         ]
     );
 
diff --git a/t/rest2/queues.t b/t/rest2/queues.t
index 25c2b06df9..48501c53af 100644
--- a/t/rest2/queues.t
+++ b/t/rest2/queues.t
@@ -134,7 +134,7 @@ my $queue_url;
         'Authorization' => $auth,
     );
     is($res->code, 200);
-    is_deeply($mech->json_response, ['Queue General: Description changed from "The default queue" to "gotta squash em all"', 'Queue Bugs: Name changed from "General" to "Bugs"']);
+    is_deeply($mech->json_response, ['Queue 1: Description changed from "The default queue" to "gotta squash em all"', 'Queue 1: Name changed from "General" to "Bugs"']);
 
     $res = $mech->get($queue_url,
         'Authorization' => $auth,
diff --git a/t/rest2/user-customfields.t b/t/rest2/user-customfields.t
index 84e1d410dc..9afc933ba1 100644
--- a/t/rest2/user-customfields.t
+++ b/t/rest2/user-customfields.t
@@ -117,7 +117,7 @@ my $no_user_cf_values = bag(
     is_deeply(
         $mech->json_response,
         [
-            "User User1: Name changed from 'user1' to 'User1'",
+            "User $user_id: Name changed from 'user1' to 'User1'",
             RT::Handle::cmp_version( $RT::VERSION, '4.4.6' ) >= 0
             ? "Could not add a new value to custom field 'Freeform': Permission Denied"
             : 'Could not add new custom field value: Permission Denied',
@@ -150,7 +150,7 @@ my $no_user_cf_values = bag(
         'Authorization' => $auth,
     );
     is($res->code, 200);
-    is_deeply($mech->json_response, ["User User1: EmailAddress changed from 'user1\@example.com' to 'user1+rt\@example.com'", 'Freeform Modified CF added']);
+    is_deeply($mech->json_response, ["User $user_id: EmailAddress changed from 'user1\@example.com' to 'user1+rt\@example.com'", 'Freeform Modified CF added']);
 
     $res = $mech->get($user_url,
         'Authorization' => $auth,
@@ -196,7 +196,7 @@ my $no_user_cf_values = bag(
         'Authorization' => $auth,
     );
     is($res->code, 200);
-    is_deeply($mech->json_response, ["User User1: EmailAddress changed from 'user1+rt\@example.com' to 'user1+rt.test\@example.com'"]);
+    is_deeply($mech->json_response, ["User $user_id: EmailAddress changed from 'user1+rt\@example.com' to 'user1+rt.test\@example.com'"]);
 
     $res = $mech->get($user_url,
         'Authorization' => $auth,
diff --git a/t/web/template.t b/t/web/template.t
index 1a02dc98d6..4bca733c65 100644
--- a/t/web/template.t
+++ b/t/web/template.t
@@ -76,7 +76,7 @@ is($m->value('Type'), 'Perl', 'now that we have ExecuteCode we can update Type t
   $m->field(Content => $content);
   $m->submit;
 
-  $m->content_contains('Template Resolved: Content updated');
+  $m->content_like(qr{Template \d+: Content updated});
 
   # next submit should not result in an update
   $m->form_name('ModifyTemplate');
@@ -86,7 +86,7 @@ is($m->value('Type'), 'Perl', 'now that we have ExecuteCode we can update Type t
 
     local $TODO = "WWW::Mechanize doesn't strip newline following <textarea> tag like browsers do";
     # this test fails because the template change makes Mech continuously add newlines where browsers dont
-    $m->content_lacks('Template Resolved: Content updated');
+    $m->content_unlike(qr{Template \d+: Content updated});
 
   }
 }

commit 33e54a7cb2c55ed5958360365a7f9236aae87acb
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu May 26 15:19:23 2022 +0800

    Use id prefix for core field update messages consistently
    
    The can('Name') check is unreliable as most records don't have Name
    method explicitly defined but automatically generated via AUTOLOAD, thus
    if the record hasn't called Name yet, can('Name') returns false,
    otherwise true.
    
    Besides that, for classes of which Name is not a unique value(e.g.
    Assets), the message would be confusing on bulk update pages if Name
    prefix is used, not mentioning that it's inconsistent with other
    non-core field update messages, which all use id prefix.

diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index 7828634bf0..2f6e02d7a4 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -992,9 +992,7 @@ sub _UpdateAttributes {
         my ( $code, $msg ) = $self->$method($value);
         my ($prefix) = ref($self) =~ /RT(?:.*)::(\w+)/;
 
-        # Default to $id, but use name if we can get it.
         my $label = $self->id;
-        $label = $self->Name if (UNIVERSAL::can($self,'Name'));
         # this requires model names to be loc'ed.
 
 =for loc

commit 932a37cf0bfb653c38aafbad12d728c08d40d94a
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue May 24 04:08:36 2022 +0800

    Avoid adding duplicated prefixes like "Ticket ID: " on bulk update pages
    
    At least for core field updates, the messages already contain prefixes
    like "Ticket ID: " added in RT::Record::_UpdateAttributes.
    
    Previously the prefix was "Asset #ID: " on asset bulk update, which was
    inconsistent with ticket bulk update and RT::Record::_UpdateAttributes.
    This commit tweaks it to be "Asset ID: " accordingly.

diff --git a/share/html/Asset/Search/Bulk.html b/share/html/Asset/Search/Bulk.html
index b84adb1968..1f7e94d9ed 100644
--- a/share/html/Asset/Search/Bulk.html
+++ b/share/html/Asset/Search/Bulk.html
@@ -297,7 +297,9 @@ elsif ( $ARGS{Update} ) {
         push @tmp_res, ProcessObjectCustomFieldUpdates( Object => $asset, ARGSRef => \%ARGS );
         push @tmp_res, ProcessRecordLinks( RecordObj => $asset, RecordId => 'Asset', ARGSRef => \%ARGS );
         push @tmp_res, ProcessRecordBulkCustomFields( RecordObj => $asset, ARGSRef => \%ARGS );
-        push @results, map { loc( "Asset #[_1]: [_2]", $asset->id, $_ ) } @tmp_res;
+
+        my $prefix = loc('Asset') . ' ' . $asset->Id . ': ';
+        push @results, map { /^$prefix/ ? $_ : loc( "Asset [_1]: [_2]", $asset->id, $_ ) } @tmp_res;
     }
 
     MaybeRedirectForResults(
diff --git a/share/html/Search/Bulk.html b/share/html/Search/Bulk.html
index 1e2fc77f5a..86844459dc 100644
--- a/share/html/Search/Bulk.html
+++ b/share/html/Search/Bulk.html
@@ -430,8 +430,8 @@ unless ( $ARGS{'AddMoreAttach'} ) {
             @updateresults, @linkresults,  @cfresults
         );
 
-        @tempresults =
-          map { loc( "Ticket [_1]: [_2]", $Ticket->Id, $_ ) } @tempresults;
+        my $prefix = loc('Ticket') . ' ' . $Ticket->Id . ': ';
+        @tempresults = map { /^$prefix/ ? $_ : loc( "Ticket [_1]: [_2]", $Ticket->Id, $_ ) } @tempresults;
 
         @results = ( @results, @tempresults );
     }

-----------------------------------------------------------------------


hooks/post-receive
-- 
rt


More information about the rt-commit mailing list