From 370674224910366cc836a424da4e27071f4e4346 Mon Sep 17 00:00:00 2001
From: Trey Terrell <trey.terrell@oregonstate.edu>
Date: Fri, 14 Aug 2015 10:40:19 -0700
Subject: [PATCH 1/5] Adjust template editor to allow for more info

---
 app/views/admin/form_templates/_form.html.erb | 13 +++++++++--
 spec/features/template_management_spec.rb     | 23 +++++++++++++++----
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/app/views/admin/form_templates/_form.html.erb b/app/views/admin/form_templates/_form.html.erb
index a768c4d..6f7c412 100644
--- a/app/views/admin/form_templates/_form.html.erb
+++ b/app/views/admin/form_templates/_form.html.erb
@@ -3,8 +3,17 @@
   <%= f.input :title %>
 
   <%= f.simple_fields_for :properties do |p| %>
-    <%= p.input :visible, :label => "Show #{p.object.name}" %>
-    <%= p.input :name, :as => :hidden %>
+    <div id="<%= p.object.name %>" class="panel panel-default template-property">
+      <div class="panel-heading">
+        <div class="panel-title">
+          <%= p.object.name %>
+        </div>
+      </div>
+      <div class="panel-body">
+        <%= p.input :visible, :label => "Show", :wrapper => :inline %>
+        <%= p.input :name, :as => :hidden %>
+      </div>
+    </div>
   <% end %>
 
   <%= f.submit "Save Template" %>
diff --git a/spec/features/template_management_spec.rb b/spec/features/template_management_spec.rb
index 2a4b70d..4c57438 100644
--- a/spec/features/template_management_spec.rb
+++ b/spec/features/template_management_spec.rb
@@ -29,7 +29,7 @@
 
       before do
         fill_in "form_template_title", :with => template_name
-        check_fields.each {|field| check "Show %s" % field}
+        check_fields.each {|field| show_field field}
         click_button "Save Template"
       end
 
@@ -59,8 +59,8 @@
           let(:updated_template_name) { "Updated test template %d" % Random.rand(99999) }
           before do
             fill_in "form_template_title", :with => updated_template_name
-            check "Show creator"
-            uncheck "Show title"
+            show_field "creator"
+            hide_field "title"
             click_button "Save Template"
           end
 
@@ -101,9 +101,22 @@ def navigate_to_template_admin_path
 
 def checkbox_states
   checkboxes = HashWithIndifferentAccess.new
-  all('.form_template_properties_visible').each do |el|
-    checkboxes[el.find("label").text.sub("Show ", "")] = el.find("input[type=checkbox]")
+  all('.template-property').each do |el|
+    checkboxes[el.find(".panel-title").text] = el.find("input[type=checkbox]")
   end
 
   checkboxes
 end
+
+def show_field(field)
+  within "##{field}" do
+    check "Show"
+  end
+end
+
+def hide_field(field)
+  within "##{field}" do
+    uncheck "Show"
+  end
+end
+

From a37a6b6b1ad34da80113c02771dcd14ac7a087ce Mon Sep 17 00:00:00 2001
From: Trey Terrell <trey.terrell@oregonstate.edu>
Date: Fri, 14 Aug 2015 10:48:55 -0700
Subject: [PATCH 2/5] Add required to template creation.

---
 app/controllers/admin/form_templates_controller.rb   |  2 +-
 app/views/admin/form_templates/_form.html.erb        |  3 ++-
 ...74059_add_required_to_form_template_properties.rb |  5 +++++
 db/schema.rb                                         |  3 ++-
 spec/features/template_management_spec.rb            | 12 +++++++++++-
 5 files changed, 21 insertions(+), 4 deletions(-)
 create mode 100644 db/migrate/20150814174059_add_required_to_form_template_properties.rb

