Refactoring: Eliminate explicit coupling of classes in ChecksImport concern

app/models/application_model/checks_import.rb

@@ -6,13 +6,17 @@ module ApplicationModel::ChecksImport
     before_create :check_attributes_protected
   end
 
+  class_methods do
+    # Use `include CanBeImported` in a class to override this method
+    def importable?
+      false
+    end
+  end
+
   def check_attributes_protected
-
-    import_class_list = ['Ticket', 'Ticket::Article', 'History', 'Ticket::State', 'Ticket::StateType', 'Ticket::Priority', 'Group', 'User', 'Role' ]
-
     # do noting, use id as it is
     return if !Setting.get('system_init_done')
-    return if Setting.get('import_mode') && import_class_list.include?(self.class.to_s)
+    return if Setting.get('import_mode') && self.class.importable?
     return if !has_attribute?(:id)
 
     self[:id] = nil

app/models/concerns/can_be_imported.rb

@@ -0,0 +1,10 @@
+module CanBeImported
+  extend ActiveSupport::Concern
+
+  # methods defined here are going to extend the class, not the instance of it
+  class_methods do
+    def importable?
+      true
+    end
+  end
+end

app/models/group.rb

@@ -1,6 +1,7 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
 class Group < ApplicationModel
+  include CanBeImported
   include HasActivityStreamLog
   include ChecksClientNotification
   include ChecksLatestChangeObserved

app/models/history.rb

@@ -1,6 +1,7 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
 class History < ApplicationModel
+  include CanBeImported
   include History::Assets
 
   self.table_name = 'histories'

app/models/role.rb

@@ -1,6 +1,7 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
 class Role < ApplicationModel
+  include CanBeImported
   include HasActivityStreamLog
   include ChecksClientNotification
   include ChecksLatestChangeObserved

app/models/ticket.rb

@@ -1,6 +1,7 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
 class Ticket < ApplicationModel
+  include CanBeImported
   include HasActivityStreamLog
   include ChecksClientNotification
   include ChecksLatestChangeObserved

app/models/ticket/article.rb

@@ -1,5 +1,6 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 class Ticket::Article < ApplicationModel
+  include CanBeImported
   include HasActivityStreamLog
   include ChecksClientNotification
   include HasHistory

app/models/ticket/priority.rb

@@ -1,5 +1,6 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 class Ticket::Priority < ApplicationModel
+  include CanBeImported
   self.table_name = 'ticket_priorities'
   validates :name, presence: true
 

app/models/ticket/state.rb

@@ -1,5 +1,6 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 class Ticket::State < ApplicationModel
+  include CanBeImported
   include ChecksLatestChangeObserved
 
   belongs_to :state_type, class_name: 'Ticket::StateType', inverse_of: :states

app/models/ticket/state_type.rb

@@ -1,5 +1,6 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 class Ticket::StateType < ApplicationModel
+  include CanBeImported
   include ChecksLatestChangeObserved
 
   has_many :states, class_name: 'Ticket::State', inverse_of: :state_type

app/models/user.rb

@@ -1,5 +1,6 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 class User < ApplicationModel
+  include CanBeImported
   include HasActivityStreamLog
   include ChecksClientNotification
   include HasHistory

spec/models/application_model/checks_import_examples.rb

@@ -1,5 +1,5 @@
-RSpec.shared_examples 'ApplicationModel::ChecksImport' do |importable: false|
-  subject(:new_instance) { build(described_class.name.underscore, id: unused_id) }
+RSpec.shared_examples 'ApplicationModel::ChecksImport' do
+  subject { build(described_class.name.underscore, id: unused_id) }
   let(:unused_id) { described_class.pluck(:id).max * 2 }
 
   context 'when Setting.get("system_init_done") is true AND Setting.get("import_mode") is false' do
@@ -7,7 +7,7 @@ RSpec.shared_examples 'ApplicationModel::ChecksImport' do |importable: false|
     before { Setting.set('import_mode', false) }
 
     it 'prevents explicit setting of #id attribute' do
-      expect { new_instance.save }.to change { new_instance.id }
+      expect { subject.save }.to change { subject.id }
     end
   end
 
@@ -15,7 +15,7 @@ RSpec.shared_examples 'ApplicationModel::ChecksImport' do |importable: false|
     before { Setting.set('system_init_done', false) }
 
     it 'allows explicit setting of #id attribute' do
-      expect { new_instance.save }.not_to change { new_instance.id }
+      expect { subject.save }.not_to change { subject.id }
     end
   end
 
@@ -24,16 +24,16 @@ RSpec.shared_examples 'ApplicationModel::ChecksImport' do |importable: false|
 
     shared_examples 'importable classes' do
       it 'allows explicit setting of #id attribute' do
