baarkerlounger
/
submit-social-housing-lettings-and-sales-data
mirror of https://github.com/communitiesuk/submit-social-housing-lettings-and-sales-data.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Refactor form UI controller

pull/143/head
baarkerlounger 4 years ago
parent
5a7d248309
commit
d6a40e2d9f
7 changed files with 357 additions and 370 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 94
      app/controllers/case_logs_controller.rb
  2. 83
      app/controllers/form_controller.rb
  3. 4
      app/controllers/users_controller.rb
  4. 6
      config/routes.rb
  5. 172
      spec/controllers/case_logs_controller_spec.rb
  6. 113
      spec/requests/case_log_controller_spec.rb
  7. 255
      spec/requests/form_controller_spec.rb

94
app/controllers/case_logs_controller.rb
Unescape View File

@ -2,6 +2,7 @@ class CaseLogsController < ApplicationController
skip_before_action :verify_authenticity_token, if: :json_api_request? skip_before_action :verify_authenticity_token, if: :json_api_request?
before_action :authenticate, if: :json_api_request? before_action :authenticate, if: :json_api_request?
before_action :authenticate_user!, unless: :json_api_request? before_action :authenticate_user!, unless: :json_api_request?
before_action :find_resource, except: %i[create index edit]
def index def index
@completed_case_logs = current_user.completed_case_logs @completed_case_logs = current_user.completed_case_logs
@ -23,11 +24,11 @@ class CaseLogsController < ApplicationController
end end
def update def update
if (case_log = CaseLog.find_by(id: params[:id])) if @case_log
if case_log.update(api_case_log_params) if @case_log.update(api_case_log_params)
render json: case_log, status: :ok render json: @case_log, status: :ok
else else
render json: { errors: case_log.errors.messages }, status: :unprocessable_entity render json: { errors: @case_log.errors.messages }, status: :unprocessable_entity
end end
else else
render_not_found_json("Case log", params[:id]) render_not_found_json("Case log", params[:id])
@ -39,8 +40,8 @@ class CaseLogsController < ApplicationController
# We don't have a dedicated non-editable show view # We don't have a dedicated non-editable show view
format.html { edit } format.html { edit }
format.json do format.json do
if (case_log = CaseLog.find_by(id: params[:id])) if @case_log
render json: case_log, status: :ok render json: @case_log, status: :ok
else else
render_not_found_json("Case log", params[:id]) render_not_found_json("Case log", params[:id])
end end
@ -58,93 +59,22 @@ class CaseLogsController < ApplicationController
end end
end end
def submit_form
form = FormHandler.instance.get_form("2021_2022")
@case_log = current_user.case_logs.find_by(id: params[:id])
if @case_log
page = form.get_page(params[:case_log][:page])
responses_for_page = responses_for_page(page)
if @case_log.update(responses_for_page) && @case_log.has_no_unresolved_soft_errors?
redirect_path = form.next_page_redirect_path(page, @case_log)
redirect_to(send(redirect_path, @case_log))
else
subsection = form.subsection_for_page(page)
render "form/page", locals: { form: form, page: page, subsection: subsection.label }, status: :unprocessable_entity
end
else
render_not_found_html
end
end
def destroy def destroy
if (case_log = CaseLog.find_by(id: params[:id])) if @case_log
if case_log.discard if @case_log.discard
head :no_content head :no_content
else else
render json: { errors: case_log.errors.messages }, status: :unprocessable_entity render json: { errors: @case_log.errors.messages }, status: :unprocessable_entity
end end
else else
render_not_found_json("Case log", params[:id]) render_not_found_json("Case log", params[:id])
end end
end end
def check_answers
form = FormHandler.instance.get_form("2021_2022")
@case_log = current_user.case_logs.find_by(id: params[:case_log_id])
if @case_log
current_url = request.env["PATH_INFO"]
subsection = form.get_subsection(current_url.split("/")[-2])
render "form/check_answers", locals: { subsection: subsection, form: form }
else
render_not_found_html
end
end
form = FormHandler.instance.get_form("2021_2022")
form.pages.map do |page|
define_method(page.id) do |_errors = {}|
@case_log = current_user.case_logs.find_by(id: params[:case_log_id])
if @case_log
subsection = form.subsection_for_page(page)
render "form/page", locals: { form: form, page: page, subsection: subsection.label }
else
render_not_found_html
end
end
end
private private
API_ACTIONS = %w[create show update destroy].freeze API_ACTIONS = %w[create show update destroy].freeze
def responses_for_page(page)
page.expected_responses.each_with_object({}) do |question, result|
question_params = params["case_log"][question.id]
if question.type == "date"
day = params["case_log"]["#{question.id}(3i)"]
month = params["case_log"]["#{question.id}(2i)"]
year = params["case_log"]["#{question.id}(1i)"]
next unless [day, month, year].any?(&:present?)
result[question.id] = if day.to_i.between?(1, 31) && month.to_i.between?(1, 12) && year.to_i.between?(2000, 2200)
Date.new(year.to_i, month.to_i, day.to_i)
else
Date.new(0, 1, 1)
end
end
next unless question_params
if %w[checkbox validation_override].include?(question.type)
question.answer_options.keys.reject { |x| x.match(/divider/) }.each do |option|
result[option] = question_params.include?(option) ? 1 : 0
end
else
result[question.id] = question_params
end
result
end
end
def json_api_request? def json_api_request?
API_ACTIONS.include?(request["action"]) && request.format.json? API_ACTIONS.include?(request["action"]) && request.format.json?
end end
@ -173,4 +103,8 @@ private
params.require(:case_log).permit(CaseLog.editable_fields) params.require(:case_log).permit(CaseLog.editable_fields)
end end
def find_resource
@case_log = CaseLog.find_by(id: params[:id])
end
end end

