diff --git a/app/controllers/bulk_upload_controller.rb b/app/controllers/bulk_upload_controller.rb index ba552cb49..28fe7bbae 100644 --- a/app/controllers/bulk_upload_controller.rb +++ b/app/controllers/bulk_upload_controller.rb @@ -1,4 +1,6 @@ class BulkUploadController < ApplicationController + before_action :authenticate_user! + def show @bulk_upload = BulkUpload.new(nil, nil) render "case_logs/bulk_upload" @@ -8,7 +10,7 @@ class BulkUploadController < ApplicationController file = upload_params.tempfile content_type = upload_params.content_type @bulk_upload = BulkUpload.new(file, content_type) - @bulk_upload.process + @bulk_upload.process(current_user) if @bulk_upload.errors.present? render "case_logs/bulk_upload", status: :unprocessable_entity else @@ -16,6 +18,8 @@ class BulkUploadController < ApplicationController end end +private + def upload_params params.require("bulk_upload")["case_log_bulk_upload"] end diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 069100299..77ccc3d6d 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -2,6 +2,7 @@ class CaseLogsController < ApplicationController skip_before_action :verify_authenticity_token, if: :json_api_request? before_action :authenticate, if: :json_api_request? before_action :authenticate_user!, unless: :json_api_request? + before_action :find_resource, except: %i[create index edit] def index @completed_case_logs = current_user.completed_case_logs @@ -23,11 +24,11 @@ class CaseLogsController < ApplicationController end def update - if (case_log = CaseLog.find_by(id: params[:id])) - if case_log.update(api_case_log_params) - render json: case_log, status: :ok + if @case_log + if @case_log.update(api_case_log_params) + render json: @case_log, status: :ok else - render json: { errors: case_log.errors.messages }, status: :unprocessable_entity + render json: { errors: @case_log.errors.messages }, status: :unprocessable_entity end else 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 format.html { edit } format.json do - if (case_log = CaseLog.find_by(id: params[:id])) - render json: case_log, status: :ok + if @case_log + render json: @case_log, status: :ok else render_not_found_json("Case log", params[:id]) end @@ -58,93 +59,22 @@ class CaseLogsController < ApplicationController 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 - if (case_log = CaseLog.find_by(id: params[:id])) - if case_log.discard + if @case_log + if @case_log.discard head :no_content else - render json: { errors: case_log.errors.messages }, status: :unprocessable_entity + render json: { errors: @case_log.errors.messages }, status: :unprocessable_entity end else render_not_found_json("Case log", params[:id]) 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 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? API_ACTIONS.include?(request["action"]) && request.format.json? end @@ -173,4 +103,8 @@ private params.require(:case_log).permit(CaseLog.editable_fields) end + + def find_resource + @case_log = CaseLog.find_by(id: params[:id]) + end end diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb new file mode 100644 index 000000000..93dc32e84 --- /dev/null +++ b/app/controllers/form_controller.rb @@ -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 diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index dcf1aa34a..a9081dd3f 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -1,6 +1,7 @@ class OrganisationsController < ApplicationController before_action :authenticate_user! - before_action :find_organisation + before_action :find_resource + before_action :authenticate_scope! def show redirect_to details_organisation_path(@organisation) @@ -20,7 +21,11 @@ class OrganisationsController < ApplicationController private - def find_organisation + def authenticate_scope! + head :not_found if current_user.organisation != @organisation + end + + def find_resource @organisation = Organisation.find(params[:id]) end end diff --git a/app/controllers/soft_validations_controller.rb b/app/controllers/soft_validations_controller.rb index 4f9881de6..0f0baf376 100644 --- a/app/controllers/soft_validations_controller.rb +++ b/app/controllers/soft_validations_controller.rb @@ -1,4 +1,6 @@ class SoftValidationsController < ApplicationController + before_action :authenticate_user! + def show @case_log = CaseLog.find(params[:case_log_id]) page_id = request.env["PATH_INFO"].split("/")[-2] diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 30afa501b..9ebdae068 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -2,12 +2,14 @@ class UsersController < ApplicationController include Devise::Controllers::SignInOut include Helpers::Email before_action :authenticate_user! + before_action :find_resource, except: %i[new create] + before_action :authenticate_scope!, except: %i[new create] def update - if current_user.update(user_params) - bypass_sign_in current_user + if @user.update(user_params) + bypass_sign_in @user flash[:notice] = I18n.t("devise.passwords.updated") - redirect_to user_path(current_user) + redirect_to user_path(@user) end end @@ -48,4 +50,12 @@ private def user_params params.require(:user).permit(:email, :name, :password, :role) end + + def find_resource + @user = User.find(params[:id]) + end + + def authenticate_scope! + head :not_found if current_user != @user + end end diff --git a/app/models/bulk_upload.rb b/app/models/bulk_upload.rb index 8bf0e2131..4eb558c10 100644 --- a/app/models/bulk_upload.rb +++ b/app/models/bulk_upload.rb @@ -15,7 +15,7 @@ class BulkUpload @content_type = content_type end - def process + def process(current_user) return unless valid_content_type? xlsx = Roo::Spreadsheet.open(@file, extension: :xlsx) @@ -30,8 +30,8 @@ class BulkUpload owning_organisation = Organisation.find(row[111]) managing_organisation = Organisation.find(row[113]) case_log = CaseLog.create!( - owning_organisation: owning_organisation, - managing_organisation: managing_organisation, + owning_organisation: current_user.organisation, + managing_organisation: current_user.organisation, ) map_row(row).each do |attr_key, attr_val| update = case_log.update(attr_key => attr_val) diff --git a/config/routes.rb b/config/routes.rb index 72e1ef208..0cc0065c0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -38,16 +38,16 @@ Rails.application.routes.draw do end member do - post "form", to: "case_logs#submit_form" + post "form", to: "form#submit_form" end 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? end 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 diff --git a/docs/adr/adr-012-controller-http-return-statuses.md b/docs/adr/adr-012-controller-http-return-statuses.md new file mode 100644 index 000000000..d28ee39d8 --- /dev/null +++ b/docs/adr/adr-012-controller-http-return-statuses.md @@ -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. diff --git a/spec/controllers/case_logs_controller_spec.rb b/spec/controllers/case_logs_controller_spec.rb deleted file mode 100644 index c2340d4ef..000000000 --- a/spec/controllers/case_logs_controller_spec.rb +++ /dev/null @@ -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 diff --git a/spec/requests/bulk_upload_controller_spec.rb b/spec/requests/bulk_upload_controller_spec.rb index 52233906c..e19247162 100644 --- a/spec/requests/bulk_upload_controller_spec.rb +++ b/spec/requests/bulk_upload_controller_spec.rb @@ -2,62 +2,89 @@ require "rails_helper" RSpec.describe BulkUploadController, type: :request do let(:url) { "/case-logs/bulk-upload" } - let(:organisation) { FactoryBot.create(:organisation) } + let(:user) { FactoryBot.create(:user) } + let(:organisation) { user.organisation } before do allow(Organisation).to receive(:find).with(107_242).and_return(organisation) end - describe "GET #show" do - before do - get url, params: {} + context "a not signed in user" do + describe "GET #show" do + it "does not let you see the bulk upload page" do + get url, headers: headers, params: {} + expect(response).to redirect_to("/users/sign-in") + end end - it "returns a success response" do - expect(response).to be_successful - end + describe "POST #bulk upload" do + before do + @file = fixture_file_upload("2021_22_lettings_bulk_upload.xlsx", "application/vnd.ms-excel") + end - it "returns a page with a file upload form" do - expect(response.body).to match(/ "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}", headers: headers, params: {} + end - 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) + it "does not show the tasklist for case logs you don't have access to" do + expect(response).to have_http_status(:not_found) + end end end end @@ -414,87 +393,4 @@ RSpec.describe CaseLogsController, type: :request do 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 diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb new file mode 100644 index 000000000..f88b1219b --- /dev/null +++ b/spec/requests/form_controller_spec.rb @@ -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 diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index be49b8c9d..d086452ac 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -2,110 +2,181 @@ require "rails_helper" RSpec.describe OrganisationsController, type: :request do let(:organisation) { user.organisation } + let(:unauthorised_organisation) { FactoryBot.create(:organisation) } let(:headers) { { "Accept" => "text/html" } } let(:page) { Capybara::Node::Simple.new(response.body) } + let(:user) { FactoryBot.create(:user, :data_coordinator) } - describe "#show" do - let(:user) { FactoryBot.create(:user, :data_coordinator) } - - before do - sign_in user - get "/organisations/#{organisation.id}", headers: headers, params: {} - end - - it "redirects to details" do - expect(response).to have_http_status(:redirect) - end - end - - context "As a data coordinator user" do - let(:user) { FactoryBot.create(:user, :data_coordinator) } - - context "details tab" do - before do - sign_in user - get "/organisations/#{organisation.id}/details", headers: headers, params: {} - end - - it "shows the tab navigation" do - expected_html = "