[Rt-commit] rt branch, 4.4/custom-role-not-update-groups, created. rt-4.4.3-42-g99bad53d7
? sunnavy
sunnavy at bestpractical.com
Mon Sep 17 15:02:46 EDT 2018
The branch, 4.4/custom-role-not-update-groups has been created
at 99bad53d76ea19db853d0ce047db0dea46665c18 (commit)
- Log -----------------------------------------------------------------
commit 5f949ae898cbff04ecbe42d258425c601698d520
Author: sunnavy <sunnavy at bestpractical.com>
Date: Fri Sep 7 04:42:01 2018 +0800
Don't enable/disable related groups when updating custom roles
This is for performance: as each ticket has a correponding group for a
custom role, it could be time consuming when there are many tickets
involved.
Since groups are not in sync now, we need to exclude disabled/unapplied
custom roles when listing/checking rights.
This also fixes a bug that when you move a ticket from queue A to B,
people in a custom role that is applied to A can still get rights via
the custom role in B, even if the custom role is not applied to B.
diff --git a/lib/RT/CustomRole.pm b/lib/RT/CustomRole.pm
index 8c9b08451..0f0ab3ae9 100644
--- a/lib/RT/CustomRole.pm
+++ b/lib/RT/CustomRole.pm
@@ -677,25 +677,13 @@ sub SetDisabled {
my $self = shift;
my $value = shift;
- $RT::Handle->BeginTransaction();
-
my ($ok, $msg) = $self->_Set( Field => 'Disabled', Value => $value );
unless ($ok) {
- $RT::Handle->Rollback();
$RT::Logger->warning("Couldn't ".(($value == 0) ? "enable" : "disable")." custom role ".$self->Name.": $msg");
return ($ok, $msg);
}
- # we can't unconditionally re-enable all role groups because
- # if you add a role to queues A and B, add users and privileges and
- # tickets on both, remove the role from B, disable the role, then re-enable
- # the role, we shouldn't re-enable B because it's still removed
- my $queues = $self->AddedTo;
- while (my $queue = $queues->Next) {
- $self->_SetGroupsDisabledForQueue($value, $queue);
- }
-
- $RT::Handle->Commit();
+ RT::Principal->InvalidateACLCache();
if ( $value == 0 ) {
return (1, $self->loc("Custom role enabled"));
diff --git a/lib/RT/ObjectCustomRole.pm b/lib/RT/ObjectCustomRole.pm
index edeaf32a3..872a4ac84 100644
--- a/lib/RT/ObjectCustomRole.pm
+++ b/lib/RT/ObjectCustomRole.pm
@@ -144,11 +144,7 @@ sub Add {
);
if ($existing->Id) {
- # there already was a role group for this queue, which means
- # this was previously added, then removed, and is now being re-added,
- # which means we have to re-enable the queue group and all the
- # ticket groups
- $role->_SetGroupsDisabledForQueue(0, $queue);
+ RT::Principal->InvalidateACLCache();
}
else {
my $group = RT::Group->new($self->CurrentUser);
@@ -179,19 +175,14 @@ Removes the custom role from the queue and disables that queue's role group.
sub Delete {
my $self = shift;
- $RT::Handle->BeginTransaction;
-
- $self->CustomRoleObj->_SetGroupsDisabledForQueue(1, $self->QueueObj);
-
# remove the ObjectCustomRole record
my ($ok, $msg) = $self->SUPER::Delete(@_);
unless ($ok) {
- $RT::Handle->Rollback;
$RT::Logger->error("Couldn't add ObjectCustomRole: $msg");
return(undef);
}
- $RT::Handle->Commit;
+ RT::Principal->InvalidateACLCache();
return ($ok, $msg);
}
diff --git a/lib/RT/Principal.pm b/lib/RT/Principal.pm
index 0d3a59adc..63eee5ac0 100644
--- a/lib/RT/Principal.pm
+++ b/lib/RT/Principal.pm
@@ -433,13 +433,37 @@ sub HasRights {
return ();
}
}
- if ( @$roles ) {
+
+ my @enabled_roles;
+ for my $role ( @$roles ) {
+ if ( $role =~ /^RT::CustomRole-(\d+)$/ ) {
+ my $custom_role = RT::CustomRole->new( RT->SystemUser );
+ $custom_role->Load( $1 );
+ if ( $custom_role->id && !$custom_role->Disabled ) {
+ my $added;
+ for my $object ( @{ $args{'EquivObjects'} } ) {
+ next unless $object->isa('RT::Queue');
+ if ( $custom_role->IsAdded( $object->id ) ) {
+ $added = 1;
+ last;
+ }
+ }
+ push @enabled_roles, $role if $added;
+ }
+ }
+ else {
+ push @enabled_roles, $role;
+ }
+ }
+
+ if ( @enabled_roles ) {
+
my $query
= "SELECT DISTINCT ACL.RightName "
. $self->_RolesWithRightQuery(
EquivObjects => $args{'EquivObjects'}
)
- . ' AND ('. join( ' OR ', map "PrincipalType = '$_'", @$roles ) .')'
+ . ' AND ('. join( ' OR ', map "PrincipalType = '$_'", @enabled_roles ) .')'
;
my $rights = $RT::Handle->dbh->selectcol_arrayref($query);
unless ($rights) {
@@ -639,7 +663,30 @@ sub RolesWithRight {
$RT::Logger->warning( $RT::Handle->dbh->errstr );
return ();
}
- return @$roles;
+
+ my @enabled_roles;
+ for my $role ( @$roles ) {
+ if ( $role =~ /^RT::CustomRole-(\d+)$/ ) {
+ my $custom_role = RT::CustomRole->new( RT->SystemUser );
+ $custom_role->Load( $1 );
+ if ( $custom_role->id && !$custom_role->Disabled ) {
+ my $added;
+ for my $object ( @{ $args{'EquivObjects'} } ) {
+ next unless $object->isa('RT::Queue');
+ if ( $custom_role->IsAdded( $object->id ) ) {
+ $added = 1;
+ last;
+ }
+ }
+ push @enabled_roles, $role if $added;
+ }
+ }
+ else {
+ push @enabled_roles, $role;
+ }
+ }
+
+ return @enabled_roles;
}
sub _RolesWithRightQuery {
diff --git a/share/html/Admin/Elements/EditRights b/share/html/Admin/Elements/EditRights
index 3961e93ca..d69383069 100644
--- a/share/html/Admin/Elements/EditRights
+++ b/share/html/Admin/Elements/EditRights
@@ -183,6 +183,11 @@ for my $category (@$Principals) {
<li class="category"><% loc($name) %></li>
<%perl>
while ( my $obj = $collection->Next ) {
+ next
+ if $obj->isa( 'RT::Group' )
+ && $obj->_CustomRoleObj
+ && ( $obj->_CustomRoleObj->Disabled
+ || ( $Context->isa( 'RT::Queue' ) && !$obj->_CustomRoleObj->IsAdded( $Context->id ) ) );
my $display = ref $col eq 'CODE' ? $col->($obj) : $obj->$col;
my $id = "acl-" . $obj->PrincipalId;
</%perl>
@@ -227,6 +232,11 @@ for my $category (@$Principals) {
for my $category (@$Principals) {
my ($name, $collection, $col, $loc) = @$category;
while ( my $obj = $collection->Next ) {
+ next
+ if $obj->isa( 'RT::Group' )
+ && $obj->_CustomRoleObj
+ && ( $obj->_CustomRoleObj->Disabled
+ || ( $Context->isa( 'RT::Queue' ) && !$obj->_CustomRoleObj->IsAdded( $Context->id ) ) );
my $display = ref $col eq 'CODE' ? $col->($obj) : $obj->$col;
my $id = "acl-" . $obj->PrincipalId;
</%perl>
diff --git a/t/customroles/rights.t b/t/customroles/rights.t
index 238e77868..c7d71b51e 100644
--- a/t/customroles/rights.t
+++ b/t/customroles/rights.t
@@ -163,7 +163,7 @@ sub sales_has_rights_for_specs_individual {
my $t = $specs_individual;
- if (!$has_right || $has_right == 2) {
+ if (!$has_right) {
is($t->RoleAddresses($sales->GroupType), '', "got no salespeople $rationale");
}
else {
@@ -176,12 +176,6 @@ sub sales_has_rights_for_specs_individual {
ok(!$ricky->HasRight(Right => 'ShowTicket', Object => $t), "ricky (ticket sales) has no right to see the ticket $rationale");
ok(!$ricky->HasRight(Right => 'CommentOnTicket', Object => $t), "ricky (ticket sales) has no right to comment on the ticket $rationale");
}
- elsif ($has_right == 2) {
- ok($moss->HasRight(Right => 'ShowTicket', Object => $t), 'moss (ticket sales) has right to see the ticket thru global sales right');
- ok(!$moss->HasRight(Right => 'CommentOnTicket', Object => $t), "moss (ticket sales) has no right to comment on the ticket $rationale");
- ok($ricky->HasRight(Right => 'ShowTicket', Object => $t), 'ricky (ticket sales) has right to see the ticket thru global sales right');
- ok(!$ricky->HasRight(Right => 'CommentOnTicket', Object => $t), "ricky (ticket sales) has no right to comment on the ticket $rationale");
- }
else {
ok($moss->HasRight(Right => 'ShowTicket', Object => $t), 'moss (ticket sales) has right to see the ticket');
ok($moss->HasRight(Right => 'CommentOnTicket', Object => $t), 'moss (ticket sales) has right to comment on the ticket');
@@ -421,7 +415,7 @@ diag 'change queue from Specs to General' if $ENV{'TEST_VERBOSE'};
sales_has_rights_for_inbox_individual(1);
sales_has_rights_for_inbox_group(1);
- sales_has_rights_for_specs_individual(2, 'queue changed to General');
+ sales_has_rights_for_specs_individual(0, 'queue changed to General');
engineer_has_no_rights_for_inbox_individual($_) for $linus, $john;
engineer_has_rights_for_specs_individual($linus => 0);
commit 99bad53d76ea19db853d0ce047db0dea46665c18
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Sep 8 05:19:26 2018 +0800
Enable previously disabled custom role groups
As we don't enable/disable them automatically when updating custom
roles, we need to make sure they are all enabled.
diff --git a/etc/upgrade/4.4.4/content b/etc/upgrade/4.4.4/content
new file mode 100644
index 000000000..c25f86ca6
--- /dev/null
+++ b/etc/upgrade/4.4.4/content
@@ -0,0 +1,18 @@
+use strict;
+use warnings;
+
+our @Final = (
+ sub {
+ my $role_groups = RT::Groups->new( RT->SystemUser );
+ $role_groups->{'find_disabled_rows'} = 1;
+ $role_groups->Limit( FIELD => 'Name', VALUE => 'RT::CustomRole-', OPERATOR => 'LIKE', CASESENSITIVE => 0 );
+ $role_groups->Limit( FIELD => 'Domain', VALUE => '-Role', OPERATOR => 'LIKE', CASESENSITIVE => 0 );
+ $role_groups->LimitToDeleted;
+
+ while ( my $role_group = $role_groups->Next ) {
+ my ( $ret, $msg ) = $role_group->SetDisabled( 0 );
+ RT->Logger->error( "Couldn't enable role group #" . $role_group->id . ": $msg" ) unless $ret;
+ }
+
+ },
+);
-----------------------------------------------------------------------
More information about the rt-commit
mailing list