Browse Source

Fixes #5268 - Inactive Ticket States selectable in Mobile View.

Co-authored-by: Rolf Schmidt <rolf.schmidt@zammad.com>
Co-authored-by: Dusan Vuckovic <dv@zammad.com>
Dusan Vuckovic 7 months ago
parent
commit
b0190017e6

+ 17 - 12
app/models/ticket/state.rb

@@ -73,25 +73,30 @@ returns:
     end
   end
 
-  private
-
-  def update_object_manager_attribute
-    return if !Setting.get('system_init_done')
-    return if callback_loop
-
+  def self.update_state_field_configuration
     attr = ObjectManager::Attribute.get(
       object: 'Ticket',
       name:   'state_id',
     )
 
-    attr.data_option[:filter]                                = Ticket::State.by_category_ids(:viewable)
-    attr.screens[:create_middle]['ticket.agent'][:filter]    = Ticket::State.by_category_ids(:viewable_agent_new)
-    attr.screens[:create_middle]['ticket.customer'][:filter] = Ticket::State.by_category_ids(:viewable_customer_new)
-    attr.screens[:edit]['ticket.agent'][:filter]             = Ticket::State.by_category_ids(:viewable_agent_edit)
-    attr.screens[:edit]['ticket.customer'][:filter]          = Ticket::State.by_category_ids(:viewable_customer_edit)
-    attr.screens[:overview_bulk]['ticket.agent'][:filter]    = Ticket::State.by_category_ids(:viewable_agent_edit)
+    active_states = Ticket::State.where(active: true)
+
+    attr.data_option[:filter]                                = active_states.by_category_ids(:viewable)
+    attr.screens[:create_middle]['ticket.agent'][:filter]    = active_states.by_category_ids(:viewable_agent_new)
+    attr.screens[:create_middle]['ticket.customer'][:filter] = active_states.by_category_ids(:viewable_customer_new)
+    attr.screens[:edit]['ticket.agent'][:filter]             = active_states.by_category_ids(:viewable_agent_edit)
+    attr.screens[:edit]['ticket.customer'][:filter]          = active_states.by_category_ids(:viewable_customer_edit)
+    attr.screens[:overview_bulk]['ticket.agent'][:filter]    = active_states.by_category_ids(:viewable_agent_edit)
 
     attr.save!
   end
 
+  private
+
+  def update_object_manager_attribute
+    return if !Setting.get('system_init_done')
+    return if callback_loop
+
+    self.class.update_state_field_configuration
+  end
 end

+ 10 - 0
db/migrate/20240717152405_issue_5268_update_state_object_manager_attribute.rb

@@ -0,0 +1,10 @@
+# Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+class Issue5268UpdateStateObjectManagerAttribute < ActiveRecord::Migration[7.0]
+  def change
+    # return if it's a new setup
+    return if !Setting.exists?(name: 'system_init_done')
+
+    Ticket::State.update_state_field_configuration
+  end
+end

+ 6 - 6
db/seeds/object_manager_attributes.rb

@@ -254,7 +254,7 @@ ObjectManager::Attribute.add(
     null:       false,
     default:    Ticket::State.find_by(default_follow_up: true).id,
     translate:  true,
-    filter:     Ticket::State.by_category_ids(:viewable),
+    filter:     Ticket::State.where(active: true).by_category_ids(:viewable),
   },
   editable:    false,
   active:      true,
@@ -263,13 +263,13 @@ ObjectManager::Attribute.add(
       'ticket.agent'    => {
         null:       false,
         item_class: 'column',
-        filter:     Ticket::State.by_category_ids(:viewable_agent_new),
+        filter:     Ticket::State.where(active: true).by_category_ids(:viewable_agent_new),
       },
       'ticket.customer' => {
         item_class: 'column',
         nulloption: false,
         null:       true,
-        filter:     Ticket::State.by_category_ids(:viewable_customer_new),
+        filter:     Ticket::State.where(active: true).by_category_ids(:viewable_customer_new),
         default:    Ticket::State.find_by(default_create: true).id,
       },
     },
@@ -277,12 +277,12 @@ ObjectManager::Attribute.add(
       'ticket.agent'    => {
         nulloption: false,
         null:       false,
-        filter:     Ticket::State.by_category_ids(:viewable_agent_edit),
+        filter:     Ticket::State.where(active: true).by_category_ids(:viewable_agent_edit),
       },
       'ticket.customer' => {
         nulloption: false,
         null:       true,
-        filter:     Ticket::State.by_category_ids(:viewable_customer_edit),
+        filter:     Ticket::State.where(active: true).by_category_ids(:viewable_customer_edit),
         default:    Ticket::State.find_by(default_follow_up: true).id,
       },
     },
@@ -291,7 +291,7 @@ ObjectManager::Attribute.add(
         nulloption: true,
         null:       true,
         default:    '',
-        filter:     Ticket::State.by_category_ids(:viewable_agent_edit),
+        filter:     Ticket::State.where(active: true).by_category_ids(:viewable_agent_edit),
       },
     }
   },

+ 33 - 0
spec/db/migrate/issue_5268_update_state_object_manager_attribute_spec.rb

