Browse Source

Fixes #5280 - It takes two clicks to show all groups when creating a new ticket.

Co-authored-by: Florian Liebe <fl@zammad.com>
Rolf Schmidt 7 months ago
parent
commit
2912628a22

+ 1 - 1
app/models/group/assets.rb

@@ -12,7 +12,7 @@ class Group
     end
 
     def authorized_asset?
-      return true if UserInfo.assets.blank? || UserInfo.assets.agent?
+      return true if UserInfo.assets.blank? || UserInfo.assets.agent? || Setting.get('customer_ticket_create_group_ids').blank?
 
       allowed_group_ids = Auth::RequestCache.fetch_value("Group/Assets/authorized_asset/groups/#{UserInfo.current_user_id}") do
         Array.wrap(Setting.get('customer_ticket_create_group_ids')).map(&:to_i) | TicketPolicy::ReadScope.new(User.find(UserInfo.current_user_id)).resolve.distinct(:group_id).pluck(:group_id)

+ 3 - 1
lib/sessions/event.rb

@@ -22,6 +22,8 @@ class Sessions::Event
       Rails.logger.error e.backtrace
       { event: 'error', data: { error: e.message, payload: params[:payload] } }
     end
+  ensure
+    UserInfo.current_user_id = nil
+    ActiveSupport::CurrentAttributes.clear_all
   end
-
 end

+ 12 - 0
lib/sessions/event/base.rb

@@ -20,6 +20,18 @@ class Sessions::Event::Base
       @reused_connection = false
       ActiveRecord::Base.establish_connection
     end
+
+    session_user_info
+  end
+
+  def session_user_info
+    return if !@session
+    return if !@session['id']
+
+    user = User.lookup(id: @session['id'])
+    return if user.blank?
+
+    UserInfo.current_user_id = user.id
   end
 
   def self.inherited(subclass)

+ 2 - 0
lib/sessions/event/core_workflow.rb

@@ -4,6 +4,8 @@ class Sessions::Event::CoreWorkflow < Sessions::Event::Base
   database_connection_required
 
   def run
+    return if !valid_session?
+
     {
       event: 'core_workflow',
       data:  CoreWorkflow.perform(payload: @payload, user: current_user)

+ 0 - 2
lib/websocket_server.rb

@@ -118,8 +118,6 @@ class WebsocketServer
     else
       log 'error', "unknown message '#{data.inspect}'", client_id
     end
-  ensure
-    ActiveSupport::CurrentAttributes.clear_all
   end
 
   def self.websocket_send(client_id, data)

+ 75 - 31
spec/system/basic/assets_spec.rb

@@ -12,12 +12,6 @@ RSpec.describe 'Assets', db_strategy: :reset, type: :system do
   end
   let(:admin)        { create(:admin, groups: [Group.find_by(name: 'Users')], note: 'hello', last_login: Time.zone.now, login_failed: 1) }
   let(:ticket)       { create(:ticket, owner: agent, group: Group.find_by(name: 'Users'), customer: customer, created_by: admin) }
-  let(:agent_groups) { create_list(:group, 3) }
-
-  before do
-    agent_groups
-    Setting.set('customer_ticket_create_group_ids', [Group.first.id])
-  end
 
   context 'groups' do
     before do
@@ -141,50 +135,96 @@ RSpec.describe 'Assets', db_strategy: :reset, type: :system do
       ].compact
     end
 
-    before do
-      visit "#ticket/zoom/#{ticket.id}"
-    end
-
     describe 'when customer', authenticated_as: :customer do
-      it 'can access customer email' do
-        expect(customer_email).not_to be_nil
-      end
+      let(:agent_groups) { create_list(:group, 3) }
 
-      it 'can not access customer note' do
-        expect(customer_note).to be_nil
-      end
+      context 'when zoom' do
+        before do
+          visit "#ticket/zoom/#{ticket.id}"
+        end
 
-      it 'can not access owner details' do
-        expect(owner_details).to be_empty
-      end
+        it 'can access customer email' do
+          expect(customer_email).not_to be_nil
+        end
 
