From 3df0fe3f072e9811beb4d005dcb8815ed2dc9b75 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 2 Jun 2026 15:14:02 -0600 Subject: [PATCH 1/5] Prevent status change for rejected and closed account requests A rejected or closed account request is a final state that you shouldn't be able to change. For that reason, adding a model validation prevents that from happening. --- app/models/account_request.rb | 14 ++++++++++++++ spec/models/account_request_spec.rb | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/app/models/account_request.rb b/app/models/account_request.rb index bd48ee6956..75c0ef40c0 100644 --- a/app/models/account_request.rb +++ b/app/models/account_request.rb @@ -25,6 +25,8 @@ class AccountRequest < ApplicationRecord validate :email_not_already_used_by_organization validate :email_not_already_used_by_user + validate :cannot_change_status_once_rejected, + :cannot_change_status_once_closed, on: :update belongs_to :ndbn_member, class_name: 'NDBNMember', optional: true @@ -100,4 +102,16 @@ def email_not_already_used_by_user errors.add(:email, 'already used by an existing User') end end + + def cannot_change_status_once_rejected + if status_changed? && status_was == "rejected" + errors.add(:status, "cannot be changed once rejected") + end + end + + def cannot_change_status_once_closed + if status_changed? && status_was == "admin_closed" + errors.add(:status, "cannot be changed once closed by an admin") + end + end end diff --git a/spec/models/account_request_spec.rb b/spec/models/account_request_spec.rb index b03c5b834b..cf984183e9 100644 --- a/spec/models/account_request_spec.rb +++ b/spec/models/account_request_spec.rb @@ -74,6 +74,30 @@ end end + describe '#status' do + it "does not regress from rejected to another status" do + rejected_request = create(:account_request, status: 'rejected') + + expect { rejected_request.confirm! } + .to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once rejected/) + end + + it "does not regress from admin_closed to another status" do + rejected_request = create(:account_request, status: 'admin_closed') + + expect { rejected_request.confirm! } + .to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once closed by an admin/) + end + + it "allows normal transitions" do + started_request = create(:account_request, status: 'started') + user_confirmed_request = create(:account_request, status: 'user_confirmed') + + expect { started_request.confirm! }.not_to raise_error + expect { user_confirmed_request.reject!('rejectable request') }.not_to raise_error + end + end + describe '.get_by_identity_token' do subject { described_class.get_by_identity_token(identity_token) } From db8fefb7b76114e9d0a0fcafd9e837dbebbdef09 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 2 Jun 2026 15:35:34 -0600 Subject: [PATCH 2/5] Prevent status change for finalized audits A finalized audit is a final state that you shouldn't be able to change. For that reason, adding a model validation prevents that from happening from the UI and from other interactions. --- app/models/audit.rb | 7 +++++++ spec/models/audit_spec.rb | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/app/models/audit.rb b/app/models/audit.rb index 5a669623ca..b387473663 100644 --- a/app/models/audit.rb +++ b/app/models/audit.rb @@ -32,6 +32,7 @@ class Audit < ApplicationRecord validate :line_items_quantity_is_not_negative validate :line_items_unique_by_item_id validate :user_is_organization_admin_of_the_organization + validate :cannot_change_status_once_finalized, on: :update def self.finalized_since?(itemizable, *location_ids) item_ids = itemizable.line_items.pluck(:item_id) @@ -86,4 +87,10 @@ def line_items_unique_by_item_id def line_items_quantity_is_not_negative line_items_quantity_is_at_least(0) end + + def cannot_change_status_once_finalized + if status_changed? && status_was == "finalized" + errors.add(:status, "cannot be changed once finalized") + end + end end diff --git a/spec/models/audit_spec.rb b/spec/models/audit_spec.rb index 10996b97fd..9da2621ab1 100644 --- a/spec/models/audit_spec.rb +++ b/spec/models/audit_spec.rb @@ -92,6 +92,23 @@ expect(audit.save).to be_truthy end + + describe '#status' do + it "does not regress from finalized to another status" do + finalized_audit = create(:audit, organization:, status: :finalized) + + expect { finalized_audit.update!(status: :confirmed) } + .to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once finalized/) + end + + it "allows normal transitions" do + in_progress_audit = create(:audit, organization:, status: :in_progress) + confirmed_audit = create(:audit, organization:, status: :confirmed) + + expect { in_progress_audit.update!(status: :confirmed) }.not_to raise_error + expect { confirmed_audit.update!(status: :finalized) }.not_to raise_error + end + end end context "Scopes >" do From 43fbcc448a8f6c14d824652fcf939ac2067c0982 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Mon, 22 Jun 2026 16:14:36 -0600 Subject: [PATCH 3/5] Prevent a finalized audit from being edited While the validation added prevented a finalized audit to have its status changed, QA showed that it was possible to confirm an edited audit that has already been finalized. With this change, we prevent any edits from being made to a finalized audit and display a message for the user to make the experience better. --- app/controllers/audits_controller.rb | 5 ++++ spec/requests/audits_requests_spec.rb | 34 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/app/controllers/audits_controller.rb b/app/controllers/audits_controller.rb index 7d08c6949a..19aef74fcd 100644 --- a/app/controllers/audits_controller.rb +++ b/app/controllers/audits_controller.rb @@ -36,6 +36,11 @@ def finalize end def update + if @audit.reload.finalized? + redirect_to audit_path(@audit), error: "This audit has been finalized and cannot be edited." + return + end + @audit.line_items.destroy_all if @audit.update(audit_params) save_audit_status_and_redirect(params) diff --git a/spec/requests/audits_requests_spec.rb b/spec/requests/audits_requests_spec.rb index eb22bdc857..0e4d7d3980 100644 --- a/spec/requests/audits_requests_spec.rb +++ b/spec/requests/audits_requests_spec.rb @@ -109,6 +109,40 @@ end end + describe "PUT #update" do + it "confirms the updated audit and redirects to the audit" do + audit = create(:audit, organization: organization, status: :in_progress) + item = create(:item) + audit.line_items << create(:line_item, quantity: 3, item: item) + + put audit_path(id: audit.to_param, audit: { + storage_location_id: storage_location.id, + line_items_attributes: {"0" => {"item_id" => item.id, "quantity" => "4"}} + }) + + expect(response).to redirect_to(audit_path(audit)) + expect(flash[:notice]).to include("Audit is confirmed.") + expect(audit.reload).to be_confirmed + end + + context "when the audit has already been finalized" do + it "does not allow updates and redirects to the finalized audit" do + finalized_audit = create(:audit, organization: organization, status: :finalized) + item = create(:item) + finalized_audit.line_items << create(:line_item, quantity: 3, item: item) + + put audit_path(id: finalized_audit.to_param, audit: { + storage_location_id: storage_location.id, + line_items_attributes: {"0" => {"item_id" => item.id, "quantity" => "4"}} + }) + + expect(response).to redirect_to(audit_path(finalized_audit)) + expect(flash[:error]).to include("This audit has been finalized and cannot be edited.") + expect(finalized_audit.line_items.first.quantity).to eq(3) + end + end + end + describe "POST #create" do context "with valid params" do it "creates a new Audit" do From 1405fe0031f4e71f756e199fe250bbf86697e7c2 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Mon, 22 Jun 2026 16:16:41 -0600 Subject: [PATCH 4/5] Minor request specs improvements - reuse an existing record instead of creating duplicate ones - assert that the correct flash notice is displayed when saving an audit --- spec/requests/audits_requests_spec.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/spec/requests/audits_requests_spec.rb b/spec/requests/audits_requests_spec.rb index 0e4d7d3980..458494392d 100644 --- a/spec/requests/audits_requests_spec.rb +++ b/spec/requests/audits_requests_spec.rb @@ -6,7 +6,7 @@ { organization_id: organization.id, storage_location_id: storage_location.id, - user_id: create(:organization_admin, organization: organization).id + user_id: organization_admin.id } end @@ -14,7 +14,7 @@ { organization_id: organization.id, storage_location_id: nil, - user_id: create(:organization_admin, organization: organization).id + user_id: organization_admin.id } end @@ -22,8 +22,6 @@ { organization_id: nil } end - let(:valid_session) { {} } - describe "while signed in as an organization admin" do before do sign_in(organization_admin) @@ -155,6 +153,7 @@ expect do post audits_path(audit: valid_attributes, save_progress: '') expect(Audit.last.in_progress?).to be_truthy + expect(flash[:notice]).to include("Audit's progress was successfully saved.") end.to change(Audit.in_progress, :count).by(1) end From 865cf14d436c6d38617933cf92bb9d0e1578cfa0 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 23 Jun 2026 16:31:04 -0600 Subject: [PATCH 5/5] Ensure a finalized audit can not be edited While the validation added prevented a finalized audit to have its status changed, QA showed that it was possible to edit and audit that has already been finalized. With this change, we prevent any edits from being made to a finalized audit and display a message for the user to make the experience better. It also refactors the check to a before_action block to be reused on any other actions for a finalized audit and to keep the controller DRY. --- app/controllers/audits_controller.rb | 12 +++++++----- spec/requests/audits_requests_spec.rb | 13 +++++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/app/controllers/audits_controller.rb b/app/controllers/audits_controller.rb index 19aef74fcd..de69df0224 100644 --- a/app/controllers/audits_controller.rb +++ b/app/controllers/audits_controller.rb @@ -2,6 +2,7 @@ class AuditsController < ApplicationController before_action :authorize_admin before_action :set_audit, only: %i(show edit update destroy finalize) + before_action :ensure_audit_is_editable, only: %i(finalize update) def index @selected_location = filter_params[:at_location] @@ -36,11 +37,6 @@ def finalize end def update - if @audit.reload.finalized? - redirect_to audit_path(@audit), error: "This audit has been finalized and cannot be edited." - return - end - @audit.line_items.destroy_all if @audit.update(audit_params) save_audit_status_and_redirect(params) @@ -85,6 +81,12 @@ def destroy private + def ensure_audit_is_editable + if @audit.reload.finalized? + redirect_to audit_path(@audit), error: "This audit has been finalized and cannot be edited." + end + end + def handle_audit_errors error_message = @audit.errors.uniq(&:attribute).map do |error| attr = (error.attribute.to_s == 'base') ? '' : error.attribute.capitalize diff --git a/spec/requests/audits_requests_spec.rb b/spec/requests/audits_requests_spec.rb index 458494392d..079d0c00df 100644 --- a/spec/requests/audits_requests_spec.rb +++ b/spec/requests/audits_requests_spec.rb @@ -203,6 +203,19 @@ expect(audit.reload).to be_finalized expect(AuditEvent.count).to eq(1) end + + context "when the audit has already been finalized" do + it "does not create a new AuditEvent and redirects to the finalized audit" do + finalized_audit = create(:audit, organization: organization, status: :finalized) + + expect do + post audit_finalize_path(audit_id: finalized_audit.to_param) + end.not_to change(AuditEvent, :count) + + expect(response).to redirect_to(audit_path(finalized_audit)) + expect(flash[:error]).to include("This audit has been finalized and cannot be edited.") + end + end end describe "DELETE #destroy" do