diff options
| -rw-r--r-- | app/models/compliance_control.rb | 11 | ||||
| -rw-r--r-- | app/workers/compliance_control_set_copy_worker.rb | 2 | ||||
| -rw-r--r-- | config/locales/compliance_controls.en.yml | 2 | ||||
| -rw-r--r-- | config/locales/compliance_controls.fr.yml | 2 | ||||
| -rw-r--r-- | lib/af83/compliance_control_set_copier.rb | 128 | ||||
| -rw-r--r-- | lib/compliance_control_set_copier.rb | 126 | ||||
| -rw-r--r-- | spec/lib/compliance_control_set_copier_spec.rb (renamed from spec/lib/af83/cloning/compliance_control_set_copier_spec.rb) | 2 | ||||
| -rw-r--r-- | spec/models/compliance_check_spec.rb | 2 | ||||
| -rw-r--r-- | spec/models/compliance_control_spec.rb | 12 | ||||
| -rw-r--r-- | spec/workers/compliance_control_set_copy_worker_spec.rb | 13 | ||||
| -rw-r--r-- | spec/workers/line_referential_sync_worker_spec.rb | 1 |
11 files changed, 158 insertions, 143 deletions
diff --git a/app/models/compliance_control.rb b/app/models/compliance_control.rb index 7e4e22636..6cf029b7d 100644 --- a/app/models/compliance_control.rb +++ b/app/models/compliance_control.rb @@ -16,15 +16,14 @@ class ComplianceControl < ActiveRecord::Base return true if compliance_control_block_id.nil? ids = [compliance_control_block.compliance_control_set_id, compliance_control_set_id] return true if ids.first == ids.last - errors.add(:coherent_control_set, I18n.t('compliance_controls.errors.incoherent_control_sets', indirect_set_id: ids.first, direct_set_id: ids.last)) + names = ids.map{|id| ComplianceControlSet.find(id).name} + errors.add(:coherent_control_set, + I18n.t('compliance_controls.errors.incoherent_control_sets', + indirect_set_name: names.first, + direct_set_name: names.last)) end class << self - def create *args - super.tap do | x | - require 'pry'; binding.pry - end - end def default_criticity; :warning end def default_code; "" end def dynamic_attributes diff --git a/app/workers/compliance_control_set_copy_worker.rb b/app/workers/compliance_control_set_copy_worker.rb index 4cbd4e0bc..a59ee756a 100644 --- a/app/workers/compliance_control_set_copy_worker.rb +++ b/app/workers/compliance_control_set_copy_worker.rb @@ -2,7 +2,7 @@ class ComplianceControlSetCopyWorker include Sidekiq::Worker def perform(cc_set_id, referential_id) - AF83.ComplianceControlSetCopier.new.copy(cc_set_id, referential_id) + ComplianceControlSetCopier.new.copy(cc_set_id, referential_id) end end diff --git a/config/locales/compliance_controls.en.yml b/config/locales/compliance_controls.en.yml index ee98367cf..cee83afee 100644 --- a/config/locales/compliance_controls.en.yml +++ b/config/locales/compliance_controls.en.yml @@ -1,7 +1,7 @@ en: compliance_controls: errors: - incoherent_control_sets: "Directly associated control_set (id: %{direct_set_id}) and indirectly associated one (id: %{indirect_set_id}) differ" + incoherent_control_sets: "Impossible to assign a control to a set (id: %{direct_set_name}) differing from the one of its group (id: %{indirect_set_name})" show: title: "Compliance control" index: diff --git a/config/locales/compliance_controls.fr.yml b/config/locales/compliance_controls.fr.yml index 0f3e586bf..f0f4ecf77 100644 --- a/config/locales/compliance_controls.fr.yml +++ b/config/locales/compliance_controls.fr.yml @@ -1,7 +1,7 @@ fr: compliance_controls: errors: - incoherent_control_sets: "ControlSet associé directement (id: %{direct_set_id}) et indirectement (id: %{indirect_set_id}) ne sont pas égeaux" + incoherent_control_sets: "Le contrôle ne peut pas être associé à un jeu de contrôle (id: %{direct_set_name}) différent de celui de son groupe (id: %{indirect_set_name})" show: title: "Jeu de controle" index: diff --git a/lib/af83/compliance_control_set_copier.rb b/lib/af83/compliance_control_set_copier.rb deleted file mode 100644 index 840e21e9e..000000000 --- a/lib/af83/compliance_control_set_copier.rb +++ /dev/null @@ -1,128 +0,0 @@ -module AF83 - # We use a class instead of a singleton object because we will use - # a cache to avoid copying instancs of ComplianceControl twice as they - # might be reachable from CCSet **and** its descendent CCBlock. - # More generally spoken, we copy a DAG, not a tree. - class ComplianceControlSetCopier - - # Naming Convention: As we are in a domain with quite long names we - # abbreviate compliance_control to cc and - # compliance_check to cck iff used as prefixes. - - attr_reader :cc_set_id, :referential_id - - def copy cc_set_id, referential_id - @cc_set_id = cc_set_id - @referential_id = referential_id - check_organisation_coherence! - copy_dag - end - - - private - - - # Workers - # ------- - def check_organisation_coherence! - return true if cc_set.organisation_id == referential.organisation_id - raise ArgumentError, "Incoherent organisation of referential" - end - - def copy_dag - # Assure cck_set's existance, just in case cc_set is _empty_. - cck_set - make_cck_blocks - make_ccks_from_ccs - end - - def make_all_cck_block_children cc_block, cck_block - cc_block - .compliance_controls - .each{ |compliance_control| make_compliance_check(compliance_control, cck_block.id) } - end - def make_cck_block cc_block - cck_block = - cck_set.compliance_check_blocks.create( - name: name_with_refid(cc_block.name)) - - make_all_cck_block_children cc_block, cck_block - end - def make_cck_blocks - cc_set.compliance_control_blocks.each(&method(:make_cck_block)) - end - - def make_ccks_from_ccs - cc_set.compliance_controls.each(&method(:make_compliance_check)) - end - - def make_compliance_check(compliance_control, cck_block_id = nil) - already_there = get_from_cache compliance_control.id - # We do not want to impose traversal order of the DAG, that would - # make the code more resistant to change... - # So we check if we need to update the compliance_check_block_id - # of a control found in the cache. - # N.B. By traversing the indirect descendents from a set first, - # or IOW, traversing the control_blocks before the controls, - # this check could go away and we could return from the - # method in case of a cache hit. - if already_there - # Purely defensive: - if already_there.compliance_check_block_id.nil? && cck_block_id - already_there.update compliance_check_block_id: cck_block_id - end - return - end - make_compliance_check!(compliance_control, cck_block_id) - end - def make_compliance_check!(compliance_control, cck_block_id) - add_to_cache( - compliance_control.id, - cck_set.compliance_checks.create( - criticity: compliance_control.criticity, - name: name_with_refid(compliance_control.name), - code: compliance_control.code, - origin_code: compliance_control.origin_code, - compliance_check_block_id: cck_block_id - )) - end - - def name_with_refid name - [name, referential.name].join('-') - end - - # Lazy Values - # ----------- - def cc_set - @__cc_set__ ||= ComplianceControlSet.find(cc_set_id) - end - def cck_set - @__cck_set__ ||= ComplianceCheckSet.create!( - compliance_control_set_id: cc_set_id, - referential_id: referential_id, - workbench_id: referential.workbench_id, - name: name_with_refid(cc_set.name), - status: 'new' - ) - end - def referential - @__referential__ ||= Referential.find(referential_id) - end - - # Copy Cache - # ---------- - def add_to_cache key, obj - # Right now we map key -> obj, in case memory consumption becomes too important - # we can map key -> obj.id and fetch the object from the database in get_from_cache - # (time vs. space tradeoff) - copy_cache.merge!(key => obj) - end - def get_from_cache key - copy_cache[key].tap do | ele | - end - end - def copy_cache - @__copy_cache__ ||= Hash.new - end - end -end diff --git a/lib/compliance_control_set_copier.rb b/lib/compliance_control_set_copier.rb new file mode 100644 index 000000000..b015735c2 --- /dev/null +++ b/lib/compliance_control_set_copier.rb @@ -0,0 +1,126 @@ +# We use a class instead of a singleton object because we will use +# a cache to avoid copying instancs of ComplianceControl twice as they +# might be reachable from CCSet **and** its descendent CCBlock. +# More generally spoken, we copy a DAG, not a tree. +class ComplianceControlSetCopier + + # Naming Convention: As we are in a domain with quite long names we + # abbreviate compliance_control to cc and + # compliance_check to cck iff used as prefixes. + + attr_reader :cc_set_id, :referential_id + + def copy cc_set_id, referential_id + @cc_set_id = cc_set_id + @referential_id = referential_id + check_organisation_coherence! + copy_dag + end + + + private + + + # Workers + # ------- + def check_organisation_coherence! + return true if cc_set.organisation_id == referential.organisation_id + raise ArgumentError, "Incoherent organisation of referential" + end + + def copy_dag + # Assure cck_set's existance, just in case cc_set is _empty_. + cck_set + make_cck_blocks + make_ccks_from_ccs + end + + def make_all_cck_block_children cc_block, cck_block + cc_block + .compliance_controls + .each{ |compliance_control| make_compliance_check(compliance_control, cck_block.id) } + end + def make_cck_block cc_block + cck_block = + cck_set.compliance_check_blocks.create( + name: name_with_refid(cc_block.name)) + + make_all_cck_block_children cc_block, cck_block + end + def make_cck_blocks + cc_set.compliance_control_blocks.each(&method(:make_cck_block)) + end + + def make_ccks_from_ccs + cc_set.compliance_controls.each(&method(:make_compliance_check)) + end + + def make_compliance_check(compliance_control, cck_block_id = nil) + already_there = get_from_cache compliance_control.id + # We do not want to impose traversal order of the DAG, that would + # make the code more resistant to change... + # So we check if we need to update the compliance_check_block_id + # of a control found in the cache. + # N.B. By traversing the indirect descendents from a set first, + # or IOW, traversing the control_blocks before the controls, + # this check could go away and we could return from the + # method in case of a cache hit. + if already_there + # Purely defensive: + if already_there.compliance_check_block_id.nil? && cck_block_id + already_there.update compliance_check_block_id: cck_block_id + end + return + end + make_compliance_check!(compliance_control, cck_block_id) + end + def make_compliance_check!(compliance_control, cck_block_id) + add_to_cache( + compliance_control.id, + cck_set.compliance_checks.create( + criticity: compliance_control.criticity, + name: name_with_refid(compliance_control.name), + code: compliance_control.code, + origin_code: compliance_control.origin_code, + compliance_check_block_id: cck_block_id + )) + end + + def name_with_refid name + [name, referential.name].join('-') + end + + # Lazy Values + # ----------- + def cc_set + @__cc_set__ ||= ComplianceControlSet.find(cc_set_id) + end + def cck_set + @__cck_set__ ||= ComplianceCheckSet.create!( + compliance_control_set_id: cc_set_id, + referential_id: referential_id, + workbench_id: referential.workbench_id, + name: name_with_refid(cc_set.name), + status: 'new' + ) + end + def referential + @__referential__ ||= Referential.find(referential_id) + end + + # Copy Cache + # ---------- + def add_to_cache key, obj + # Right now we map key -> obj, in case memory consumption becomes too important + # we can map key -> obj.id and fetch the object from the database in get_from_cache + # (time vs. space tradeoff) + copy_cache.merge!(key => obj) + end + def get_from_cache key + copy_cache[key].tap do | ele | + end + end + def copy_cache + @__copy_cache__ ||= Hash.new + end +end diff --git a/spec/lib/af83/cloning/compliance_control_set_copier_spec.rb b/spec/lib/compliance_control_set_copier_spec.rb index aacd9d7c9..b620d0657 100644 --- a/spec/lib/af83/cloning/compliance_control_set_copier_spec.rb +++ b/spec/lib/compliance_control_set_copier_spec.rb @@ -1,4 +1,4 @@ -RSpec.describe AF83::ComplianceControlSetCopier do +RSpec.describe ComplianceControlSetCopier do subject{ described_class.new } diff --git a/spec/models/compliance_check_spec.rb b/spec/models/compliance_check_spec.rb index acdcc3ebf..710cd4b4f 100644 --- a/spec/models/compliance_check_spec.rb +++ b/spec/models/compliance_check_spec.rb @@ -1,5 +1,3 @@ -require 'rails_helper' - RSpec.describe ComplianceCheck, type: :model do it 'should have a valid factory' do expect(FactoryGirl.build(:compliance_check)).to be_valid diff --git a/spec/models/compliance_control_spec.rb b/spec/models/compliance_control_spec.rb index 7c58eaf3e..db73dab21 100644 --- a/spec/models/compliance_control_spec.rb +++ b/spec/models/compliance_control_spec.rb @@ -43,9 +43,17 @@ RSpec.describe ComplianceControl, type: :model do compliance_control_block_id: compliance_control_block.id, compliance_control_set_id: create( :compliance_control_set ).id expect(compliance_control).to_not be_valid + + direct_name = compliance_control.compliance_control_set.name + indirect_name = compliance_control_block.compliance_control_set.name + expected_message = "Le contrôle ne peut pas être associé à un jeu de contrôle (id: #{direct_name}) différent de celui de son groupe (id: #{indirect_name})" + selected_error_message = - compliance_control.errors.messages[:coherent_control_set].grep(%r{ControlSet associé}) - expect( selected_error_message ).to_not be_empty + compliance_control + .errors + .messages[:coherent_control_set] + .first + expect( selected_error_message ).to eq(expected_message) end end end diff --git a/spec/workers/compliance_control_set_copy_worker_spec.rb b/spec/workers/compliance_control_set_copy_worker_spec.rb new file mode 100644 index 000000000..3456ad398 --- /dev/null +++ b/spec/workers/compliance_control_set_copy_worker_spec.rb @@ -0,0 +1,13 @@ +RSpec.describe ComplianceControlSetCopyWorker do + let( :copier ){ ComplianceControlSetCopier } + + let( :compliance_control_set_id ){ double('compliance_control_set_id') } + let( :referential_id ){ double('referential_id') } + + before do + expect_any_instance_of( copier ).to receive(:copy).with(compliance_control_set_id, referential_id) + end + it 'delegates to ComplianceControlSetCopier' do + described_class.new.perform(compliance_control_set_id, referential_id) + end +end diff --git a/spec/workers/line_referential_sync_worker_spec.rb b/spec/workers/line_referential_sync_worker_spec.rb index f1a63c9db..f8d7eed91 100644 --- a/spec/workers/line_referential_sync_worker_spec.rb +++ b/spec/workers/line_referential_sync_worker_spec.rb @@ -1,4 +1,3 @@ -require 'rails_helper' RSpec.describe LineReferentialSyncWorker, type: :worker do let!(:line_referential_sync) { create :line_referential_sync } |