diff --git a/app/controllers/admin/form_templates_controller.rb b/app/controllers/admin/form_templates_controller.rb
index 9475953..ceb4b89 100644
--- a/app/controllers/admin/form_templates_controller.rb
+++ b/app/controllers/admin/form_templates_controller.rb
@@ -60,7 +60,7 @@ def find_template
   end
 
   def form_template_params
-    params.require(:form_template).permit(:title, {:properties_attributes => [:id, :name, :visible]})
+    params.require(:form_template).permit(:title, {:properties_attributes => [:id, :name, :visible, :required]})
   end
 
   def template_class
diff --git a/app/views/admin/form_templates/_form.html.erb b/app/views/admin/form_templates/_form.html.erb
index 6f7c412..eb68766 100644
--- a/app/views/admin/form_templates/_form.html.erb
+++ b/app/views/admin/form_templates/_form.html.erb
@@ -10,7 +10,8 @@
         </div>
       </div>
       <div class="panel-body">
-        <%= p.input :visible, :label => "Show", :wrapper => :inline %>
+        <%= p.input :visible, :label => "Show" %>
+        <%= p.input :required %>
         <%= p.input :name, :as => :hidden %>
       </div>
     </div>
diff --git a/db/migrate/20150814174059_add_required_to_form_template_properties.rb b/db/migrate/20150814174059_add_required_to_form_template_properties.rb
new file mode 100644
index 0000000..e11f341
--- /dev/null
+++ b/db/migrate/20150814174059_add_required_to_form_template_properties.rb
@@ -0,0 +1,5 @@
+class AddRequiredToFormTemplateProperties < ActiveRecord::Migration
+  def change
+    add_column :form_template_properties, :required, :boolean
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f9eb03c..028901b 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150803175310) do
+ActiveRecord::Schema.define(version: 20150814174059) do
 
   create_table "bookmarks", force: :cascade do |t|
     t.integer  "user_id",       null: false
@@ -38,6 +38,7 @@
     t.integer "form_template_id"
     t.string  "name",                             null: false
     t.boolean "visible",          default: false
+    t.boolean "required"
   end
 
   create_table "form_templates", force: :cascade do |t|
diff --git a/spec/features/template_management_spec.rb b/spec/features/template_management_spec.rb
index 4c57438..77a0558 100644
--- a/spec/features/template_management_spec.rb
+++ b/spec/features/template_management_spec.rb
@@ -30,6 +30,7 @@
       before do
         fill_in "form_template_title", :with => template_name
         check_fields.each {|field| show_field field}
+        require_field :title
         click_button "Save Template"
       end
 
@@ -53,6 +54,9 @@
           (GenericAssetForm.terms - check_fields).each do |t|
             expect(checkboxes[t]).not_to be_checked
           end
+          within "#title .form_template_properties_required" do
+            expect(find("input[type=checkbox]")).to be_checked
+          end
         end
 
         context "when submitting changes" do
@@ -102,7 +106,7 @@ def navigate_to_template_admin_path
 def checkbox_states
   checkboxes = HashWithIndifferentAccess.new
   all('.template-property').each do |el|
-    checkboxes[el.find(".panel-title").text] = el.find("input[type=checkbox]")
+    checkboxes[el.find(".panel-title").text] = el.find(".form_template_properties_visible input[type=checkbox]")
   end
 
   checkboxes
@@ -120,3 +124,9 @@ def hide_field(field)
   end
 end
 
+def require_field(field)
+  within "##{field}" do
+    check "Required"
+  end
+end
+

From e7054760fb96e9a5c70830e0d82d0bc8b068b337 Mon Sep 17 00:00:00 2001
From: Trey Terrell <trey.terrell@oregonstate.edu>
Date: Fri, 14 Aug 2015 11:12:02 -0700
Subject: [PATCH 3/5] Enforce required fields at form layer.

