[Rt-commit] rt branch 5.0/clean-up-bulk-update-messages created. rt-5.0.2-248-g4c1f0c7ef8
BPS Git Server
git at git.bestpractical.com
Thu May 26 14:59:06 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 created
at 4c1f0c7ef84573c86da9b97d4c628a96a797baf4 (commit)
- Log -----------------------------------------------------------------
commit 4c1f0c7ef84573c86da9b97d4c628a96a797baf4
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 c1c6f467ea7c49d9ed267e4125862516094a2f1d
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 00971c2e2519d620c869cda6e29e201497b7c3c6
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..211ec22a73 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 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
commit 1438b1bc45b737736d8dcf26912238f26a2988d4
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 0027cba7ca..91bd53782a 100644
--- a/share/html/Asset/Search/Bulk.html
+++ b/share/html/Asset/Search/Bulk.html
@@ -304,7 +304,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 a62e6b3042..e639a6afaa 100644
--- a/share/html/Search/Bulk.html
+++ b/share/html/Search/Bulk.html
@@ -550,8 +550,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