From 53eaa682a80a661e4353ec6979275d72b7efef61 Mon Sep 17 00:00:00 2001 From: Olexii Kasianenko Date: Thu, 7 Aug 2025 23:03:24 +0300 Subject: [PATCH 01/11] Add Inventory Flow tab with quantity tracking and date filtering --- .rubocop_todo.yml | 8 --- .../storage_locations_controller.rb | 23 +++--- app/models/item.rb | 14 +++- app/models/line_item.rb | 22 ++++++ app/queries/items_in_query.rb | 29 -------- app/queries/items_in_total_query.rb | 29 -------- app/queries/items_out_query.rb | 29 -------- app/queries/items_out_total_query.rb | 26 ------- .../storage_locations/_item_row.html.erb | 7 ++ app/views/storage_locations/show.html.erb | 53 +++++++------- spec/models/item_spec.rb | 32 +++++++++ spec/models/line_item_spec.rb | 42 +++++++++++ spec/queries/items_in_query_spec.rb | 43 ----------- spec/queries/items_in_total_query_spec.rb | 53 -------------- spec/queries/items_out_query_spec.rb | 30 -------- spec/queries/items_out_total_query_spec.rb | 24 ------- spec/system/storage_location_system_spec.rb | 72 +++++++++++++------ 17 files changed, 207 insertions(+), 329 deletions(-) delete mode 100644 app/queries/items_in_query.rb delete mode 100644 app/queries/items_in_total_query.rb delete mode 100644 app/queries/items_out_query.rb delete mode 100644 app/queries/items_out_total_query.rb create mode 100644 app/views/storage_locations/_item_row.html.erb delete mode 100644 spec/queries/items_in_query_spec.rb delete mode 100644 spec/queries/items_in_total_query_spec.rb delete mode 100644 spec/queries/items_out_query_spec.rb delete mode 100644 spec/queries/items_out_total_query_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index fc9a7dccba..84863fa2fa 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -86,10 +86,6 @@ Layout/ExtraSpacing: - 'Guardfile' - 'app/models/barcode_item.rb' - 'app/queries/items_by_storage_collection_query.rb' - - 'app/queries/items_in_query.rb' - - 'app/queries/items_in_total_query.rb' - - 'app/queries/items_out_query.rb' - - 'app/queries/items_out_total_query.rb' - 'config/environments/development.rb' - 'config/environments/test.rb' - 'config/puma.rb' @@ -169,10 +165,6 @@ Layout/MultilineMethodCallIndentation: - 'app/models/partner_distribution.rb' - 'app/models/storage_location.rb' - 'app/queries/items_by_storage_collection_query.rb' - - 'app/queries/items_in_query.rb' - - 'app/queries/items_in_total_query.rb' - - 'app/queries/items_out_query.rb' - - 'app/queries/items_out_total_query.rb' - 'app/services/reports/acquisition_report_service.rb' - 'app/services/reports/adult_incontinence_report_service.rb' - 'app/services/reports/children_served_report_service.rb' diff --git a/app/controllers/storage_locations_controller.rb b/app/controllers/storage_locations_controller.rb index 9b22e22836..68bc10fcd0 100644 --- a/app/controllers/storage_locations_controller.rb +++ b/app/controllers/storage_locations_controller.rb @@ -54,15 +54,15 @@ def edit @storage_location = current_organization.storage_locations.find(params[:id]) end - # TODO: Move these queries to Query Object def show + setup_date_range_picker @storage_location = current_organization.storage_locations.find(params[:id]) version_date = params[:version_date].presence&.to_date - # TODO: Find a way to do these with less hard SQL. These queries have to be manually updated because they're not in-sync with the Model - @items_out = ItemsOutQuery.new(organization: current_organization, storage_location: @storage_location).call - @items_out_total = ItemsOutTotalQuery.new(organization: current_organization, storage_location: @storage_location).call - @items_in = ItemsInQuery.new(organization: current_organization, storage_location: @storage_location).call - @items_in_total = ItemsInTotalQuery.new(organization: current_organization, storage_location: @storage_location).call + @items = current_organization.items.order(:name) + @items = @items.created_between(*date_range) if filter_params[:date_range].present? + @total_quantity_in = @items.sum { |item| item.quantity_in_storage(@storage_location.id) } + @total_quantity_out = @items.sum { |item| item.quantity_out_storage(@storage_location.id) } + @total_quantity_change = @total_quantity_in - @total_quantity_out if View::Inventory.within_snapshot?(current_organization.id, version_date) @inventory = View::Inventory.new(current_organization.id, event_time: version_date) else @@ -157,9 +157,16 @@ def include_omitted_items(existing_item_ids = []) end helper_method \ - def filter_params + def filter_params return {} unless params.key?(:filters) - params.require(:filters).permit(:containing) + params.require(:filters).permit(:containing, :date_range, :date_range_label) + end + + def date_range + date_range = filter_params[:date_range].split(" - ") + start_date = Date.parse(date_range[0]) + end_date = Date.parse(date_range[1]) + [start_date, end_date] end end diff --git a/app/models/item.rb b/app/models/item.rb index 15e7b1382c..26b4c1742f 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -69,7 +69,7 @@ class Item < ApplicationRecord scope :period_supplies, -> { where(reporting_category: [:pads, :tampons, :period_liners, :period_underwear, :period_other]) } - + scope :created_between, ->(start_date, end_date) { where(created_at: start_date..end_date) } enum :reporting_category, { adult_incontinence: "adult_incontinence", cloth_diapers: "cloth_diapers", @@ -193,6 +193,18 @@ def sync_request_units!(unit_ids) end end + def quantity_in_storage(storage_location_id) + line_items.inventory_in_storage(storage_location_id).sum(:quantity) + end + + def quantity_out_storage(storage_location_id) + line_items.inventory_out_storage(storage_location_id).sum(:quantity) + end + + def quantity_change(storage_location_id) + quantity_in_storage(storage_location_id) - quantity_out_storage(storage_location_id) + end + private def set_default_distribution_quantity diff --git a/app/models/line_item.rb b/app/models/line_item.rb index 68bc74a132..c6cf439f5a 100644 --- a/app/models/line_item.rb +++ b/app/models/line_item.rb @@ -25,6 +25,28 @@ class LineItem < ApplicationRecord scope :active, -> { joins(:item).where(items: { active: true }) } + scope :inventory_in_storage, ->(storage_location_id) do + joins(" + LEFT OUTER JOIN donations ON donations.id = line_items.itemizable_id AND line_items.itemizable_type = 'Donation' + LEFT OUTER JOIN purchases ON purchases.id = line_items.itemizable_id AND line_items.itemizable_type = 'Purchase' + LEFT OUTER JOIN adjustments ON adjustments.id = line_items.itemizable_id AND line_items.itemizable_type = 'Adjustment' + LEFT OUTER JOIN transfers ON transfers.id = line_items.itemizable_id AND line_items.itemizable_type = 'Transfer'") + .where("donations.storage_location_id = :storage_location_id OR + purchases.storage_location_id = :storage_location_id OR + (adjustments.storage_location_id = :storage_location_id and line_items.quantity < 0) OR + transfers.to_id = :storage_location_id", storage_location_id: storage_location_id) + end + + scope :inventory_out_storage, ->(storage_location_id) do + joins(" + LEFT OUTER JOIN distributions ON distributions.id = line_items.itemizable_id AND line_items.itemizable_type = 'Distribution' + LEFT OUTER JOIN adjustments ON adjustments.id = line_items.itemizable_id AND line_items.itemizable_type = 'Adjustment' + LEFT OUTER JOIN transfers ON transfers.id = line_items.itemizable_id AND line_items.itemizable_type = 'Transfer'") + .where("distributions.storage_location_id = :storage_location_id OR + (adjustments.storage_location_id = :storage_location_id and line_items.quantity > 0) OR + transfers.from_id = :storage_location_id", storage_location_id: storage_location_id) + end + delegate :name, to: :item # Used in a distribution that was initialized from a request. The `item_request` will be diff --git a/app/queries/items_in_query.rb b/app/queries/items_in_query.rb deleted file mode 100644 index 5792d8b8da..0000000000 --- a/app/queries/items_in_query.rb +++ /dev/null @@ -1,29 +0,0 @@ -# Creates a query object for retrieving the items, grouped by storage location -# We're using query objects for some of these more complicated queries to get -# the raw SQL out of the models and encapsulate it. -class ItemsInQuery - attr_reader :organization - attr_reader :filter_params - attr_reader :storage_location - - def initialize(organization:, storage_location:, filter_params: nil) - @organization = organization - @storage_location = storage_location - @filter_params = filter_params - end - - # rubocop:disable Naming/MemoizedInstanceVariableName - def call - @items ||= LineItem.joins(" -LEFT OUTER JOIN donations ON donations.id = line_items.itemizable_id AND line_items.itemizable_type = 'Donation' -LEFT OUTER JOIN purchases ON purchases.id = line_items.itemizable_id AND line_items.itemizable_type = 'Purchase' -LEFT OUTER JOIN items ON items.id = line_items.item_id -LEFT OUTER JOIN adjustments ON adjustments.id = line_items.itemizable_id AND line_items.itemizable_type = 'Adjustment' -LEFT OUTER JOIN transfers ON transfers.id = line_items.itemizable_id AND line_items.itemizable_type = 'Transfer'") - .where("(donations.storage_location_id = :id or purchases.storage_location_id = :id or (adjustments.storage_location_id = :id and line_items.quantity > 0) or transfers.to_id = :id) and items.organization_id = :organization_id", id: @storage_location.id, organization_id: @organization.id) - .select("sum(line_items.quantity) as quantity, items.id AS item_id, items.name") - .group("items.name, items.id") - .order("items.name") - end - # rubocop:enable Naming/MemoizedInstanceVariableName -end diff --git a/app/queries/items_in_total_query.rb b/app/queries/items_in_total_query.rb deleted file mode 100644 index fa6dc3f23b..0000000000 --- a/app/queries/items_in_total_query.rb +++ /dev/null @@ -1,29 +0,0 @@ -# Creates a query object for retrieving the items, grouped by storage location -# We're using query objects for some of these more complicated queries to get -# the raw SQL out of the models and encapsulate it. -class ItemsInTotalQuery - attr_reader :organization - attr_reader :filter_params - attr_reader :storage_location - - def initialize(organization:, storage_location:, filter_params: nil) - @organization = organization - @storage_location = storage_location - @filter_params = filter_params - end - - # rubocop:disable Naming/MemoizedInstanceVariableName - def call - @items ||= LineItem.joins(" -LEFT OUTER JOIN donations ON donations.id = line_items.itemizable_id AND line_items.itemizable_type = 'Donation' -LEFT OUTER JOIN purchases ON purchases.id = line_items.itemizable_id AND line_items.itemizable_type = 'Purchase' -LEFT OUTER JOIN items ON items.id = line_items.item_id -LEFT OUTER JOIN adjustments ON adjustments.id = line_items.itemizable_id AND line_items.itemizable_type = 'Adjustment' -LEFT OUTER JOIN transfers ON transfers.id = line_items.itemizable_id AND line_items.itemizable_type = 'Transfer'") - .where("(donations.storage_location_id = :id or purchases.storage_location_id = :id or (adjustments.storage_location_id = :id and line_items.quantity > 0) or transfers.to_id = :id) and items.organization_id = :organization_id", id: @storage_location.id, organization_id: @organization.id) - .sum("line_items.quantity") - end - # rubocop:enable Naming/MemoizedInstanceVariableName -end - - diff --git a/app/queries/items_out_query.rb b/app/queries/items_out_query.rb deleted file mode 100644 index 016245c6b2..0000000000 --- a/app/queries/items_out_query.rb +++ /dev/null @@ -1,29 +0,0 @@ -# Creates a query object for retrieving the items, grouped by storage location -# We're using query objects for some of these more complicated queries to get -# the raw SQL out of the models and encapsulate it. -class ItemsOutQuery - attr_reader :organization - attr_reader :filter_params - attr_reader :storage_location - - def initialize(organization:, storage_location:, filter_params: nil) - @organization = organization - @storage_location = storage_location - @filter_params = filter_params - end - - # rubocop:disable Naming/MemoizedInstanceVariableName - def call - @items ||= LineItem.joins(" -LEFT OUTER JOIN distributions ON distributions.id = line_items.itemizable_id AND line_items.itemizable_type = 'Distribution' -LEFT OUTER JOIN items ON items.id = line_items.item_id -LEFT OUTER JOIN adjustments ON adjustments.id = line_items.itemizable_id AND line_items.itemizable_type = 'Adjustment' -LEFT OUTER JOIN transfers ON transfers.id = line_items.itemizable_id AND line_items.itemizable_type = 'Transfer'") - .where("(distributions.storage_location_id = :id or (adjustments.storage_location_id= :id and line_items.quantity < 0) or transfers.from_id = :id) and items.organization_id= :organization_id", - id: @storage_location.id, organization_id: @organization.id) - .select("sum( case when line_items.quantity < 0 then -1*line_items.quantity else line_items.quantity END ) as quantity, items.id AS item_id, items.name") - .group("items.name, items.id") - .order("items.name") - end - # rubocop:enable Naming/MemoizedInstanceVariableName -end diff --git a/app/queries/items_out_total_query.rb b/app/queries/items_out_total_query.rb deleted file mode 100644 index ad0d39e124..0000000000 --- a/app/queries/items_out_total_query.rb +++ /dev/null @@ -1,26 +0,0 @@ -# Creates a query object for retrieving the items, grouped by storage location -# We're using query objects for some of these more complicated queries to get -# the raw SQL out of the models and encapsulate it. -class ItemsOutTotalQuery - attr_reader :organization - attr_reader :filter_params - attr_reader :storage_location - - def initialize(organization:, storage_location:, filter_params: nil) - @organization = organization - @storage_location = storage_location - @filter_params = filter_params - end - - # rubocop:disable Naming/MemoizedInstanceVariableName - def call - @items ||= LineItem.joins(" -LEFT OUTER JOIN distributions ON distributions.id = line_items.itemizable_id AND line_items.itemizable_type = 'Distribution' -LEFT OUTER JOIN items ON items.id = line_items.item_id -LEFT OUTER JOIN adjustments ON adjustments.id = line_items.itemizable_id AND line_items.itemizable_type = 'Adjustment' -LEFT OUTER JOIN transfers ON transfers.id = line_items.itemizable_id AND line_items.itemizable_type = 'Transfer'") - .where("(distributions.storage_location_id = :id or (adjustments.storage_location_id= :id and line_items.quantity < 0) or transfers.from_id = :id) and items.organization_id= :organization_id", id: @storage_location.id, organization_id: @organization.id) - .sum("case when line_items.quantity < 0 then -1*line_items.quantity else line_items.quantity END") - end - # rubocop:enable Naming/MemoizedInstanceVariableName -end diff --git a/app/views/storage_locations/_item_row.html.erb b/app/views/storage_locations/_item_row.html.erb new file mode 100644 index 0000000000..a0faa7c324 --- /dev/null +++ b/app/views/storage_locations/_item_row.html.erb @@ -0,0 +1,7 @@ +<% css_class = item_row.quantity_change(@storage_location.id).negative? ? 'modal-body-warning-text' : '' %> + + <%= link_to item_row.name, item_path(item_row.id) %> + <%= item_row.quantity_in_storage(@storage_location.id) %> + <%= item_row.quantity_out_storage(@storage_location.id) %> + <%= item_row.quantity_change(@storage_location.id) %> + \ No newline at end of file diff --git a/app/views/storage_locations/show.html.erb b/app/views/storage_locations/show.html.erb index 066dc3a7ec..c27ca0619b 100644 --- a/app/views/storage_locations/show.html.erb +++ b/app/views/storage_locations/show.html.erb @@ -68,14 +68,11 @@
@@ -131,41 +128,39 @@
-
- - - - - - - - - <%= render partial: "line_item_row", collection: @items_in %> - - - - - - - -
ItemQuantity
Total<%= @items_in_total %>
-
- -
+
+ <%= form_for @storage_location, method: :get do %> + <%= label_tag "Date Range" %> +
+
+ <%= render partial: "shared/date_range_picker", locals: {css_class: "form-control"} %> +
+
+ <%= filter_button %> + <%= clear_filter_button %> +
+
+ <% end %> +