-        expect { new_instance.save }.not_to change { new_instance.id }
+        expect { subject.save }.not_to change { subject.id }
       end
     end
 
     shared_examples 'non-importable classes' do
       it 'prevents explicit setting of #id attribute' do
-        expect { new_instance.save }.to change { new_instance.id }
+        expect { subject.save }.to change { subject.id }
       end
     end
 
-    include_examples importable ? 'importable classes' : 'non-importable classes'
+    include_examples described_class.importable? ? 'importable classes' : 'non-importable classes'
   end
 end

spec/models/concerns/can_be_imported_examples.rb

@@ -0,0 +1,7 @@
+RSpec.shared_examples 'CanBeImported' do
+  describe '.importable?' do
+    it 'returns true' do
+      expect(described_class.importable?).to be(true)
+    end
+  end
+end

spec/models/group_spec.rb

@@ -1,6 +1,8 @@
 require 'rails_helper'
 require 'models/application_model_examples'
+require 'models/concerns/can_be_imported_examples'
 
 RSpec.describe Group, type: :model do
   it_behaves_like 'ApplicationModel'
+  it_behaves_like 'CanBeImported'
 end

spec/models/history_spec.rb

@@ -1,6 +1,8 @@
 require 'rails_helper'
 require 'models/application_model_examples'
+require 'models/concerns/can_be_imported_examples'
 
 RSpec.describe History, type: :model do
   it_behaves_like 'ApplicationModel'
+  it_behaves_like 'CanBeImported'
 end

spec/models/role_spec.rb

@@ -1,9 +1,11 @@
 require 'rails_helper'
 require 'models/application_model_examples'
+require 'models/concerns/can_be_imported_examples'
 require 'models/concerns/has_groups_examples'
 
 RSpec.describe Role do
   it_behaves_like 'ApplicationModel'
+  it_behaves_like 'CanBeImported'
   it_behaves_like 'HasGroups', group_access_factory: :role
 
   subject(:role) { create(:role) }

spec/models/ticket/article_spec.rb

@@ -1,8 +1,10 @@
 require 'rails_helper'
 require 'models/application_model_examples'
+require 'models/concerns/can_be_imported_examples'
 
 RSpec.describe Ticket::Article, type: :model do
   it_behaves_like 'ApplicationModel'
+  it_behaves_like 'CanBeImported'
 
   describe '.create' do
     it 'handles NULL byte in subject or body' do

spec/models/ticket/priority_spec.rb

@@ -1,8 +1,10 @@
 require 'rails_helper'
 require 'models/application_model_examples'
+require 'models/concerns/can_be_imported_examples'
 
 RSpec.describe Ticket::Priority, type: :model do
   it_behaves_like 'ApplicationModel'
+  it_behaves_like 'CanBeImported'
 
   describe 'Default state' do
     describe 'of whole table:' do

spec/models/ticket/state_spec.rb

@@ -1,8 +1,10 @@
 require 'rails_helper'
 require 'models/application_model_examples'
+require 'models/concerns/can_be_imported_examples'
 
 RSpec.describe Ticket::State, type: :model do
   it_behaves_like 'ApplicationModel'
+  it_behaves_like 'CanBeImported'
 
   describe '.by_category' do
     it 'looks up states by category' do

spec/models/ticket/state_type_spec.rb

@@ -1,6 +1,8 @@
 require 'rails_helper'
 require 'models/application_model_examples'
+require 'models/concerns/can_be_imported_examples'
 
 RSpec.describe Ticket::StateType, type: :model do
   it_behaves_like 'ApplicationModel'
+  it_behaves_like 'CanBeImported'
 end

spec/models/ticket_spec.rb

@@ -1,9 +1,11 @@
 require 'rails_helper'
 require 'models/application_model_examples'
+require 'models/concerns/can_be_imported_examples'
 require 'models/concerns/can_lookup_examples'
 
 RSpec.describe Ticket, type: :model do
   it_behaves_like 'ApplicationModel'
+  it_behaves_like 'CanBeImported'
   it_behaves_like 'CanLookup'
 
   describe '#merge_to' do

spec/models/user_spec.rb

@@ -3,6 +3,7 @@ require 'models/application_model_examples'
 require 'models/concerns/has_groups_examples'
 require 'models/concerns/has_roles_examples'
 require 'models/concerns/has_groups_permissions_examples'
+require 'models/concerns/can_be_imported_examples'
 require 'models/concerns/can_lookup_examples'
 
 RSpec.describe User do
@@ -10,6 +11,7 @@ RSpec.describe User do
   it_behaves_like 'HasGroups', group_access_factory: :agent_user
   it_behaves_like 'HasRoles', group_access_factory: :agent_user
   it_behaves_like 'HasGroups and Permissions', group_access_no_permission_factory: :user
+  it_behaves_like 'CanBeImported'
   it_behaves_like 'CanLookup'
 
   subject(:user) { create(:user) }