---
 app/forms/generic_asset_form.rb       | 16 +++++++++++++++-
 app/models/form_template.rb           |  4 ++++
 spec/factories/form_templates.rb      |  6 ++++++
 spec/forms/generic_asset_form_spec.rb | 27 +++++++++++++++++++++++++--
 spec/models/form_template_spec.rb     |  7 +++++++
 5 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/app/forms/generic_asset_form.rb b/app/forms/generic_asset_form.rb
index d9a250b..538904f 100644
--- a/app/forms/generic_asset_form.rb
+++ b/app/forms/generic_asset_form.rb
@@ -26,12 +26,22 @@ def template
   end
 
   def template_terms
-    template.visible_property_names
+    template.visible_property_names.map(&:to_sym)
   end
 
   def hidden_terms
     self.class.terms - template_terms.map(&:to_sym)
   end
+
+  def required_fields
+    self.class.required_fields | template_required_property_names
+  end
+
+  private
+
+  def template_required_property_names
+    template.required_property_names.map(&:to_sym)
+  end
 end
 
 # Provides a compatible API for using forms that have no template
@@ -45,4 +55,8 @@ def id
   def visible_property_names
     property_names
   end
+
+  def required_property_names
+    []
+  end
 end
diff --git a/app/models/form_template.rb b/app/models/form_template.rb
index af5e38e..4fe813a 100644
--- a/app/models/form_template.rb
+++ b/app/models/form_template.rb
@@ -21,6 +21,10 @@ def visible_property_names
     properties.select {|property| property.visible?}.collect(&:name)
   end
 
+  def required_property_names
+    properties.select {|property| property.required?}.collect(&:name)
+  end
+
   def property_map
     @property_map || rebuild_property_map
   end
diff --git a/spec/factories/form_templates.rb b/spec/factories/form_templates.rb
index 282d147..fac2860 100644
--- a/spec/factories/form_templates.rb
+++ b/spec/factories/form_templates.rb
@@ -8,6 +8,12 @@
       end
     end
 
+    trait :with_required_title do
+      after(:build) do |obj|
+        obj.properties << build(:form_template_property, :name => "title", :required => true)
+      end
+    end
+
     trait :with_desc do
       after(:build) do |obj|
         obj.properties << build(:form_template_property, :name => "description")
diff --git a/spec/forms/generic_asset_form_spec.rb b/spec/forms/generic_asset_form_spec.rb
index 90bdfb1..74da65b 100644
--- a/spec/forms/generic_asset_form_spec.rb
+++ b/spec/forms/generic_asset_form_spec.rb
@@ -42,7 +42,7 @@
       end
     end
     context "when there is a template" do
-      let(:template) { instance_double("FormTemplate") }
+      let(:template) { instance_double(FormTemplate) }
       before do
         generic_form.template = template
         allow(described_class).to receive(:terms).and_return([:foo, :banana])
@@ -63,7 +63,7 @@
     end
 
     context "when there is a template" do
-      let(:template) { instance_double("FormTemplate") }
+      let(:template) { instance_double(FormTemplate) }
       before do
         generic_form.template = template
         allow(template).to receive(:visible_property_names).and_return(["foo", "bar"])
@@ -74,4 +74,27 @@
       end
     end
   end
+  
+  describe "#required_fields" do
+    context "when there is no template" do
+      it "should fall back to class-level required fields" do
+        expect(generic_form.required_fields).to eq GenericAssetForm.required_fields
+      end
+    end
+    context "when there is a template" do
+      let(:template) { instance_double(FormTemplate) }
+      before do
+        generic_form.template = template
+        allow(template).to receive(:required_property_names).and_return(["foo"])
+      end
+      it "should return the template's required properties" do
+        expect(generic_form.required_fields).to eq [:foo]
+        expect(generic_form.required?(:foo)).to eq true
+      end
+      it "should not lose the class' required fields" do
+        allow(generic_form.class).to receive(:required_fields).and_return([:bar])
+        expect(generic_form.required_fields).to eq [:bar, :foo]
+      end
+    end
+  end
 end
