Merge branch 'fnf-extract-form-fixes' into develop

app/controllers/projects/extracts_controller.rb

@@ -10,14 +10,20 @@ def show
       redirect_to @project, notice: msg
       return
     end
+
+    @value_set = Dataset::ValueSet.new
   end
 
   def create
     authorize! :read, @project
 
-    if @project.submissions.create(submission_params)
+    @value_set = Dataset::ValueSet.new(extract_params)
+    submission = @project.submissions.new(submission_params)
+
+    if submission.save
       redirect_to project_extract_path
     else
+      flash.now[:error] = _("Extraction couldn't be saved.")
       render :show
     end
   end
@@ -53,7 +59,7 @@ def submission_params
     {
       user: current_user,
       info_request: @info_request,
-      resource: Dataset::ValueSet.new(extract_params)
+      resource: @value_set
     }
   end
 end

app/models/dataset/value.rb

@@ -19,6 +19,8 @@ class Dataset::Value < ApplicationRecord
   belongs_to :value_set, foreign_key: 'dataset_value_set_id'
   belongs_to :key, foreign_key: 'dataset_key_id'
 
-  validates :value_set, :key, :value, presence: true
-  validates :value, format: { with: -> (value) { value.key&.format_regexp } }
+  validates :value_set, :key, presence: true
+  validates :value,
+            format: { with: -> (value) { value.key&.format_regexp } },
+            allow_blank: true
 end

app/models/dataset/value_set.rb

@@ -19,17 +19,23 @@ class Dataset::ValueSet < ApplicationRecord
   belongs_to :key_set, foreign_key: 'dataset_key_set_id'
   has_many :values, foreign_key: 'dataset_value_set_id', inverse_of: :value_set
 
-  accepts_nested_attributes_for :values, reject_if: proc { |attributes|
-    attributes['dataset_key_id'].blank?
-  }
+  accepts_nested_attributes_for :values
 
   RESOURCE_TYPES = %w[
     InfoRequest
     IncomingMessage
     FoiAttachment
   ].freeze
 
-  validates :key_set, presence: true
+  validates :key_set, :values, presence: true
   validates :resource_type, inclusion: { in: RESOURCE_TYPES }, if: :resource
   validates_associated :values
+  validate :check_at_least_one_value_is_present
+
+  private
+
+  def check_at_least_one_value_is_present
+    return unless values.map(&:value).all?(&:blank?)
+    errors.add :values, :emtpy
+  end
 end

app/views/projects/dataset/keys/_boolean_key.html.erb

@@ -1,4 +1,4 @@
-<div class="dataset-key boolean-checkbox">
-  <%= f.check_box :value %>
+<div class="dataset-key boolean-select">
   <%= f.label :value, key.title %>
+  <%= f.select :value, [[_('No'), 0], [_('Yes'), 1]], include_blank: true %>
 </div>

app/views/projects/dataset/keys/_key.html.erb

@@ -1,4 +1,5 @@
-<%= f.fields_for :values, Dataset::Value.new do |nested_form| %>
+<% value = f.object.values.find { |v| v.key.order == key.order } %>
+<%= f.fields_for :values, value || Dataset::Value.new do |nested_form| %>
   <%= render partial: "projects/dataset/keys/#{key.format}_key",
              locals: { f: nested_form, key: key } %>
   <%= nested_form.hidden_field :dataset_key_id, value: key.to_param %>

app/views/projects/extracts/_form.html.erb

@@ -1,8 +1,5 @@
 <div class="extract-answer-form">
-  <%= form_for Dataset::ValueSet.new,
-               url: project_extract_path,
-               as: :extract do |f| %>
-
+  <%= form_for value_set, url: project_extract_path, as: :extract do |f| %>
     <%= hidden_field_tag :url_title, info_request.url_title %>
     <%= render project.key_set, f: f %>
 

app/views/projects/extracts/_sidebar.html.erb

@@ -3,6 +3,7 @@
     <%= js_correspondence_navigation %>
 
     <%= render partial: 'form', locals: { project: project,
-                                          info_request: info_request } %>
+                                          info_request: info_request,
+                                          value_set: value_set } %>
   </div>
 </div>

app/views/projects/extracts/show.html.erb

@@ -29,5 +29,6 @@
     </div>
   </div>
   <%= render partial: 'sidebar', locals: { project: @project,
-                                           info_request: @info_request } %>
+                                           info_request: @info_request,
+                                           value_set: @value_set } %>
 </div>

spec/controllers/projects/extracts_controller_spec.rb

@@ -32,6 +32,10 @@
         expect(assigns[:project]).to eq(project)
       end
 
