Merge pull request #84 from af83/4629-comp-check-set-creation

4629 comp check set creation *Mergeable*

13 files changed, 287 insertions, 31 deletions

diff --git a/app/models/compliance_check_block.rb b/app/models/compliance_check_block.rb
index 035c03ed9..ee60a8bb1 100644
--- a/app/models/compliance_check_block.rb
+++ b/app/models/compliance_check_block.rb
@@ -1,3 +1,5 @@
 class ComplianceCheckBlock < ActiveRecord::Base
   belongs_to :compliance_check_set
+
+  has_many :compliance_checks
 end
diff --git a/app/models/compliance_check_set.rb b/app/models/compliance_check_set.rb
index e4146e0e2..536afc705 100644
--- a/app/models/compliance_check_set.rb
+++ b/app/models/compliance_check_set.rb
@@ -6,6 +6,9 @@ class ComplianceCheckSet < ActiveRecord::Base
   belongs_to :workbench
   belongs_to :parent, polymorphic: true
 
+  has_many :compliance_check_blocks
+  has_many :compliance_checks
+
   has_many :compliance_check_resources
   has_many :compliance_check_messages
 
diff --git a/app/models/compliance_control.rb b/app/models/compliance_control.rb
index 33a075e40..363ef5d61 100644
--- a/app/models/compliance_control.rb
+++ b/app/models/compliance_control.rb
@@ -12,6 +12,17 @@ class ComplianceControl < ActiveRecord::Base
   validates :origin_code, presence: true
   validates :compliance_control_set, presence: true
 
+  validate def coherent_control_set
+    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
+    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 default_criticity; :warning end
     def default_code; "" end
diff --git a/config/locales/compliance_controls.en.yml b/config/locales/compliance_controls.en.yml
index bef39515c..cee83afee 100644
--- a/config/locales/compliance_controls.en.yml
+++ b/config/locales/compliance_controls.en.yml
@@ -1,5 +1,7 @@
 en:
   compliance_controls:
+    errors:
+      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 05e47665c..f0f4ecf77 100644
--- a/config/locales/compliance_controls.fr.yml
+++ b/config/locales/compliance_controls.fr.yml
@@ -1,5 +1,7 @@
 fr:
   compliance_controls:
+    errors:
+      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/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/compliance_control_set_copier_spec.rb b/spec/lib/compliance_control_set_copier_spec.rb
new file mode 100644
index 000000000..b620d0657
--- /dev/null
+++ b/spec/lib/compliance_control_set_copier_spec.rb
@@ -0,0 +1,88 @@
+RSpec.describe ComplianceControlSetCopier do
+
+  subject{ described_class.new }
+
+  let( :cc_set ){ create :compliance_control_set }
+
+  context 'Copying empty set' do 
+    context 'incorrect organisation' do
+      # Assuring the organisation missmatch
+      before { referential.organisation_id = cc_set.organisation_id.succ }
+      it 'fails' do
+        expect{ subject.copy(cc_set.id, referential.id) }.to raise_error(ArgumentError)
+      end
+      it 'does not create any objects in the database' do
+        expect{ subject.copy(cc_set.id, referential.id) rescue nil }.to_not change{ComplianceCheckSet.count}
+        expect{ subject.copy(cc_set.id, referential.id) rescue nil }.to_not change{ComplianceCheckBlock.count}
+        expect{ subject.copy(cc_set.id, referential.id) rescue nil }.to_not change{ComplianceCheck.count}
+      end
+    end
+
+    context 'correct organisation' do 
+      let(:ref){ create :referential, organisation_id: cc_set.organisation_id }
+
+      let(:cc_blox){
+        3.times.map{ |_| create :compliance_control_block, compliance_control_set: cc_set }
+      }
+      let!(:direct_ccs){
+        3.times.map{ |n| create :compliance_control, compliance_control_set: cc_set, name: "direct #{n.succ}" }
+      }
+      # Needed to check we do not dulicate a node (compliance_control) twice
+      let!(:indirect_ccs){
+        # Create 1 child for each block and also associate first of the direct ccs to the first block
+        #                                                  seconf of the direct css to the second block
+        cc_blox.take(2).zip(direct_ccs.take(2)).each do | cc_block, cc |
+          cc.update compliance_control_block_id: cc_block.id
+        end
+        cc_blox.each_with_index.map{ | cc_block, n |
+          create(:compliance_control, compliance_control_set: cc_set, compliance_control_block: cc_block, name: "indirect #{n.succ}")
+        }
+      }
+
+      let( :cck_set ){ ComplianceCheckSet.last }
+      let( :cck_blox ){ cck_set.compliance_check_blocks }
+      let( :ccks ){ cck_set.compliance_checks }
+
+      it 'correctly creates a cck_set for a complete DAG' do
+        # Slowness of tests constrain us to create a minimum of objects in the DB,
+        # hence only one example :(
+        counts = object_counts
+        subject.copy(cc_set.id, ref.id)
+
+        # Did not change the original objects
+        # Correct numbers
+        expect( ComplianceControlSet.count).to eq(counts.cc_set_count)
+        expect( ComplianceControlBlock.count).to eq(counts.cc_block_count)
+        expect( ComplianceControl.count).to eq(counts.cc_count)
+
+        expect( ComplianceCheckSet.count ).to eq(counts.cck_set_count + 1)
+        expect( cck_blox.count ).to eq(counts.cck_block_count + cc_blox.size)
+        expect( ccks.count ).to eq(counts.cck_count + direct_ccs.size + indirect_ccs.size)
+
+        # Correcly associated
+        expect( cck_blox.map(&:compliance_checks).map(&:size) )
+          .to eq([2, 2, 1])
+        expect( ComplianceCheck.where(name: mk_name('direct 1')).first.compliance_check_block_id )
+          .to eq( cck_blox.first.id )
+        expect( ComplianceCheck.where(name: mk_name('direct 3')).first.compliance_check_block_id ).to be_nil
+      end
+
+    end
+
+
+    def object_counts
+      OpenStruct.new \
+        cc_set_count: ComplianceControlSet.count,
+        cc_block_count: ComplianceControlBlock.count,
+        cc_count: ComplianceControl.count,
+        cck_set_count: ComplianceCheckSet.count,
+        cck_block_count: ComplianceCheckBlock.count,
+        cck_count: ComplianceCheck.count
+    end
+
+      def mk_name name
+        [name, ref.name].join('-')
+      end
+  end
+
+end
diff --git a/spec/models/compliance_check_block_spec.rb b/spec/models/compliance_check_block_spec.rb
index f581d5085..a3d98d459 100644
--- a/spec/models/compliance_check_block_spec.rb
+++ b/spec/models/compliance_check_block_spec.rb
@@ -6,4 +6,5 @@ RSpec.describe ComplianceCheckBlock, type: :model do
   end
 
   it { should belong_to :compliance_check_set }
