Merge pull request #4123 from benwbrum/3768-error-saving-field-based-…

…page-fix

3768 - Fix error saving field-based page

app/helpers/transcription_field_helper.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module TranscriptionFieldHelper
   def field_order(collection)
     @fields = collection.transcription_fields.order(:line_number).order(:position).group_by(&:line_number)
@@ -22,4 +24,35 @@ def field_layout(array)
     @field_array = array.zip(@values)
   end
 
+  def generate_field_input(field, cell)
+    input_name = formatted_field_name(field)
+
+    label = label_tag(field.label.parameterize, field.label)
+
+    content = cell&.content
+
+    case field.input_type
+    when 'text'
+      input = text_field_tag(input_name, content, class: 'field-input')
+    when 'date'
+      input = text_field_tag(input_name, content, class: 'field-input edtf',
+                                                  data: { inputmask: '"alias": "datetime", "inputFormat": "isoDate"' })
+    when 'select'
+      options = field.options&.split(';')
+      input = select_tag(input_name, options_for_select(options, content), class: 'field-input')
+    when 'description'
+      input = hidden_field_tag(input_name, content, class: 'field-input')
+    when 'alt text'
+      input = text_area_tag(input_name, content, class: 'field-input alt-text', maxlength: '255') +
+              content_tag(:div, class: 'character-count')
+    else
+      input = text_area_tag(input_name, content, class: 'field-input')
+    end
+
+    label + input
+  end
+
+  def formatted_field_name(field)
+    "fields[#{field.id}][#{field.label.parameterize}]"
+  end
 end

app/models/transcription_field.rb

@@ -1,20 +1,24 @@
+# frozen_string_literal: true
+
 class TranscriptionField < ApplicationRecord
   belongs_to :collection, optional: true
-  acts_as_list :scope => :collection
+
+  acts_as_list scope: :collection
+
   has_many :table_cells
-  has_many :spreadsheet_columns, -> { order 'position' }, :dependent => :destroy
-  validates :options, presence: true, if: Proc.new {|field| field.input_type == 'select'}, on: [:create, :update]
+  has_many :spreadsheet_columns, -> { order 'position' }, dependent: :destroy
 
+  validates :options, presence: true, if: Proc.new { |field| field.input_type == 'select' }, on: [:create, :update]
   validates :percentage, numericality: { allow_nil: true, greater_than: 0, less_than_or_equal_to: 100 }
   validates :page_number, numericality: { allow_nil: true, greater_than: 0, less_than_or_equal_to: 1000 }
-
+  validates :label, format: { without: /[\[\]]/, message: "cannot contain '[' or ']'" }
 
   module FieldType
     TRANSCRIPTION = 'transcription'
     METADATA = 'metadata'
   end
 
-
-  TRANSCRIPTION_INPUTS = ["text", "select", "date", "textarea", "description", "instruction", "spreadsheet", "alt text"]
-  METADATA_INPUTS = ["text", "select", "date", "multiselect", "textarea", "instruction", "alt text"]
+  TRANSCRIPTION_INPUTS = ['text', 'select', 'date', 'textarea',
+                          'description', 'instruction', 'spreadsheet', 'alt text'].freeze
+  METADATA_INPUTS = ['text', 'select', 'date', 'multiselect', 'textarea', 'instruction', 'alt text'].freeze
 end

app/views/transcription_field/_field_layout.html.slim

@@ -14,32 +14,17 @@
           -span_width = !field.percentage.blank? ? field.percentage : @width
           .field-wrapper(style="width: #{span_width}%")
             -if field.input_type == "instruction"
-              div id="fields[#{field.id}][#{field.label}]" class="field-instructions"
+              div id=formatted_field_name(field) class="field-instructions"
                 h5 =t('.instructions')
-                p ==field.label
+                p ==field.label.titleize
             -else
