From ee40e92ee777e1baf0e0cbdd078c17c8365d7c02 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Tue, 8 Feb 2022 16:51:22 +0000 Subject: [PATCH] Track who created or changed records via UI --- Gemfile | 2 + Gemfile.lock | 4 + app/controllers/application_controller.rb | 8 + spec/requests/case_logs_controller_spec.rb | 140 ++++++++++-------- spec/requests/form_controller_spec.rb | 7 + .../requests/organisations_controller_spec.rb | 7 + spec/requests/users_controller_spec.rb | 7 + 7 files changed, 115 insertions(+), 60 deletions(-) diff --git a/Gemfile b/Gemfile index 590a0a28c..45832489b 100644 --- a/Gemfile +++ b/Gemfile @@ -47,6 +47,8 @@ gem "view_component" gem "aws-sdk-s3" # Track changes to models for auditing or versioning. gem "paper_trail" +# Store active record objects in version whodunnits +gem "paper_trail-globalid" group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console diff --git a/Gemfile.lock b/Gemfile.lock index 59ca0abf3..937bb3f80 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -267,6 +267,9 @@ GEM paper_trail (12.2.0) activerecord (>= 5.2) request_store (~> 1.1) + paper_trail-globalid (0.2.0) + globalid + paper_trail (>= 3.0.0) parallel (1.21.0) parser (3.1.0.0) ast (~> 2.4.1) @@ -469,6 +472,7 @@ DEPENDENCIES notifications-ruby-client overcommit (>= 0.37.0) paper_trail + paper_trail-globalid pg (~> 1.1) postcodes_io pry-byebug diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 776259fab..78b367d80 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,4 +1,6 @@ class ApplicationController < ActionController::Base + before_action :set_paper_trail_whodunnit + def render_not_found render "errors/not_found", status: :not_found end @@ -6,4 +8,10 @@ class ApplicationController < ActionController::Base def render_not_found_json(class_name, id) render json: { error: "#{class_name} #{id} not found" }, status: :not_found end + +protected + + def user_for_paper_trail + current_user + end end diff --git a/spec/requests/case_logs_controller_spec.rb b/spec/requests/case_logs_controller_spec.rb index de33846cf..cf6cd64c9 100644 --- a/spec/requests/case_logs_controller_spec.rb +++ b/spec/requests/case_logs_controller_spec.rb @@ -34,84 +34,104 @@ RSpec.describe CaseLogsController, type: :request do let(:in_progress) { "in_progress" } let(:completed) { "completed" } - let(:params) do - { - "owning_organisation_id": owning_organisation.id, - "managing_organisation_id": managing_organisation.id, - "tenant_code": tenant_code, - "age1": age1, - "property_postcode": property_postcode, - "offered": offered, - } - end - - before do - post "/logs", headers: headers, params: params.to_json - end - - it "returns http success" do - expect(response).to have_http_status(:success) - end - - it "returns a serialized Case Log" do - json_response = JSON.parse(response.body) - expect(json_response.keys).to match_array(CaseLog.new.attributes.keys) - end + context "when API" do + let(:params) do + { + "owning_organisation_id": owning_organisation.id, + "managing_organisation_id": managing_organisation.id, + "tenant_code": tenant_code, + "age1": age1, + "property_postcode": property_postcode, + "offered": offered, + } + end - it "creates a case log with the values passed" do - json_response = JSON.parse(response.body) - expect(json_response["tenant_code"]).to eq(tenant_code) - expect(json_response["age1"]).to eq(age1) - expect(json_response["property_postcode"]).to eq(property_postcode) - end + before do + post "/logs", headers: headers, params: params.to_json + end - context "with invalid json parameters" do - let(:age1) { 2000 } - let(:offered) { 21 } + it "returns http success" do + expect(response).to have_http_status(:success) + end - it "validates case log parameters" do + it "returns a serialized Case Log" do json_response = JSON.parse(response.body) - expect(response).to have_http_status(:unprocessable_entity) - expect(json_response["errors"]).to match_array([["offered", [I18n.t("validations.property.offered.relet_number")]], ["age1", [I18n.t("validations.household.age.must_be_valid", lower_bound: 16)]]]) + expect(json_response.keys).to match_array(CaseLog.new.attributes.keys) end - end - context "with a partial case log submission" do - it "marks the record as in_progress" do + it "creates a case log with the values passed" do json_response = JSON.parse(response.body) - expect(json_response["status"]).to eq(in_progress) + expect(json_response["tenant_code"]).to eq(tenant_code) + expect(json_response["age1"]).to eq(age1) + expect(json_response["property_postcode"]).to eq(property_postcode) end - end - context "with a complete case log submission" do - let(:org_params) do - { - "case_log" => { - "owning_organisation_id" => owning_organisation.id, - "managing_organisation_id" => managing_organisation.id, - }, - } + context "with invalid json parameters" do + let(:age1) { 2000 } + let(:offered) { 21 } + + it "validates case log parameters" do + json_response = JSON.parse(response.body) + expect(response).to have_http_status(:unprocessable_entity) + expect(json_response["errors"]).to match_array([["offered", [I18n.t("validations.property.offered.relet_number")]], ["age1", [I18n.t("validations.household.age.must_be_valid", lower_bound: 16)]]]) + end end - let(:case_log_params) { JSON.parse(File.open("spec/fixtures/complete_case_log.json").read) } - let(:params) do - case_log_params.merge(org_params) { |_k, a_val, b_val| a_val.merge(b_val) } + + context "with a partial case log submission" do + it "marks the record as in_progress" do + json_response = JSON.parse(response.body) + expect(json_response["status"]).to eq(in_progress) + end end - it "marks the record as completed" do - json_response = JSON.parse(response.body) + context "with a complete case log submission" do + let(:org_params) do + { + "case_log" => { + "owning_organisation_id" => owning_organisation.id, + "managing_organisation_id" => managing_organisation.id, + }, + } + end + let(:case_log_params) { JSON.parse(File.open("spec/fixtures/complete_case_log.json").read) } + let(:params) do + case_log_params.merge(org_params) { |_k, a_val, b_val| a_val.merge(b_val) } + end + + it "marks the record as completed" do + json_response = JSON.parse(response.body) + + expect(json_response).not_to have_key("errors") + expect(json_response["status"]).to eq(completed) + end + end + + context "with a request containing invalid credentials" do + let(:basic_credentials) do + ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops") + end - expect(json_response).not_to have_key("errors") - expect(json_response["status"]).to eq(completed) + it "returns 401" do + expect(response).to have_http_status(:unauthorized) + end end end - context "with a request containing invalid credentials" do - let(:basic_credentials) do - ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops") + context "when UI" do + let(:user) { FactoryBot.create(:user) } + let(:headers) { { "Accept" => "text/html" } } + + before do + RequestHelper.stub_http_requests + sign_in user + post "/logs", headers: headers end - it "returns 401" do - expect(response).to have_http_status(:unauthorized) + it "tracks who created the record" do + created_id = response.location.match(/[1-9]+/)[0] + whodunnit_actor = CaseLog.find_by(id: created_id).versions.last.actor + expect(whodunnit_actor).to be_a(User) + expect(whodunnit_actor.id).to eq(user.id) end end end diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index 3d77492b7..f4b49e129 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -155,6 +155,13 @@ RSpec.describe FormController, type: :request do expect(case_log.age1).to eq(answer) expect(case_log.age2).to be nil end + + it "tracks who updated the record" do + case_log.reload + whodunnit_actor = case_log.versions.last.actor + expect(whodunnit_actor).to be_a(User) + expect(whodunnit_actor.id).to eq(user.id) + end end end diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 24ca82101..b92c8da53 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -185,6 +185,13 @@ RSpec.describe OrganisationsController, type: :request do follow_redirect! expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") end + + it "tracks who updated the record" do + organisation.reload + whodunnit_actor = organisation.versions.last.actor + expect(whodunnit_actor).to be_a(User) + expect(whodunnit_actor.id).to eq(user.id) + end end context "with an organisation that the user does not belong to" do diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 59997f3ce..66cbeb5c2 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -196,6 +196,13 @@ RSpec.describe UsersController, type: :request do user.reload expect(user.name).to eq(new_value) end + + it "tracks who updated the record" do + user.reload + whodunnit_actor = user.versions.last.actor + expect(whodunnit_actor).to be_a(User) + expect(whodunnit_actor.id).to eq(user.id) + end end context "when the update fails to persist" do