Moved default and follow up state/priority from source code into model. Related to issue #689 - OTRS import breaks a lot of assumptions in Zammad.

app/assets/javascripts/app/controllers/customer_ticket_create.coffee

@@ -102,12 +102,12 @@ class Index extends App.ControllerContent
 
     # set prio
     if !params.priority_id
-      priority = App.TicketPriority.findByAttribute( 'name', '2 normal' )
+      priority = App.TicketPriority.findByAttribute( 'default_create', true )
       params.priority_id = priority.id
 
     # set state
     if !params.state_id
-      state = App.TicketState.findByAttribute( 'name', 'new' )
+      state = App.TicketState.findByAttribute( 'default_create', true )
       params.state_id = state.id
 
     # fillup params

app/controllers/first_steps_controller.rb

@@ -188,10 +188,7 @@ class FirstStepsController < ApplicationController
     ticket = Ticket.create(
       group_id: Group.find_by(active: true, name: 'Users').id,
       customer_id: customer.id,
-      owner_id: User.find_by(login: '-').id,
       title: result[:subject],
-      state_id: Ticket::State.find_by(name: 'new').id,
-      priority_id: Ticket::Priority.find_by(name: '2 normal').id,
     )
     article = Ticket::Article.create(
       ticket_id: ticket.id,

app/controllers/form_controller.rb

@@ -98,8 +98,6 @@ class FormController < ApplicationController
       group_id: 1,
       customer_id: customer.id,
       title: params[:title],
-      state_id: Ticket::State.find_by(name: 'new').id,
-      priority_id: Ticket::Priority.find_by(name: '2 normal').id,
     )
     article = Ticket::Article.create(
       ticket_id: ticket.id,

app/models/channel/email_parser.rb

@@ -481,10 +481,11 @@ returns
 
         end
 
-        # set ticket to open again
+        # set ticket to open again or keep create state
         if !mail['x-zammad-ticket-followup-state'.to_sym] && !mail['x-zammad-ticket-followup-state_id'.to_sym]
-          if state_type.name != 'new' && !mail['x-zammad-out-of-office'.to_sym]
-            ticket.state = Ticket::State.find_by(name: 'open')
+          new_state = Ticket::State.find_by(default_create: true)
+          if ticket.state_id != new_state.id && !mail['x-zammad-out-of-office'.to_sym]
+            ticket.state = Ticket::State.find_by(default_follow_up: true)
             ticket.save!
           end
         end
@@ -514,8 +515,6 @@ returns
         ticket = Ticket.new(
           group_id: group.id,
           title: mail[:subject] || '',
-          state_id: Ticket::State.find_by(name: 'new').id,
-          priority_id: Ticket::Priority.find_by(name: '2 normal').id,
           preferences: preferences,
         )
         set_attributes_by_x_headers(ticket, 'ticket', mail)

app/models/observer/ticket/reset_new_state.rb

@@ -19,10 +19,10 @@ class Observer::Ticket::ResetNewState < ActiveRecord::Observer
 
     # if current ticket state is still new
     ticket = Ticket.lookup(id: record.ticket_id)
-    return true if ticket.state.state_type.name != 'new'
+    new_state = Ticket::State.find_by(default_create: true)
+    return true if ticket.state_id != new_state.id
 
-    # TODO: add config option to state managment in UI
-    state = Ticket::State.lookup(name: 'open')
+    state = Ticket::State.find_by(default_follow_up: true)
     return if !state
 
     # set ticket to open

app/models/ticket.rb

@@ -18,13 +18,11 @@ class Ticket < ApplicationModel
   extend Ticket::Search
 
   store          :preferences
-  before_create  :check_generate, :check_defaults, :check_title, :check_escalation_update
+  before_create  :check_generate, :check_defaults, :check_title, :check_escalation_update, :set_default_state, :set_default_priority
   before_update  :check_defaults, :check_title, :reset_pending_time, :check_escalation_update
   before_destroy :destroy_dependencies
 
   validates :group_id, presence: true
-  validates :priority_id, presence: true
-  validates :state_id, presence: true
 
   activity_stream_permission 'ticket.agent'
 
@@ -925,4 +923,22 @@ result
     # destroy online notifications
     OnlineNotification.remove(self.class.to_s, id)
   end
+
+  def set_default_state
+    return if state_id
+
+    default_ticket_state = Ticket::State.find_by(default_create: true)
+    return if !default_ticket_state
+
+    self.state_id = default_ticket_state.id
+  end
+
+  def set_default_priority
+    return if priority_id
+
+    default_ticket_priority = Ticket::Priority.find_by(default_create: true)
+    return if !default_ticket_priority
+
+    self.priority_id = default_ticket_priority.id
+  end
 end

app/models/ticket/priority.rb

@@ -2,4 +2,33 @@
 class Ticket::Priority < ApplicationModel
   self.table_name = 'ticket_priorities'
   validates :name, presence: true
+
+  after_create  :ensure_defaults
+  after_update  :ensure_defaults
+  after_destroy :ensure_defaults
+
+  attr_accessor :callback_loop
+
+  def ensure_defaults
+    return if callback_loop
+    priorities_with_default = Ticket::Priority.where(default_create: true)
+    return if priorities_with_default.count == 1
+
+    if priorities_with_default.count.zero?
+      priority = Ticket::Priority.where(active: true).order(id: :asc).first
+      priority.default_create = true
+      priority.callback_loop = true
+      priority.save!
+      return
+    end
+
+    if priorities_with_default.count > 1
+      Ticket::Priority.all.each { |local_priority|
+        next if local_priority.id == id
+        local_priority.default_create = false
+        local_priority.callback_loop = true
+        local_priority.save!
+      }
+    end
+  end
 end

app/models/ticket/state.rb

@@ -2,10 +2,16 @@
 class Ticket::State < ApplicationModel
   include LatestChangeObserved
 
+  after_create  :ensure_defaults
+  after_update  :ensure_defaults
+  after_destroy :ensure_defaults
+
   belongs_to    :state_type, class_name: 'Ticket::StateType'
   belongs_to    :next_state, class_name: 'Ticket::State'
   validates     :name, presence: true
 
+  attr_accessor :callback_loop
+
 =begin
 
 list tickets by customer
@@ -69,4 +75,31 @@ returns:
     return true if ignore_escalation
     false
   end
+
+  def ensure_defaults
+    return if callback_loop
+
+    %w(default_create default_follow_up).each do |default_field|
+      states_with_default = Ticket::State.where(default_field => true)
+      next if states_with_default.count == 1
+
+      if states_with_default.count.zero?
+        state = Ticket::State.where(active: true).order(id: :asc).first
+        state[default_field] = true
+        state.callback_loop = true
+        state.save!
+        next
+      end
+
+      Ticket::State.all.each { |local_state|
+        next if local_state.id == id
+        next if local_state[default_field] == false
+        local_state[default_field] = false
+        local_state.callback_loop = true
+        local_state.save!
+        next
+      }
+    end
+  end
+
 end

db/migrate/20120101000010_create_ticket.rb

@@ -14,6 +14,8 @@ class CreateTicket < ActiveRecord::Migration
       t.column :name,                 :string, limit: 250,  null: false
       t.column :next_state_id,        :integer,             null: true
       t.column :ignore_escalation,    :boolean,             null: false, default: false
+      t.column :default_create,       :boolean,             null: false, default: false
+      t.column :default_follow_up,    :boolean,             null: false, default: false
       t.column :note,                 :string, limit: 250,  null: true
       t.column :active,               :boolean,             null: false, default: true
       t.column :updated_by_id,        :integer,             null: false
@@ -21,9 +23,12 @@ class CreateTicket < ActiveRecord::Migration
       t.timestamps limit: 3, null: false
     end
     add_index :ticket_states, [:name], unique: true
+    add_index :ticket_states, [:default_create]
+    add_index :ticket_states, [:default_follow_up]
 
     create_table :ticket_priorities do |t|
       t.column :name,                 :string, limit: 250, null: false
+      t.column :default_create,       :boolean,            null: false, default: false
       t.column :note,                 :string, limit: 250, null: true
       t.column :active,               :boolean,            null: false, default: true
       t.column :updated_by_id,        :integer,            null: false
@@ -31,6 +36,7 @@ class CreateTicket < ActiveRecord::Migration
       t.timestamps limit: 3, null: false
     end
     add_index :ticket_priorities, [:name], unique: true
+    add_index :ticket_priorities, [:default_create]
 
     create_table :tickets do |t|
       t.references :group,                                                null: false

db/migrate/20170207081400_ticket_state_priority_defaults.rb

@@ -0,0 +1,57 @@
+class TicketStatePriorityDefaults < ActiveRecord::Migration
+  def up
+
+    # return if it's a new setup
+    return if !Setting.find_by(name: 'system_init_done')
+
+    add_column :ticket_states, :default_create, :boolean, null: false, default: false
+    add_index  :ticket_states, :default_create
+    add_column :ticket_states, :default_follow_up, :boolean, null: false, default: false
+    add_index  :ticket_states, :default_follow_up
+
+    add_column :ticket_priorities, :default_create, :boolean, null: false, default: false
+    add_index  :ticket_priorities, :default_create
+
+    # Set defaults
+    ticket_state_new = Ticket::State.find_by(name: 'new')
+    if !ticket_state_new
+      ticket_state_new = Ticket::State.first
+    end
+    if ticket_state_new
+      ticket_state_new.default_create = true
+      ticket_state_new.save!
+    end
+
+    ticket_state_open = Ticket::State.find_by(name: 'open')
+    if !ticket_state_open
+      ticket_state_open = Ticket::State.first
+    end
+    if ticket_state_open
+      ticket_state_open.default_follow_up = true
+      ticket_state_open.save!
+    end
+
+    ticket_priority = Ticket::Priority.find_by(name: '2 normal')
+    if !ticket_priority
+      ticket_priority = Ticket::Priority.first
+    end
+    if ticket_priority
+      ticket_priority.default_create = true
+      ticket_priority.save!
+    end
+
+    Cache.clear
+  end
+
+  def down
+    remove_index  :ticket_states, :default_create
+    remove_column :ticket_states, :default_create, :boolean
+    remove_index  :ticket_states, :default_follow_up
+    remove_column :ticket_states, :default_follow_up, :boolean
+
+    remove_index  :ticket_priorities, :default_create
+    remove_column :ticket_priorities, :default_create, :boolean
+
+    Cache.clear
+  end
+end

db/seeds.rb

@@ -3181,16 +3181,53 @@ Ticket::StateType.create_if_not_exists(id: 5, name: 'closed')
 Ticket::StateType.create_if_not_exists(id: 6, name: 'merged')
 Ticket::StateType.create_if_not_exists(id: 7, name: 'removed')
 
-Ticket::State.create_if_not_exists(id: 1, name: 'new', state_type_id: Ticket::StateType.find_by(name: 'new').id)
-Ticket::State.create_if_not_exists(id: 2, name: 'open', state_type_id: Ticket::StateType.find_by(name: 'open').id)
-Ticket::State.create_if_not_exists(id: 3, name: 'pending reminder', state_type_id: Ticket::StateType.find_by(name: 'pending reminder').id, ignore_escalation: true)
-Ticket::State.create_if_not_exists(id: 4, name: 'closed', state_type_id: Ticket::StateType.find_by(name: 'closed').id, ignore_escalation: true)
-Ticket::State.create_if_not_exists(id: 5, name: 'merged', state_type_id: Ticket::StateType.find_by(name: 'merged').id, ignore_escalation: true)
-Ticket::State.create_if_not_exists(id: 6, name: 'removed', state_type_id: Ticket::StateType.find_by(name: 'removed').id, active: false, ignore_escalation: true)
-Ticket::State.create_if_not_exists(id: 7, name: 'pending close', state_type_id: Ticket::StateType.find_by(name: 'pending action').id, next_state_id: 4, ignore_escalation: true)
+Ticket::State.create_if_not_exists(
+  id: 1,
+  name: 'new',
+  state_type_id: Ticket::StateType.find_by(name: 'new').id,
+  default_create: true,
+)
+Ticket::State.create_if_not_exists(
+  id: 2,
+  name: 'open',
+  state_type_id: Ticket::StateType.find_by(name: 'open').id,
+  default_follow_up: true,
+)
+Ticket::State.create_if_not_exists(
+  id: 3,
+  name: 'pending reminder',
+  state_type_id: Ticket::StateType.find_by(name: 'pending reminder').id,
+  ignore_escalation: true,
+)
+Ticket::State.create_if_not_exists(
+  id: 4,
+  name: 'closed',
+  state_type_id: Ticket::StateType.find_by(name: 'closed').id,
+  ignore_escalation: true,
+)
+Ticket::State.create_if_not_exists(
+  id: 5,
+  name: 'merged',
+  state_type_id: Ticket::StateType.find_by(name: 'merged').id,
+  ignore_escalation: true,
+)
+Ticket::State.create_if_not_exists(
+  id: 6,
+  name: 'removed',
+  state_type_id: Ticket::StateType.find_by(name: 'removed').id,
+  active: false,
+  ignore_escalation: true,
+)
+Ticket::State.create_if_not_exists(
+  id: 7,
+  name: 'pending close',
+  state_type_id: Ticket::StateType.find_by(name: 'pending action').id,
+  next_state_id: Ticket::State.find_by(name: 'closed').id,
+  ignore_escalation: true,
+)
 
 Ticket::Priority.create_if_not_exists(id: 1, name: '1 low')
-Ticket::Priority.create_if_not_exists(id: 2, name: '2 normal')
+Ticket::Priority.create_if_not_exists(id: 2, name: '2 normal', default_create: true)
 Ticket::Priority.create_if_not_exists(id: 3, name: '3 high')
 
 Ticket::Article::Type.create_if_not_exists(id: 1, name: 'email', communication: true)
@@ -3232,7 +3269,6 @@ UserInfo.current_user_id = user_community.id
 ticket = Ticket.create(
   group_id: Group.find_by(name: 'Users').id,
   customer_id: User.find_by(login: 'nicole.braun@zammad.org').id,
-  owner_id: User.find_by(login: '-').id,
   title: 'Welcome to Zammad!',
   state_id: Ticket::State.find_by(name: 'new').id,
   priority_id: Ticket::Priority.find_by(name: '2 normal').id,

lib/facebook.rb

@@ -351,11 +351,11 @@ result
       return ticket.state
     end
 
-    state = Ticket::State.find_by(name: 'new')
+    state = Ticket::State.find_by(default_create: true)
     return state if !ticket
 
-    return ticket.state if ticket.state.name == 'new'
-    Ticket::State.find_by(name: 'open')
+    return ticket.state if ticket.state_id == state.id
+    Ticket::State.find_by(default_follow_up: true)
   end
 
   def access_token_for_page(lookup)

lib/tweet_base.rb

@@ -337,10 +337,10 @@ class TweetBase
       return ticket.state
     end
 
-    state = Ticket::State.find_by(name: 'new')
+    state = Ticket::State.find_by(default_create: true)
     return state if !ticket
-    return ticket.state if ticket.state.name == 'new'
-    Ticket::State.find_by(name: 'open')
+    return ticket.state if ticket.state_id == state.id
+    Ticket::State.find_by(default_follow_up: true)
   end
 
   def tweet_limit_reached(tweet)

test/controllers/tickets_controller_test.rb

@@ -242,6 +242,27 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
     assert_equal(@agent.id, result['created_by_id'])
   end
 
+  test '01.09 ticket create with agent - minimal article with guess customer' do
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    params = {
+      title: 'a new ticket #9',
+      group: 'Users',
+      customer_id: 'guess:some_new_customer@example.com',
+      article: {
+        body: 'some test 123',
+      },
+    }
+    post '/api/v1/tickets', params.to_json, @headers.merge('Authorization' => credentials)
+    assert_response(201)
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
+    assert_equal('a new ticket #9', result['title'])
+    assert_equal(User.lookup(email: 'some_new_customer@example.com').id, result['customer_id'])
+    assert_equal(@agent.id, result['updated_by_id'])
+    assert_equal(@agent.id, result['created_by_id'])
+  end
+
   test '02.02 ticket create with agent' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {

test/unit/ticket_priority_test.rb

@@ -0,0 +1,61 @@
+# encoding: utf-8
+require 'test_helper'
+
+class TicketPriorityTest < ActiveSupport::TestCase
+
+  test 'base' do
+
+    # check current state
+    assert_equal(1, Ticket::Priority.where(default_create: true).count)
+    priority_create = Ticket::Priority.find_by(default_create: true)
+
+    # add new state
+    priority_new2 = Ticket::Priority.create_if_not_exists(
+      name: 'priority 2',
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    # verify states
+    assert_equal(1, Ticket::Priority.where(default_create: true).count)
+    assert_equal(priority_create.id, Ticket::Priority.find_by(default_create: true).id)
+
+    # cleanup
+    priority_new2.destroy
+
+    # verify states
+    assert_equal(1, Ticket::Priority.where(default_create: true).count)
+    assert_equal(priority_create.id, Ticket::Priority.find_by(default_create: true).id)
+
+    # add new state
+    priority_new3 = Ticket::Priority.create_if_not_exists(
+      name: 'priority 3',
+      default_create: true,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    # verify states
+    assert_equal(1, Ticket::Priority.where(default_create: true).count)
+    assert_equal(priority_new3.id, Ticket::Priority.find_by(default_create: true).id)
+    assert_not_equal(priority_create.id, Ticket::Priority.find_by(default_create: true).id)
+
+    # cleanup
+    priority_new3.destroy
+
+    # verify states
+    assert_equal(1, Ticket::Priority.where(default_create: true).count)
+    assert_equal(Ticket::Priority.first, Ticket::Priority.find_by(default_create: true))
+
+    # cleanup
+    priority_create.reload
+    priority_create.default_create = true
+    priority_create.save!
+
+    # verify states
+    assert_equal(1, Ticket::Priority.where(default_create: true).count)
+    assert_equal(priority_create.id, Ticket::Priority.find_by(default_create: true).id)
+
+  end
+
+end

test/unit/ticket_state_test.rb

@@ -0,0 +1,97 @@
+# encoding: utf-8
+require 'test_helper'
+
+class TicketStateTest < ActiveSupport::TestCase
+
+  test 'base' do
+
+    # check current state
+    assert_equal(1, Ticket::State.where(default_create: true).count)
+    assert_equal(1, Ticket::State.where(default_follow_up: true).count)
+    state_create = Ticket::State.find_by(default_create: true)
+    state_follow_up = Ticket::State.find_by(default_follow_up: true)
+
+    # add new state
+    state_new2 = Ticket::State.create_if_not_exists(
+      name: 'new 2',
+      state_type_id: Ticket::StateType.find_by(name: 'new').id,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    state_follow_up2 = Ticket::State.create_if_not_exists(
+      name: 'open 2',
+      state_type_id: Ticket::StateType.find_by(name: 'open').id,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    # verify states
+    assert_equal(1, Ticket::State.where(default_create: true).count)
+    assert_equal(1, Ticket::State.where(default_follow_up: true).count)
+    assert_equal(state_create.id, Ticket::State.find_by(default_create: true).id)
+    assert_equal(state_follow_up.id, Ticket::State.find_by(default_follow_up: true).id)
+
+    # cleanup
+    state_new2.destroy
+    state_follow_up2.destroy
+
+    # verify states
+    assert_equal(1, Ticket::State.where(default_create: true).count)
+    assert_equal(1, Ticket::State.where(default_follow_up: true).count)
+    assert_equal(state_create.id, Ticket::State.find_by(default_create: true).id)
+    assert_equal(state_follow_up.id, Ticket::State.find_by(default_follow_up: true).id)
+
+    # add new state
+    state_new3 = Ticket::State.create_if_not_exists(
+      name: 'new 3',
+      state_type_id: Ticket::StateType.find_by(name: 'new').id,
+      default_create: true,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    state_follow_up3 = Ticket::State.create_if_not_exists(
+      name: 'open 3',
+      state_type_id: Ticket::StateType.find_by(name: 'open').id,
+      default_follow_up: true,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    # verify states
+    assert_equal(1, Ticket::State.where(default_create: true).count)
+    assert_equal(1, Ticket::State.where(default_follow_up: true).count)
+    assert_not_equal(state_create.id, Ticket::State.find_by(default_create: true).id)
+    assert_equal(state_new3.id, Ticket::State.find_by(default_create: true).id)
+    assert_not_equal(state_follow_up.id, Ticket::State.find_by(default_follow_up: true).id)
+    assert_equal(state_follow_up3.id, Ticket::State.find_by(default_follow_up: true).id)
+
+    # cleanup
+    state_new3.destroy
+    state_follow_up3.destroy
+
+    # verify states
+    assert_equal(1, Ticket::State.where(default_create: true).count)
+    assert_equal(1, Ticket::State.where(default_follow_up: true).count)
+    assert_equal(state_create.id, Ticket::State.find_by(default_create: true).id)
+    assert_not_equal(state_follow_up.id, Ticket::State.find_by(default_follow_up: true).id)
+
+    # cleanup
+    state_create.reload
+    state_create.default_create = true
+    state_create.save!
+
+    state_follow_up.reload
+    state_follow_up.default_follow_up = true
+    state_follow_up.save!
+
+    # verify states
+    assert_equal(1, Ticket::State.where(default_create: true).count)
+    assert_equal(1, Ticket::State.where(default_follow_up: true).count)
+    assert_equal(state_create.id, Ticket::State.find_by(default_create: true).id)
+    assert_equal(state_follow_up.id, Ticket::State.find_by(default_follow_up: true).id)
+
+  end
+
+end