diff --git a/.rubocop/cop/zammad/exists_reset_column_information.rb b/.rubocop/cop/zammad/exists_reset_column_information.rb index 5047a3953..064d38425 100644 --- a/.rubocop/cop/zammad/exists_reset_column_information.rb +++ b/.rubocop/cop/zammad/exists_reset_column_information.rb @@ -54,7 +54,9 @@ and check if there are reset_column_information function calls existing for the end def reset_class(node) - node.children[0].children[1].to_s + # simplify namespaced class names + # Rubocop can't reliably convert table names to namespaced class names + node.children[0].const_name.gsub '::', '' end def table_class(node) diff --git a/app/assets/javascripts/app/controllers/_application_controller/_modal_generic_history.coffee b/app/assets/javascripts/app/controllers/_application_controller/_modal_generic_history.coffee index 228190c1f..5470580c7 100644 --- a/app/assets/javascripts/app/controllers/_application_controller/_modal_generic_history.coffee +++ b/app/assets/javascripts/app/controllers/_application_controller/_modal_generic_history.coffee @@ -44,6 +44,8 @@ class App.GenericHistory extends App.ControllerModal if item.object is 'Ticket::Article' item.object = 'Article' + if item.object is 'Ticket::SharedDraftZoom' + item.object = 'Draft' data = item data.created_by = App.User.find( item.created_by_id ) diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 69a365c65..06be66854 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -71,7 +71,7 @@ class Ticket < ApplicationModel :article_count, :preferences - history_relation_object 'Ticket::Article', 'Mention' + history_relation_object 'Ticket::Article', 'Mention', 'Ticket::SharedDraftZoom' sanitized_html :note diff --git a/app/models/ticket/shared_draft_start.rb b/app/models/ticket/shared_draft_start.rb index 1e622d927..2cde948fc 100644 --- a/app/models/ticket/shared_draft_start.rb +++ b/app/models/ticket/shared_draft_start.rb @@ -5,6 +5,8 @@ class Ticket::SharedDraftStart < ApplicationModel include ChecksClientNotification belongs_to :group + belongs_to :created_by, class_name: 'User' + belongs_to :updated_by, class_name: 'User' validates :name, presence: true diff --git a/app/models/ticket/shared_draft_zoom.rb b/app/models/ticket/shared_draft_zoom.rb index 317c9b2e6..117c8ccb5 100644 --- a/app/models/ticket/shared_draft_zoom.rb +++ b/app/models/ticket/shared_draft_zoom.rb @@ -3,14 +3,31 @@ class Ticket::SharedDraftZoom < ApplicationModel include CanCloneAttachments include ChecksClientNotification + include HasHistory belongs_to :ticket, touch: true + belongs_to :created_by, class_name: 'User' + belongs_to :updated_by, class_name: 'User' store :new_article store :ticket_attributes + history_attributes_ignored :new_article, + :ticket_attributes + # required by CanCloneAttachments def content_type 'text/html' end + + def history_log_attributes + { + related_o_id: self['ticket_id'], + related_history_object: 'Ticket', + } + end + + def history_destroy + history_log('removed', created_by_id) + end end diff --git a/db/migrate/20120101000010_create_ticket.rb b/db/migrate/20120101000010_create_ticket.rb index e102c4fe0..f3cd815bd 100644 --- a/db/migrate/20120101000010_create_ticket.rb +++ b/db/migrate/20120101000010_create_ticket.rb @@ -593,10 +593,8 @@ class CreateTicket < ActiveRecord::Migration[4.2] t.references :ticket, null: false, foreign_key: { to_table: :tickets } t.text :new_article t.text :ticket_attributes - - t.column :created_by_id, :integer, null: true - t.column :updated_by_id, :integer, null: true - + t.column :created_by_id, :integer, null: false + t.column :updated_by_id, :integer, null: false t.timestamps limit: 3 end @@ -604,10 +602,8 @@ class CreateTicket < ActiveRecord::Migration[4.2] t.references :group, null: false, foreign_key: { to_table: :groups } t.string :name t.text :content - - t.column :created_by_id, :integer, null: true - t.column :updated_by_id, :integer, null: true - + t.column :created_by_id, :integer, null: false + t.column :updated_by_id, :integer, null: false t.timestamps limit: 3 end end diff --git a/db/migrate/20220228095014_fix_draft_user_required.rb b/db/migrate/20220228095014_fix_draft_user_required.rb new file mode 100644 index 000000000..7e12d28e3 --- /dev/null +++ b/db/migrate/20220228095014_fix_draft_user_required.rb @@ -0,0 +1,16 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +class FixDraftUserRequired < ActiveRecord::Migration[6.0] + def change + # return if it's a new setup + return if !Setting.exists?(name: 'system_init_done') + + change_column :ticket_shared_draft_zooms, :created_by_id, :integer, null: false + change_column :ticket_shared_draft_zooms, :updated_by_id, :integer, null: false + change_column :ticket_shared_draft_starts, :created_by_id, :integer, null: false + change_column :ticket_shared_draft_starts, :updated_by_id, :integer, null: false + + Ticket::SharedDraftStart.reset_column_information + Ticket::SharedDraftZoom.reset_column_information + end +end diff --git a/spec/factories/ticket/shared_draft/start.rb b/spec/factories/ticket/shared_draft/start.rb index b36f6ff62..5225449d0 100644 --- a/spec/factories/ticket/shared_draft/start.rb +++ b/spec/factories/ticket/shared_draft/start.rb @@ -5,5 +5,7 @@ FactoryBot.define do name { Faker::Name.unique.name } group { create(:group) } content { { content: true } } + updated_by_id { 1 } + created_by_id { 1 } end end diff --git a/spec/factories/ticket/shared_draft/zooms.rb b/spec/factories/ticket/shared_draft/zooms.rb index 47dbef0a7..423a99915 100644 --- a/spec/factories/ticket/shared_draft/zooms.rb +++ b/spec/factories/ticket/shared_draft/zooms.rb @@ -5,5 +5,7 @@ FactoryBot.define do ticket { create(:ticket) } new_article { { new_article: true } } ticket_attributes { { ticket_attributes: true } } + updated_by_id { 1 } + created_by_id { 1 } end end diff --git a/spec/models/ticket/shared_draft_zoom_spec.rb b/spec/models/ticket/shared_draft_zoom_spec.rb index 3ad941f5b..5b9692e65 100644 --- a/spec/models/ticket/shared_draft_zoom_spec.rb +++ b/spec/models/ticket/shared_draft_zoom_spec.rb @@ -8,4 +8,18 @@ RSpec.describe Ticket::SharedDraftZoom, type: :model do it { is_expected.to belong_to :ticket } it { expect(shared_draft_zoom.new_article).to be_a(Hash) } it { expect(shared_draft_zoom.ticket_attributes).to be_a(Hash) } + + describe 'Draft Sharing: Add history entry for updating and deleting of a draft #3983' do + it 'does create a history entry for the new draft' do + expect(shared_draft_zoom.ticket.history_get) + .to include(include('object' => 'Ticket::SharedDraftZoom', 'type' => 'created')) + end + + it 'does add a history entry for removing the draft' do + shared_draft_zoom.destroy + + expect(shared_draft_zoom.ticket.history_get) + .to include(include('object' => 'Ticket::SharedDraftZoom', 'type' => 'removed')) + end + end end diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 8769f71e1..16b195f35 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -26,7 +26,7 @@ RSpec.describe Ticket, type: :model do it_behaves_like 'CanBeImported' it_behaves_like 'CanCsvImport' it_behaves_like 'ChecksCoreWorkflow' - it_behaves_like 'HasHistory', history_relation_object: ['Ticket::Article', 'Mention'] + it_behaves_like 'HasHistory', history_relation_object: ['Ticket::Article', 'Mention', 'Ticket::SharedDraftZoom'] it_behaves_like 'HasTags' it_behaves_like 'TagWritesToTicketHistory' it_behaves_like 'HasTaskbars' diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 403dfcdfe..e5fcda784 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -961,8 +961,8 @@ RSpec.describe User, type: :model do 'Ticket::Article::Type' => { 'created_by_id' => 0, 'updated_by_id' => 0 }, 'Ticket::Article::Flag' => { 'created_by_id' => 0 }, 'Ticket::Priority' => { 'created_by_id' => 0, 'updated_by_id' => 0 }, - 'Ticket::SharedDraftStart' => { 'created_by_id' => 0, 'updated_by_id' => 0 }, - 'Ticket::SharedDraftZoom' => { 'created_by_id' => 0, 'updated_by_id' => 0 }, + 'Ticket::SharedDraftStart' => { 'created_by_id' => 1, 'updated_by_id' => 0 }, + 'Ticket::SharedDraftZoom' => { 'created_by_id' => 1, 'updated_by_id' => 0 }, 'Ticket::TimeAccounting' => { 'created_by_id' => 0 }, 'Ticket::State' => { 'created_by_id' => 0, 'updated_by_id' => 0 }, 'Ticket::Flag' => { 'created_by_id' => 0 }, @@ -1006,7 +1006,7 @@ RSpec.describe User, type: :model do 'Mention' => { 'created_by_id' => 1, 'updated_by_id' => 0, 'user_id' => 1 }, 'Channel' => { 'created_by_id' => 0, 'updated_by_id' => 0 }, 'Role' => { 'created_by_id' => 0, 'updated_by_id' => 0 }, - 'History' => { 'created_by_id' => 4 }, + 'History' => { 'created_by_id' => 5 }, 'Webhook' => { 'created_by_id' => 0, 'updated_by_id' => 0 }, 'Overview' => { 'created_by_id' => 1, 'updated_by_id' => 0 }, 'ActivityStream' => { 'created_by_id' => 0 }, @@ -1034,6 +1034,8 @@ RSpec.describe User, type: :model do chat_session = create(:'chat/session', user: user) chat_message = create(:'chat/message', chat_session: chat_session) chat_message2 = create(:'chat/message', chat_session: chat_session, created_by: user) + draft_start = create(:ticket_shared_draft_start, created_by: user) + draft_zoom = create(:ticket_shared_draft_zoom, created_by: user) expect(overview.reload.user_ids).to eq([user.id]) # create a chat agent for admin user (id=1) before agent user @@ -1094,6 +1096,8 @@ RSpec.describe User, type: :model do .to change(user_created_by, :created_by_id).to(1) .and change(user_created_by, :updated_by_id).to(1) .and change(user_created_by, :out_of_office_replacement_id).to(1) + expect { draft_start.reload }.to change(draft_start, :created_by_id).to(1) + expect { draft_zoom.reload }.to change(draft_zoom, :created_by_id).to(1) end it 'does delete cache after user deletion' do