Fixed #2132 by putting the I-doit objects in ticket preferences by creating the ticket.

.gitlab-ci.yml

@@ -1055,3 +1055,18 @@ test:browser:integration:zendesk_chrome:
     - script/build/test_startup.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 1
     - ruby -I test/ test/integration/zendesk_import_browser_test.rb || script/build/test_shutdown.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 1 1
     - script/build/test_shutdown.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 0 1
+
+test:browser:integration:idoit_chrome:
+  stage: browser-integration
+  dependencies:
+    - browser:build
+  tags:
+    - browser
+  script:
+    - export BROWSER=chrome
+    - export BROWSER_URL=http://$IP:$BROWSER_PORT
+    - RAILS_ENV=test rake db:create
+    - cp contrib/auto_wizard_test.json auto_wizard.json
+    - script/build/test_startup.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 1
+    - ruby -I test/ test/integration/idoit_browser_test.rb || script/build/test_shutdown.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 1 1
+    - script/build/test_shutdown.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 0 1

app/assets/javascripts/app/controllers/agent_ticket_create.coffee

@@ -508,6 +508,10 @@ class App.TicketCreate extends App.Controller
           if !confirm(App.i18n.translateContent('You use %s in text but no attachment is attached. Do you want to continue?', matchingWord))
             return
 
+    # add sidebar params
+    if @sidebarWidget && @sidebarWidget.postParams
+      @sidebarWidget.postParams(ticket: ticket)
+
     # disable form
     @submitDisable(e)
     ui = @

app/assets/javascripts/app/controllers/agent_ticket_create/sidebar.coffee

@@ -13,6 +13,11 @@ class App.TicketCreateSidebar extends App.Controller
       if backend && backend.commit
         backend.commit(args)
 
+  postParams: (args) =>
+    for key, backend of @sidebarBackends
+      if backend && backend.postParams
+        backend.postParams(args)
+
   render: (params) =>
     if params
       @params = params

app/assets/javascripts/app/controllers/ticket_zoom.coffee

@@ -839,6 +839,10 @@ class App.TicketZoom extends App.Controller
 
       ticket.article = article
 
+    # add sidebar params
+    if @sidebarWidget && @sidebarWidget.postParams
+      @sidebarWidget.postParams(ticket: ticket)
+
     if !ticket.article
       @submitPost(e, ticket, macro)
       return

app/assets/javascripts/app/controllers/ticket_zoom/sidebar.coffee

@@ -14,6 +14,11 @@ class App.TicketZoomSidebar extends App.ObserverController
       if backend && backend.commit
         backend.commit(args)
 
+  postParams: (args) =>
+    for key, backend of @sidebarBackends
+      if backend && backend.postParams
+        backend.postParams(args)
+
   render: (ticket) =>
     @sidebarBackends ||= {}
     @sidebarItems = []

app/assets/javascripts/app/controllers/ticket_zoom/sidebar_idoit.coffee

@@ -102,13 +102,14 @@ class SidebarIdoit extends App.Controller
       @updateTicket(@ticket.id, @objectIds)
     @showObjectsContent()
 
-  commit: (args) =>
-    return if @ticket && @ticket.id
+  postParams: (args) =>
+    return if !args.ticket
+    return if args.ticket.created_at
     return if !@objectIds
     return if _.isEmpty(@objectIds)