83
app/controllers/form_controller.rb
Unescape View File

@ -0,0 +1,83 @@
class FormController < ApplicationController
before_action :authenticate_user!
before_action :find_resource, only: [:submit_form]
before_action :find_resource_by_named_id, except: [:submit_form]
def submit_form
form = FormHandler.instance.get_form("2021_2022")
if @case_log
page = form.get_page(params[:case_log][:page])
responses_for_page = responses_for_page(page)
if @case_log.update(responses_for_page) && @case_log.has_no_unresolved_soft_errors?
redirect_path = form.next_page_redirect_path(page, @case_log)
redirect_to(send(redirect_path, @case_log))
else
subsection = form.subsection_for_page(page)
render "form/page", locals: { form: form, page: page, subsection: subsection.label }, status: :unprocessable_entity
end
else
render_not_found_html
end
end
def check_answers
form = FormHandler.instance.get_form("2021_2022")
if @case_log
current_url = request.env["PATH_INFO"]
subsection = form.get_subsection(current_url.split("/")[-2])
render "form/check_answers", locals: { subsection: subsection, form: form }
else
render_not_found_html
end
end
form = FormHandler.instance.get_form("2021_2022")
form.pages.map do |page|
define_method(page.id) do |_errors = {}|
if @case_log
subsection = form.subsection_for_page(page)
render "form/page", locals: { form: form, page: page, subsection: subsection.label }
else
render_not_found_html
end
end
end
private
def responses_for_page(page)
page.expected_responses.each_with_object({}) do |question, result|
question_params = params["case_log"][question.id]
if question.type == "date"
day = params["case_log"]["#{question.id}(3i)"]
month = params["case_log"]["#{question.id}(2i)"]
year = params["case_log"]["#{question.id}(1i)"]
next unless [day, month, year].any?(&:present?)
result[question.id] = if day.to_i.between?(1, 31) && month.to_i.between?(1, 12) && year.to_i.between?(2000, 2200)
Date.new(year.to_i, month.to_i, day.to_i)
else
Date.new(0, 1, 1)
end
end
next unless question_params
if %w[checkbox validation_override].include?(question.type)
question.answer_options.keys.reject { |x| x.match(/divider/) }.each do |option|
result[option] = question_params.include?(option) ? 1 : 0
end
else
result[question.id] = question_params
end
result
end
end
def find_resource
@case_log = current_user.case_logs.find_by(id: params[:id])
end
def find_resource_by_named_id
@case_log = current_user.case_logs.find_by(id: params[:case_log_id])
end
end

