From da79e051908f51b7245f54dcd226086bb0798d3a Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 28 Apr 2026 12:00:17 -0600 Subject: [PATCH 1/7] Fix typo --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6476dd4b70..7babb1147a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -198,7 +198,7 @@ Only take multiple issues if they are related and you can solve all of them at t Users that are frequent contributors and are involved in discussion (join the slack channel! :)) may be given direct Contributor access to the Repo so they can submit Pull Requests directly instead of Forking first. ## Debugging -If starting server directly, via `rail s` or `rail console`, or built-in debugger in RubyMine, or running `bundle exec rspec path/to/spec.rb:line_no`, then you can use `binding.pry` to debug. Drop the pry where you want the execution to pause. +If starting server directly, via `rails s` or `rails console`, or built-in debugger in RubyMine, or running `bundle exec rspec path/to/spec.rb:line_no`, then you can use `binding.pry` to debug. Drop the pry where you want the execution to pause. If starting via Procfile with `bin/start`, then drop a ``binding.remote_pry`` into the line where you want execution to pause at. Then run ``pry-remote`` in the terminal to connect to it. https://github.com/Mon-Ouie/pry-remote From a95fe2ef1817e55558eed3d0cbdaa995c2ed5f9c Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 28 Apr 2026 12:02:12 -0600 Subject: [PATCH 2/7] Add ability to see cancelled requests The current Requests view does not include cancelled requests. A bank requested that cancelled requests can be listed/exported in this view. When the 'Include Cancelled' field is checked, the view displays cancelled requests. When the field is checked, and the 'Filter by status' field is selected to 'Cancelled', the view lists only cancelled requests. The same is applied to exports. To be consistent with our current wording everywhere else, we are renaming the 'discarded' status to 'cancelled'. Since the DB status column is an integer, renaming it in the enum does not need any work prior to renaming it. --- app/controllers/requests_controller.rb | 6 + app/models/request.rb | 2 +- app/services/request_destroy_service.rb | 4 +- app/views/requests/_status.html.erb | 2 +- docs/user_guide/bank/essentials_requests.md | 2 +- docs/user_guide/bank/exports.md | 2 +- spec/factories/requests.rb | 5 +- spec/requests/partners/requests_spec.rb | 2 +- spec/requests/requests_requests_spec.rb | 15 +- spec/services/request_destroy_service_spec.rb | 8 +- spec/system/dashboard_system_spec.rb | 4 +- spec/system/request_system_spec.rb | 146 ++++++++++++++++-- 12 files changed, 169 insertions(+), 29 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index ce5677da0c..2315a67486 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -8,6 +8,11 @@ def index .undiscarded .during(helpers.selected_range) .class_filter(filter_params) + + if params[:include_cancelled] + @requests = @requests.with_discarded + end + @unfulfilled_requests_count = current_organization.requests.where(status: [:pending, :started]).during(helpers.selected_range).class_filter(filter_params).count @paginated_requests = @requests.includes(:partner).page(params[:page]) @calculate_product_totals = RequestsTotalItemsService.new(requests: @requests).calculate @@ -20,6 +25,7 @@ def index @selected_request_item = filter_params[:by_request_item_id] @selected_partner = filter_params[:by_partner] @selected_status = filter_params[:by_status] + @include_cancelled = params[:include_cancelled] respond_to do |format| format.html diff --git a/app/models/request.rb b/app/models/request.rb index 9d54f11fab..cabf3678c8 100644 --- a/app/models/request.rb +++ b/app/models/request.rb @@ -31,7 +31,7 @@ class Request < ApplicationRecord accepts_nested_attributes_for :item_requests, allow_destroy: true, reject_if: proc { |attributes| attributes["quantity"].blank? } has_many :child_item_requests, through: :item_requests - enum :status, { pending: 0, started: 1, fulfilled: 2, discarded: 3 }, prefix: true + enum :status, { pending: 0, started: 1, fulfilled: 2, cancelled: 3 }, prefix: true enum :request_type, %w[quantity individual child].map { |v| [v, v] }.to_h validates :distribution_id, uniqueness: true, allow_nil: true diff --git a/app/services/request_destroy_service.rb b/app/services/request_destroy_service.rb index 1aeb9d6019..09e8b33f6a 100644 --- a/app/services/request_destroy_service.rb +++ b/app/services/request_destroy_service.rb @@ -11,7 +11,7 @@ def call request.discarded_at = Time.current request.discard_reason = reason - request.status = :discarded + request.status = :cancelled request.save! unless request.partner.deactivated? @@ -29,7 +29,7 @@ def valid? if request.blank? errors.add(:base, 'request_id is invalid') elsif request.discarded_at.present? - errors.add(:base, 'request already discarded') + errors.add(:base, 'request already cancelled') end errors.none? diff --git a/app/views/requests/_status.html.erb b/app/views/requests/_status.html.erb index 7f2cdc5027..4280cbac1d 100644 --- a/app/views/requests/_status.html.erb +++ b/app/views/requests/_status.html.erb @@ -6,5 +6,5 @@ <% when "fulfilled" %> Fulfilled <% else %> - Errored + Cancelled <% end %> diff --git a/docs/user_guide/bank/essentials_requests.md b/docs/user_guide/bank/essentials_requests.md index 1e2eefcfb4..56e7d19ae3 100644 --- a/docs/user_guide/bank/essentials_requests.md +++ b/docs/user_guide/bank/essentials_requests.md @@ -29,7 +29,7 @@ The list contains: - pending -- haven't started fulfilling it yet - started -- have started fulfilling, but haven't saved the resulting distribution - fulfilled -- have created the distribution for this Request - - discarded -- have cancelled the Request + - cancelled -- have cancelled the Request - and the actions you can take on that Request ### Filtering your Requests diff --git a/docs/user_guide/bank/exports.md b/docs/user_guide/bank/exports.md index 3dbed2243d..2d3ccde6dd 100644 --- a/docs/user_guide/bank/exports.md +++ b/docs/user_guide/bank/exports.md @@ -404,7 +404,7 @@ Click 'Requests' in the left-hand menu, then "Export Requests" You can filter the exported Requests by the following: - Item - Partner -- Status (Pending, Started, Fulfilled, Discarded) +- Status (Pending, Started, Fulfilled, Cancelled) - Date Range Make your selection, then click 'Filter' before clicking 'Export Requests' diff --git a/spec/factories/requests.rb b/spec/factories/requests.rb index 017a36b787..c4e77031ba 100644 --- a/spec/factories/requests.rb +++ b/spec/factories/requests.rb @@ -74,8 +74,9 @@ def random_request_items status { 'pending' } end - trait :discarded do - status { 'discarded' } + trait :cancelled do + status { 'cancelled' } + discarded_at { Time.current } end trait :with_varied_quantities do diff --git a/spec/requests/partners/requests_spec.rb b/spec/requests/partners/requests_spec.rb index 55b88df00a..c81a98c127 100644 --- a/spec/requests/partners/requests_spec.rb +++ b/spec/requests/partners/requests_spec.rb @@ -438,7 +438,7 @@ let(:partner_user) { partner1.primary_user } let!(:pending_request) { create(:request, :with_item_requests, :pending, partner: partner1, request_items: [{ item_id: item1.id, quantity: '100' }]) } let!(:started_request) { create(:request, :with_item_requests, :started, partner: partner1, request_items: [{ item_id: item2.id, quantity: '50' }]) } - let!(:discarded_request) { create(:request, :with_item_requests, :discarded, partner: partner1, request_items: [{ item_id: item2.id, quantity: '30' }]) } + let!(:cancelled_request) { create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ item_id: item2.id, quantity: '30' }]) } let!(:fulfilled_request) { create(:request, :with_item_requests, :fulfilled, partner: partner1, request_items: [{ item_id: item2.id, quantity: '20' }]) } before do diff --git a/spec/requests/requests_requests_spec.rb b/spec/requests/requests_requests_spec.rb index 7bde1cc7b1..10d83d52d9 100644 --- a/spec/requests/requests_requests_spec.rb +++ b/spec/requests/requests_requests_spec.rb @@ -34,11 +34,24 @@ create(:request, :pending) create(:request, :started) create(:request, :fulfilled) - create(:request, :discarded) + create(:request, :cancelled) get requests_path expect(response.body).to include('Print Unfulfilled Picklists (2)') + expect(response.body).not_to match(%r{\s*Cancelled\s*}) + end + end + + context "when 'include_cancelled' param is present" do + it "shows cancelled requests" do + Request.delete_all + + create(:request, :cancelled) + + get requests_path, params: {include_cancelled: "1"} + + expect(response.body).to match(%r{\s*Cancelled\s*}) end end diff --git a/spec/services/request_destroy_service_spec.rb b/spec/services/request_destroy_service_spec.rb index abe8cfa6ac..7a033133b2 100644 --- a/spec/services/request_destroy_service_spec.rb +++ b/spec/services/request_destroy_service_spec.rb @@ -16,13 +16,13 @@ end end - context 'when the request is already discarded' do + context 'when the request is already cancelled' do before do request.discard! end it 'should not be successful and have errors' do - expect(subject.errors.full_messages).to eq(['request already discarded']) + expect(subject.errors.full_messages).to eq(['request already cancelled']) end end @@ -37,7 +37,7 @@ end it 'should update the status column on the request' do - expect { subject }.to change { request.reload.status_discarded? }.from(false).to(true) + expect { subject }.to change { request.reload.status_cancelled? }.from(false).to(true) end it 'should send a email notification to the partner' do @@ -51,7 +51,7 @@ let(:request) { create(:request, partner: partner) } it 'should update the status column on the request' do - expect { subject }.to change { request.reload.status_discarded? }.from(false).to(true) + expect { subject }.to change { request.reload.status_cancelled? }.from(false).to(true) end it 'should not send a email notification to the partner' do diff --git a/spec/system/dashboard_system_spec.rb b/spec/system/dashboard_system_spec.rb index d329f81b80..f3ce2ef9a4 100644 --- a/spec/system/dashboard_system_spec.rb +++ b/spec/system/dashboard_system_spec.rb @@ -109,8 +109,8 @@ # expect(org_dashboard_page.outstanding_requests).to be_empty end - it "does not display a discarded request" do - create :request, :discarded + it "does not display a cancelled request" do + create :request, :cancelled org_dashboard_page.visit org_dashboard_page.outstanding_section # wait for the section expect(org_dashboard_page.outstanding_requests).to be_empty diff --git a/spec/system/request_system_spec.rb b/spec/system/request_system_spec.rb index 1ddba256d0..d677ae7135 100644 --- a/spec/system/request_system_spec.rb +++ b/spec/system/request_system_spec.rb @@ -30,24 +30,92 @@ create(:request, :with_item_requests, :pending, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '16' }]) end - it "lists requests" do + it "excludes cancelled requests by default" do + _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '7' }]) + visit subject + + expect(find_field("include_cancelled")).not_to be_checked + expect(page).to have_xpath("//h1", text: "Requests") + expect(page.find("table")).to have_content('Started', count: 3) + expect(page.find("table")).to have_content('Fulfilled', count: 1) + expect(page.find("table")).to have_content('Pending', count: 1) + expect(page.find("table")).not_to have_content("Cancelled") end - it "can be exported in CSV" do - visit subject - click_on "Export Requests" + context "when 'Include Cancelled?' is checked" do + it "includes requests that are cancelled" do + _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) + + visit subject + + check "include_cancelled" + click_on 'Filter' + + expect(find_field("include_cancelled")).to be_checked + + expect(page).to have_xpath("//h1", text: "Requests") + expect(page.find("table")).to have_content('Started', count: 3) + expect(page.find("table")).to have_content('Fulfilled', count: 1) + expect(page.find("table")).to have_content('Pending', count: 1) + expect(page.find("table")).to have_content("Cancelled", count: 1) + end + + it 'does not display the Cancel button for cancelled requests' do + _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) + + visit subject + + check "Include Cancelled?" + click_on 'Filter' + + within "table tbody" do + expect(page).to have_content("Cancelled", count: 1) + expect(page).to have_link("Cancel", count: 5) # 5 requests created in the before block + end + end + end + + context "exporting requests" do + it "exports the requests CSV excluding cancelled requests by default" do + _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) + + visit subject + click_on "Export Requests" + + wait_for_download + expect(downloads.length).to eq(1) + expect(download).to match(/.*\.csv/) + + headers, *rows = download_content.split("\n") + + expect(rows.size).to eq(5) + expect(rows.join).to have_text(partner1.name, count: 4) + expect(rows.join).not_to have_text('Cancelled') + expect(headers).to have_text(item2.name, count: 1) + end + + it "exports the requests CSV including cancelled requests when 'Include Cancelled' is checked" do + _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) + + visit subject + check "Include Cancelled?" + click_on 'Filter' + + click_on "Export Requests" - wait_for_download - expect(downloads.length).to eq(1) - expect(download).to match(/.*\.csv/) + wait_for_download + expect(downloads.length).to eq(1) + expect(download).to match(/.*\.csv/) - headers, *rows = download_content.split("\n") + headers, *rows = download_content.split("\n") - expect(rows.size).to eq(5) - expect(rows.join).to have_text(partner1.name, count: 4) - expect(headers).to have_text(item2.name, count: 1) + expect(rows.size).to eq(6) + expect(rows.join).to have_text(partner1.name, count: 5) + expect(rows.join).to have_text('Cancelled') + expect(headers).to have_text(item2.name, count: 1) + end end context "when filtering on the index page" do @@ -81,7 +149,9 @@ end context "when filtering by status" do - it "constrains the list" do + it "constrains the list excluding cancelled requests by default" do + _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) + visit subject # check for all requests expect(page).to have_css("table tbody tr", count: 5) @@ -91,6 +161,20 @@ # check for filtered requests expect(page).to have_css("table tbody tr", count: 1) end + + context "when 'Include Cancelled?' is checked and filter by Cancelled" do + it "constrains the list including cancelled requests" do + _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) + + visit subject + + check "Include Cancelled?" + select "Cancelled", from: "Filter by status" + click_on 'Filter' + + expect(page).to have_css("table tbody tr", count: 1) + end + end end context "when exporting as CSV" do @@ -112,8 +196,31 @@ expect(rows.join).to have_text('13', count: 1) expect(rows.join).to have_text(partner1.name, count: 1) end + + it "exports only the cancelled requests CSV when 'Include Cancelled' is checked and 'Filter by Status' is 'Cancelled'" do + _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) + + visit subject + check "Include Cancelled?" + select "Cancelled", from: "Filter by status" + click_on 'Filter' + + click_on "Export Requests" + + wait_for_download + expect(downloads.length).to eq(1) + expect(download).to match(/.*\.csv/) + + headers, *rows = download_content.split("\n") + + expect(rows.size).to eq(1) + expect(rows.join).to have_text(partner1.name, count: 1) + expect(rows.join).to have_text('Cancelled') + expect(headers).to have_text(item1.name, count: 1) + end end end + it_behaves_like "Date Range Picker", Request, :created_at it "doesn't display New Quantity Request link" do @@ -207,6 +314,19 @@ expect(page).to have_content("334") end + context 'when the request has a cancelled status' do + it 'does not display the Cancel and Fulfill request buttons' do + cancelled_request = create(:request, :with_item_requests, :cancelled, organization: organization) + + visit request_path(cancelled_request.id) + + expect(page).to have_content('Cancelled') + expect(page).not_to have_button('Cancel') + expect(page).not_to have_button('Fulfill request') + expect(page).to have_content('Print') + end + end + context "change status request" do before do visit subject @@ -253,7 +373,7 @@ visit requests_path end - it 'should set the request as canceled/discarded and contain the reason' do + it 'should set the request as canceled and contain the reason' do click_on 'Cancel' fill_in 'Cancellation reason *', with: reason click_on 'Yes. Cancel Request' From e39536ae421245ad72b1f7c486ae3f9f4acb92a1 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 28 Apr 2026 12:07:29 -0600 Subject: [PATCH 3/7] Cancelled requests actions update - Cancelled requests will not have the "cancel" button beside them - Do not show "Fulfil request" -- once a request is cancelled, it is cancelled. - Do not show the "Cancel" button --- app/views/requests/_request_row.html.erb | 6 ++++-- app/views/requests/index.html.erb | 9 ++++++--- app/views/requests/show.html.erb | 6 ++++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/views/requests/_request_row.html.erb b/app/views/requests/_request_row.html.erb index 58b4fdc3df..66af59903d 100644 --- a/app/views/requests/_request_row.html.erb +++ b/app/views/requests/_request_row.html.erb @@ -22,7 +22,9 @@ <%= view_button_to request_path(request_row) %> - <%= cancel_button_to new_request_cancelation_path(request_id: request_row.id), method: :get, data: { disable_with: "Please wait..." }, size: "xs", type: "danger" %> + <% unless request_row.status_cancelled? %> + <%= cancel_button_to new_request_cancelation_path(request_id: request_row.id), method: :get, data: { disable_with: "Please wait..." }, size: "xs", type: "danger" %> + <% end %> <%= print_button_to print_picklist_request_path(request_row), { format: :pdf, text: "Print", size: "xs" } %> - + diff --git a/app/views/requests/index.html.erb b/app/views/requests/index.html.erb index 57462d00f5..c7152f2158 100644 --- a/app/views/requests/index.html.erb +++ b/app/views/requests/index.html.erb @@ -57,8 +57,11 @@ <%= label_tag "Date Range" %> <%= render partial: "shared/date_range_picker", locals: {css_class: "form-control"} %> - - +
+ <%= filter_checkbox(label: "Include Cancelled?", scope: "include_cancelled", selected: @include_cancelled) %> +
+ +
@@ -73,7 +76,7 @@ text: "Print Unfulfilled Picklists (#{@unfulfilled_requests_count})", size: "md") %> <% end %> - <%= download_button_to(requests_path(format: :csv, filters: filter_params.merge(date_range: date_range_params)), {text: "Export Requests", size: "md"}) if @requests.any? %> + <%= download_button_to(requests_path(format: :csv, include_cancelled: @include_cancelled, filters: filter_params.merge(date_range: date_range_params)), {text: "Export Requests", size: "md"}) if @requests.any? %> <%= modal_button_to("#newRequest", text: "New Quantity Request", icon: "plus", type: "success") if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
diff --git a/app/views/requests/show.html.erb b/app/views/requests/show.html.erb index 19a44d5cd5..8e3d1b6a99 100644 --- a/app/views/requests/show.html.erb +++ b/app/views/requests/show.html.erb @@ -104,10 +104,12 @@
- <%= submit_button_to start_request_path(@request), {text: "Fulfill request", size: "md"} unless @request.distribution %> + <% unless @request.status_cancelled? %> + <%= submit_button_to start_request_path(@request), {text: "Fulfill request", size: "md"} unless @request.distribution %> + <% end %> <%= view_button_to(distribution_path(@request.distribution), {text: "View Associated Distribution", size: "md"}) if @request.distribution %> <%= print_button_to print_picklist_request_path(@request), { format: :pdf, text: "Print", size: "md" } %> - <% unless @request.status_fulfilled? %> + <% unless @request.status_fulfilled? || @request.status_cancelled? %> <%= button_to 'Cancel', new_request_cancelation_path(request_id: @request.id), method: :get, data: { disable_with: "Please wait..." }, form_class: 'd-inline', class: 'btn btn-danger btn-md' %> <% end %> From 94f7949fe01e202cc585b4efc5361f907a260a51 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 28 Apr 2026 12:08:32 -0600 Subject: [PATCH 4/7] Ensure cancelled requests cannot have their status changed The UI does not display the buttons to change a cancelled requests' status but adding model validations ensures data integrity. --- app/models/request.rb | 10 +++++++++- spec/models/request_spec.rb | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/app/models/request.rb b/app/models/request.rb index cabf3678c8..6c4590975b 100644 --- a/app/models/request.rb +++ b/app/models/request.rb @@ -37,7 +37,9 @@ class Request < ApplicationRecord validates :distribution_id, uniqueness: true, allow_nil: true validate :item_requests_uniqueness_by_item_id validate :not_completely_empty - validate :cannot_change_status_once_fulfilled, on: :update + validate :cannot_change_status_once_fulfilled, + :cannot_change_status_once_cancelled, + on: :update after_validation :sanitize_items_data @@ -93,4 +95,10 @@ def cannot_change_status_once_fulfilled errors.add(:status, "cannot be changed once fulfilled") end end + + def cannot_change_status_once_cancelled + if status_changed? && status_was == "cancelled" + errors.add(:status, "cannot be changed once cancelled") + end + end end diff --git a/spec/models/request_spec.rb b/spec/models/request_spec.rb index 16db16da2a..e203179deb 100644 --- a/spec/models/request_spec.rb +++ b/spec/models/request_spec.rb @@ -40,6 +40,32 @@ .to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once fulfilled/) end + it "does not regress from fulfilled to cancelled" do + expect { request_fulfilled.status_cancelled! } + .to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once fulfilled/) + end + + it "does not regress from cancelled to started" do + cancelled_request = create(:request, :cancelled) + + expect { cancelled_request.status_started! } + .to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once cancelled/) + end + + it "does not regress from cancelled to pending" do + cancelled_request = create(:request, :cancelled) + + expect { cancelled_request.status_pending! } + .to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once cancelled/) + end + + it "does not regress from cancelled to fulfilled" do + cancelled_request = create(:request, :cancelled) + + expect { cancelled_request.status_fulfilled! } + .to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once cancelled/) + end + it "allows normal transitions" do expect { request_pending.status_started! }.not_to raise_error expect { request_started.status_fulfilled! }.not_to raise_error From 2dbc5c87d4074091cba552bccee80106b8eff0f0 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 28 Apr 2026 12:09:17 -0600 Subject: [PATCH 5/7] Create factories only when necessary Small test improvement to only create test data that is needed. --- spec/system/request_system_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/system/request_system_spec.rb b/spec/system/request_system_spec.rb index d677ae7135..34187d6c0a 100644 --- a/spec/system/request_system_spec.rb +++ b/spec/system/request_system_spec.rb @@ -4,8 +4,8 @@ let(:item1) { create(:item, name: "Good item") } let(:item2) { create(:item, name: "Crap item") } - let(:partner1) { create(:partner, organization:, name: "This Guy", email: "thisguy@example.com") } - let(:partner2) { create(:partner, organization:, name: "That Guy", email: "ntg@example.com") } + let(:partner1) { build(:partner, organization:, name: "This Guy", email: "thisguy@example.com") } + let(:partner2) { build(:partner, organization:, name: "That Guy", email: "ntg@example.com") } let!(:storage_location) { create(:storage_location, organization: organization) } before do From c920b367215c29c0994fac7f2a95b3e9c290fae0 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Mon, 4 May 2026 14:59:11 -0600 Subject: [PATCH 6/7] Move system tests to requests System tests are costly to maintain. Leaving just the tests that make more sense to be tested in the browser. --- spec/requests/requests_requests_spec.rb | 23 ++++++++++++++- spec/system/request_system_spec.rb | 38 +------------------------ 2 files changed, 23 insertions(+), 38 deletions(-) diff --git a/spec/requests/requests_requests_spec.rb b/spec/requests/requests_requests_spec.rb index 10d83d52d9..a538ba1ae0 100644 --- a/spec/requests/requests_requests_spec.rb +++ b/spec/requests/requests_requests_spec.rb @@ -44,29 +44,50 @@ end context "when 'include_cancelled' param is present" do - it "shows cancelled requests" do + it "shows print unfulfilled picklists button with correct quantity including cancelled requests" do Request.delete_all + create(:request, :pending) + create(:request, :started) create(:request, :cancelled) get requests_path, params: {include_cancelled: "1"} + expect(response.body).to include('Print Unfulfilled Picklists (2)') expect(response.body).to match(%r{\s*Cancelled\s*}) end end + context "when 'Include Cancelled?' is checked and filter by Cancelled" do + it "constrains the list for cancelled requests only" do + Request.delete_all + + create(:request, :started, comments: "Need more supplies") + create(:request, :pending, comments: "Awaiting for confirmation") + create(:request, :cancelled, comments: 'Not necessary anymore') + + get requests_path, params: {include_cancelled: "1", filters: { by_status: :cancelled}} + + expect(response.body).to include("Not necessary anymore") + expect(response.body).not_to include("Need more supplies") + expect(response.body).not_to include("Awaiting for confirmation") + end + end + context "when there is a filter applied" do it "shows only filtered requests, print unfulfilled picklists button with correct quantity" do Request.delete_all create(:request, :started, comments: "Started request - should appear") create(:request, :pending, comments: "Pending request - should not appear") + create(:request, :cancelled, comments: 'Cancelled request - a comment') get requests_path({ filters: { by_status: :started} }) expect(response.body).to include("Print Unfulfilled Picklists (1)") expect(response.body).to include("Started request - should appear") expect(response.body).not_to include("Pending request - should not appear") + expect(response.body).not_to include("Cancelled request - a comment") end end end diff --git a/spec/system/request_system_spec.rb b/spec/system/request_system_spec.rb index 34187d6c0a..baf2d0ed00 100644 --- a/spec/system/request_system_spec.rb +++ b/spec/system/request_system_spec.rb @@ -31,8 +31,6 @@ end it "excludes cancelled requests by default" do - _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '7' }]) - visit subject expect(find_field("include_cancelled")).not_to be_checked @@ -41,27 +39,9 @@ expect(page.find("table")).to have_content('Started', count: 3) expect(page.find("table")).to have_content('Fulfilled', count: 1) expect(page.find("table")).to have_content('Pending', count: 1) - expect(page.find("table")).not_to have_content("Cancelled") end context "when 'Include Cancelled?' is checked" do - it "includes requests that are cancelled" do - _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) - - visit subject - - check "include_cancelled" - click_on 'Filter' - - expect(find_field("include_cancelled")).to be_checked - - expect(page).to have_xpath("//h1", text: "Requests") - expect(page.find("table")).to have_content('Started', count: 3) - expect(page.find("table")).to have_content('Fulfilled', count: 1) - expect(page.find("table")).to have_content('Pending', count: 1) - expect(page.find("table")).to have_content("Cancelled", count: 1) - end - it 'does not display the Cancel button for cancelled requests' do _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) @@ -149,9 +129,7 @@ end context "when filtering by status" do - it "constrains the list excluding cancelled requests by default" do - _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) - + it "constrains the list" do visit subject # check for all requests expect(page).to have_css("table tbody tr", count: 5) @@ -161,20 +139,6 @@ # check for filtered requests expect(page).to have_css("table tbody tr", count: 1) end - - context "when 'Include Cancelled?' is checked and filter by Cancelled" do - it "constrains the list including cancelled requests" do - _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) - - visit subject - - check "Include Cancelled?" - select "Cancelled", from: "Filter by status" - click_on 'Filter' - - expect(page).to have_css("table tbody tr", count: 1) - end - end end context "when exporting as CSV" do From 58f16d552ac05874afd4f2a72ec529e024b4ee18 Mon Sep 17 00:00:00 2001 From: Stefanni Brasil Date: Tue, 5 May 2026 10:30:49 -0600 Subject: [PATCH 7/7] Move more system tests to request specs System tests are costly to maintain. Leaving just the tests that make more sense to be tested in the browser. --- spec/requests/requests_requests_spec.rb | 74 ++++++++++++++--- spec/system/request_system_spec.rb | 105 +++--------------------- 2 files changed, 72 insertions(+), 107 deletions(-) diff --git a/spec/requests/requests_requests_spec.rb b/spec/requests/requests_requests_spec.rb index a538ba1ae0..5339fc2b8d 100644 --- a/spec/requests/requests_requests_spec.rb +++ b/spec/requests/requests_requests_spec.rb @@ -11,10 +11,6 @@ response end - before do - create(:request) - end - context "html" do let(:response_format) { 'html' } @@ -25,12 +21,37 @@ let(:response_format) { 'csv' } it { is_expected.to be_successful } + + context 'when exporting as CSV' do + it "exports only the cancelled requests CSV when 'Include Cancelled' is checked and 'Filter by Status' is 'Cancelled'" do + create(:request, :started) + create(:request, :cancelled) + + get requests_path(format: :csv, params: {include_cancelled: "1", filters: { by_status: :cancelled}}) + + csv = CSV.parse(response.body, headers: true) + + expect(csv.count).to eq(1) + expect(csv.first["Status"]).to eq("Cancelled") + end + + it "exports the requests CSV including cancelled requests when 'Include Cancelled' is checked" do + create(:request, :started) + create(:request, :cancelled) + + get requests_path(format: :csv, params: {include_cancelled: "1"}) + + csv = CSV.parse(response.body, headers: true) + + expect(csv.count).to eq(2) + expect(csv[0]["Status"]).to eq("Started") + expect(csv[1]["Status"]).to eq("Cancelled") + end + end end context "when there are pending or started requests" do - it "shows print unfulfilled picklists button with correct quantity" do - Request.delete_all - + it "shows print unfulfilled picklists button with correct quantity, excluding cancelled requests by default" do create(:request, :pending) create(:request, :started) create(:request, :fulfilled) @@ -44,9 +65,22 @@ end context "when 'include_cancelled' param is present" do - it "shows print unfulfilled picklists button with correct quantity including cancelled requests" do - Request.delete_all + it 'does not display the Cancel button for cancelled requests' do + pending_request = create(:request, :pending) + cancelled_request = create(:request, :cancelled) + + get requests_path, params: {include_cancelled: "1"} + page = Nokogiri::HTML(response.body) + + cancelled_request_cancel_button = page.at_css("form[action='/requests/#{cancelled_request.id}/cancelation/new']") + expect(cancelled_request_cancel_button).to be_nil + + pending_request_cancel_button = page.at_css("a[href='/requests/#{pending_request.id}/cancelation/new']") + expect(pending_request_cancel_button).to be_present + end + + it "shows print unfulfilled picklists button with correct quantity including cancelled requests" do create(:request, :pending) create(:request, :started) create(:request, :cancelled) @@ -60,8 +94,6 @@ context "when 'Include Cancelled?' is checked and filter by Cancelled" do it "constrains the list for cancelled requests only" do - Request.delete_all - create(:request, :started, comments: "Need more supplies") create(:request, :pending, comments: "Awaiting for confirmation") create(:request, :cancelled, comments: 'Not necessary anymore') @@ -76,8 +108,6 @@ context "when there is a filter applied" do it "shows only filtered requests, print unfulfilled picklists button with correct quantity" do - Request.delete_all - create(:request, :started, comments: "Started request - should appear") create(:request, :pending, comments: "Pending request - should not appear") create(:request, :cancelled, comments: 'Cancelled request - a comment') @@ -203,6 +233,24 @@ expect(cancel_button).not_to be_present end end + + context 'when the request has a cancelled status' do + it 'does not display the Cancel and Fulfill request buttons' do + cancelled_request = create(:request, :cancelled, organization:) + + get request_path(cancelled_request) + + page = Nokogiri::HTML(response.body) + + cancel_button = page.at_css("form[action='/requests/#{cancelled_request.id}/cancelation/new']") + fulfill_button = page.at_css("form[action='/requests/#{cancelled_request.id}/start']") + expect(cancel_button).to be_nil + expect(fulfill_button).to be_nil + + print_link = page.at_css("a[href='/requests/#{cancelled_request.id}/print_picklist']") + expect(print_link).to be_present + end + end end describe 'POST #start' do diff --git a/spec/system/request_system_spec.rb b/spec/system/request_system_spec.rb index baf2d0ed00..c1d3c936b9 100644 --- a/spec/system/request_system_spec.rb +++ b/spec/system/request_system_spec.rb @@ -30,72 +30,24 @@ create(:request, :with_item_requests, :pending, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '16' }]) end - it "excludes cancelled requests by default" do + it "lists requests" do visit subject - - expect(find_field("include_cancelled")).not_to be_checked - expect(page).to have_xpath("//h1", text: "Requests") - expect(page.find("table")).to have_content('Started', count: 3) - expect(page.find("table")).to have_content('Fulfilled', count: 1) - expect(page.find("table")).to have_content('Pending', count: 1) end - context "when 'Include Cancelled?' is checked" do - it 'does not display the Cancel button for cancelled requests' do - _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) + it "can be exported in CSV" do + visit subject + click_on "Export Requests" - visit subject + wait_for_download + expect(downloads.length).to eq(1) + expect(download).to match(/.*\.csv/) - check "Include Cancelled?" - click_on 'Filter' + headers, *rows = download_content.split("\n") - within "table tbody" do - expect(page).to have_content("Cancelled", count: 1) - expect(page).to have_link("Cancel", count: 5) # 5 requests created in the before block - end - end - end - - context "exporting requests" do - it "exports the requests CSV excluding cancelled requests by default" do - _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) - - visit subject - click_on "Export Requests" - - wait_for_download - expect(downloads.length).to eq(1) - expect(download).to match(/.*\.csv/) - - headers, *rows = download_content.split("\n") - - expect(rows.size).to eq(5) - expect(rows.join).to have_text(partner1.name, count: 4) - expect(rows.join).not_to have_text('Cancelled') - expect(headers).to have_text(item2.name, count: 1) - end - - it "exports the requests CSV including cancelled requests when 'Include Cancelled' is checked" do - _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) - - visit subject - check "Include Cancelled?" - click_on 'Filter' - - click_on "Export Requests" - - wait_for_download - expect(downloads.length).to eq(1) - expect(download).to match(/.*\.csv/) - - headers, *rows = download_content.split("\n") - - expect(rows.size).to eq(6) - expect(rows.join).to have_text(partner1.name, count: 5) - expect(rows.join).to have_text('Cancelled') - expect(headers).to have_text(item2.name, count: 1) - end + expect(rows.size).to eq(5) + expect(rows.join).to have_text(partner1.name, count: 4) + expect(headers).to have_text(item2.name, count: 1) end context "when filtering on the index page" do @@ -160,28 +112,6 @@ expect(rows.join).to have_text('13', count: 1) expect(rows.join).to have_text(partner1.name, count: 1) end - - it "exports only the cancelled requests CSV when 'Include Cancelled' is checked and 'Filter by Status' is 'Cancelled'" do - _cancelled_request = create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ "item_id": item1.id, "quantity": '6' }]) - - visit subject - check "Include Cancelled?" - select "Cancelled", from: "Filter by status" - click_on 'Filter' - - click_on "Export Requests" - - wait_for_download - expect(downloads.length).to eq(1) - expect(download).to match(/.*\.csv/) - - headers, *rows = download_content.split("\n") - - expect(rows.size).to eq(1) - expect(rows.join).to have_text(partner1.name, count: 1) - expect(rows.join).to have_text('Cancelled') - expect(headers).to have_text(item1.name, count: 1) - end end end @@ -278,19 +208,6 @@ expect(page).to have_content("334") end - context 'when the request has a cancelled status' do - it 'does not display the Cancel and Fulfill request buttons' do - cancelled_request = create(:request, :with_item_requests, :cancelled, organization: organization) - - visit request_path(cancelled_request.id) - - expect(page).to have_content('Cancelled') - expect(page).not_to have_button('Cancel') - expect(page).not_to have_button('Fulfill request') - expect(page).to have_content('Print') - end - end - context "change status request" do before do visit subject