+      it 'assigns the value set' do
+        expect(assigns[:value_set]).to be_a(Dataset::ValueSet)
+      end
+
       it 'renders the project template' do
         expect(response).to render_template('projects/extracts/show')
       end
@@ -165,13 +169,18 @@ def post_extract(extract = nil)
       include_context 'with a logged in user who can read the project'
 
       let(:submissions) { double(:submissions_collection) }
-      before { allow(project).to receive(:submissions).and_return(submissions) }
+      let(:submission) { instance_double(Project::Submission) }
+
+      before do
+        allow(project).to receive(:submissions).and_return(submissions)
+        allow(submissions).to receive(:new).and_return(submission)
+      end
     end
 
     context 'submission created' do
       include_context 'extraction can be submitted'
 
-      before { allow(submissions).to receive(:create).and_return(true) }
+      before { allow(submission).to receive(:save).and_return(true) }
 
       it 'initialises new value set with request' do
         params = {
@@ -192,9 +201,10 @@ def post_extract(extract = nil)
       it 'creates project submission' do
         value_set = instance_double(Dataset::ValueSet)
         allow(Dataset::ValueSet).to receive(:new).and_return(value_set)
-        expect(submissions).to receive(:create).with(
+        expect(submissions).to receive(:new).with(
           user: user, info_request: info_request, resource: value_set
-        )
+        ).and_return(submission)
+        expect(submission).to receive(:save)
         post_extract
       end
 
@@ -207,7 +217,7 @@ def post_extract(extract = nil)
     context 'submission validation fails' do
       include_context 'extraction can be submitted'
 
-      before { expect(submissions).to receive(:create).and_return(false) }
+      before { expect(submission).to receive(:save).and_return(false) }
 
       it 'assigns the project' do
         post_extract
@@ -219,6 +229,16 @@ def post_extract(extract = nil)
         expect(assigns[:info_request]).to eq(info_request)
       end
 
+      it 'assigns the value set' do
+        post_extract
+        expect(assigns[:value_set]).to be_a(Dataset::ValueSet)
+      end
+
+      it 'sets flash now error' do
+        post_extract
+        expect(flash.now[:error]).to eq("Extraction couldn't be saved.")
+      end
+
       it 'renders show template' do
         post_extract
         expect(response).to render_template('show')

spec/factories/dataset_value_sets.rb

@@ -30,11 +30,14 @@
     end
 
     transient do
-      value_count { 0 }
+      value_count { 1 }
     end
 
-    after(:create) do |value_set, evaluator|
-      create_list(:dataset_value, evaluator.value_count, value_set: value_set)
+    after(:build) do |value_set, evaluator|
+      next if value_set.values.count > 0 || evaluator.value_count.zero?
+      value_set.values = build_list(
+        :dataset_value, evaluator.value_count, value_set: value_set
+      )
     end
   end
 end

spec/models/dataset/value_set_spec.rb

@@ -48,6 +48,21 @@
     end
   end
 
+  describe 'nested attibutes' do
+    it 'accpets attributes for values' do
+      key = FactoryBot.create(:dataset_key)
+      value_set = FactoryBot.create(
+        :dataset_value_set,
+        value_count: 0,
+        values_attributes: [{ dataset_key_id: key.id, value: '1' }]
+      )
+      value = value_set.values.first
+      expect(value).to be_a Dataset::Value
+      expect(value.key).to eq key
+      expect(value.value).to eq '1'
+    end
+  end
+
   describe 'validations' do
     it { is_expected.to be_valid }
 
@@ -71,5 +86,15 @@
       value_set.key_set = nil
       is_expected.not_to be_valid
     end
+
+    it 'requires values' do
+      value_set.values = []
+      is_expected.not_to be_valid
+    end
+
+    it 'requires at least one value to be present' do
+      value_set.values.each { |v| v.value = '' }
+      is_expected.to_not be_valid
+    end
   end
 end

spec/models/dataset/value_spec.rb

@@ -39,6 +39,7 @@ def invalid(value_to_test)
 
       it 'checks text values' do
         value.key = FactoryBot.build(:dataset_key, :text)
+        valid('')
         valid('A string')
         valid('1234')
         valid('1')
@@ -47,6 +48,7 @@ def invalid(value_to_test)
 
       it 'checks numeric values' do
         value.key = FactoryBot.build(:dataset_key, :numeric)
+        valid('')
         invalid('A string')
         valid('1234')
         valid('1')
@@ -55,6 +57,7 @@ def invalid(value_to_test)
 
       it 'checks boolean values' do
         value.key = FactoryBot.build(:dataset_key, :boolean)
+        valid('')
         invalid('A string')
         invalid('1234')
         valid('1')