4
app/controllers/users_controller.rb
Unescape View File

@ -2,8 +2,8 @@ class UsersController < ApplicationController
include Devise::Controllers::SignInOut include Devise::Controllers::SignInOut
include Helpers::Email include Helpers::Email
before_action :authenticate_user! before_action :authenticate_user!
before_action :find_resource, except: [:new, :create] before_action :find_resource, except: %i[new create]
before_action :authenticate_scope!, except: [:new, :create] before_action :authenticate_scope!, except: %i[new create]
def update def update
if @user.update(user_params) if @user.update(user_params)

6
config/routes.rb
Unescape View File

@ -38,16 +38,16 @@ Rails.application.routes.draw do
end end
member do member do
post "form", to: "case_logs#submit_form" post "form", to: "form#submit_form"
end end
form.pages.map do |page| form.pages.map do |page|
get page.id.to_s.dasherize, to: "case_logs##{page.id}" get page.id.to_s.dasherize, to: "form##{page.id}"
get "#{page.id.to_s.dasherize}/soft-validations", to: "soft_validations#show" if page.has_soft_validations? get "#{page.id.to_s.dasherize}/soft-validations", to: "soft_validations#show" if page.has_soft_validations?
end end
form.subsections.map do |subsection| form.subsections.map do |subsection|
get "#{subsection.id.to_s.dasherize}/check-answers", to: "case_logs#check_answers" get "#{subsection.id.to_s.dasherize}/check-answers", to: "form#check_answers"
end end
end end
end end

172
spec/controllers/case_logs_controller_spec.rb
Unescape View File