-              =label_tag field.label, field.label
-              -content = cell.nil? ? nil : cell.content
-              -if field.input_type == "text"
-                =text_field_tag "fields[#{field.id}][#{field.label}]", content, class: 'field-input'
-              -elsif field.input_type == "date"
-                =text_field_tag "fields[#{field.id}][#{field.label}]", content, class: 'field-input edtf', data: { inputmask: '"alias": "datetime", "inputFormat": "isoDate"'}
-              -elsif field.input_type == "select"
-                -options = field.options.split(";") unless field.options.nil?
-                =select_tag("fields[#{field.id}][#{field.label}]", options_for_select(options, content), class: 'field-input')
-              -elsif field.input_type == "description"
-                =hidden_field_tag "fields[#{field.id}][#{field.label}]", content, class: 'field-input'
-              -elsif field.input_type == "alt text"
-                =text_area_tag "fields[#{field.id}][#{field.label}]", content, class: 'field-input alt-text', maxlength: '255'
-                div.character-count
-              -else
-                =text_area_tag "fields[#{field.id}][#{field.label}]", content, class: 'field-input'
+              =generate_field_input(field, cell)
 
 -content_for :javascript
   javascript:
     function countChars() {
       var chars = $(this).val().length
-      $(this).next().text(chars + " / 180"); 
+      $(this).next().text(chars + " / 180");
 
       if (chars > 180) {
         $(this).css('background-color', '#F5B7AC');
@@ -58,4 +43,4 @@
         countChars.call($(this));
       });
     });
-    $('.edtf').inputmask();
+    $('.edtf').inputmask();

spec/features/field_based_spec.rb

@@ -1,4 +1,3 @@
-
 require 'spec_helper'
 
 describe "collection settings js tasks", :order => :defined do
@@ -39,16 +38,16 @@
   end
 
   it "checks the field preview on edit page" do
-    #check the field preview
+    # check the field preview
     visit collection_path(@collection.owner, @collection)
-    page.find('.tabs').click_link("Fields")
-    expect(page.find('div.fields-preview')).to have_content("First field")
-    expect(page.find('div.fields-preview')).to have_content("Second field")
-    expect(page.find('div.fields-preview')).to have_content("Third field")
-    #check field width for first field (set to 20%)
-    expect(page.find('div.fields-preview .field-wrapper[1]')[:style]).to eq "width: 20%"
-    #check field width for second field (not set)
-    expect(page.find('div.fields-preview .field-wrapper[2]')[:style]).not_to eq "width: 20%"
+    page.find('.tabs').click_link('Fields')
+    expect(page.find('div.fields-preview')).to have_content('First field')
+    expect(page.find('div.fields-preview')).to have_content('Second field')
+    expect(page.find('div.fields-preview')).to have_content('Third field')
+    # check field width for first field (set to 20%)
+    expect(page.find('div.fields-preview .field-wrapper[1]')[:style]).to eq 'width: 20%'
+    # check field width for second field (not set)
+    expect(page.find('div.fields-preview .field-wrapper[2]')[:style]).not_to eq 'width: 20%'
     expect(TranscriptionField.count).to eq 3
   end
 
@@ -80,18 +79,18 @@
     expect(TranscriptionField.all.count).to eq 3
     visit collection_transcribe_page_path(@collection.owner, @collection, work, field_page)
     expect(TranscriptionField.all.count).to eq 3
-    expect(page).not_to have_content("Autolink")
-    expect(page).to have_content("First field")
-    expect(page).to have_content("Second field")
-    expect(page).to have_content("Third field")
-    page.fill_in('fields_1_First_field', with: "Field one")
-    page.fill_in('fields_2_Second_field', with: "Field < three")
-    page.fill_in('fields_3_Third_field', with: "Field three")
+    expect(page).not_to have_content('Autolink')
+    expect(page).to have_content('First field')
+    expect(page).to have_content('Second field')
+    expect(page).to have_content('Third field')
+    page.fill_in('fields_1_first-field', with: 'Field one')
+    page.fill_in('fields_2_second-field', with: 'Field < three')
+    page.fill_in('fields_3_third-field', with: 'Field three')
     find('#save_button_top').click
     click_button 'Preview', match: :first
-    expect(page.find('.page-preview')).to have_content("First field: Field one")
+    expect(page.find('.page-preview')).to have_content('first-field: Field one')
     click_button 'Edit', match: :first
-    expect(page.find('.page-editarea')).to have_selector('#fields_1_First_field')
+    expect(page.find('.page-editarea')).to have_selector('#fields_1_first-field')
   end
 
   it "deletes a transcription field" do
@@ -106,7 +105,7 @@
     test_page = @collection.works.first.pages.second
     #next page arrow
     visit collection_transcribe_page_path(@collection.owner, @collection, test_page.work, test_page)
-    page.fill_in('fields_1_First_field', with: "Field one")
+    page.fill_in('fields_1_first-field', with: "Field one")
     message = accept_alert do
       page.click_link("Next page")
     end