diff --git a/spec/models/form_template_spec.rb b/spec/models/form_template_spec.rb
index 6e85553..fa2f488 100644
--- a/spec/models/form_template_spec.rb
+++ b/spec/models/form_template_spec.rb
@@ -24,6 +24,13 @@
     end
   end
 
+  describe "#required_property_names" do
+    it "should return names of properties which are required" do
+      template = FactoryGirl.build(:form_template, :with_required_title, :with_desc)
+      expect(template.required_property_names).to contain_exactly("title")
+    end
+  end
+
   describe "#property_names=" do
     it "should build hidden properties" do
       template.property_names = ["foo", "bar"]

From af3b28f55960266a1c9b90248c5f93411d529d2f Mon Sep 17 00:00:00 2001
From: Trey Terrell <trey.terrell@oregonstate.edu>
Date: Fri, 14 Aug 2015 11:18:21 -0700
Subject: [PATCH 4/5] Fix up spec.

---
 spec/forms/generic_asset_form_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/forms/generic_asset_form_spec.rb b/spec/forms/generic_asset_form_spec.rb
index 74da65b..93031be 100644
--- a/spec/forms/generic_asset_form_spec.rb
+++ b/spec/forms/generic_asset_form_spec.rb
@@ -70,7 +70,7 @@
       end
 
       it "should return template.visible_property_names" do
-        expect(generic_form.template_terms).to eq(["foo", "bar"])
+        expect(generic_form.template_terms).to eq([:foo, :bar])
       end
     end
   end

From 8f3c5ba098bb782c5950e8979ad9caa7e6385403 Mon Sep 17 00:00:00 2001
From: Trey Terrell <trey.terrell@oregonstate.edu>
Date: Fri, 14 Aug 2015 12:58:21 -0700
Subject: [PATCH 5/5] Update template form for improved accessibility.

---
 app/views/admin/form_templates/_form.html.erb | 18 ++++++------------
 spec/features/template_management_spec.rb     |  2 +-
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/app/views/admin/form_templates/_form.html.erb b/app/views/admin/form_templates/_form.html.erb
index eb68766..5d6cf33 100644
--- a/app/views/admin/form_templates/_form.html.erb
+++ b/app/views/admin/form_templates/_form.html.erb
@@ -3,18 +3,12 @@
   <%= f.input :title %>
 
   <%= f.simple_fields_for :properties do |p| %>
-    <div id="<%= p.object.name %>" class="panel panel-default template-property">
-      <div class="panel-heading">
-        <div class="panel-title">
-          <%= p.object.name %>
-        </div>
-      </div>
-      <div class="panel-body">
-        <%= p.input :visible, :label => "Show" %>
-        <%= p.input :required %>
-        <%= p.input :name, :as => :hidden %>
-      </div>
-    </div>
+    <fieldset id="<%= p.object.name.underscore %>" class="template-property">
+      <legend><%= p.object.name %></legend>
+      <%= p.input :visible, :label => "Show" %>
+      <%= p.input :required %>
+      <%= p.input :name, :as => :hidden %>
+    </fieldset>
   <% end %>
 
   <%= f.submit "Save Template" %>
diff --git a/spec/features/template_management_spec.rb b/spec/features/template_management_spec.rb
index 77a0558..0bb5436 100644
--- a/spec/features/template_management_spec.rb
+++ b/spec/features/template_management_spec.rb
@@ -106,7 +106,7 @@ def navigate_to_template_admin_path
 def checkbox_states
   checkboxes = HashWithIndifferentAccess.new
   all('.template-property').each do |el|
-    checkboxes[el.find(".panel-title").text] = el.find(".form_template_properties_visible input[type=checkbox]")
+    checkboxes[el.find("legend").text] = el.find(".form_template_properties_visible input[type=checkbox]")
   end
 
   checkboxes