@ -1,172 +0,0 @@
require "rails_helper"
RSpec.describe CaseLogsController, type: :controller do
let(:valid_session) { {} }
let(:user) { FactoryBot.create(:user) }
let(:case_log) do
FactoryBot.create(
:case_log,
owning_organisation: user.organisation,
managing_organisation: user.organisation,
)
end
let(:id) { case_log.id }
before do
sign_in user
end
context "Collection routes" do
describe "GET #index" do
it "returns a success response" do
get :index, params: {}, session: valid_session
expect(response).to be_successful
end
end
describe "Post #create" do
let(:owning_organisation) { FactoryBot.create(:organisation) }
let(:managing_organisation) { owning_organisation }
let(:params) do
{
"owning_organisation_id": owning_organisation.id,
"managing_organisation_id": managing_organisation.id,
}
end
it "creates a new case log record" do
expect {
post :create, params: params, session: valid_session
}.to change(CaseLog, :count).by(1)
end
it "redirects to that case log" do
post :create, params: params, session: valid_session
expect(response.status).to eq(302)
end
end
end
context "Instance routes" do
describe "GET #show" do
it "returns a success response" do
get :show, params: { id: id }
expect(response).to be_successful
end
end
describe "GET #edit" do
it "returns a success response" do
get :edit, params: { id: id }
expect(response).to be_successful
end
end
end
describe "submit_form" do
let(:case_log_form_params) do
{ accessibility_requirements:
%w[ housingneeds_a
housingneeds_b
housingneeds_c],
page: "accessibility_requirements" }
end
let(:new_case_log_form_params) do
{
accessibility_requirements: %w[housingneeds_c],
page: "accessibility_requirements",
}
end
it "sets checked items to true" do
post :submit_form, params: { id: id, case_log: case_log_form_params }
case_log.reload
expect(case_log.housingneeds_a).to eq("Yes")
expect(case_log.housingneeds_b).to eq("Yes")
expect(case_log.housingneeds_c).to eq("Yes")
end
it "sets previously submitted items to false when resubmitted with new values" do
post :submit_form, params: { id: id, case_log: new_case_log_form_params }
case_log.reload
expect(case_log.housingneeds_a).to eq("No")
expect(case_log.housingneeds_b).to eq("No")
expect(case_log.housingneeds_c).to eq("Yes")
end
context "given a page with checkbox and non-checkbox questions" do
let(:tenant_code) { "BZ355" }
let(:case_log_form_params) do
{ accessibility_requirements:
%w[ housingneeds_a
housingneeds_b
housingneeds_c],
tenant_code: tenant_code,
page: "accessibility_requirements" }
end
let(:questions_for_page) do
[
Form::Question.new(
"accessibility_requirements",
{
"type" => "checkbox",
"answer_options" =>
{ "housingneeds_a" => "Fully wheelchair accessible housing",
"housingneeds_b" => "Wheelchair access to essential rooms",
"housingneeds_c" => "Level access housing",
"housingneeds_f" => "Other disability requirements",
"housingneeds_g" => "No disability requirements",
"divider_a" => true,
"housingneeds_h" => "Do not know",
"divider_b" => true,
"accessibility_requirements_prefer_not_to_say" => "Prefer not to say" },
}, nil
),
Form::Question.new("tenant_code", { "type" => "text" }, nil),
]
end
it "updates both question fields" do
allow_any_instance_of(Form::Page).to receive(:expected_responses).and_return(questions_for_page)
post :submit_form, params: { id: id, case_log: case_log_form_params }
case_log.reload
expect(case_log.housingneeds_a).to eq("Yes")
expect(case_log.housingneeds_b).to eq("Yes")
expect(case_log.housingneeds_c).to eq("Yes")
expect(case_log.tenant_code).to eq(tenant_code)
end
end
context "conditional routing" do
before do
allow_any_instance_of(CaseLogValidator).to receive(:validate_pregnancy).and_return(true)
end
let(:case_log_form_conditional_question_yes_params) do
{
preg_occ: "Yes",
page: "conditional_question",
}
end
let(:case_log_form_conditional_question_no_params) do
{
preg_occ: "No",
page: "conditional_question",
}
end
it "routes to the appropriate conditional page based on the question answer of the current page" do
post :submit_form, params: { id: id, case_log: case_log_form_conditional_question_yes_params }
expect(response).to redirect_to("/case-logs/#{id}/conditional-question-yes-page")
post :submit_form, params: { id: id, case_log: case_log_form_conditional_question_no_params }
expect(response).to redirect_to("/case-logs/#{id}/conditional-question-no-page")
end
end
end
end

113
spec/requests/case_log_controller_spec.rb
Unescape View File

@ -231,36 +231,6 @@ RSpec.describe CaseLogsController, type: :request do
end end
end end
end end
context "form pages" do
let(:headers) { { "Accept" => "text/html" } }
context "case logs that are not owned or managed by your organisation" do
before do
sign_in user
get "/case-logs/#{unauthorized_case_log.id}/person-1-age", headers: headers, params: {}
end
it "does not show form pages for case logs you don't have access to" do
expect(response).to have_http_status(:not_found)
end
end
end
context "check answers pages" do
let(:headers) { { "Accept" => "text/html" } }
context "case logs that are not owned or managed by your organisation" do
before do
sign_in user
get "/case-logs/#{unauthorized_case_log.id}/household-characteristics/check-answers", headers: headers, params: {}
end
it "does not show a check answers for case logs you don't have access to" do
expect(response).to have_http_status(:not_found)
end
end
end
end end
end end
@ -414,87 +384,4 @@ RSpec.describe CaseLogsController, type: :request do
end end
end end
end end
describe "Submit Form" do
let(:user) { FactoryBot.create(:user) }
let(:form) { Form.new("spec/fixtures/forms/test_form.json") }
let(:organisation) { user.organisation }
let(:case_log) do
FactoryBot.create(
:case_log,
owning_organisation: organisation,
managing_organisation: organisation,
)
end
let(:page_id) { "person_1_age" }
let(:params) do
{
id: case_log.id,
case_log: {
page: page_id,
age1: answer,
},
}
end
before do
allow(FormHandler.instance).to receive(:get_form).and_return(form)
sign_in user
post "/case-logs/#{case_log.id}/form", params: params
end
context "invalid answers" do
let(:answer) { 2000 }
it "re-renders the same page with errors if validation fails" do
expect(response).to have_http_status(:unprocessable_entity)
end
end
context "valid answers" do
let(:answer) { 20 }
it "re-renders the same page with errors if validation fails" do
expect(response).to have_http_status(:redirect)
end
let(:params) do
{
id: case_log.id,
case_log: {
page: page_id,
age1: answer,
age2: 2000,
},
}
end
it "only updates answers that apply to the page being submitted" do
case_log.reload
expect(case_log.age1).to eq(answer)
expect(case_log.age2).to be nil
end
end
context "case logs that are not owned or managed by your organisation" do
let(:answer) { 25 }
let(:other_organisation) { FactoryBot.create(:organisation) }
let(:unauthorized_case_log) do
FactoryBot.create(
:case_log,
owning_organisation: other_organisation,
managing_organisation: other_organisation,
)
end
before do
sign_in user
post "/case-logs/#{unauthorized_case_log.id}/form", params: params
end
it "does not let you post form answers to case logs you don't have access to" do
expect(response).to have_http_status(:not_found)
end
end
end
end end

