Browse Source
* Scope auth for org pages * Scope user methods * Refactor form UI controller * More tests for access * Consistently return not found for scoped auth * Add ADR * Authenticate bulk uploads * Authenticate soft validations controller * Scope bulk upload to user's org for now * Pass through current user * Update docs/adr/adr-012-controller-http-return-statuses.md Co-authored-by: Dushan <47317567+dushan-madetech@users.noreply.github.com> * Update spec/requests/case_log_controller_spec.rb Co-authored-by: Dushan <47317567+dushan-madetech@users.noreply.github.com> Co-authored-by: Dushan <47317567+dushan-madetech@users.noreply.github.com>pull/147/head
baarkerlounger
3 years ago
committed by
GitHub
16 changed files with 832 additions and 565 deletions
@ -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 |
||||||
@ -0,0 +1,15 @@ |
|||||||
|
### ADR - 012: Controller HTTP return statuses |
||||||
|
|
||||||
|
Controllers assess authentication by 3 criteria: |
||||||
|
|
||||||
|
1. Are you signed in at all? |
||||||
|
2. Are you signed in and requesting an action that your role/user type has access to? |
||||||
|
3. Are you signed in, requesting an action that your role/user type has access to and requesting a resource that your user has access to. |
||||||
|
|
||||||
|
When these aren't met they fail with the following response types: |
||||||
|
|
||||||
|
1. 401: Unauthorized. Redirect to sign-in page. |
||||||
|
2. 401: Unauthorized |
||||||
|
3. 404: Not found. |
||||||
|
|
||||||
|
This helps make it harder to determine whether a resource exists or not just by enumerating ids. |
||||||
@ -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 |
|
||||||
@ -0,0 +1,278 @@ |
|||||||
|
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" } } |
||||||
|
|
||||||
|
context "a not signed in user" do |
||||||
|
describe "GET" do |
||||||
|
it "does not let you get case logs pages you don't have access to" do |
||||||
|
get "/case-logs/#{case_log.id}/person-1-age", headers: headers, params: {} |
||||||
|
expect(response).to redirect_to("/users/sign-in") |
||||||
|
end |
||||||
|
|
||||||
|
it "does not let you get case log check answer pages you don't have access to" do |
||||||
|
get "/case-logs/#{case_log.id}/household-characteristics/check-answers", headers: headers, params: {} |
||||||
|
expect(response).to redirect_to("/users/sign-in") |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
describe "POST" do |
||||||
|
it "does not let you post form answers to case logs you don't have access to" do |
||||||
|
post "/case-logs/#{case_log.id}/form", params: {} |
||||||
|
expect(response).to redirect_to("/users/sign-in") |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "a signed in user" do |
||||||
|
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 |
||||||
|
end |
||||||
Loading…
Reference in new issue