Follow-up: 276c45b2e9
- Improved ticket policy scope handling.
This commit is contained in:
parent
c4ccbc46ff
commit
880b2fd131
2 changed files with 107 additions and 11 deletions
|
@ -13,7 +13,7 @@ class TicketPolicy < ApplicationPolicy
|
||||||
super
|
super
|
||||||
end
|
end
|
||||||
|
|
||||||
def resolve # rubocop:disable Metrics/AbcSize
|
def resolve # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
|
||||||
raise NoMethodError, <<~ERR.chomp if instance_of?(TicketPolicy::BaseScope)
|
raise NoMethodError, <<~ERR.chomp if instance_of?(TicketPolicy::BaseScope)
|
||||||
specify an access type using a subclass of TicketPolicy::BaseScope
|
specify an access type using a subclass of TicketPolicy::BaseScope
|
||||||
ERR
|
ERR
|
||||||
|
@ -26,12 +26,19 @@ class TicketPolicy < ApplicationPolicy
|
||||||
bind.push(user.group_ids_access(self.class::ACCESS_TYPE))
|
bind.push(user.group_ids_access(self.class::ACCESS_TYPE))
|
||||||
end
|
end
|
||||||
|
|
||||||
if user.organization&.shared
|
if user.permissions?('ticket.customer')
|
||||||
sql.push('(tickets.customer_id = ? OR tickets.organization_id = ?)')
|
if user.organization&.shared
|
||||||
bind.push(user.id, user.organization.id)
|
sql.push('(tickets.customer_id = ? OR tickets.organization_id = ?)')
|
||||||
else
|
bind.push(user.id, user.organization.id)
|
||||||
sql.push('tickets.customer_id = ?')
|
else
|
||||||
bind.push(user.id)
|
sql.push('tickets.customer_id = ?')
|
||||||
|
bind.push(user.id)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# The report permission can access all tickets.
|
||||||
|
if sql.empty? && !user.permissions?('report')
|
||||||
|
sql.push '0 = 1' # Forbid unlimited access for all other permissions.
|
||||||
end
|
end
|
||||||
|
|
||||||
scope.where sql.join(' OR '), *bind
|
scope.where sql.join(' OR '), *bind
|
||||||
|
|
|
@ -12,15 +12,13 @@ RSpec.shared_examples 'for agent user' do |access_type|
|
||||||
|
|
||||||
shared_examples 'shown' do
|
shared_examples 'shown' do
|
||||||
it 'returns its groups’ tickets' do
|
it 'returns its groups’ tickets' do
|
||||||
expect(scope.resolve)
|
expect(scope.resolve).to match_array(Ticket.where(group: member_groups))
|
||||||
.to match_array(Ticket.where(group: member_groups))
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
shared_examples 'hidden' do
|
shared_examples 'hidden' do
|
||||||
it 'does not return its groups’ tickets' do
|
it 'does not return its groups’ tickets' do
|
||||||
expect(scope.resolve)
|
expect(scope.resolve).to be_empty
|
||||||
.to be_empty
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -47,6 +45,62 @@ RSpec.shared_examples 'for agent user' do |access_type|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'without any role permission' do
|
||||||
|
|
||||||
|
let(:role_without_rights) { create(:role) }
|
||||||
|
let(:user) { create(:agent, groups: member_groups, role_ids: [ role_without_rights.id ]) }
|
||||||
|
|
||||||
|
context 'when checking for "full" access' do
|
||||||
|
# this is already true by default, but it doesn't hurt to be explicit
|
||||||
|
before { user.user_groups.each { |ug| ug.update_columns(access: 'full') } }
|
||||||
|
|
||||||
|
include_examples 'hidden'
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when limited to "read" access' do
|
||||||
|
before { user.user_groups.each { |ug| ug.update_columns(access: 'read') } }
|
||||||
|
|
||||||
|
include_examples 'hidden'
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when limited to "overview" access' do
|
||||||
|
before { user.user_groups.each { |ug| ug.update_columns(access: 'overview') } }
|
||||||
|
|
||||||
|
include_examples 'hidden'
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with report permission' do
|
||||||
|
|
||||||
|
let(:report_role) { create(:role).tap { |role| role.permission_grant('report') } }
|
||||||
|
let(:user) { create(:agent, groups: member_groups, role_ids: [ report_role.id ]) }
|
||||||
|
|
||||||
|
context 'when checking for "full" access' do
|
||||||
|
# this is already true by default, but it doesn't hurt to be explicit
|
||||||
|
before { user.user_groups.each { |ug| ug.update_columns(access: 'full') } }
|
||||||
|
|
||||||
|
it 'grants access to all tickets' do
|
||||||
|
expect(scope.resolve).to match_array(Ticket.all)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when limited to "read" access' do
|
||||||
|
before { user.user_groups.each { |ug| ug.update_columns(access: 'read') } }
|
||||||
|
|
||||||
|
it 'grants access to all tickets' do
|
||||||
|
expect(scope.resolve).to match_array(Ticket.all)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when limited to "overview" access' do
|
||||||
|
before { user.user_groups.each { |ug| ug.update_columns(access: 'overview') } }
|
||||||
|
|
||||||
|
it 'grants access to all tickets' do
|
||||||
|
expect(scope.resolve).to match_array(Ticket.all)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context 'with indirect access via Role#groups' do
|
context 'with indirect access via Role#groups' do
|
||||||
let(:user) { create(:agent).tap { |u| u.roles << role } }
|
let(:user) { create(:agent).tap { |u| u.roles << role } }
|
||||||
let(:role) { create(:role, groups: member_groups) }
|
let(:role) { create(:role, groups: member_groups) }
|
||||||
|
@ -70,6 +124,41 @@ RSpec.shared_examples 'for agent user' do |access_type|
|
||||||
include_examples access_type == 'overview' ? 'shown' : 'hidden'
|
include_examples access_type == 'overview' ? 'shown' : 'hidden'
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when checking access via customer organization but no customer permissions' do
|
||||||
|
let(:user) { create(:agent, organization: organization) }
|
||||||
|
|
||||||
|
let(:teammate) { create(:agent, organization: organization) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
create_list(:ticket, 2, customer: user)
|
||||||
|
create_list(:ticket, 2, customer: teammate)
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with no #organization' do
|
||||||
|
let(:organization) { nil }
|
||||||
|
|
||||||
|
it 'returns no tickets' do
|
||||||
|
expect(scope.resolve).to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with a non-shared #organization' do
|
||||||
|
let(:organization) { create(:organization, shared: false) }
|
||||||
|
|
||||||
|
it 'returns no tickets' do
|
||||||
|
expect(scope.resolve).to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with a shared #organization (default)' do
|
||||||
|
let(:organization) { create(:organization, shared: true) }
|
||||||
|
|
||||||
|
it 'returns no tickets' do
|
||||||
|
expect(scope.resolve).to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
RSpec.shared_examples 'for agent user with predefined but impossible context' do
|
RSpec.shared_examples 'for agent user with predefined but impossible context' do
|
||||||
|
|
Loading…
Reference in a new issue