255
spec/requests/form_controller_spec.rb
Unescape View File

@ -0,0 +1,255 @@
require "rails_helper"
RSpec.describe FormController, type: :request do
let(:user) { FactoryBot.create(:user) }
let(:organisation) { user.organisation }
let(:other_organisation) { FactoryBot.create(:organisation) }
let!(:case_log) do
FactoryBot.create(
:case_log,
owning_organisation: organisation,
managing_organisation: organisation,
)
end
let!(:unauthorized_case_log) do
FactoryBot.create(
:case_log,
owning_organisation: other_organisation,
managing_organisation: other_organisation,
)
end
let(:headers) { { "Accept" => "text/html" } }
before do
sign_in user
end
describe "GET" do
context "form pages" do
context "case logs that are not owned or managed by your organisation" do
it "does not show form pages for case logs you don't have access to" do
get "/case-logs/#{unauthorized_case_log.id}/person-1-age", headers: headers, params: {}
expect(response).to have_http_status(:not_found)
end
end
end
context "check answers pages" do
context "case logs that are not owned or managed by your organisation" do
it "does not show a check answers for case logs you don't have access to" do
get "/case-logs/#{unauthorized_case_log.id}/household-characteristics/check-answers", headers: headers, params: {}
expect(response).to have_http_status(:not_found)
end
end
end
end
describe "Submit Form" do
context "a form page" do
let(:user) { FactoryBot.create(:user) }
let(:form) { Form.new("spec/fixtures/forms/test_form.json") }
let(:organisation) { user.organisation }
let(:case_log) do
FactoryBot.create(
:case_log,
owning_organisation: organisation,
managing_organisation: organisation,
)
end
let(:page_id) { "person_1_age" }
let(:params) do
{
id: case_log.id,
case_log: {
page: page_id,
age1: answer,
},
}
end
before do
allow(FormHandler.instance).to receive(:get_form).and_return(form)
post "/case-logs/#{case_log.id}/form", params: params
end
context "invalid answers" do
let(:answer) { 2000 }
it "re-renders the same page with errors if validation fails" do
expect(response).to have_http_status(:unprocessable_entity)
end
end
context "valid answers" do
let(:answer) { 20 }
it "re-renders the same page with errors if validation fails" do
expect(response).to have_http_status(:redirect)
end
let(:params) do
{
id: case_log.id,
case_log: {
page: page_id,
age1: answer,
age2: 2000,
},
}
end
it "only updates answers that apply to the page being submitted" do
case_log.reload
expect(case_log.age1).to eq(answer)
expect(case_log.age2).to be nil
end
end
end
context "checkbox questions" do
let(:case_log_form_params) do
{
id: case_log.id,
case_log: {
page: "accessibility_requirements",
accessibility_requirements:
%w[ housingneeds_a
housingneeds_b
housingneeds_c],
},
}
end
let(:new_case_log_form_params) do
{
id: case_log.id,
case_log: {
page: "accessibility_requirements",
accessibility_requirements: %w[housingneeds_c],
},
}
end
it "sets checked items to true" do
post "/case-logs/#{case_log.id}/form", params: case_log_form_params
case_log.reload
expect(case_log.housingneeds_a).to eq("Yes")
expect(case_log.housingneeds_b).to eq("Yes")
expect(case_log.housingneeds_c).to eq("Yes")
end
it "sets previously submitted items to false when resubmitted with new values" do
post "/case-logs/#{case_log.id}/form", params: new_case_log_form_params
case_log.reload
expect(case_log.housingneeds_a).to eq("No")
expect(case_log.housingneeds_b).to eq("No")
expect(case_log.housingneeds_c).to eq("Yes")
end
context "given a page with checkbox and non-checkbox questions" do
let(:tenant_code) { "BZ355" }
let(:case_log_form_params) do
{
id: case_log.id,
case_log: {
page: "accessibility_requirements",
accessibility_requirements:
%w[ housingneeds_a
housingneeds_b
housingneeds_c],
tenant_code: tenant_code,
},
}
end
let(:questions_for_page) do
[
Form::Question.new(
"accessibility_requirements",
{
"type" => "checkbox",
"answer_options" =>
{ "housingneeds_a" => "Fully wheelchair accessible housing",
"housingneeds_b" => "Wheelchair access to essential rooms",
"housingneeds_c" => "Level access housing",
"housingneeds_f" => "Other disability requirements",
"housingneeds_g" => "No disability requirements",
"divider_a" => true,
"housingneeds_h" => "Do not know",
"divider_b" => true,
"accessibility_requirements_prefer_not_to_say" => "Prefer not to say" },
}, nil
),
Form::Question.new("tenant_code", { "type" => "text" }, nil),
]
end
it "updates both question fields" do
allow_any_instance_of(Form::Page).to receive(:expected_responses).and_return(questions_for_page)
post "/case-logs/#{case_log.id}/form", params: case_log_form_params
case_log.reload
expect(case_log.housingneeds_a).to eq("Yes")
expect(case_log.housingneeds_b).to eq("Yes")
expect(case_log.housingneeds_c).to eq("Yes")
expect(case_log.tenant_code).to eq(tenant_code)
end
end
end
context "conditional routing" do
before do
allow_any_instance_of(CaseLogValidator).to receive(:validate_pregnancy).and_return(true)
end
let(:case_log_form_conditional_question_yes_params) do
{
id: case_log.id,
case_log: {
page: "conditional_question",
preg_occ: "Yes",
},
}
end
let(:case_log_form_conditional_question_no_params) do
{
id: case_log.id,
case_log: {
page: "conditional_question",
preg_occ: "No",
},
}
end
it "routes to the appropriate conditional page based on the question answer of the current page" do
post "/case-logs/#{case_log.id}/form", params: case_log_form_conditional_question_yes_params
expect(response).to redirect_to("/case-logs/#{case_log.id}/conditional-question-yes-page")
post "/case-logs/#{case_log.id}/form", params: case_log_form_conditional_question_no_params
expect(response).to redirect_to("/case-logs/#{case_log.id}/conditional-question-no-page")
end
end
context "case logs that are not owned or managed by your organisation" do
let(:answer) { 25 }
let(:other_organisation) { FactoryBot.create(:organisation) }
let(:unauthorized_case_log) do
FactoryBot.create(
:case_log,
owning_organisation: other_organisation,
managing_organisation: other_organisation,
)
end
before do
post "/case-logs/#{unauthorized_case_log.id}/form", params: {}
end
it "does not let you post form answers to case logs you don't have access to" do
expect(response).to have_http_status(:not_found)
end
end
end
end
Write Preview
Loading…
Cancel
Save