@@ -0,0 +1,33 @@
+# Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe Issue5268UpdateStateObjectManagerAttribute, type: :db_migration do
+  context 'when the ticket state attribute has inactive states in filter values', db_strategy: :reset do
+    let(:inactive_state)  { create(:ticket_state, state_type: Ticket::StateType.find_by(name: 'closed'), active: false) }
+    let(:state_attribute) { ObjectManager::Attribute.get(name: 'state_id', object: 'Ticket') }
+
+    before do
+      inactive_state
+
+      # Purposefully include inactive state in the object manager attribute options.
+      state_attribute.data_option[:filter] = Ticket::State.by_category_ids(:viewable)
+      state_attribute.screens[:create_middle][:'ticket.agent'][:filter] = Ticket::State.by_category_ids(:viewable_agent_new)
+      state_attribute.screens[:create_middle][:'ticket.customer'][:filter] = Ticket::State.by_category_ids(:viewable_customer_new)
+      state_attribute.screens[:edit][:'ticket.agent'][:filter] = Ticket::State.by_category_ids(:viewable_agent_edit)
+      state_attribute.screens[:edit][:'ticket.customer'][:filter] = Ticket::State.by_category_ids(:viewable_customer_edit)
+      state_attribute.screens[:overview_bulk][:'ticket.agent'][:filter] = Ticket::State.by_category_ids(:viewable_agent_edit)
+      state_attribute.save!
+    end
+
+    it 'updates filter values to not include the inactive state' do
+      expect { migrate }
+        .to change  { state_attribute.reload.data_option[:filter] }.from(include(inactive_state.id)).to(not_include(inactive_state.id))
+        .and change { state_attribute.reload.screens[:create_middle][:'ticket.agent'][:filter] }.from(include(inactive_state.id)).to(not_include(inactive_state.id))
+        .and change { state_attribute.reload.screens[:create_middle][:'ticket.customer'][:filter] }.from(include(inactive_state.id)).to(not_include(inactive_state.id))
+        .and change { state_attribute.reload.screens[:edit][:'ticket.agent'][:filter] }.from(include(inactive_state.id)).to(not_include(inactive_state.id))
+        .and change { state_attribute.reload.screens[:edit][:'ticket.customer'][:filter] }.from(include(inactive_state.id)).to(not_include(inactive_state.id))
+        .and change { state_attribute.reload.screens[:overview_bulk][:'ticket.agent'][:filter] }.from(include(inactive_state.id)).to(not_include(inactive_state.id))
+    end
+  end
+end

+ 2 - 2
spec/graphql/gql/queries/object_manager/frontend_attributes_spec.rb

@@ -390,7 +390,7 @@ RSpec.describe Gql::Queries::ObjectManager::FrontendAttributes, type: :graphql d
                 'null'       => false,
                 'default'    => 2,
                 'translate'  => true,
-                'filter'     => Ticket::State.by_category_ids(:viewable),
+                'filter'     => Ticket::State.where(active: true).by_category_ids(:viewable),
                 'maxlength'  => 255,
                 'belongs_to' => 'state',
               },
@@ -520,7 +520,7 @@ RSpec.describe Gql::Queries::ObjectManager::FrontendAttributes, type: :graphql d
                 'null'       => false,
                 'default'    => 2,
                 'translate'  => true,
-                'filter'     => Ticket::State.by_category_ids(:viewable),
+                'filter'     => Ticket::State.where(active: true).by_category_ids(:viewable),
                 'maxlength'  => 255,
                 'belongs_to' => 'state',
               },

+ 16 - 0
spec/models/ticket/state_spec.rb

@@ -238,6 +238,22 @@ RSpec.describe Ticket::State, type: :model do
           )
       end
 
+      it 'updated state_id attribute does not include inactive states (#5268)' do
+        state.update! active: false
+
+        expect(attr.screens)
+          .to include(
+            'create_middle' => include(
+              'ticket.agent'    => include('filter' => not_include(state.id)),
+              'ticket.customer' => include('filter' => not_include(state.id))
+            ),
+            'edit'          => include(
+              'ticket.agent'    => include('filter' => not_include(state.id)),
+              'ticket.customer' => include('filter' => not_include(state.id))
+            )
+          )
+      end
+
       it 'updates state_id attribute when a state is destroyed' do
         state.destroy!
 

+ 5 - 5
spec/system/ticket/create_spec.rb

@@ -613,11 +613,11 @@ RSpec.describe 'Ticket Create', type: :system do
         object: 'Ticket',
         name:   'state_id',
       )
-      attribute.data_option[:filter] = Ticket::State.by_category_ids(:viewable)
-      attribute.screens[:create_middle]['ticket.agent'][:filter] = Ticket::State.by_category_ids(:viewable_agent_new)
-      attribute.screens[:create_middle]['ticket.customer'][:filter] = Ticket::State.by_category_ids(:viewable_customer_new)
-      attribute.screens[:edit]['ticket.agent'][:filter] = Ticket::State.by_category_ids(:viewable_agent_edit)
-      attribute.screens[:edit]['ticket.customer'][:filter] = Ticket::State.by_category_ids(:viewable_customer_edit)
+      attribute.data_option[:filter] = Ticket::State.where(active: true).by_category_ids(:viewable)
+      attribute.screens[:create_middle]['ticket.agent'][:filter] = Ticket::State.where(active: true).by_category_ids(:viewable_agent_new)
+      attribute.screens[:create_middle]['ticket.customer'][:filter] = Ticket::State.where(active: true).by_category_ids(:viewable_customer_new)
+      attribute.screens[:edit]['ticket.agent'][:filter] = Ticket::State.where(active: true).by_category_ids(:viewable_agent_edit)
+      attribute.screens[:edit]['ticket.customer'][:filter] = Ticket::State.where(active: true).by_category_ids(:viewable_customer_edit)
       attribute.save!
     end