Refs: #4629@1h; Implementing CR + missing worker spec

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 }