+  it { should have_many :compliance_checks }
 end
diff --git a/spec/models/compliance_check_set_spec.rb b/spec/models/compliance_check_set_spec.rb
index 6e53c9def..8afea5b3e 100644
--- a/spec/models/compliance_check_set_spec.rb
+++ b/spec/models/compliance_check_set_spec.rb
@@ -9,4 +9,7 @@ RSpec.describe ComplianceCheckSet, type: :model do
   it { should belong_to :workbench }
   it { should belong_to :compliance_control_set }
   it { should belong_to :parent }
+
+  it { should have_many :compliance_checks }
+  it { should have_many :compliance_check_blocks }
 end
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_set_spec.rb b/spec/models/compliance_control_set_spec.rb
index edc684bbc..04d1c418c 100644
--- a/spec/models/compliance_control_set_spec.rb
+++ b/spec/models/compliance_control_set_spec.rb
@@ -7,6 +7,7 @@ RSpec.describe ComplianceControlSet, type: :model do
 
   it { should belong_to :organisation }
   it { should have_many(:compliance_controls).dependent(:destroy) }
+  it { should have_many(:compliance_control_blocks).dependent(:destroy) }
 
   it { should validate_presence_of :name }
 end
diff --git a/spec/models/compliance_control_spec.rb b/spec/models/compliance_control_spec.rb
index 50c2b7b8d..db73dab21 100644
--- a/spec/models/compliance_control_spec.rb
+++ b/spec/models/compliance_control_spec.rb
@@ -1,39 +1,59 @@
 RSpec.describe ComplianceControl, type: :model do
 
-  let(:compliance_control) { create :compliance_control }
+  context 'standard validation' do
 
-  it 'should have a valid factory' do
-    expect(compliance_control).to be_valid
-  end
+    let(:compliance_control) { build_stubbed :compliance_control }
 
-  it { should belong_to :compliance_control_set }
-  it { should belong_to :compliance_control_block }
+    it 'should have a valid factory' do
+      expect(compliance_control).to be_valid
+    end
 
-  it 'should validate_presence_of criticity' do
-    compliance_control.criticity = nil
-    expect(compliance_control).not_to be_valid
-  end
+    it { should belong_to :compliance_control_set }
+    it { should belong_to :compliance_control_block }
 
-  it 'should validate_presence_of name' do
-    compliance_control.name = nil
-    expect(compliance_control).not_to be_valid
-  end
 
-  it 'should validate_presence_of code' do
-    compliance_control.code = nil
-    expect(compliance_control).not_to be_valid
-  end
+    it { should validate_presence_of :criticity }
+    it 'should validate_presence_of :name' do
+      expect( build :compliance_control, name: '' ).to_not be_valid
+    end
+    it { should validate_presence_of :code }
+    it { should validate_presence_of :origin_code }
 
-  it 'should validate_presence_of origin_code' do
-    compliance_control.origin_code = nil
-    expect(compliance_control).not_to be_valid
   end
 
-  #TODO dont know why the 'shortcuts' below to validates presence dont work
-  # That's why we dont it 'manually'
-  # it { should validate_presence_of :criticity }
-  # it { should validate_presence_of :name }
-  # it { should validate_presence_of :code }
-  # it { should validate_presence_of :origin_code }
-
+  context 'validates that direct and indirect (via control_block) control_set are not different instances' do
+
+    it 'not attached to control_block -> valid' do
+      compliance_control = create :compliance_control, compliance_control_block_id: nil
+      expect(compliance_control).to be_valid
+    end
+
+    it 'attached to a control_block belonging to the same control_set -> valid' do
+      compliance_control_block = create :compliance_control_block
+      compliance_control = create :compliance_control,
+        compliance_control_block_id: compliance_control_block.id,
+        compliance_control_set_id: compliance_control_block.compliance_control_set.id # DO NOT change the last . to _
+                                                                                      # We need to be sure that is is not nil
+      expect(compliance_control).to be_valid
+    end
+
+    it 'attached to a control_block **not** belonging to the same control_set -> invalid' do
+      compliance_control_block = create :compliance_control_block
+      compliance_control = build :compliance_control,
+        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]
+          .first
+      expect( selected_error_message ).to eq(expected_message)
+    end
+  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 }