Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Required Template Fields #258

Merged
merged 5 commits into from Aug 14, 2015
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/admin/form_templates_controller.rb
Expand Up @@ -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
Expand Down
16 changes: 15 additions & 1 deletion app/forms/generic_asset_form.rb
Expand Up @@ -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
Expand All @@ -45,4 +55,8 @@ def id
def visible_property_names
property_names
end

def required_property_names
[]
end
end
4 changes: 4 additions & 0 deletions app/models/form_template.rb
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions app/views/admin/form_templates/_form.html.erb
Expand Up @@ -3,8 +3,18 @@
<%= 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" %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're hurting accessibility here by removing the full object name from the label. Consider a styled fieldset if you really want to convert to shorter labels.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might also be aria attributes you can put on the panel header, but I'm not sure if those exist or are as effective as a fieldset with legend

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're the best. Will do.

<%= p.input :required %>
<%= p.input :name, :as => :hidden %>
</div>
</div>
<% end %>

<%= f.submit "Save Template" %>
Expand Down
@@ -0,0 +1,5 @@
class AddRequiredToFormTemplateProperties < ActiveRecord::Migration
def change
add_column :form_template_properties, :required, :boolean
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Expand Up @@ -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
Expand All @@ -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|
Expand Down
6 changes: 6 additions & 0 deletions spec/factories/form_templates.rb
Expand Up @@ -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")
Expand Down
33 changes: 28 additions & 5 deletions spec/features/template_management_spec.rb
Expand Up @@ -29,7 +29,8 @@

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}
require_field :title
click_button "Save Template"
end

Expand All @@ -53,14 +54,17 @@
(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
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

Expand Down Expand Up @@ -101,9 +105,28 @@ 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(".form_template_properties_visible 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

def require_field(field)
within "##{field}" do
check "Required"
end
end

29 changes: 26 additions & 3 deletions spec/forms/generic_asset_form_spec.rb
Expand Up @@ -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])
Expand All @@ -63,14 +63,37 @@
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"])
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

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
Expand Down
7 changes: 7 additions & 0 deletions spec/models/form_template_spec.rb
Expand Up @@ -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"]
Expand Down