- + + + - <%= render partial: "line_item_row", collection: @items_out %> + <%= render partial: "item_row", collection: @items %> - + + + <% css_class = @total_quantity_change.negative? ? 'modal-body-warning-text' : '' %> +
ItemQuantityQuantity InQuantity OutChange
Total<%= @items_out_total %><%= @total_quantity_in %><%= @total_quantity_out %><%= @total_quantity_change %>
diff --git a/spec/models/item_spec.rb b/spec/models/item_spec.rb index 5f9e28e485..6326d36227 100644 --- a/spec/models/item_spec.rb +++ b/spec/models/item_spec.rb @@ -175,6 +175,21 @@ expect(ai_items).to_not include(child_disposable_item, child_cloth_item, liner_item) end end + + describe "->created_between" do + let!(:start_date) { 2.days.ago } + let!(:end_date) { 1.day.ago } + let!(:item1) { create(:item, created_at: start_date, organization:) } + let!(:item2) { create(:item, created_at: end_date, organization:) } + let!(:item3) { create(:item, created_at: 3.days.ago, organization:) } + let(:items_in_range) { Item.created_between(start_date, end_date) } + + it "returns items created within the specified date range" do + expect(items_in_range.count).to eq(2) + expect(items_in_range).to include(item1, item2) + expect(items_in_range).to_not include(item3) + end + end end describe "->period_supplies" do @@ -387,6 +402,23 @@ end end end + + describe "#quantity_change" do + let(:storage_location) { create(:storage_location) } + let(:item) { create(:item) } + + before do + create(:donation, :with_items, item: item, item_quantity: 5, storage_location: storage_location) + create(:purchase, :with_items, item: item, item_quantity: 10, storage_location: storage_location) + create(:distribution, :with_items, item: item, item_quantity: 5, storage_location: storage_location) + create(:adjustment, :with_items, item: item, item_quantity: -2, storage_location: storage_location) + create(:transfer, :with_items, item: item, item_quantity: 3, from: create(:storage_location), to: storage_location) + end + + it "returns the total quantity change for the item" do + expect(item.quantity_change(storage_location.id)).to eq(5 + 10 - 5 - 2 + 3) + end + end end describe "default_quantity" do diff --git a/spec/models/line_item_spec.rb b/spec/models/line_item_spec.rb index f8721b1e5d..89369d6c82 100644 --- a/spec/models/line_item_spec.rb +++ b/spec/models/line_item_spec.rb @@ -55,6 +55,48 @@ end.to change { described_class.active.size }.by(1) end end + + describe "->inventory_in_storage" do + let(:storage_location) { create(:storage_location) } + let(:item) { create(:item) } + + before do + create(:donation, :with_items, item: item, item_quantity: 5, storage_location: storage_location) + create(:purchase, :with_items, item: item, item_quantity: 10, storage_location: storage_location) + create(:adjustment, :with_items, item: item, item_quantity: -2, storage_location: storage_location) + create(:transfer, :with_items, item: item, item_quantity: 3, from: create(:storage_location), to: storage_location) + end + + it "returns line items that are in the specified storage location" do + expect(described_class.inventory_in_storage(storage_location.id).count).to eq(4) + end + + it "returns an empty collection if no line items are in the specified storage location" do + other_storage_location = create(:storage_location) + expect(described_class.inventory_in_storage(other_storage_location.id)).to be_empty + end + end + + describe "->inventory_out_storage" do + let(:storage_location) { create(:storage_location) } + let(:item) { create(:item) } + + before do + create(:donation, :with_items, item: item, item_quantity: 10, storage_location: storage_location) + create(:distribution, :with_items, item: item, item_quantity: 5, storage_location: storage_location) + create(:adjustment, :with_items, item: item, item_quantity: -2, storage_location: storage_location) + create(:transfer, :with_items, item: item, item_quantity: 3, from: storage_location, to: create(:storage_location)) + end + + it "returns line items that are out of the specified storage location" do + expect(described_class.inventory_out_storage(storage_location.id).count).to eq(2) + end + + it "returns an empty collection if no line items are out of the specified storage location" do + other_storage_location = create(:storage_location) + expect(described_class.inventory_out_storage(other_storage_location.id)).to be_empty + end + end end describe 'Methods >' do diff --git a/spec/queries/items_in_query_spec.rb b/spec/queries/items_in_query_spec.rb deleted file mode 100644 index 25632c92e7..0000000000 --- a/spec/queries/items_in_query_spec.rb +++ /dev/null @@ -1,43 +0,0 @@ -# Spec for /app/queries/items_in_query.rb - -RSpec.describe ItemsInQuery do - let(:organization) { create(:organization) } - let!(:storage_location) { create(:storage_location, organization: organization) } - let(:other_item) { create(:item, organization: organization) } - let!(:other_storage_location) { create(:storage_location, :with_items, item: other_item, item_quantity: 10, organization: organization) } - - subject { ItemsInQuery.new(storage_location: storage_location, organization: organization).call } - - describe "items_in" do - before do - create_list(:donation, 2, :with_items, item: create(:item, organization: organization), item_quantity: 5, storage_location: storage_location, organization: organization) - create_list(:purchase, 2, :with_items, item: create(:item, organization: organization), item_quantity: 5, storage_location: storage_location, organization: organization) - create_list(:adjustment, 2, :with_items, item: create(:item, organization: organization), item_quantity: 5, storage_location: storage_location, organization: organization) - create_list(:transfer, 2, :with_items, item_quantity: 5, item: other_item, from: other_storage_location, to: storage_location, organization: organization) - end - - it "returns a collection with the fields name, item_id, quantity" do - expect(subject.first).to be_respond_to(:name) - expect(subject.first).to be_respond_to(:quantity) - expect(subject.first).to be_respond_to(:item_id) - end - - it "includes donations, purchases, adjustments, transfers among sources" do - expect(subject.to_a.size).to eq(4) - end - - it "does not count negative adjustments towards in-flow" do - shared_item = create(:item, organization: organization) - create(:donation, :with_items, item: shared_item, item_quantity: 10, storage_location: storage_location, organization: organization) - create(:adjustment, :with_items, item: shared_item, item_quantity: -10, storage_location: storage_location, organization: organization) - expect(subject.to_a.size).to eq(5) - end - - it "does not count transfers going in the negative direction" do - shared_item = create(:item, organization: organization) - create(:donation, :with_items, item: shared_item, item_quantity: 10, storage_location: storage_location, organization: organization) - create(:transfer, :with_items, item_quantity: 10, item: shared_item, from: storage_location, to: other_storage_location, organization: organization) - expect(subject.to_a.size).to eq(5) - end - end -end diff --git a/spec/queries/items_in_total_query_spec.rb b/spec/queries/items_in_total_query_spec.rb deleted file mode 100644 index 00c18b9bf7..0000000000 --- a/spec/queries/items_in_total_query_spec.rb +++ /dev/null @@ -1,53 +0,0 @@ -# Spec for /app/queries/items_in_total_query.rb - -RSpec.describe ItemsInTotalQuery do - let(:organization) { create(:organization) } - let!(:storage_location) { create(:storage_location, organization: organization) } - let(:other_item) { create(:item, organization: organization) } - let!(:other_storage_location) { create(:storage_location, :with_items, item: other_item, item_quantity: 10, organization: organization) } - let!(:shared_item) { create(:item, organization: organization) } - - subject { ItemsInTotalQuery.new(storage_location: storage_location, organization: organization).call } - - describe "items_in_total_query" do - before do - create(:donation, :with_items, item: create(:item, organization: organization), item_quantity: 10, storage_location: storage_location, organization: organization) - create(:purchase, :with_items, item: create(:item, organization: organization), item_quantity: 10, storage_location: storage_location, organization: organization) - create(:adjustment, :with_items, item: create(:item, organization: organization), item_quantity: 10, storage_location: storage_location, organization: organization) - create(:transfer, :with_items, item_quantity: 10, item: other_item, from: other_storage_location, to: storage_location, organization: organization) - end - - it "returns a sum total of all in-flows" do - expect(subject).to eq(40) - end - - it "counts shared items together" do - create(:donation, :with_items, item: shared_item, item_quantity: 50, storage_location: storage_location, organization: organization) - create(:purchase, :with_items, item: shared_item, item_quantity: 100, storage_location: storage_location, organization: organization) - create(:adjustment, :with_items, item: shared_item, item_quantity: 50, storage_location: storage_location, organization: organization) - create(:donation, :with_items, item: shared_item, item_quantity: 100, storage_location: storage_location, organization: organization) - create(:purchase, :with_items, item: shared_item, item_quantity: 50, storage_location: storage_location, organization: organization) - create(:adjustment, :with_items, item: shared_item, item_quantity: 100, storage_location: storage_location, organization: organization) - expect(subject).to eq(490) - end - - it "does not count negative adjustments towards in-flow" do - create(:donation, :with_items, item: shared_item, item_quantity: 10, storage_location: storage_location, organization: organization) - create(:adjustment, :with_items, item: shared_item, item_quantity: -10, storage_location: storage_location, organization: organization) - expect(subject).to eq(50) - end - - it "does not count transfers going in the negative direction" do - create(:donation, :with_items, item: shared_item, item_quantity: 10, storage_location: storage_location, organization: organization) - create(:transfer, :with_items, item_quantity: 10, item: shared_item, from: storage_location, to: other_storage_location, organization: organization) - expect(subject).to eq(50) - end - - it "does not count donations, purchases, adjustments to other storage locations" do - create(:donation, :with_items, item: create(:item, organization: organization), item_quantity: 100, storage_location: other_storage_location, organization: organization) - create(:purchase, :with_items, item: create(:item, organization: organization), item_quantity: 10, storage_location: other_storage_location, organization: organization) - create(:adjustment, :with_items, item: create(:item, organization: organization), item_quantity: 10, storage_location: other_storage_location, organization: organization) - expect(subject).to eq(40) - end - end -end diff --git a/spec/queries/items_out_query_spec.rb b/spec/queries/items_out_query_spec.rb deleted file mode 100644 index 9bbe735ac7..0000000000 --- a/spec/queries/items_out_query_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# Spec for /app/queries/items_out_query.rb - -RSpec.describe ItemsOutQuery do - let(:organization) { create(:organization) } - let!(:storage_location) { create(:storage_location, organization: organization) } - subject { ItemsOutQuery.new(storage_location: storage_location, organization: organization).call } - - describe "items_out_query" do - before do - items = create_list(:item, 3, organization: organization) - TestInventory.create_inventory(storage_location.organization, { - storage_location.id => items.to_h { |i| [i.id, 10] } - }) - other_storage_location = create(:storage_location, organization: organization) - create(:transfer, :with_items, item_quantity: 8, item: items[0], to: other_storage_location, from: storage_location, organization: organization) - create(:distribution, :with_items, item: items[1], item_quantity: 9, storage_location: storage_location, organization: organization) - create(:adjustment, :with_items, item: items[2], item_quantity: -10, storage_location: storage_location, organization: organization) - end - - it "returns a collection with the fields name, item_id, quantity" do - expect(subject.first).to be_respond_to(:name) - expect(subject.first).to be_respond_to(:quantity) - expect(subject.first).to be_respond_to(:item_id) - end - - it "includes distributions, adjustments, transfers among sources" do - expect(subject.to_a.size).to eq(3) - end - end -end diff --git a/spec/queries/items_out_total_query_spec.rb b/spec/queries/items_out_total_query_spec.rb deleted file mode 100644 index deab96d3ed..0000000000 --- a/spec/queries/items_out_total_query_spec.rb +++ /dev/null @@ -1,24 +0,0 @@ -# Spec for /app/queries/items_out_total_query.rb - -RSpec.describe ItemsOutTotalQuery do - let(:organization) { create(:organization) } - let!(:storage_location) { create(:storage_location, organization: organization) } - subject { ItemsOutTotalQuery.new(storage_location: storage_location, organization: organization).call } - - describe "items_out_total_query" do - before do - items = create_list(:item, 3, organization: organization) - TestInventory.create_inventory(storage_location.organization, { - storage_location.id => items.to_h { |i| [i.id, 10] } - }) - other_storage_location = create(:storage_location, organization: organization) - create(:transfer, :with_items, item_quantity: 10, item: items[0], to: other_storage_location, from: storage_location, organization: organization) - create(:distribution, :with_items, item: items[1], item_quantity: 10, storage_location: storage_location, organization: organization) - create(:adjustment, :with_items, item: items[2], item_quantity: -10, storage_location: storage_location, organization: organization) - end - - it "returns a sum total of all out-flows" do - expect(subject).to eq(30) - end - end -end diff --git a/spec/system/storage_location_system_spec.rb b/spec/system/storage_location_system_spec.rb index 662af5f3a3..02a3581898 100644 --- a/spec/system/storage_location_system_spec.rb +++ b/spec/system/storage_location_system_spec.rb @@ -99,13 +99,6 @@ click_on "View", match: :first - find("#custom-tabs-inventory-in-tab").click - - within "#custom-tabs-inventory-in" do - expect(page).to have_content("Needle") - expect(page).to have_content(100) - end - find("#custom-tabs-inventory-tab").click within "#custom-tabs-inventory" do @@ -189,24 +182,63 @@ end context "when viewing an existing storage location" do - let(:item) { create(:item, name: "AAA Diapers") } - let!(:storage_location) { create(:storage_location, :with_items, item: item, name: "here") } - let!(:adjustment) { create(:adjustment, :with_items, storage_location: storage_location) } + let(:items) { create_list(:item, 2) } + let!(:storage_location) { create(:storage_location, name: "here") } subject { storage_location_path(storage_location.id) } - it "Items in (adjustments)" do - visit subject - find("#custom-tabs-inventory-in-tab").click + context "Inventory Flow Tab" do + before do + create(:donation, :with_items, item: items[0], item_quantity: 10, storage_location: storage_location) + create(:distribution, :with_items, item: items[0], item_quantity: 5, storage_location: storage_location) + create(:donation, :with_items, item: items[1], item_quantity: 3, storage_location: storage_location) + create(:adjustment, :with_items, item: items[1], item_quantity: 3, storage_location: storage_location) + create(:transfer, :with_items, item: items[1], item_quantity: 2, from: storage_location, to: create(:storage_location)) - expect(page.find("#custom-tabs-inventory-in", visible: true)).to have_content "100" - end + visit subject + find("#custom-tabs-inventory-flow-tab").click + end - it "Items out (distributions)" do - create(:distribution, :with_items, storage_location: storage_location) - visit subject - find("#custom-tabs-inventory-out-tab").click + it "shows the inventory flow for the storage location" do + within("#custom-tabs-inventory-flow table tbody") do + items.each do |item| + row = find(:css, "tr[id='#{item.id}']") + change_column_css = item.quantity_change(storage_location).negative? ? "td.modal-body-warning-text" : "td" + expect(row).to have_link(item.name, href: item_path(item.id)) + expect(row).to have_css("td", text: item.quantity_in_storage(storage_location.id)) + expect(row).to have_css("td", text: item.quantity_out_storage(storage_location.id)) + expect(row).to have_css(change_column_css, text: item.quantity_change(storage_location.id)) + end + end + within("#custom-tabs-inventory-flow table tfoot") do + expect(page).to have_css("td", text: "Total") + expect(page).to have_css("td", text: items.sum { |item| item.quantity_in_storage(storage_location.id) }) + expect(page).to have_css("td", text: items.sum { |item| item.quantity_out_storage(storage_location.id) }) + expect(page).to have_css("td", text: items.sum { |item| item.quantity_change(storage_location.id) }) + end + end - expect(page.find("#custom-tabs-inventory-out", visible: true)).to have_content "100" + context "date range filter" do + let!(:start_date) { 2.days.ago } + let!(:end_date) { 1.day.ago } + let!(:item) { create(:item, name: "Filtered Item", created_at: start_date) } + before do + create(:donation, :with_items, item: item, item_quantity: 10, storage_location: storage_location) + fill_in "filters[date_range]", with: "#{start_date} - #{end_date}" + click_button "Filter" + find("#custom-tabs-inventory-flow-tab").click + end + + it "filters the inventory flow by date range" do + within("#custom-tabs-inventory-flow table tbody") do + expect(page).to have_css("tr", count: 1) + row = find(:css, "tr[id='#{item.id}']") + expect(row).to have_link(item.name, href: item_path(item.id)) + expect(row).to have_css("td", text: item.quantity_in_storage(storage_location.id)) + expect(row).to have_css("td", text: item.quantity_out_storage(storage_location.id)) + expect(row).to have_css("td", text: item.quantity_change(storage_location.id)) + end + end + end end end end From bb568791ffd73b904aa431579068ff0da62d6aae Mon Sep 17 00:00:00 2001 From: Olexii Kasianenko Date: Sun, 21 Sep 2025 19:43:33 +0300 Subject: [PATCH 02/11] Refactor inventory flow to use ItemsFlowQuery for improved data handling and display --- .../storage_locations_controller.rb | 7 +- app/queries/items_flow_query.rb | 77 +++++++++++++++++++ .../storage_locations/_item_row.html.erb | 12 +-- spec/queries/items_flow_query_spec.rb | 49 ++++++++++++ 4 files changed, 135 insertions(+), 10 deletions(-) create mode 100644 app/queries/items_flow_query.rb create mode 100644 spec/queries/items_flow_query_spec.rb diff --git a/app/controllers/storage_locations_controller.rb b/app/controllers/storage_locations_controller.rb index 68bc10fcd0..ffbf43b496 100644 --- a/app/controllers/storage_locations_controller.rb +++ b/app/controllers/storage_locations_controller.rb @@ -58,10 +58,9 @@ def show setup_date_range_picker @storage_location = current_organization.storage_locations.find(params[:id]) version_date = params[:version_date].presence&.to_date - @items = current_organization.items.order(:name) - @items = @items.created_between(*date_range) if filter_params[:date_range].present? - @total_quantity_in = @items.sum { |item| item.quantity_in_storage(@storage_location.id) } - @total_quantity_out = @items.sum { |item| item.quantity_out_storage(@storage_location.id) } + @items = ItemsFlowQuery.new(storage_location: @storage_location, organization: current_organization).call.to_a + @total_quantity_in = @items.first["total_quantity_in"].to_i + @total_quantity_out = @items.first["total_quantity_out"].to_i @total_quantity_change = @total_quantity_in - @total_quantity_out if View::Inventory.within_snapshot?(current_organization.id, version_date) @inventory = View::Inventory.new(current_organization.id, event_time: version_date) diff --git a/app/queries/items_flow_query.rb b/app/queries/items_flow_query.rb new file mode 100644 index 0000000000..08038ea5c8 --- /dev/null +++ b/app/queries/items_flow_query.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +class ItemsFlowQuery + attr_reader :organization + attr_reader :filter_params + attr_reader :storage_location + + def initialize(organization:, storage_location:, filter_params: nil) + @organization = organization + @storage_location = storage_location + @filter_params = filter_params + end + + def call + query = <<~SQL + WITH line_items_with_flags AS ( + SELECT + li.item_id, + it.name AS item_name, + -- in quantity for this row (0 if not matching) + CASE + WHEN (donations.storage_location_id = :id + OR purchases.storage_location_id = :id + OR (adjustments.storage_location_id = :id AND li.quantity > 0) + OR transfers.to_id = :id) + AND it.organization_id = :organization_id + THEN li.quantity + ELSE 0 + END AS quantity_in, + -- out quantity normalized to positive numbers (0 if not matching) + CASE + WHEN (distributions.storage_location_id = :id + OR (adjustments.storage_location_id = :id AND li.quantity < 0) + OR transfers.from_id = :id) + AND it.organization_id = :organization_id + THEN CASE WHEN li.quantity < 0 THEN -li.quantity ELSE li.quantity END + ELSE 0 + END AS quantity_out, + -- mark rows that are relevant for the overall WHERE in original query + CASE + WHEN (donations.storage_location_id = :id + OR purchases.storage_location_id = :id + OR distributions.storage_location_id = :id + OR transfers.from_id = :id + OR transfers.to_id = :id + OR adjustments.storage_location_id = :id) + AND it.organization_id = :organization_id + THEN 1 ELSE 0 + END AS relevant + FROM line_items li + LEFT JOIN donations ON donations.id = li.itemizable_id AND li.itemizable_type = 'Donation' + LEFT JOIN purchases ON purchases.id = li.itemizable_id AND li.itemizable_type = 'Purchase' + LEFT JOIN distributions ON distributions.id = li.itemizable_id AND li.itemizable_type = 'Distribution' + LEFT JOIN adjustments ON adjustments.id = li.itemizable_id AND li.itemizable_type = 'Adjustment' + LEFT JOIN transfers ON transfers.id = li.itemizable_id AND li.itemizable_type = 'Transfer' + LEFT JOIN items it ON it.id = li.item_id + ) + SELECT + item_id, + item_name, + SUM(quantity_in) AS quantity_in, + SUM(quantity_out) AS quantity_out, + SUM(quantity_in) - SUM(quantity_out) AS change, + SUM(SUM(quantity_in)) OVER () AS total_quantity_in, + SUM(SUM(quantity_out)) OVER () AS total_quantity_out, + SUM(SUM(quantity_in) - SUM(quantity_out)) OVER () AS total_change + FROM line_items_with_flags + WHERE relevant = 1 + GROUP BY item_id, item_name + ORDER BY item_name; + SQL + + ActiveRecord::Base.connection.exec_query( + ActiveRecord::Base.send(:sanitize_sql_array, [query, {id: @storage_location.id, organization_id: @organization.id}]) + ) + end +end diff --git a/app/views/storage_locations/_item_row.html.erb b/app/views/storage_locations/_item_row.html.erb index a0faa7c324..08e1c57149 100644 --- a/app/views/storage_locations/_item_row.html.erb +++ b/app/views/storage_locations/_item_row.html.erb @@ -1,7 +1,7 @@ -<% css_class = item_row.quantity_change(@storage_location.id).negative? ? 'modal-body-warning-text' : '' %> - - <%= link_to item_row.name, item_path(item_row.id) %> - <%= item_row.quantity_in_storage(@storage_location.id) %> - <%= item_row.quantity_out_storage(@storage_location.id) %> - <%= item_row.quantity_change(@storage_location.id) %> +<% css_class = item_row["change"].negative? ? 'modal-body-warning-text' : '' %> +"> + <%= link_to item_row["item_name"], item_path(item_row["item_id"]) %> + <%= item_row["quantity_in"] %> + <%= item_row["quantity_out"] %> + <%= item_row["change"] %> \ No newline at end of file diff --git a/spec/queries/items_flow_query_spec.rb b/spec/queries/items_flow_query_spec.rb new file mode 100644 index 0000000000..7f267012e9 --- /dev/null +++ b/spec/queries/items_flow_query_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require "rspec" + +RSpec.describe ItemsFlowQuery do + let(:items) { create_list(:item, 2) } + let!(:storage_location) { create(:storage_location, name: "here") } + let(:organization) { storage_location.organization } + let!(:result) do + [ + { + item_id: items[0].id, + item_name: items[0].name, + quantity_in: 10, + quantity_out: 5, + change: 5, + total_quantity_in: 16, + total_quantity_out: 7, + total_change: 9 + }, + { + item_id: items[1].id, + item_name: items[1].name, + quantity_in: 6, + quantity_out: 2, + change: 4, + total_quantity_in: 16, + total_quantity_out: 7, + total_change: 9 + } + ].map(&:with_indifferent_access) + end + + before do + create(:donation, :with_items, item: items[0], item_quantity: 10, storage_location: storage_location) + create(:distribution, :with_items, item: items[0], item_quantity: 5, storage_location: storage_location) + create(:donation, :with_items, item: items[1], item_quantity: 3, storage_location: storage_location) + create(:adjustment, :with_items, item: items[1], item_quantity: 3, storage_location: storage_location) + create(:transfer, :with_items, item: items[1], item_quantity: 2, from: storage_location, to: create(:storage_location)) + end + + subject { described_class.new(organization: organization, storage_location: storage_location).call } + + context "without filter params" do + it "returns array of hashes" do + expect(subject.to_a).to match_array(result) + end + end +end From 6e923a6641b3d8943431283c12a67a4dc6edbe9e Mon Sep 17 00:00:00 2001 From: Olexii Kasianenko Date: Mon, 22 Sep 2025 17:21:12 +0300 Subject: [PATCH 03/11] Add date range filtering to ItemsFlowQuery and update Inventory Flow tab --- .../storage_locations_controller.rb | 8 ++-- app/queries/items_flow_query.rb | 8 +++- app/views/storage_locations/show.html.erb | 12 ++--- spec/queries/items_flow_query_spec.rb | 47 +++++++++++++++++++ 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/app/controllers/storage_locations_controller.rb b/app/controllers/storage_locations_controller.rb index ffbf43b496..dd0f428668 100644 --- a/app/controllers/storage_locations_controller.rb +++ b/app/controllers/storage_locations_controller.rb @@ -58,9 +58,9 @@ def show setup_date_range_picker @storage_location = current_organization.storage_locations.find(params[:id]) version_date = params[:version_date].presence&.to_date - @items = ItemsFlowQuery.new(storage_location: @storage_location, organization: current_organization).call.to_a - @total_quantity_in = @items.first["total_quantity_in"].to_i - @total_quantity_out = @items.first["total_quantity_out"].to_i + @items = ItemsFlowQuery.new(storage_location: @storage_location, organization: current_organization, filter_params: date_range).call.to_a + @total_quantity_in = @items.count.positive? ? @items.first["total_quantity_in"].to_i : 0 + @total_quantity_out = @items.count.positive? ? @items.first["total_quantity_out"].to_i : 0 @total_quantity_change = @total_quantity_in - @total_quantity_out if View::Inventory.within_snapshot?(current_organization.id, version_date) @inventory = View::Inventory.new(current_organization.id, event_time: version_date) @@ -163,6 +163,8 @@ def filter_params end def date_range + return unless filter_params[:date_range].present? + date_range = filter_params[:date_range].split(" - ") start_date = Date.parse(date_range[0]) end_date = Date.parse(date_range[1]) diff --git a/app/queries/items_flow_query.rb b/app/queries/items_flow_query.rb index 08038ea5c8..883df45848 100644 --- a/app/queries/items_flow_query.rb +++ b/app/queries/items_flow_query.rb @@ -54,6 +54,7 @@ def call LEFT JOIN adjustments ON adjustments.id = li.itemizable_id AND li.itemizable_type = 'Adjustment' LEFT JOIN transfers ON transfers.id = li.itemizable_id AND li.itemizable_type = 'Transfer' LEFT JOIN items it ON it.id = li.item_id + WHERE it.created_at BETWEEN :start_date AND :end_date ) SELECT item_id, @@ -71,7 +72,12 @@ def call SQL ActiveRecord::Base.connection.exec_query( - ActiveRecord::Base.send(:sanitize_sql_array, [query, {id: @storage_location.id, organization_id: @organization.id}]) + ActiveRecord::Base.send(:sanitize_sql_array, [query, { + id: @storage_location.id, + organization_id: @organization.id, + start_date: @filter_params ? @filter_params[0] : 20.years.ago, + end_date: @filter_params ? @filter_params[1] : Time.current.end_of_day + }]) ) end end diff --git a/app/views/storage_locations/show.html.erb b/app/views/storage_locations/show.html.erb index c27ca0619b..3a01ba6771 100644 --- a/app/views/storage_locations/show.html.erb +++ b/app/views/storage_locations/show.html.erb @@ -68,18 +68,18 @@
-
+
<%= form_for @storage_location, method: :get do %> Show Inventory at Date: <%= date_field_tag 'version_date', params[:version_date], min: InventoryItem::EARLIEST_VERSION, autocomplete: "on" %> <%= filter_button(text: 'View') %> @@ -128,7 +128,7 @@
-
+
<%= form_for @storage_location, method: :get do %> <%= label_tag "Date Range" %>
diff --git a/spec/queries/items_flow_query_spec.rb b/spec/queries/items_flow_query_spec.rb index 7f267012e9..40a1316819 100644 --- a/spec/queries/items_flow_query_spec.rb +++ b/spec/queries/items_flow_query_spec.rb @@ -46,4 +46,51 @@ expect(subject.to_a).to match_array(result) end end + + context "with filter params" do + let(:filter_params) { [11.days.ago, 9.days.ago] } + let!(:old_items) do + [ + create(:item, organization: organization, created_at: 10.days.ago.beginning_of_day), + create(:item, organization: organization, created_at: 10.days.ago.end_of_day) + ] + end + let(:other_location) { create(:storage_location, organization: organization) } + + subject { described_class.new(organization: organization, storage_location: storage_location, filter_params: filter_params).call } + + before do + create(:donation, :with_items, item: old_items[0], item_quantity: 10, storage_location: storage_location) + create(:distribution, :with_items, item: old_items[1], item_quantity: 5, storage_location: storage_location) + end + + let(:filtered_result) do + [ + { + item_id: old_items[0].id, + item_name: old_items[0].name, + quantity_in: 10, + quantity_out: 0, + change: 10, + total_quantity_in: 10, + total_quantity_out: 5, + total_change: 5 + }, + { + item_id: old_items[1].id, + item_name: old_items[1].name, + quantity_in: 0, + quantity_out: 5, + change: -5, + total_quantity_in: 10, + total_quantity_out: 5, + total_change: 5 + } + ].map(&:with_indifferent_access) + end + + it "returns array of hashes" do + expect(subject.to_a).to match_array(filtered_result) + end + end end From 2439b142add6ce18eb82fa53314f443a29e4a54c Mon Sep 17 00:00:00 2001 From: Olexii Kasianenko Date: Mon, 22 Sep 2025 18:09:11 +0300 Subject: [PATCH 04/11] Remove unused inventory storage methods and related tests for cleaner code --- .../storage_locations_controller.rb | 2 +- app/models/item.rb | 14 +--- app/models/line_item.rb | 22 ------ .../storage_locations/_item_row.html.erb | 2 +- spec/models/item_spec.rb | 32 --------- spec/models/line_item_spec.rb | 42 ------------ spec/system/storage_location_system_spec.rb | 68 +++++++++++++++---- 7 files changed, 56 insertions(+), 126 deletions(-) diff --git a/app/controllers/storage_locations_controller.rb b/app/controllers/storage_locations_controller.rb index dd0f428668..df5bfa4f58 100644 --- a/app/controllers/storage_locations_controller.rb +++ b/app/controllers/storage_locations_controller.rb @@ -163,7 +163,7 @@ def filter_params end def date_range - return unless filter_params[:date_range].present? + return if filter_params[:date_range].blank? date_range = filter_params[:date_range].split(" - ") start_date = Date.parse(date_range[0]) diff --git a/app/models/item.rb b/app/models/item.rb index 26b4c1742f..15e7b1382c 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -69,7 +69,7 @@ class Item < ApplicationRecord scope :period_supplies, -> { where(reporting_category: [:pads, :tampons, :period_liners, :period_underwear, :period_other]) } - scope :created_between, ->(start_date, end_date) { where(created_at: start_date..end_date) } + enum :reporting_category, { adult_incontinence: "adult_incontinence", cloth_diapers: "cloth_diapers", @@ -193,18 +193,6 @@ def sync_request_units!(unit_ids) end end - def quantity_in_storage(storage_location_id) - line_items.inventory_in_storage(storage_location_id).sum(:quantity) - end - - def quantity_out_storage(storage_location_id) - line_items.inventory_out_storage(storage_location_id).sum(:quantity) - end - - def quantity_change(storage_location_id) - quantity_in_storage(storage_location_id) - quantity_out_storage(storage_location_id) - end - private def set_default_distribution_quantity diff --git a/app/models/line_item.rb b/app/models/line_item.rb index c6cf439f5a..68bc74a132 100644 --- a/app/models/line_item.rb +++ b/app/models/line_item.rb @@ -25,28 +25,6 @@ class LineItem < ApplicationRecord scope :active, -> { joins(:item).where(items: { active: true }) } - scope :inventory_in_storage, ->(storage_location_id) do - joins(" - LEFT OUTER JOIN donations ON donations.id = line_items.itemizable_id AND line_items.itemizable_type = 'Donation' - LEFT OUTER JOIN purchases ON purchases.id = line_items.itemizable_id AND line_items.itemizable_type = 'Purchase' - LEFT OUTER JOIN adjustments ON adjustments.id = line_items.itemizable_id AND line_items.itemizable_type = 'Adjustment' - LEFT OUTER JOIN transfers ON transfers.id = line_items.itemizable_id AND line_items.itemizable_type = 'Transfer'") - .where("donations.storage_location_id = :storage_location_id OR - purchases.storage_location_id = :storage_location_id OR - (adjustments.storage_location_id = :storage_location_id and line_items.quantity < 0) OR - transfers.to_id = :storage_location_id", storage_location_id: storage_location_id) - end - - scope :inventory_out_storage, ->(storage_location_id) do - joins(" - LEFT OUTER JOIN distributions ON distributions.id = line_items.itemizable_id AND line_items.itemizable_type = 'Distribution' - LEFT OUTER JOIN adjustments ON adjustments.id = line_items.itemizable_id AND line_items.itemizable_type = 'Adjustment' - LEFT OUTER JOIN transfers ON transfers.id = line_items.itemizable_id AND line_items.itemizable_type = 'Transfer'") - .where("distributions.storage_location_id = :storage_location_id OR - (adjustments.storage_location_id = :storage_location_id and line_items.quantity > 0) OR - transfers.from_id = :storage_location_id", storage_location_id: storage_location_id) - end - delegate :name, to: :item # Used in a distribution that was initialized from a request. The `item_request` will be diff --git a/app/views/storage_locations/_item_row.html.erb b/app/views/storage_locations/_item_row.html.erb index 08e1c57149..5992eff1fe 100644 --- a/app/views/storage_locations/_item_row.html.erb +++ b/app/views/storage_locations/_item_row.html.erb @@ -4,4 +4,4 @@ <%= item_row["quantity_in"] %> <%= item_row["quantity_out"] %> <%= item_row["change"] %> - \ No newline at end of file + diff --git a/spec/models/item_spec.rb b/spec/models/item_spec.rb index 6326d36227..5f9e28e485 100644 --- a/spec/models/item_spec.rb +++ b/spec/models/item_spec.rb @@ -175,21 +175,6 @@ expect(ai_items).to_not include(child_disposable_item, child_cloth_item, liner_item) end end - - describe "->created_between" do - let!(:start_date) { 2.days.ago } - let!(:end_date) { 1.day.ago } - let!(:item1) { create(:item, created_at: start_date, organization:) } - let!(:item2) { create(:item, created_at: end_date, organization:) } - let!(:item3) { create(:item, created_at: 3.days.ago, organization:) } - let(:items_in_range) { Item.created_between(start_date, end_date) } - - it "returns items created within the specified date range" do - expect(items_in_range.count).to eq(2) - expect(items_in_range).to include(item1, item2) - expect(items_in_range).to_not include(item3) - end - end end describe "->period_supplies" do @@ -402,23 +387,6 @@ end end end - - describe "#quantity_change" do - let(:storage_location) { create(:storage_location) } - let(:item) { create(:item) } - - before do - create(:donation, :with_items, item: item, item_quantity: 5, storage_location: storage_location) - create(:purchase, :with_items, item: item, item_quantity: 10, storage_location: storage_location) - create(:distribution, :with_items, item: item, item_quantity: 5, storage_location: storage_location) - create(:adjustment, :with_items, item: item, item_quantity: -2, storage_location: storage_location) - create(:transfer, :with_items, item: item, item_quantity: 3, from: create(:storage_location), to: storage_location) - end - - it "returns the total quantity change for the item" do - expect(item.quantity_change(storage_location.id)).to eq(5 + 10 - 5 - 2 + 3) - end - end end describe "default_quantity" do diff --git a/spec/models/line_item_spec.rb b/spec/models/line_item_spec.rb index 89369d6c82..f8721b1e5d 100644 --- a/spec/models/line_item_spec.rb +++ b/spec/models/line_item_spec.rb @@ -55,48 +55,6 @@ end.to change { described_class.active.size }.by(1) end end - - describe "->inventory_in_storage" do - let(:storage_location) { create(:storage_location) } - let(:item) { create(:item) } - - before do - create(:donation, :with_items, item: item, item_quantity: 5, storage_location: storage_location) - create(:purchase, :with_items, item: item, item_quantity: 10, storage_location: storage_location) - create(:adjustment, :with_items, item: item, item_quantity: -2, storage_location: storage_location) - create(:transfer, :with_items, item: item, item_quantity: 3, from: create(:storage_location), to: storage_location) - end - - it "returns line items that are in the specified storage location" do - expect(described_class.inventory_in_storage(storage_location.id).count).to eq(4) - end - - it "returns an empty collection if no line items are in the specified storage location" do - other_storage_location = create(:storage_location) - expect(described_class.inventory_in_storage(other_storage_location.id)).to be_empty - end - end - - describe "->inventory_out_storage" do - let(:storage_location) { create(:storage_location) } - let(:item) { create(:item) } - - before do - create(:donation, :with_items, item: item, item_quantity: 10, storage_location: storage_location) - create(:distribution, :with_items, item: item, item_quantity: 5, storage_location: storage_location) - create(:adjustment, :with_items, item: item, item_quantity: -2, storage_location: storage_location) - create(:transfer, :with_items, item: item, item_quantity: 3, from: storage_location, to: create(:storage_location)) - end - - it "returns line items that are out of the specified storage location" do - expect(described_class.inventory_out_storage(storage_location.id).count).to eq(2) - end - - it "returns an empty collection if no line items are out of the specified storage location" do - other_storage_location = create(:storage_location) - expect(described_class.inventory_out_storage(other_storage_location.id)).to be_empty - end - end end describe 'Methods >' do diff --git a/spec/system/storage_location_system_spec.rb b/spec/system/storage_location_system_spec.rb index 02a3581898..15dc14f3ac 100644 --- a/spec/system/storage_location_system_spec.rb +++ b/spec/system/storage_location_system_spec.rb @@ -184,6 +184,30 @@ context "when viewing an existing storage location" do let(:items) { create_list(:item, 2) } let!(:storage_location) { create(:storage_location, name: "here") } + let(:result) do + [ + { + item_id: items[0].id, + item_name: items[0].name, + quantity_in: 10, + quantity_out: 5, + change: 5, + total_quantity_in: 16, + total_quantity_out: 7, + total_change: 9 + }, + { + item_id: items[1].id, + item_name: items[1].name, + quantity_in: 6, + quantity_out: 2, + change: 4, + total_quantity_in: 16, + total_quantity_out: 7, + total_change: 9 + } + ].map(&:with_indifferent_access) + end subject { storage_location_path(storage_location.id) } context "Inventory Flow Tab" do @@ -200,20 +224,20 @@ it "shows the inventory flow for the storage location" do within("#custom-tabs-inventory-flow table tbody") do - items.each do |item| - row = find(:css, "tr[id='#{item.id}']") - change_column_css = item.quantity_change(storage_location).negative? ? "td.modal-body-warning-text" : "td" - expect(row).to have_link(item.name, href: item_path(item.id)) - expect(row).to have_css("td", text: item.quantity_in_storage(storage_location.id)) - expect(row).to have_css("td", text: item.quantity_out_storage(storage_location.id)) - expect(row).to have_css(change_column_css, text: item.quantity_change(storage_location.id)) + result.each do |item| + row = find(:css, "tr[id='#{item[:item_id]}']") + change_column_css = item[:change].negative? ? "td.modal-body-warning-text" : "td" + expect(row).to have_link(item[:name], href: item_path(item[:item_id])) + expect(row).to have_css("td", text: item[:quantity_in]) + expect(row).to have_css("td", text: item[:quantity_out]) + expect(row).to have_css(change_column_css, text: item[:change]) end end within("#custom-tabs-inventory-flow table tfoot") do expect(page).to have_css("td", text: "Total") - expect(page).to have_css("td", text: items.sum { |item| item.quantity_in_storage(storage_location.id) }) - expect(page).to have_css("td", text: items.sum { |item| item.quantity_out_storage(storage_location.id) }) - expect(page).to have_css("td", text: items.sum { |item| item.quantity_change(storage_location.id) }) + expect(page).to have_css("td", text: result.first[:total_quantity_in]) + expect(page).to have_css("td", text: result.first[:total_quantity_out]) + expect(page).to have_css("td", text: result.first[:total_change]) end end @@ -221,6 +245,20 @@ let!(:start_date) { 2.days.ago } let!(:end_date) { 1.day.ago } let!(:item) { create(:item, name: "Filtered Item", created_at: start_date) } + let(:result) do + [ + { + item_id: item.id, + item_name: item.name, + quantity_in: 10, + quantity_out: 0, + change: 10, + total_quantity_in: 10, + total_quantity_out: 0, + total_change: 10 + } + ].map(&:with_indifferent_access) + end before do create(:donation, :with_items, item: item, item_quantity: 10, storage_location: storage_location) fill_in "filters[date_range]", with: "#{start_date} - #{end_date}" @@ -231,11 +269,11 @@ it "filters the inventory flow by date range" do within("#custom-tabs-inventory-flow table tbody") do expect(page).to have_css("tr", count: 1) - row = find(:css, "tr[id='#{item.id}']") - expect(row).to have_link(item.name, href: item_path(item.id)) - expect(row).to have_css("td", text: item.quantity_in_storage(storage_location.id)) - expect(row).to have_css("td", text: item.quantity_out_storage(storage_location.id)) - expect(row).to have_css("td", text: item.quantity_change(storage_location.id)) + row = find(:css, "tr[id='#{result.first[:item_id]}']") + expect(row).to have_link(result.first[:name], href: item_path(result.first[:item_id])) + expect(row).to have_css("td", text: result.first[:quantity_in]) + expect(row).to have_css("td", text: result.first[:quantity_out]) + expect(row).to have_css("td", text: result.first[:change]) end end end From 5233723c6aa39caf1eee75fb0ef904898308e7f2 Mon Sep 17 00:00:00 2001 From: Olexii Kasianenko Date: Fri, 26 Sep 2025 17:37:00 +0300 Subject: [PATCH 05/11] Fix date filtering in ItemsFlowQuery to use item creation timestamps --- app/controllers/storage_locations_controller.rb | 4 ++-- app/queries/items_flow_query.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/storage_locations_controller.rb b/app/controllers/storage_locations_controller.rb index df5bfa4f58..db19606952 100644 --- a/app/controllers/storage_locations_controller.rb +++ b/app/controllers/storage_locations_controller.rb @@ -166,8 +166,8 @@ def date_range return if filter_params[:date_range].blank? date_range = filter_params[:date_range].split(" - ") - start_date = Date.parse(date_range[0]) - end_date = Date.parse(date_range[1]) + start_date = Date.parse(date_range[0]).beginning_of_day + end_date = Date.parse(date_range[1]).end_of_day [start_date, end_date] end end diff --git a/app/queries/items_flow_query.rb b/app/queries/items_flow_query.rb index 883df45848..7302d7addc 100644 --- a/app/queries/items_flow_query.rb +++ b/app/queries/items_flow_query.rb @@ -54,7 +54,7 @@ def call LEFT JOIN adjustments ON adjustments.id = li.itemizable_id AND li.itemizable_type = 'Adjustment' LEFT JOIN transfers ON transfers.id = li.itemizable_id AND li.itemizable_type = 'Transfer' LEFT JOIN items it ON it.id = li.item_id - WHERE it.created_at BETWEEN :start_date AND :end_date + WHERE li.created_at >= :start_date AND li.created_at <= :end_date ) SELECT item_id, From 0eda0354215c9c51f1d4edd7f5882e5567b6017e Mon Sep 17 00:00:00 2001 From: Olexii Kasianenko Date: Fri, 26 Sep 2025 19:11:32 +0300 Subject: [PATCH 06/11] Fix date range filtering in storage location system spec to ensure accurate item creation timestamps --- spec/system/storage_location_system_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/system/storage_location_system_spec.rb b/spec/system/storage_location_system_spec.rb index 15dc14f3ac..6d3f20a27d 100644 --- a/spec/system/storage_location_system_spec.rb +++ b/spec/system/storage_location_system_spec.rb @@ -244,7 +244,7 @@ context "date range filter" do let!(:start_date) { 2.days.ago } let!(:end_date) { 1.day.ago } - let!(:item) { create(:item, name: "Filtered Item", created_at: start_date) } + let!(:item) { create(:item, name: "Filtered Item") } let(:result) do [ { @@ -260,7 +260,8 @@ ].map(&:with_indifferent_access) end before do - create(:donation, :with_items, item: item, item_quantity: 10, storage_location: storage_location) + donation = create(:donation, :with_items, item: item, item_quantity: 10, storage_location: storage_location) + donation.line_items.update_all(created_at: start_date) fill_in "filters[date_range]", with: "#{start_date} - #{end_date}" click_button "Filter" find("#custom-tabs-inventory-flow-tab").click From 216dc2c4a535027cfabc2631a23a67affc4fa10c Mon Sep 17 00:00:00 2001 From: Olexii Kasianenko Date: Fri, 26 Sep 2025 19:23:10 +0300 Subject: [PATCH 07/11] Refactor item creation in flow query specs for improved clarity and accuracy --- spec/queries/items_flow_query_spec.rb | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/spec/queries/items_flow_query_spec.rb b/spec/queries/items_flow_query_spec.rb index 40a1316819..70138c58c1 100644 --- a/spec/queries/items_flow_query_spec.rb +++ b/spec/queries/items_flow_query_spec.rb @@ -49,19 +49,16 @@ context "with filter params" do let(:filter_params) { [11.days.ago, 9.days.ago] } - let!(:old_items) do - [ - create(:item, organization: organization, created_at: 10.days.ago.beginning_of_day), - create(:item, organization: organization, created_at: 10.days.ago.end_of_day) - ] - end + let!(:old_items) { create_list(:item, 2) } let(:other_location) { create(:storage_location, organization: organization) } subject { described_class.new(organization: organization, storage_location: storage_location, filter_params: filter_params).call } before do - create(:donation, :with_items, item: old_items[0], item_quantity: 10, storage_location: storage_location) - create(:distribution, :with_items, item: old_items[1], item_quantity: 5, storage_location: storage_location) + donation = create(:donation, :with_items, item: old_items[0], item_quantity: 10, storage_location: storage_location) + donation.line_items.update_all(created_at: 10.days.ago) + distribution = create(:distribution, :with_items, item: old_items[1], item_quantity: 5, storage_location: storage_location) + distribution.line_items.update_all(created_at: 10.days.ago) end let(:filtered_result) do From 0d50f31009399af907b4d336fc8c3c4195d3e90e Mon Sep 17 00:00:00 2001 From: Olexii Kasianenko Date: Thu, 2 Oct 2025 18:01:35 +0300 Subject: [PATCH 08/11] Refactor inventory flow query to utilize event-based data for improved accuracy and clarity --- app/queries/items_flow_query.rb | 94 ++++++++++----------- spec/queries/items_flow_query_spec.rb | 17 ++-- spec/system/storage_location_system_spec.rb | 14 +-- 3 files changed, 63 insertions(+), 62 deletions(-) diff --git a/app/queries/items_flow_query.rb b/app/queries/items_flow_query.rb index 7302d7addc..3831ccf782 100644 --- a/app/queries/items_flow_query.rb +++ b/app/queries/items_flow_query.rb @@ -13,62 +13,56 @@ def initialize(organization:, storage_location:, filter_params: nil) def call query = <<~SQL - WITH line_items_with_flags AS ( - SELECT - li.item_id, - it.name AS item_name, + with events_with_flags as ( + select it.id as item_id, + it.name as item_name, -- in quantity for this row (0 if not matching) - CASE - WHEN (donations.storage_location_id = :id - OR purchases.storage_location_id = :id - OR (adjustments.storage_location_id = :id AND li.quantity > 0) - OR transfers.to_id = :id) - AND it.organization_id = :organization_id - THEN li.quantity - ELSE 0 - END AS quantity_in, + case + when (e.type = 'DonationEvent' and (item->>'to_storage_location')::int = :id) + or (e.type = 'PurchaseEvent' and (item->>'to_storage_location')::int = :id) + or (e.type = 'AdjustmentEvent' and (item->>'to_storage_location')::int = :id) + or (e.type = 'TransferEvent' and (item->>'to_storage_location')::int = :id) + and e.organization_id = :organization_id + then (item->>'quantity')::int + else 0 + end as quantity_in, -- out quantity normalized to positive numbers (0 if not matching) - CASE - WHEN (distributions.storage_location_id = :id - OR (adjustments.storage_location_id = :id AND li.quantity < 0) - OR transfers.from_id = :id) - AND it.organization_id = :organization_id - THEN CASE WHEN li.quantity < 0 THEN -li.quantity ELSE li.quantity END - ELSE 0 - END AS quantity_out, + case + when (e.type = 'DistributionEvent' and (item->>'from_storage_location')::int = :id) + or (e.type = 'AdjustmentEvent' and (item->>'from_storage_location')::int = :id) + or (e.type = 'TransferEvent' and (item->>'from_storage_location')::int = :id) + and e.organization_id = :organization_id + then case when (item->>'quantity')::int < 0 then -(item->>'quantity')::int else (item->>'quantity')::int end + else 0 + end as quantity_out, -- mark rows that are relevant for the overall WHERE in original query - CASE - WHEN (donations.storage_location_id = :id - OR purchases.storage_location_id = :id - OR distributions.storage_location_id = :id - OR transfers.from_id = :id - OR transfers.to_id = :id - OR adjustments.storage_location_id = :id) - AND it.organization_id = :organization_id - THEN 1 ELSE 0 - END AS relevant - FROM line_items li - LEFT JOIN donations ON donations.id = li.itemizable_id AND li.itemizable_type = 'Donation' - LEFT JOIN purchases ON purchases.id = li.itemizable_id AND li.itemizable_type = 'Purchase' - LEFT JOIN distributions ON distributions.id = li.itemizable_id AND li.itemizable_type = 'Distribution' - LEFT JOIN adjustments ON adjustments.id = li.itemizable_id AND li.itemizable_type = 'Adjustment' - LEFT JOIN transfers ON transfers.id = li.itemizable_id AND li.itemizable_type = 'Transfer' - LEFT JOIN items it ON it.id = li.item_id - WHERE li.created_at >= :start_date AND li.created_at <= :end_date + case + when ( (e.type = 'DonationEvent' and (item->>'to_storage_location')::int = :id) + or (e.type = 'PurchaseEvent' and (item->>'to_storage_location')::int = :id) + or (e.type = 'DistributionEvent' and (item->>'from_storage_location')::int = :id) + or (e.type = 'TransferEvent' and ((item->>'from_storage_location')::int = :id or (item->>'to_storage_location')::int = :id)) + or (e.type = 'AdjustmentEvent' and (item->>'from_storage_location')::int = :id or (item->>'to_storage_location')::int = :id) + ) and e.organization_id = :organization_id + then 1 else 0 + end as relevant + from events e + left join lateral jsonb_array_elements(data->'items') as item on true + left join items it on it.id = (item->>'item_id')::int and it.organization_id = :organization_id + where e.created_at >= :start_date and e.created_at <= :end_date ) - SELECT + select item_id, item_name, - SUM(quantity_in) AS quantity_in, - SUM(quantity_out) AS quantity_out, - SUM(quantity_in) - SUM(quantity_out) AS change, - SUM(SUM(quantity_in)) OVER () AS total_quantity_in, - SUM(SUM(quantity_out)) OVER () AS total_quantity_out, - SUM(SUM(quantity_in) - SUM(quantity_out)) OVER () AS total_change - FROM line_items_with_flags - WHERE relevant = 1 - GROUP BY item_id, item_name - ORDER BY item_name; + sum(quantity_in) as quantity_in, + sum(quantity_out) as quantity_out, + sum(quantity_in) - sum(quantity_out) as change, + sum(sum(quantity_in)) over () as total_quantity_in, + sum(sum(quantity_out)) over () as total_quantity_out, + sum(sum(quantity_in) - sum(quantity_out)) over () as total_change + from events_with_flags + where relevant = 1 + group by item_id, item_name + order by item_name; SQL ActiveRecord::Base.connection.exec_query( diff --git a/spec/queries/items_flow_query_spec.rb b/spec/queries/items_flow_query_spec.rb index 70138c58c1..d290b1a764 100644 --- a/spec/queries/items_flow_query_spec.rb +++ b/spec/queries/items_flow_query_spec.rb @@ -33,10 +33,13 @@ before do create(:donation, :with_items, item: items[0], item_quantity: 10, storage_location: storage_location) - create(:distribution, :with_items, item: items[0], item_quantity: 5, storage_location: storage_location) + distribution = create(:distribution, :with_items, item: items[0], item_quantity: 5, storage_location: storage_location) + DistributionEvent.publish(distribution) create(:donation, :with_items, item: items[1], item_quantity: 3, storage_location: storage_location) - create(:adjustment, :with_items, item: items[1], item_quantity: 3, storage_location: storage_location) - create(:transfer, :with_items, item: items[1], item_quantity: 2, from: storage_location, to: create(:storage_location)) + adjustment = create(:adjustment, :with_items, item: items[1], item_quantity: 3, storage_location: storage_location) + AdjustmentEvent.publish(adjustment) + transfer = create(:transfer, :with_items, item: items[1], item_quantity: 2, from: storage_location, to: create(:storage_location)) + TransferEvent.publish(transfer) end subject { described_class.new(organization: organization, storage_location: storage_location).call } @@ -55,10 +58,12 @@ subject { described_class.new(organization: organization, storage_location: storage_location, filter_params: filter_params).call } before do - donation = create(:donation, :with_items, item: old_items[0], item_quantity: 10, storage_location: storage_location) - donation.line_items.update_all(created_at: 10.days.ago) + create(:donation, :with_items, item: old_items[0], item_quantity: 10, storage_location: storage_location) + Event.last.update(created_at: 10.days.ago) + create(:donation, :with_items, item: old_items[1], item_quantity: 8, storage_location: storage_location) distribution = create(:distribution, :with_items, item: old_items[1], item_quantity: 5, storage_location: storage_location) - distribution.line_items.update_all(created_at: 10.days.ago) + DistributionEvent.publish(distribution) + Event.last.update(created_at: 10.days.ago) end let(:filtered_result) do diff --git a/spec/system/storage_location_system_spec.rb b/spec/system/storage_location_system_spec.rb index 6d3f20a27d..e2d8b3b6b6 100644 --- a/spec/system/storage_location_system_spec.rb +++ b/spec/system/storage_location_system_spec.rb @@ -213,11 +213,13 @@ context "Inventory Flow Tab" do before do create(:donation, :with_items, item: items[0], item_quantity: 10, storage_location: storage_location) - create(:distribution, :with_items, item: items[0], item_quantity: 5, storage_location: storage_location) + distribution = create(:distribution, :with_items, item: items[0], item_quantity: 5, storage_location: storage_location) + DistributionEvent.publish(distribution) create(:donation, :with_items, item: items[1], item_quantity: 3, storage_location: storage_location) - create(:adjustment, :with_items, item: items[1], item_quantity: 3, storage_location: storage_location) - create(:transfer, :with_items, item: items[1], item_quantity: 2, from: storage_location, to: create(:storage_location)) - + adjustment = create(:adjustment, :with_items, item: items[1], item_quantity: 3, storage_location: storage_location) + AdjustmentEvent.publish(adjustment) + transfer = create(:transfer, :with_items, item: items[1], item_quantity: 2, from: storage_location, to: create(:storage_location)) + TransferEvent.publish(transfer) visit subject find("#custom-tabs-inventory-flow-tab").click end @@ -260,8 +262,8 @@ ].map(&:with_indifferent_access) end before do - donation = create(:donation, :with_items, item: item, item_quantity: 10, storage_location: storage_location) - donation.line_items.update_all(created_at: start_date) + create(:donation, :with_items, item: item, item_quantity: 10, storage_location: storage_location) + Event.last.update(created_at: start_date) fill_in "filters[date_range]", with: "#{start_date} - #{end_date}" click_button "Filter" find("#custom-tabs-inventory-flow-tab").click From bfd1db67ce221ee450d1033ea369beeb6c4d62ac Mon Sep 17 00:00:00 2001 From: Olexii Kasianenko Date: Mon, 27 Oct 2025 16:18:16 +0200 Subject: [PATCH 09/11] Enhance inventory flow query to include AuditEvent handling and update related specs for accurate quantity calculations --- app/queries/items_flow_query.rb | 2 ++ spec/queries/items_flow_query_spec.rb | 14 ++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/queries/items_flow_query.rb b/app/queries/items_flow_query.rb index 3831ccf782..2b72d0769a 100644 --- a/app/queries/items_flow_query.rb +++ b/app/queries/items_flow_query.rb @@ -22,6 +22,7 @@ def call or (e.type = 'PurchaseEvent' and (item->>'to_storage_location')::int = :id) or (e.type = 'AdjustmentEvent' and (item->>'to_storage_location')::int = :id) or (e.type = 'TransferEvent' and (item->>'to_storage_location')::int = :id) + or (e.type = 'AuditEvent' and (item->>'to_storage_location')::int = :id) and e.organization_id = :organization_id then (item->>'quantity')::int else 0 @@ -31,6 +32,7 @@ def call when (e.type = 'DistributionEvent' and (item->>'from_storage_location')::int = :id) or (e.type = 'AdjustmentEvent' and (item->>'from_storage_location')::int = :id) or (e.type = 'TransferEvent' and (item->>'from_storage_location')::int = :id) + or (e.type = 'AuditEvent' and (item->>'from_storage_location')::int = :id) and e.organization_id = :organization_id then case when (item->>'quantity')::int < 0 then -(item->>'quantity')::int else (item->>'quantity')::int end else 0 diff --git a/spec/queries/items_flow_query_spec.rb b/spec/queries/items_flow_query_spec.rb index d290b1a764..6634be8bff 100644 --- a/spec/queries/items_flow_query_spec.rb +++ b/spec/queries/items_flow_query_spec.rb @@ -14,19 +14,19 @@ quantity_in: 10, quantity_out: 5, change: 5, - total_quantity_in: 16, + total_quantity_in: 19, total_quantity_out: 7, - total_change: 9 + total_change: 12 }, { item_id: items[1].id, item_name: items[1].name, - quantity_in: 6, + quantity_in: 9, quantity_out: 2, - change: 4, - total_quantity_in: 16, + change: 7, + total_quantity_in: 19, total_quantity_out: 7, - total_change: 9 + total_change: 12 } ].map(&:with_indifferent_access) end @@ -40,6 +40,8 @@ AdjustmentEvent.publish(adjustment) transfer = create(:transfer, :with_items, item: items[1], item_quantity: 2, from: storage_location, to: create(:storage_location)) TransferEvent.publish(transfer) + audit = create(:audit, :with_items, item: items[1], item_quantity: 3, adjustment: adjustment, storage_location: storage_location) + AuditEvent.publish(audit) end subject { described_class.new(organization: organization, storage_location: storage_location).call } From 1898ec6e3b7261ec5a767d48dc38346ba0a0c77c Mon Sep 17 00:00:00 2001 From: Brock Wilcox Date: Sun, 19 Jul 2026 18:00:39 -0400 Subject: [PATCH 10/11] Fix flow accounting: event versions, kit events, and audit deltas Validating against a production snapshot showed the flow tab's numbers diverging from inventory by millions of units. Three causes: 1. Editing a donation/purchase/distribution/transfer/adjustment republishes the full event for the same eventable, and inventory replay diffs against the previous version - but the query counted every version at full value. Dedupe to the latest version per eventable (destroy events publish zeroed items, so destroyed records net to nothing and drop out). 2. Kit allocation/deallocation events were not counted at all. On the heaviest production location that is 1.6M items consumed into kits that the tab silently omitted. They now flow like any other event (each row applies in full, matching InventoryAggregate). 3. Audits record the absolute counted quantity, not a delta, so the tab showed e.g. 'audited at 5000' as 5000 in. Audit contributions are now computed as deltas via a single replay pass through the event log. Also collapse the per-type to/from conditions (which had and/or precedence bugs) into generic to/from matching over an explicit list of flow event types, filter by event_time rather than created_at to match replay semantics, drop all-zero rows, and move totals to Ruby. With these fixes, snapshot + in - out reconciles with current inventory to within a few hundred units over multi-year windows moving 7M+ units on production data (residuals trace to UpdateExistingEvent group handling quirks), and the page renders in under a second on the largest production org. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01BNy2e1J81K8xbvXraCeN8e --- app/queries/items_flow_query.rb | 177 +++++++++++++++----- spec/queries/items_flow_query_spec.rb | 83 +++++++-- spec/system/storage_location_system_spec.rb | 2 +- 3 files changed, 208 insertions(+), 54 deletions(-) diff --git a/app/queries/items_flow_query.rb b/app/queries/items_flow_query.rb index 2b72d0769a..dc60c06e51 100644 --- a/app/queries/items_flow_query.rb +++ b/app/queries/items_flow_query.rb @@ -5,6 +5,27 @@ class ItemsFlowQuery attr_reader :filter_params attr_reader :storage_location + # Event types where editing the record republishes the full event for the + # same eventable and inventory replay diffs against the previous version + # (see InventoryAggregate). For flow purposes only the latest version per + # eventable counts. Destroy events publish zeroed line items, so destroyed + # records net out to nothing. + VERSIONED_EVENT_TYPES = %w[ + DonationEvent PurchaseEvent DistributionEvent TransferEvent AdjustmentEvent + DonationDestroyEvent PurchaseDestroyEvent DistributionDestroyEvent + TransferDestroyEvent UpdateExistingEvent + ].freeze + + # Event types where every event stands alone and applies in full + # ("ignore previous events" in InventoryAggregate). + # + # AuditEvent deliberately appears in neither list: audit payloads store the + # absolute counted quantity rather than a movement, so their flow + # contribution can't be summed in SQL - see #apply_audit_deltas, which + # derives each audit's delta by replaying the event log. Adding AuditEvent + # here would double-count it. + STANDALONE_EVENT_TYPES = %w[KitAllocateEvent KitDeallocateEvent].freeze + def initialize(organization:, storage_location:, filter_params: nil) @organization = organization @storage_location = storage_location @@ -12,68 +33,140 @@ def initialize(organization:, storage_location:, filter_params: nil) end def call + flows = sql_flows + apply_audit_deltas(flows) + + rows = flows.filter_map do |item_id, flow| + next if flow[:in].zero? && flow[:out].zero? + + { + "item_id" => item_id, + "item_name" => flow[:name], + "quantity_in" => flow[:in], + "quantity_out" => flow[:out], + "change" => flow[:in] - flow[:out] + } + end + rows.sort_by! { |row| row["item_name"].to_s } + + total_in = rows.sum { |row| row["quantity_in"] } + total_out = rows.sum { |row| row["quantity_out"] } + rows.each do |row| + row["total_quantity_in"] = total_in + row["total_quantity_out"] = total_out + row["total_change"] = total_in - total_out + end + + rows + end + + private + + def start_date + @filter_params ? @filter_params[0] : 20.years.ago + end + + def end_date + @filter_params ? @filter_params[1] : Time.current.end_of_day + end + + # @return [Hash] item_id => {name:, in:, out:} + def sql_flows query = <<~SQL - with events_with_flags as ( + with latest_versioned_events as ( + select distinct on (eventable_type, eventable_id) * + from events + where organization_id = :organization_id + and type in (:versioned_types) + order by eventable_type, eventable_id, updated_at desc + ), + flow_events as ( + select id, data from latest_versioned_events + where event_time >= :start_date and event_time <= :end_date + union all + select id, data from events + where organization_id = :organization_id + and type in (:standalone_types) + and event_time >= :start_date and event_time <= :end_date + ), + item_flows as ( select it.id as item_id, it.name as item_name, - -- in quantity for this row (0 if not matching) - case - when (e.type = 'DonationEvent' and (item->>'to_storage_location')::int = :id) - or (e.type = 'PurchaseEvent' and (item->>'to_storage_location')::int = :id) - or (e.type = 'AdjustmentEvent' and (item->>'to_storage_location')::int = :id) - or (e.type = 'TransferEvent' and (item->>'to_storage_location')::int = :id) - or (e.type = 'AuditEvent' and (item->>'to_storage_location')::int = :id) - and e.organization_id = :organization_id + case when (item->>'to_storage_location')::int = :id then (item->>'quantity')::int else 0 end as quantity_in, - -- out quantity normalized to positive numbers (0 if not matching) - case - when (e.type = 'DistributionEvent' and (item->>'from_storage_location')::int = :id) - or (e.type = 'AdjustmentEvent' and (item->>'from_storage_location')::int = :id) - or (e.type = 'TransferEvent' and (item->>'from_storage_location')::int = :id) - or (e.type = 'AuditEvent' and (item->>'from_storage_location')::int = :id) - and e.organization_id = :organization_id - then case when (item->>'quantity')::int < 0 then -(item->>'quantity')::int else (item->>'quantity')::int end + -- out quantities normalized to positive numbers + case when (item->>'from_storage_location')::int = :id + then abs((item->>'quantity')::int) else 0 - end as quantity_out, - -- mark rows that are relevant for the overall WHERE in original query - case - when ( (e.type = 'DonationEvent' and (item->>'to_storage_location')::int = :id) - or (e.type = 'PurchaseEvent' and (item->>'to_storage_location')::int = :id) - or (e.type = 'DistributionEvent' and (item->>'from_storage_location')::int = :id) - or (e.type = 'TransferEvent' and ((item->>'from_storage_location')::int = :id or (item->>'to_storage_location')::int = :id)) - or (e.type = 'AdjustmentEvent' and (item->>'from_storage_location')::int = :id or (item->>'to_storage_location')::int = :id) - ) and e.organization_id = :organization_id - then 1 else 0 - end as relevant - from events e - left join lateral jsonb_array_elements(data->'items') as item on true + end as quantity_out + from flow_events e + join lateral jsonb_array_elements(e.data->'items') as item on true left join items it on it.id = (item->>'item_id')::int and it.organization_id = :organization_id - where e.created_at >= :start_date and e.created_at <= :end_date + where (item->>'to_storage_location')::int = :id + or (item->>'from_storage_location')::int = :id ) select item_id, item_name, sum(quantity_in) as quantity_in, - sum(quantity_out) as quantity_out, - sum(quantity_in) - sum(quantity_out) as change, - sum(sum(quantity_in)) over () as total_quantity_in, - sum(sum(quantity_out)) over () as total_quantity_out, - sum(sum(quantity_in) - sum(quantity_out)) over () as total_change - from events_with_flags - where relevant = 1 + sum(quantity_out) as quantity_out + from item_flows group by item_id, item_name - order by item_name; SQL - ActiveRecord::Base.connection.exec_query( + result = ActiveRecord::Base.connection.exec_query( ActiveRecord::Base.send(:sanitize_sql_array, [query, { id: @storage_location.id, organization_id: @organization.id, - start_date: @filter_params ? @filter_params[0] : 20.years.ago, - end_date: @filter_params ? @filter_params[1] : Time.current.end_of_day + versioned_types: VERSIONED_EVENT_TYPES, + standalone_types: STANDALONE_EVENT_TYPES, + start_date: start_date, + end_date: end_date }]) ) + + result.each_with_object({}) do |row, flows| + flows[row["item_id"]] = { + name: row["item_name"], + in: row["quantity_in"].to_i, + out: row["quantity_out"].to_i + } + end + end + + # Audits record the absolute counted quantity, not a delta, so their flow + # contribution is (counted quantity - quantity just before the audit). + # Replay the event log once, tracking the audited items' quantities at this + # location, to turn each in-range audit into a delta. + def apply_audit_deltas(flows) + audits = AuditEvent.where(organization_id: @organization.id, event_time: start_date..end_date) + audited_item_ids = audits.flat_map { |audit| audit.data.items.map(&:item_id) }.uniq + return if audited_item_ids.empty? + + audit_ids = audits.map(&:id).to_set + quantities_before = Hash.new(0) + + InventoryAggregate.inventory_for(@organization.id) do |event, inventory| + if audit_ids.include?(event.id) + event.data.items.each do |line_item| + next unless line_item.to_storage_location == @storage_location.id + + delta = line_item.quantity - quantities_before[line_item.item_id] + flow = flows[line_item.item_id] ||= {name: Item.find_by(id: line_item.item_id)&.name, in: 0, out: 0} + if delta.positive? + flow[:in] += delta + else + flow[:out] += delta.abs + end + end + end + + location = inventory.storage_locations[@storage_location.id] + audited_item_ids.each do |item_id| + quantities_before[item_id] = location&.items&.[](item_id)&.quantity.to_i + end + end end end diff --git a/spec/queries/items_flow_query_spec.rb b/spec/queries/items_flow_query_spec.rb index 6634be8bff..a7bc847d44 100644 --- a/spec/queries/items_flow_query_spec.rb +++ b/spec/queries/items_flow_query_spec.rb @@ -14,19 +14,22 @@ quantity_in: 10, quantity_out: 5, change: 5, - total_quantity_in: 19, - total_quantity_out: 7, - total_change: 12 + total_quantity_in: 16, + total_quantity_out: 8, + total_change: 8 }, { + # Donation (+3) and adjustment (+3) flow in; the transfer (-2) flows + # out. The audit counts an absolute quantity of 3 when the running + # quantity is 4, so it contributes its delta: 1 more out. item_id: items[1].id, item_name: items[1].name, - quantity_in: 9, - quantity_out: 2, - change: 7, - total_quantity_in: 19, - total_quantity_out: 7, - total_change: 12 + quantity_in: 6, + quantity_out: 3, + change: 3, + total_quantity_in: 16, + total_quantity_out: 8, + total_change: 8 } ].map(&:with_indifferent_access) end @@ -61,11 +64,11 @@ before do create(:donation, :with_items, item: old_items[0], item_quantity: 10, storage_location: storage_location) - Event.last.update(created_at: 10.days.ago) + Event.last.update(event_time: 10.days.ago) create(:donation, :with_items, item: old_items[1], item_quantity: 8, storage_location: storage_location) distribution = create(:distribution, :with_items, item: old_items[1], item_quantity: 5, storage_location: storage_location) DistributionEvent.publish(distribution) - Event.last.update(created_at: 10.days.ago) + Event.last.update(event_time: 10.days.ago) end let(:filtered_result) do @@ -97,4 +100,62 @@ expect(subject.to_a).to match_array(filtered_result) end end + + context "when a donation has been edited" do + let!(:fresh_organization) { create(:organization) } + let(:location) { create(:storage_location, organization: fresh_organization) } + let(:item) { create(:item, organization: fresh_organization) } + + subject { described_class.new(organization: fresh_organization, storage_location: location).call } + + it "only counts the latest version of the donation" do + donation = create(:donation, :with_items, item: item, item_quantity: 10, storage_location: location, organization: fresh_organization) + donation.line_items.first.update!(quantity: 25) + DonationEvent.publish(donation) + + row = subject.to_a.find { |r| r["item_id"] == item.id } + expect(row["quantity_in"]).to eq(25) + end + end + + context "when a donation has been destroyed" do + let!(:fresh_organization) { create(:organization) } + let(:location) { create(:storage_location, organization: fresh_organization) } + let(:item) { create(:item, organization: fresh_organization) } + + subject { described_class.new(organization: fresh_organization, storage_location: location).call } + + it "does not count the destroyed donation or list its item" do + donation = create(:donation, :with_items, item: item, item_quantity: 10, storage_location: location, organization: fresh_organization) + DonationDestroyEvent.publish(donation) + + expect(subject.to_a).to be_empty + end + end + + context "with kit allocations" do + let!(:fresh_organization) { create(:organization) } + let(:location) { create(:storage_location, organization: fresh_organization) } + let(:content_item) { create(:item, organization: fresh_organization) } + let(:kit) do + kit_params = { + organization_id: fresh_organization.id, + name: "Flow Test Kit", + line_items_attributes: [{item_id: content_item.id, quantity: 2}] + } + KitCreateService.new(organization_id: fresh_organization.id, kit_params: kit_params).tap(&:call).kit + end + + subject { described_class.new(organization: fresh_organization, storage_location: location).call } + + it "counts kit contents flowing out and assembled kits flowing in" do + create(:donation, :with_items, item: content_item, item_quantity: 10, storage_location: location, organization: fresh_organization) + KitAllocateEvent.publish(kit, location.id, 3) + + rows = subject.to_a.index_by { |r| r["item_id"] } + expect(rows[kit.id]["quantity_in"]).to eq(3) + expect(rows[content_item.id]["quantity_in"]).to eq(10) + expect(rows[content_item.id]["quantity_out"]).to eq(6) + end + end end diff --git a/spec/system/storage_location_system_spec.rb b/spec/system/storage_location_system_spec.rb index 2cf53936fb..f1ab5e1314 100644 --- a/spec/system/storage_location_system_spec.rb +++ b/spec/system/storage_location_system_spec.rb @@ -265,7 +265,7 @@ end before do create(:donation, :with_items, item: item, item_quantity: 10, storage_location: storage_location) - Event.last.update(created_at: start_date) + Event.last.update(event_time: start_date) fill_in "filters[date_range]", with: "#{start_date} - #{end_date}" click_button "Filter" find("#custom-tabs-inventory-flow-tab").click From d141971088d4c1d89473d2b1d13d1eae13a47134 Mon Sep 17 00:00:00 2001 From: Brock Wilcox Date: Sun, 19 Jul 2026 20:52:20 -0400 Subject: [PATCH 11/11] Add start/end quantity columns and a separate adjustment/audit column The flow tab now shows, per item: Quantity at Start, Quantity In, Quantity Out, Quantity Adjustment/Audit, Quantity Change, and Quantity at End, so each row self-validates (start + in - out + adjustment = end) and users can see stock levels bracketing the flows. - Start/end quantities come from raw InventoryAggregate state at the window boundaries (View::Inventory prunes inactive items, so the raw aggregate is used instead) - Adjustments move out of in/out into the combined adjustment/audit column, netted with their signed quantities; audit deltas land there too. The audit replay pass now doubles as the window-end state computation, so the whole page needs only two replays - The effective window start is clamped to just after the organization's most recent usable snapshot: inventory state cannot be derived from events before it, and unclamped ranges silently double-counted pre-snapshot events against the snapshot baseline - Rows appear when they have flow activity or a start/end difference Validated against a production snapshot: row identity holds for all but 1-7 rows per organization (rare event-correction corner cases in aggregate diff ordering, off by tens of units against multi-million unit flows), totals reconcile start to end, and the page renders in under a second on the largest organizations. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01BNy2e1J81K8xbvXraCeN8e --- .../storage_locations_controller.rb | 10 +- app/queries/items_flow_query.rb | 175 +++++++++++------- .../storage_locations/_item_row.html.erb | 3 + app/views/storage_locations/show.html.erb | 8 +- spec/queries/items_flow_query_spec.rb | 57 ++++-- spec/system/storage_location_system_spec.rb | 13 +- 6 files changed, 182 insertions(+), 84 deletions(-) diff --git a/app/controllers/storage_locations_controller.rb b/app/controllers/storage_locations_controller.rb index 31d89d91ca..1b3001a773 100644 --- a/app/controllers/storage_locations_controller.rb +++ b/app/controllers/storage_locations_controller.rb @@ -59,9 +59,13 @@ def show @storage_location = current_organization.storage_locations.find(params[:id]) version_date = params[:version_date].presence&.to_date @items = ItemsFlowQuery.new(storage_location: @storage_location, organization: current_organization, filter_params: date_range).call.to_a - @total_quantity_in = @items.count.positive? ? @items.first["total_quantity_in"].to_i : 0 - @total_quantity_out = @items.count.positive? ? @items.first["total_quantity_out"].to_i : 0 - @total_quantity_change = @total_quantity_in - @total_quantity_out + totals = @items.first || {} + @total_quantity_start = totals["total_quantity_start"].to_i + @total_quantity_in = totals["total_quantity_in"].to_i + @total_quantity_out = totals["total_quantity_out"].to_i + @total_quantity_adjustment = totals["total_quantity_adjustment"].to_i + @total_quantity_change = totals["total_change"].to_i + @total_quantity_end = totals["total_quantity_end"].to_i if View::Inventory.within_snapshot?(current_organization.id, version_date) @inventory = View::Inventory.new(current_organization.id, event_time: version_date) else diff --git a/app/queries/items_flow_query.rb b/app/queries/items_flow_query.rb index dc60c06e51..5be2ee6c26 100644 --- a/app/queries/items_flow_query.rb +++ b/app/queries/items_flow_query.rb @@ -21,8 +21,8 @@ class ItemsFlowQuery # # AuditEvent deliberately appears in neither list: audit payloads store the # absolute counted quantity rather than a movement, so their flow - # contribution can't be summed in SQL - see #apply_audit_deltas, which - # derives each audit's delta by replaying the event log. Adding AuditEvent + # contribution can't be summed in SQL - see the replay in #call, which + # derives each audit's delta from the running quantities. Adding AuditEvent # here would double-count it. STANDALONE_EVENT_TYPES = %w[KitAllocateEvent KitDeallocateEvent].freeze @@ -34,43 +34,116 @@ def initialize(organization:, storage_location:, filter_params: nil) def call flows = sql_flows - apply_audit_deltas(flows) + start_quantities = quantities_at(start_cutoff) + end_quantities = apply_audit_deltas_and_end_quantities(flows) - rows = flows.filter_map do |item_id, flow| - next if flow[:in].zero? && flow[:out].zero? + item_ids = (flows.keys + start_quantities.keys + end_quantities.keys).uniq + names = Item.where(id: item_ids).pluck(:id, :name).to_h + + rows = item_ids.filter_map do |item_id| + flow = flows[item_id] || {in: 0, out: 0, adjustment: 0} + start_qty = start_quantities[item_id] || 0 + end_qty = end_quantities[item_id] || 0 + next if flow[:in].zero? && flow[:out].zero? && flow[:adjustment].zero? && start_qty == end_qty { "item_id" => item_id, - "item_name" => flow[:name], + "item_name" => flow[:name] || names[item_id], + "quantity_start" => start_qty, "quantity_in" => flow[:in], "quantity_out" => flow[:out], - "change" => flow[:in] - flow[:out] + "quantity_adjustment" => flow[:adjustment], + "change" => flow[:in] - flow[:out] + flow[:adjustment], + "quantity_end" => end_qty } end rows.sort_by! { |row| row["item_name"].to_s } - total_in = rows.sum { |row| row["quantity_in"] } - total_out = rows.sum { |row| row["quantity_out"] } - rows.each do |row| - row["total_quantity_in"] = total_in - row["total_quantity_out"] = total_out - row["total_change"] = total_in - total_out - end + totals = { + "total_quantity_start" => rows.sum { |row| row["quantity_start"] }, + "total_quantity_in" => rows.sum { |row| row["quantity_in"] }, + "total_quantity_out" => rows.sum { |row| row["quantity_out"] }, + "total_quantity_adjustment" => rows.sum { |row| row["quantity_adjustment"] }, + "total_change" => rows.sum { |row| row["change"] }, + "total_quantity_end" => rows.sum { |row| row["quantity_end"] } + } + rows.each { |row| row.merge!(totals) } rows end private - def start_date - @filter_params ? @filter_params[0] : 20.years.ago + # Inventory state can only be derived back to the most recent usable + # snapshot, so the effective window starts no earlier than just after it. + def start_cutoff + @start_cutoff ||= begin + requested = @filter_params ? @filter_params[0] : 20.years.ago + snapshot = Event.most_recent_snapshot(@organization.id) + if snapshot && requested <= snapshot.event_time + snapshot.event_time + 1.second + else + requested + end + end + end + + def end_cutoff + @end_cutoff ||= begin + requested = @filter_params ? @filter_params[1] : Time.current.end_of_day + [requested, start_cutoff].max + end + end + + # @return [Hash] item_id => quantity at this location + def quantities_at(event_time) + inventory = InventoryAggregate.inventory_for(@organization.id, event_time: event_time) + location_quantities(inventory) end - def end_date - @filter_params ? @filter_params[1] : Time.current.end_of_day + def location_quantities(inventory) + items = inventory.storage_locations[@storage_location.id]&.items || {} + items.transform_values(&:quantity).reject { |_, quantity| quantity.zero? } end - # @return [Hash] item_id => {name:, in:, out:} + # Audits record the absolute counted quantity, not a delta, so their flow + # contribution is (counted quantity - quantity just before the audit). + # Replay the event log once up to the window end, tracking the audited + # items' running quantities at this location to turn each in-range audit + # into a delta; the same replay's final state is the window-end inventory. + # @return [Hash] item_id => quantity at the window end + def apply_audit_deltas_and_end_quantities(flows) + audits = AuditEvent.where(organization_id: @organization.id, event_time: start_cutoff..end_cutoff) + audited_item_ids = audits.flat_map { |audit| audit.data.items.map(&:item_id) }.uniq + audit_ids = audits.map(&:id).to_set + quantities_before = Hash.new(0) + + # The aggregate only applies an event_time cutoff that is after the last + # snapshot; when the window ends now-ish, replay without a cutoff. + cutoff = (end_cutoff >= Time.current) ? nil : end_cutoff + final_inventory = InventoryAggregate.inventory_for(@organization.id, event_time: cutoff) do |event, inventory| + next if audited_item_ids.empty? + + if audit_ids.include?(event.id) + event.data.items.each do |line_item| + next unless line_item.to_storage_location == @storage_location.id + + delta = line_item.quantity - quantities_before[line_item.item_id] + flow = flows[line_item.item_id] ||= {name: nil, in: 0, out: 0, adjustment: 0} + flow[:adjustment] += delta + end + end + + location = inventory.storage_locations[@storage_location.id] + audited_item_ids.each do |item_id| + quantities_before[item_id] = location&.items&.[](item_id)&.quantity.to_i + end + end + + location_quantities(final_inventory) + end + + # @return [Hash] item_id => {name:, in:, out:, adjustment:} def sql_flows query = <<~SQL with latest_versioned_events as ( @@ -81,26 +154,32 @@ def sql_flows order by eventable_type, eventable_id, updated_at desc ), flow_events as ( - select id, data from latest_versioned_events - where event_time >= :start_date and event_time <= :end_date + select id, type, data from latest_versioned_events + where event_time > :start_date and event_time <= :end_date union all - select id, data from events + select id, type, data from events where organization_id = :organization_id and type in (:standalone_types) - and event_time >= :start_date and event_time <= :end_date + and event_time > :start_date and event_time <= :end_date ), item_flows as ( select it.id as item_id, it.name as item_name, - case when (item->>'to_storage_location')::int = :id + case when e.type != 'AdjustmentEvent' and (item->>'to_storage_location')::int = :id then (item->>'quantity')::int else 0 end as quantity_in, -- out quantities normalized to positive numbers - case when (item->>'from_storage_location')::int = :id + case when e.type != 'AdjustmentEvent' and (item->>'from_storage_location')::int = :id then abs((item->>'quantity')::int) else 0 - end as quantity_out + end as quantity_out, + -- adjustments carry signed quantities; net them into their own column + case when e.type = 'AdjustmentEvent' + then case when (item->>'to_storage_location')::int = :id then (item->>'quantity')::int else 0 end + - case when (item->>'from_storage_location')::int = :id then (item->>'quantity')::int else 0 end + else 0 + end as quantity_adjustment from flow_events e join lateral jsonb_array_elements(e.data->'items') as item on true left join items it on it.id = (item->>'item_id')::int and it.organization_id = :organization_id @@ -111,7 +190,8 @@ def sql_flows item_id, item_name, sum(quantity_in) as quantity_in, - sum(quantity_out) as quantity_out + sum(quantity_out) as quantity_out, + sum(quantity_adjustment) as quantity_adjustment from item_flows group by item_id, item_name SQL @@ -122,8 +202,8 @@ def sql_flows organization_id: @organization.id, versioned_types: VERSIONED_EVENT_TYPES, standalone_types: STANDALONE_EVENT_TYPES, - start_date: start_date, - end_date: end_date + start_date: start_cutoff, + end_date: end_cutoff }]) ) @@ -131,42 +211,9 @@ def sql_flows flows[row["item_id"]] = { name: row["item_name"], in: row["quantity_in"].to_i, - out: row["quantity_out"].to_i + out: row["quantity_out"].to_i, + adjustment: row["quantity_adjustment"].to_i } end end - - # Audits record the absolute counted quantity, not a delta, so their flow - # contribution is (counted quantity - quantity just before the audit). - # Replay the event log once, tracking the audited items' quantities at this - # location, to turn each in-range audit into a delta. - def apply_audit_deltas(flows) - audits = AuditEvent.where(organization_id: @organization.id, event_time: start_date..end_date) - audited_item_ids = audits.flat_map { |audit| audit.data.items.map(&:item_id) }.uniq - return if audited_item_ids.empty? - - audit_ids = audits.map(&:id).to_set - quantities_before = Hash.new(0) - - InventoryAggregate.inventory_for(@organization.id) do |event, inventory| - if audit_ids.include?(event.id) - event.data.items.each do |line_item| - next unless line_item.to_storage_location == @storage_location.id - - delta = line_item.quantity - quantities_before[line_item.item_id] - flow = flows[line_item.item_id] ||= {name: Item.find_by(id: line_item.item_id)&.name, in: 0, out: 0} - if delta.positive? - flow[:in] += delta - else - flow[:out] += delta.abs - end - end - end - - location = inventory.storage_locations[@storage_location.id] - audited_item_ids.each do |item_id| - quantities_before[item_id] = location&.items&.[](item_id)&.quantity.to_i - end - end - end end diff --git a/app/views/storage_locations/_item_row.html.erb b/app/views/storage_locations/_item_row.html.erb index 5992eff1fe..3ed11818b2 100644 --- a/app/views/storage_locations/_item_row.html.erb +++ b/app/views/storage_locations/_item_row.html.erb @@ -1,7 +1,10 @@ <% css_class = item_row["change"].negative? ? 'modal-body-warning-text' : '' %> "> <%= link_to item_row["item_name"], item_path(item_row["item_id"]) %> + <%= item_row["quantity_start"] %> <%= item_row["quantity_in"] %> <%= item_row["quantity_out"] %> + <%= item_row["quantity_adjustment"] %> <%= item_row["change"] %> + <%= item_row["quantity_end"] %> diff --git a/app/views/storage_locations/show.html.erb b/app/views/storage_locations/show.html.erb index 3a01ba6771..7deb8f4a34 100644 --- a/app/views/storage_locations/show.html.erb +++ b/app/views/storage_locations/show.html.erb @@ -146,9 +146,12 @@ Item + Quantity at Start Quantity In Quantity Out - Change + Quantity Adjustment/Audit + Quantity Change + Quantity at End @@ -157,10 +160,13 @@ Total + <%= @total_quantity_start %> <%= @total_quantity_in %> <%= @total_quantity_out %> + <%= @total_quantity_adjustment %> <% css_class = @total_quantity_change.negative? ? 'modal-body-warning-text' : '' %> <%= @total_quantity_change %> + <%= @total_quantity_end %> diff --git a/spec/queries/items_flow_query_spec.rb b/spec/queries/items_flow_query_spec.rb index a7bc847d44..dbd75bc226 100644 --- a/spec/queries/items_flow_query_spec.rb +++ b/spec/queries/items_flow_query_spec.rb @@ -11,25 +11,38 @@ { item_id: items[0].id, item_name: items[0].name, + quantity_start: 0, quantity_in: 10, quantity_out: 5, + quantity_adjustment: 0, change: 5, - total_quantity_in: 16, - total_quantity_out: 8, - total_change: 8 + quantity_end: 5, + total_quantity_start: 0, + total_quantity_in: 13, + total_quantity_out: 7, + total_quantity_adjustment: 2, + total_change: 8, + total_quantity_end: 8 }, { - # Donation (+3) and adjustment (+3) flow in; the transfer (-2) flows - # out. The audit counts an absolute quantity of 3 when the running - # quantity is 4, so it contributes its delta: 1 more out. + # Donation (+3) flows in and the transfer (-2) flows out. The + # adjustment (+3) lands in the adjustment column, as does the audit: + # it counts an absolute quantity of 3 when the running quantity is 4, + # so it contributes its delta of -1, netting +2. item_id: items[1].id, item_name: items[1].name, - quantity_in: 6, - quantity_out: 3, + quantity_start: 0, + quantity_in: 3, + quantity_out: 2, + quantity_adjustment: 2, change: 3, - total_quantity_in: 16, - total_quantity_out: 8, - total_change: 8 + quantity_end: 3, + total_quantity_start: 0, + total_quantity_in: 13, + total_quantity_out: 7, + total_quantity_adjustment: 2, + total_change: 8, + total_quantity_end: 8 } ].map(&:with_indifferent_access) end @@ -66,6 +79,7 @@ create(:donation, :with_items, item: old_items[0], item_quantity: 10, storage_location: storage_location) Event.last.update(event_time: 10.days.ago) create(:donation, :with_items, item: old_items[1], item_quantity: 8, storage_location: storage_location) + Event.last.update(event_time: 12.days.ago) distribution = create(:distribution, :with_items, item: old_items[1], item_quantity: 5, storage_location: storage_location) DistributionEvent.publish(distribution) Event.last.update(event_time: 10.days.ago) @@ -76,22 +90,36 @@ { item_id: old_items[0].id, item_name: old_items[0].name, + quantity_start: 0, quantity_in: 10, quantity_out: 0, + quantity_adjustment: 0, change: 10, + quantity_end: 10, + total_quantity_start: 8, total_quantity_in: 10, total_quantity_out: 5, - total_change: 5 + total_quantity_adjustment: 0, + total_change: 5, + total_quantity_end: 13 }, { + # Donated (+8) before the window, so it only shows in the starting + # quantity; the in-window distribution (-5) flows out. item_id: old_items[1].id, item_name: old_items[1].name, + quantity_start: 8, quantity_in: 0, quantity_out: 5, + quantity_adjustment: 0, change: -5, + quantity_end: 3, + total_quantity_start: 8, total_quantity_in: 10, total_quantity_out: 5, - total_change: 5 + total_quantity_adjustment: 0, + total_change: 5, + total_quantity_end: 13 } ].map(&:with_indifferent_access) end @@ -115,6 +143,7 @@ row = subject.to_a.find { |r| r["item_id"] == item.id } expect(row["quantity_in"]).to eq(25) + expect(row["quantity_end"]).to eq(25) end end @@ -154,8 +183,10 @@ rows = subject.to_a.index_by { |r| r["item_id"] } expect(rows[kit.id]["quantity_in"]).to eq(3) + expect(rows[kit.id]["quantity_end"]).to eq(3) expect(rows[content_item.id]["quantity_in"]).to eq(10) expect(rows[content_item.id]["quantity_out"]).to eq(6) + expect(rows[content_item.id]["quantity_end"]).to eq(4) end end end diff --git a/spec/system/storage_location_system_spec.rb b/spec/system/storage_location_system_spec.rb index f1ab5e1314..f2a08eff0a 100644 --- a/spec/system/storage_location_system_spec.rb +++ b/spec/system/storage_location_system_spec.rb @@ -191,20 +191,27 @@ { item_id: items[0].id, item_name: items[0].name, + quantity_start: 0, quantity_in: 10, quantity_out: 5, + quantity_adjustment: 0, change: 5, - total_quantity_in: 16, + quantity_end: 5, + total_quantity_in: 13, total_quantity_out: 7, total_change: 9 }, { + # The adjustment (+3) shows in the adjustment column, not as in. item_id: items[1].id, item_name: items[1].name, - quantity_in: 6, + quantity_start: 0, + quantity_in: 3, quantity_out: 2, + quantity_adjustment: 3, change: 4, - total_quantity_in: 16, + quantity_end: 4, + total_quantity_in: 13, total_quantity_out: 7, total_change: 9 }