Просмотр исходного кода

Fixes #4248 - "My Replacement Tickets" overview is shown even if I'm not selected for replacement.

Mantas 2 лет назад
Родитель
Сommit
0d2ae4defd

+ 10 - 1
app/models/user.rb

@@ -260,8 +260,17 @@ returns
     User.where(id: out_of_office_agent_of_recursive(user_id: id))
   end
 
+  scope :out_of_office, lambda { |user, interval_start = Time.zone.today, interval_end = Time.zone.today|
+    where(active: true, out_of_office: true, out_of_office_replacement_id: user)
+      .where('out_of_office_start_at <= ? AND out_of_office_end_at >= ?', interval_start, interval_end)
+  }
+
+  def someones_out_of_office_replacement?
+    self.class.out_of_office(self).exists?
+  end
+
   def out_of_office_agent_of_recursive(user_id:, result: [])
-    User.where(active: true, out_of_office: true, out_of_office_replacement_id: user_id).where('out_of_office_start_at <= ? AND out_of_office_end_at >= ?', Time.zone.today, Time.zone.today).each do |user|
+    self.class.out_of_office(user_id).each do |user|
 
       # stop if users are occuring multiple times to prevent endless loops
       break if result.include?(user.id)

+ 11 - 32
app/policies/ticket/overviews_policy/scope.rb

@@ -3,52 +3,31 @@
 class Ticket::OverviewsPolicy < ApplicationPolicy
   class Scope < ApplicationPolicy::Scope
     def resolve
-      if user.permissions?('ticket.customer')
-        customer_scope
-      elsif user.permissions?('ticket.agent')
-        agent_scope
-      else
-        empty_scope
-      end
-    end
+      return scope.none if !user.permissions?(%w[ticket.customer ticket.agent])
 
-    private
+      scope = base_query
 
-    def customer_scope
-      if user.shared_organizations?
-        base_query
-      else
-        base_query.where(organization_shared: false)
+      if !user.shared_organizations?
+        scope = scope.where(organization_shared: false)
       end
-    end
 
-    def agent_scope
-      if user_is_someones_out_of_office_replacement?
-        base_query
-      else
-        base_query.where.not(out_of_office: true)
+      if !user.someones_out_of_office_replacement?
+        scope = scope.where.not(out_of_office: true)
       end
-    end
 
-    def empty_scope
-      scope.where(id: nil)
+      scope
     end
 
+    private
+
     def base_query
-      scope.joins(roles: :users)
+      scope.distinct
+              .joins(roles: :users)
               .where(active: true)
               .where(roles: { active: true })
               .where(users: { id: user.id, active: true })
               .left_joins(:users)
               .where(overviews_users: { user_id: [nil, user.id] })
     end
-
-    def user_is_someones_out_of_office_replacement?
-      User.where(out_of_office: true)
-          .where('out_of_office_start_at <= ?', Time.zone.today)
-          .where('out_of_office_end_at >= ?', Time.zone.today)
-          .where(out_of_office_replacement_id: user.id)
-          .exists?(active: true)
-    end
   end
 end

+ 17 - 0
spec/models/user_spec.rb

@@ -173,6 +173,23 @@ RSpec.describe User, type: :model do
       end
     end
 
+    describe '#someones_out_of_office_replacement?' do
+      it 'returns true when is replacing someone' do
+        create(:agent).update!(
+          out_of_office:                true,
+          out_of_office_start_at:       1.day.ago,
+          out_of_office_end_at:         1.day.from_now,
+          out_of_office_replacement_id: user.id,
+        )
+
+        expect(user).to be_someones_out_of_office_replacement
+      end
+
+      it 'returns false when is not replacing anyone' do
+        expect(user).not_to be_someones_out_of_office_replacement
+      end
+    end
+
     describe '#out_of_office_agent' do
       it { is_expected.to respond_to(:out_of_office_agent) }
 

+ 126 - 0
spec/policies/ticket/overviews_policy/scope_spec.rb

@@ -0,0 +1,126 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe Ticket::OverviewsPolicy::Scope do
+  subject(:scope) { described_class.new(user, original_collection) }
+
+  let(:original_collection) { Overview }
+
+  let(:overview_a) { create(:overview) }
+  let(:overview_b) { create(:overview, organization_shared: true) }
+  let(:overview_c) { create(:overview, out_of_office: true) }
+
+  before do
+    Overview.destroy_all
+
+    overview_a && overview_b && overview_c
+  end
+
+  describe '#resolve' do
+    context 'without user' do
+      let(:user) { nil }
+
+      it 'throws exception' do
+        expect { scope.resolve }.to raise_error %r{Authentication required}
+      end
+    end
+
+    context 'with customer' do
+      let(:user) { create(:customer, organization: create(:organization, shared: false)) }
+
+      it 'returns base' do
+        expect(scope.resolve).to match_array [overview_a]
+      end
+
+      context 'with shared organization' do
+        before do
+          user.organization.update! shared: true
+        end
+
+        it 'returns base and shared' do
+          expect(scope.resolve).to match_array [overview_a, overview_b]
+        end
+      end
+    end
+
+    context 'with agent' do
+      let(:user) { create(:agent) }
+
+      it 'returns base' do
+        expect(scope.resolve).to match_array [overview_a]
+      end
+
+      context 'when out of office replacement' do
+        before do
+          create(:agent).update!(
+            out_of_office:                true,
+            out_of_office_start_at:       1.day.ago,
+            out_of_office_end_at:         1.day.from_now,
+            out_of_office_replacement_id: user.id,
+          )
+        end
+
+        it 'returns base and out of office' do
+          expect(scope.resolve).to match_array [overview_a, overview_c]
+        end
+      end
+    end
+
+    context 'with agent-customer' do
+      let(:user) { create(:agent_and_customer, organization: create(:organization, shared: false)) }
+
+      it 'returns base' do
+        expect(scope.resolve).to match_array [overview_a]
+      end
+
+      context 'with shared organization' do
+        before do
+          user.organization.update! shared: true
+        end
+
+        it 'returns base and shared' do
+          expect(scope.resolve).to match_array [overview_a, overview_b]
+        end
+
+        context 'when out of office replacement' do
+          before do
+            create(:agent).update!(
+              out_of_office:                true,
+              out_of_office_start_at:       1.day.ago,
+              out_of_office_end_at:         1.day.from_now,
+              out_of_office_replacement_id: user.id,
+            )
+          end
+
+          it 'returns all' do
+            expect(scope.resolve).to match_array [overview_a, overview_b, overview_c]
+          end
+        end
+      end
+
+      context 'when out of office replacement' do
+        before do
+          create(:agent).update!(
+            out_of_office:                true,
+            out_of_office_start_at:       1.day.ago,
+            out_of_office_end_at:         1.day.from_now,
+            out_of_office_replacement_id: user.id,
+          )
+        end
+
+        it 'returns base and out of office' do
+          expect(scope.resolve).to match_array [overview_a, overview_c]
+        end
+      end
+    end
+
+    context 'without ticket permission' do
+      let(:user) { create(:admin_only) }
+
+      it 'returns nothing' do
+        expect(scope.resolve).to be_empty
+      end
+    end
+  end
+end