-      it 'can access owner firstname' do
-        expect(owner_firstname).not_to be_nil
-      end
+        it 'can not access customer note' do
+          expect(customer_note).to be_nil
+        end
+
+        it 'can not access owner details' do
+          expect(owner_details).to be_empty
+        end
+
+        it 'can access owner firstname' do
+          expect(owner_firstname).not_to be_nil
+        end
 
-      it 'can access not owner owner accounts' do
-        expect(owner_accounts).to be_nil
+        it 'can access not owner owner accounts' do
+          expect(owner_accounts).to be_nil
+        end
+
+        context 'when groups are restricted', authenticated_as: :authenticate do
+          def authenticate
+            agent_groups
+            Setting.set('customer_ticket_create_group_ids', [Group.first.id])
+            customer
+          end
+
+          it 'can not access agent groups' do
+            expect(customer_available_group_count).to eq(1)
+          end
+
+          context 'when there are old tickets for the customer', authenticated_as: :authenticate do
+            def authenticate
+              agent_groups
+              create(:ticket, group: agent_groups.first, customer: customer)
+              Setting.set('customer_ticket_create_group_ids', [Group.first.id])
+              customer
+            end
+
+            it 'can access one of the agent groups' do
+              expect(customer_available_group_count).to eq(2)
+            end
+          end
+        end
       end
 
-      context 'when groups are restricted' do
-        it 'can not access agent groups' do
-          expect(customer_available_group_count).to eq(1)
+      context 'when ticket create' do
+        before do
+          visit '#customer_ticket_new'
         end
 
-        context 'when there are old tickets for the customer', authenticated_as: :authenticate do
+        context 'when there are no customer groups', authenticated_as: :authenticate do
           def authenticate
-            create(:ticket, group: agent_groups.first, customer: customer)
+            agent_groups
+            Setting.set('customer_ticket_create_group_ids', [])
             customer
           end
 
-          it 'can access one of the agent groups' do
-            expect(customer_available_group_count).to eq(2)
+          it 'can create tickets in all groups' do
+            expect(customer_available_group_count).to eq(5)
+          end
+        end
+
+        context 'when there are customer groups', authenticated_as: :authenticate do
+          def authenticate
+            agent_groups
+            Setting.set('customer_ticket_create_group_ids', [Group.first.id])
+            customer
+          end
+
+          it 'can create tickets in configured groups' do
+            expect(customer_available_group_count).to eq(1)
           end
         end
       end
     end
 
     describe 'when agent', authenticated_as: :agent do
+      before do
+        visit "#ticket/zoom/#{ticket.id}"
+      end
+
       it 'can access customer email' do
         expect(customer_email).not_to be_nil
       end
@@ -207,6 +247,10 @@ RSpec.describe 'Assets', db_strategy: :reset, type: :system do
     end
 
     describe 'when admin', authenticated_as: :admin do
+      before do
+        visit "#ticket/zoom/#{ticket.id}"
+      end
+
       it 'can access customer email' do
         expect(customer_email).not_to be_nil
       end

+ 4 - 4
test/unit/chat_test.rb

@@ -66,7 +66,7 @@ class ChatTest < ActiveSupport::TestCase
     message = Sessions::Event.run(
       event:     'login',
       payload:   {},
-      session:   123,
+      session:   {},
       remote_ip: '127.0.0.1',
       client_id: '123',
       clients:   {
@@ -85,7 +85,7 @@ class ChatTest < ActiveSupport::TestCase
     message = Sessions::Event.run(
       event:     'chat_status_customer',
       payload:   {},
-      session:   123,
+      session:   {},
       remote_ip: '127.0.0.1',
       client_id: '123',
       clients:   {
@@ -107,7 +107,7 @@ class ChatTest < ActiveSupport::TestCase
     message = Sessions::Event.run(
       event:     'login',
       payload:   {},
-      session:   123,
+      session:   {},
       remote_ip: '127.0.0.1',
       client_id: '123',
       clients:   {},
@@ -119,7 +119,7 @@ class ChatTest < ActiveSupport::TestCase
     message = Sessions::Event.run(
       event:     'chat_status_customer',
       payload:   {},
-      session:   123,
+      session:   {},
       remote_ip: '127.0.0.1',
       client_id: '123',
       clients:   {},