[Rt-commit] rt branch 5.0/clean-up-bulk-update-messages updated. rt-5.0.2-248-ge583e4a3a4

BPS Git Server git at git.bestpractical.com
Thu May 26 14:14:34 UTC 2022


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 updated
       via  e583e4a3a42f4c61c4e27d7ba93e6ce7e4775ade (commit)
       via  77e0fd3ee8ac13ada4d3a0adf21c39cf09a202c6 (commit)
       via  6774aeafe4002853c03ce672eac443ce0c4293ca (commit)
       via  2e48e824e7b5177e6f32f8ef4019c0168eb662c5 (commit)
      from  1438b1bc45b737736d8dcf26912238f26a2988d4 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit e583e4a3a42f4c61c4e27d7ba93e6ce7e4775ade
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 77e0fd3ee8ac13ada4d3a0adf21c39cf09a202c6
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 6774aeafe4002853c03ce672eac443ce0c4293ca
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 8020242631..2b41fcac3b 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',
+        [   'Article 1: 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"',
+        [   'Article 1: 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"',
+        [   'Article 1: 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"'] );
+        ['Article 1: 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..b84cd0c588 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, ['Asset 1: Permission Denied', 'Asset 1: 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 1: Name changed from 'Asset creation using REST' to 'Asset update using REST'", "Asset 1: 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 1: Name changed from 'Asset update using REST' to 'More updates using REST'", "Asset 1: 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 1: 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 2e48e824e7b5177e6f32f8ef4019c0168eb662c5
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 5ae9184c55..ca07e9cfed 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -986,9 +986,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

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

Summary of changes:
 lib/RT/Record.pm                                   |  2 --
 t/assets/web.t                                     | 23 +++++++++++++++++++++-
 t/rest2/article-customfields.t                     |  8 ++++----
 t/rest2/articles.t                                 |  4 ++--
 t/rest2/asset-customfields.t                       |  8 ++++----
 t/rest2/assets.t                                   |  4 ++--
 t/rest2/catalogs.t                                 |  2 +-
 t/rest2/classes.t                                  |  4 ++--
 t/rest2/queues.t                                   |  2 +-
 t/rest2/user-customfields.t                        |  6 +++---
 ...ch_bulk_update_links.t => search_bulk_update.t} | 12 ++++++++++-
 t/web/template.t                                   |  4 ++--
 12 files changed, 54 insertions(+), 25 deletions(-)
 rename t/web/{search_bulk_update_links.t => search_bulk_update.t} (93%)


hooks/post-receive
-- 
rt


More information about the rt-commit mailing list