-    return if !args
-    return if !args.ticket_id
-    @updateTicket(args.ticket_id, @objectIds)
+    args.ticket.preferences ||= {}
+    args.ticket.preferences.idoit ||= {}
+    args.ticket.preferences.idoit.object_ids = @objectIds
 
   updateTicket: (ticket_id, objectIds, callback) =>
     App.Ajax.request(

app/assets/javascripts/app/models/ticket.coffee

@@ -1,5 +1,5 @@
 class App.Ticket extends App.Model
-  @configure 'Ticket', 'number', 'title', 'group_id', 'owner_id', 'customer_id', 'state_id', 'priority_id', 'article', 'tags', 'links', 'updated_at'
+  @configure 'Ticket', 'number', 'title', 'group_id', 'owner_id', 'customer_id', 'state_id', 'priority_id', 'article', 'tags', 'links', 'updated_at', 'preferences'
   @extend Spine.Model.Ajax
   @url: @apiPath + '/tickets'
   @configure_attributes = [

app/controllers/integration/idoit_controller.rb

@@ -38,10 +38,12 @@ class Integration::IdoitController < ApplicationController
   def update
     params[:object_ids] ||= []
     ticket = Ticket.find(params[:ticket_id])
+    ticket.with_lock do
       access!(ticket, 'read')
       ticket.preferences[:idoit] ||= {}
       ticket.preferences[:idoit][:object_ids] = Array(params[:object_ids]).uniq
       ticket.save!
+    end
 
     render json: {
       result: 'ok',

app/controllers/ticket_articles_controller.rb

@@ -112,6 +112,9 @@ class TicketArticlesController < ApplicationController
     clean_params = Ticket::Article.association_name_to_id_convert(params)
     clean_params = Ticket::Article.param_cleanup(clean_params, true)
 
+    # only apply preferences changes (keep not updated keys/values)
+    clean_params = article.param_preferences_merge(clean_params)
+
     article.update!(clean_params)
 
     if response_expand?

app/controllers/tickets_controller.rb

@@ -222,6 +222,9 @@ class TicketsController < ApplicationController
     clean_params = Ticket.association_name_to_id_convert(params)
     clean_params = Ticket.param_cleanup(clean_params, true)
 
+    # only apply preferences changes (keep not updated keys/values)
+    clean_params = ticket.param_preferences_merge(clean_params)
+
     # overwrite params
     if !current_user.permissions?('ticket.agent')
       %i[owner owner_id customer customer_id organization organization_id preferences].each do |key|

app/models/application_model/can_cleanup_param.rb

@@ -33,30 +33,30 @@ returns
 
       data = {}
       params.each do |key, value|
-        data[key.to_sym] = value
+        data[key.to_s] = value
       end
 
       # ignore id for new objects
       if new_object && params[:id]
-        data.delete(:id)
+        data.delete('id')
       end
 
       # only use object attributes
-      clean_params = {}
+      clean_params = ActiveSupport::HashWithIndifferentAccess.new
       new.attributes.each_key do |attribute|
-        next if !data.key?(attribute.to_sym)
+        next if !data.key?(attribute)
 
         # check reference records, referenced by _id attributes
         reflect_on_all_associations.map do |assoc|
           class_name = assoc.options[:class_name]
           next if !class_name
-          name = "#{assoc.name}_id".to_sym
+          name = "#{assoc.name}_id"
           next if !data.key?(name)
           next if data[name].blank?
           next if assoc.klass.lookup(id: data[name])
           raise ArgumentError, "Invalid value for param '#{name}': #{data[name].inspect}"
         end
-        clean_params[attribute.to_sym] = data[attribute.to_sym]
+        clean_params[attribute] = data[attribute]
       end
 
       # we do want to set this via database
@@ -89,5 +89,23 @@ returns
       end
       data
     end
+
+  end
+
+=begin
+
+merge preferences param
+
+  record = Model.find(123)
+
+  new_preferences = record.param_preferences_merge(param_preferences)
+
+=end
+
+  def param_preferences_merge(new_params)
+    return new_params if new_params.blank?
+    return new_params if preferences.blank?
+    new_params[:preferences] = preferences.merge(new_params[:preferences] || {})
+    new_params
   end
 end

test/controllers/ticket_articles_controller_test.rb

@@ -152,6 +152,46 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     assert_equal('some_file.txt', result['attachments'][0]['filename'])
     assert_equal('8', result['attachments'][0]['size'])
     assert_equal('text/plain', result['attachments'][0]['preferences']['Mime-Type'])
+
+    params = {
+      ticket_id: result['ticket_id'],
+      content_type: 'text/plain',
+      body: 'some body',
+      type: 'note',
+      preferences: {
+        some_key1: 123,
+      },
+    }
+    post '/api/v1/ticket_articles', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
+    assert_response(201)
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert_nil(result['subject'])
+    assert_equal('some body', result['body'])
+    assert_equal('text/plain', result['content_type'])
+    assert_equal(@agent.id, result['updated_by_id'])
+    assert_equal(@agent.id, result['created_by_id'])
+    assert_equal(123, result['preferences']['some_key1'])
+    assert_equal(5, ticket.articles.count)
+
+    params = {
+      body: 'some body 2',
+      preferences: {
+        some_key2: 'abc',
+      },
+    }
+    put "/api/v1/ticket_articles/#{result['id']}", params: params.to_json, headers: @headers.merge('Authorization' => credentials)
+    assert_response(200)
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert_nil(result['subject'])
+    assert_equal('some body 2', result['body'])
+    assert_equal('text/plain', result['content_type'])
+    assert_equal(@agent.id, result['updated_by_id'])
+    assert_equal(@agent.id, result['created_by_id'])
+    assert_equal(123, result['preferences']['some_key1'])
+    assert_equal('abc', result['preferences']['some_key2'])
+
   end
 
   test '02.01 ticket create with customer and articles' do
@@ -243,7 +283,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     assert_equal(0, ticket.articles[3].attachments.count)
 
     # add internal article
-    article = Ticket::Article.create(
+    article = Ticket::Article.create!(
       ticket_id: ticket.id,
       from: 'some_sender@example.com',
       to: 'some_recipient@example.com',
@@ -297,6 +337,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     assert_response(201)
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
+    assert_equal('a new ticket #1', result['title'])
 
     article = Ticket::Article.find_by(ticket_id: result['id'])
     assert_equal(@customer_without_org.id, article.origin_by_id)

test/controllers/tickets_controller_test.rb

@@ -767,6 +767,9 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
       priority: Ticket::Priority.lookup(name: '2 normal'),
       updated_by_id: 1,
       created_by_id: 1,
+      preferences: {
+        some_key1: 123,
+      },
     )
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     get "/api/v1/tickets/#{ticket.id}", params: {}, headers: @headers.merge('Authorization' => credentials)
@@ -778,10 +781,14 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     assert_equal(ticket.customer_id, result['customer_id'])
     assert_equal(1, result['updated_by_id'])
     assert_equal(1, result['created_by_id'])
+    assert_equal(123, result['preferences']['some_key1'])
 
     params = {
       title: "#{title} - 2",
       customer_id: @agent.id,
+      preferences: {
+        some_key2: 'abc',
+      },
     }
     put "/api/v1/tickets/#{ticket.id}", params: params.to_json, headers: @headers.merge('Authorization' => credentials)
     assert_response(200)
@@ -792,6 +799,8 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     assert_equal(@agent.id, result['customer_id'])
     assert_equal(@agent.id, result['updated_by_id'])
     assert_equal(1, result['created_by_id'])
+    assert_equal(123, result['preferences']['some_key1'])
+    assert_equal('abc', result['preferences']['some_key2'])
 
     params = {
       ticket_id: ticket.id,

test/integration/idoit_browser_test.rb

@@ -0,0 +1,111 @@
+
+require 'browser_test_helper'
+
+class IntegrationIdoitTest < TestCase
+
+  def test_idoit_objects_corrects_saves_on_ticket_creation
+    # Read I-doit credentials from ENV
+    if !ENV['IDOIT_API_TOKEN']
+      raise "ERROR: Need IDOIT_API_TOKEN - hint IDOIT_API_TOKEN='1234'"
+    end
+    api_token = ENV['IDOIT_API_TOKEN']
+    if !ENV['IDOIT_API_ENDPOINT']
+      raise "ERROR: Need IDOIT_API_ENDPOINT - hint IDOIT_API_ENDPOINT='1234'"
+    end
+    api_endpoint = ENV['IDOIT_API_ENDPOINT']
+    if !ENV['IDOIT_API_CATEGORY']
+      raise "ERROR: Need IDOIT_API_CATEGORY - hint IDOIT_API_CATEGORY='Building'"
+    end
+    api_category = ENV['IDOIT_API_CATEGORY']
+
+    id = rand(99_999_999)
+    @browser = instance = browser_instance
+    login(
+      username: 'master@example.com',
+      password: 'test',
+      url: browser_url,
+    )
+
+    # turn on I-doit integration
+    click(css: 'a[href="#manage"]')
+    click(css: 'a[href="#system/integration"]')
+    click(css: 'a[href="#system/integration/idoit"]')
+    switch(
+      css: '.content.active .js-switch',
+      type: 'on'
+    )
+
+    # fill in I-doit login details
+    set(
+      css: '.content.active .main input[name="api_token"]',
+      value: api_token,
+    )
+    set(
+      css: '.content.active .main input[name="endpoint"]',
+      value: api_endpoint,
+    )
+    click(css: '.content.active .main .js-submit')
+
+    watch_for(
+      css: '#notify',
+      value: 'update successful',
+    )
+
+    # new create a new ticket with an I-doit object
+    ticket = ticket_create(
+      data: {
+        customer: 'nico',
+        group: 'Users',
+        title: 'subject - I-doit integration',
+        body: 'body - I-doit integration',
+      },
+      do_not_submit: true,
+    )
+
+    # open the I-doit selection modal
+    click(css: '.content.active .tabsSidebar svg.icon-printer')
+    click(css: '.content.active .sidebar[data-tab="idoit"] .js-headline')
+    click(css: '.content.active .sidebar[data-tab="idoit"] .dropdown-menu')
+
+    # wait for the API call to populate the dropdown menu
+    watch_for(css: '.content.active .modal form input.js-input')
+    # open the dropdown menu and choose the Building option
+    click(css: '.content.active .modal form input.js-input')
+    click(css: ".content.active .modal form li.js-option[title='#{api_category}']")
+    # wait for the building results to populate from the API
+    watch_for(css: '.content.active .modal form.js-result table.table')
+
+    # click the check box from the first row and note its entry ID
+    checkbox = instance.find_elements(css: '.content.active .modal form.js-result tbody :first-child input')[0]
+    entry_id = checkbox.attribute('value')
+    checkbox.click()
+
+    # submit the I-doit object selections
+    click(css: '.content.active .modal form button.js-submit')
+
+    # confirm that the entry have been successfully recorded
+    watch_for(
+      css: ".content.active .sidebar[data-tab='idoit'] a[href='#{api_endpoint}/?objID=#{entry_id}']",
+    )
+
+    # now submit the ticket
+    click(css: '.content.active .newTicket button.js-submit')
+    sleep 5
+
+    # open the I-doit sidebar again and verify that the entry is still there
+    click(css: '.content.active .tabsSidebar svg.icon-printer')
+    watch_for(
+      css: ".content.active .sidebar[data-tab='idoit'] a[href='#{api_endpoint}/?objID=#{entry_id}']",
+    )
+
+    # finally turn off I-doit integration
+    click(css: 'a[href="#manage"]')
+    click(css: 'a[href="#system/integration"]')
+    click(css: 'a[href="#system/integration/idoit"]')
+
+    switch(
+      css: '.content.active .js-switch',
+      type: 'off'
+    )
+  end
+end

test/unit/model_test.rb

@@ -337,4 +337,51 @@ class ModelTest < ActiveSupport::TestCase
     assert_not(result.key?(:controller))
   end
 
+  test 'param_preferences_merge test' do
+    params = {
+      id: 123,
+      firstname: '123',
+      created_by_id: 1,
+      created_at: Time.zone.now,
+      updated_by_id: 1,
+      updated_at: Time.zone.now,
+      preferences: {},
+    }
+    user = User.new(params)
+    assert(user.preferences.blank?)
+
+    user.preferences = { A: 1, B: 2 }
+    assert(user.preferences.present?)
+
+    params = {
+      firstname: '123 ABC',
+      preferences: { 'B' => 3, 'C': 4 },
+    }
+    clean_params = User.param_cleanup(params)
+    clean_user_params = user.param_preferences_merge(clean_params)
+    assert_equal(clean_user_params[:firstname], '123 ABC')
+    assert(clean_user_params[:preferences].present?)
+    assert_equal(clean_user_params[:preferences]['A'], 1)
+    assert_equal(clean_user_params[:preferences]['B'], 3)
+    assert_equal(clean_user_params[:preferences]['C'], 4)
+    assert_equal(clean_user_params[:preferences][:A], 1)
+    assert_equal(clean_user_params[:preferences][:B], 3)
+    assert_equal(clean_user_params[:preferences][:C], 4)
+
+    params = {
+      firstname: '123 ABCD',
+      preferences: {},
+    }
+    clean_params = User.param_cleanup(params)
+    clean_user_params = user.param_preferences_merge(clean_params)
+    assert_equal(clean_user_params[:firstname], '123 ABCD')
+    assert(clean_user_params[:preferences].present?)
+    assert_equal(clean_user_params[:preferences]['A'], 1)
+    assert_equal(clean_user_params[:preferences]['B'], 2)
+    assert_nil(clean_user_params[:preferences]['C'])
+    assert_equal(clean_user_params[:preferences][:A], 1)
+    assert_equal(clean_user_params[:preferences][:B], 2)
+    assert_nil(clean_user_params[:preferences][:C])
+  end
+
 end