Merge branch 'main' into 4217-kit-various-refactors

app/controllers/items_controller.rb

@@ -39,7 +39,7 @@ def create
     result = create.call
 
     if result.success?
-      redirect_to items_path, notice: "#{result.item.name} added!"
+      redirect_to items_path, notice: "#{result.value.name} added!"
     else
       @base_items = BaseItem.without_kit.alphabetized
       # Define a @item to be used in the `new` action to be rendered with

app/controllers/partners/family_requests_controller.rb

@@ -34,12 +34,14 @@ def create
     def validate
       family_requests_attributes = build_family_requests_attributes(params)
 
-      @partner_request = Partners::FamilyRequestCreateService.new(
+      create_service = Partners::FamilyRequestCreateService.new(
         partner_user_id: current_user.id,
         family_requests_attributes: family_requests_attributes,
         request_type: "child"
       ).initialize_only
-      if @partner_request.valid?
+
+      if create_service.errors.none?
+        @partner_request = create_service.partner_request
         @total_items = @partner_request.total_items
         @quota_exceeded = current_partner.quota_exceeded?(@total_items)
         body = render_to_string(template: 'partners/requests/validate', formats: [:html], layout: false)

app/controllers/partners/individuals_requests_controller.rb

@@ -34,13 +34,15 @@ def create
     end
 
     def validate
-      @partner_request = Partners::FamilyRequestCreateService.new(
+      create_service = Partners::FamilyRequestCreateService.new(
         partner_user_id: current_user.id,
         comments: individuals_request_params[:comments],
         family_requests_attributes: individuals_request_params[:items_attributes]&.values,
         request_type: "individual"
       ).initialize_only
-      if @partner_request.valid?
+
+      if create_service.errors.none?
+        @partner_request = create_service.partner_request
         @total_items = @partner_request.total_items
         @quota_exceeded = current_partner.quota_exceeded?(@total_items)
         body = render_to_string(template: 'partners/requests/validate', formats: [:html], layout: false)

app/controllers/partners/requests_controller.rb

@@ -43,14 +43,15 @@ def create
     end
 
     def validate
-      @partner_request = Partners::RequestCreateService.new(
+      create_service = Partners::RequestCreateService.new(
         request_type: "quantity",
         partner_user_id: current_user.id,
         comments: partner_request_params[:comments],
         item_requests_attributes: partner_request_params[:item_requests_attributes]&.values || []
       ).initialize_only
 
-      if @partner_request.valid?
+      if create_service.errors.none?
+        @partner_request = create_service.partner_request
         @total_items = @partner_request.total_items
         @quota_exceeded = current_partner.quota_exceeded?(@total_items)
         body = render_to_string(template: 'partners/requests/validate', formats: [:html], layout: false)

app/controllers/partners_controller.rb

@@ -1,5 +1,5 @@
 # Provides full CRUD for Partners. These are minimal representations of corresponding Partner records in PartnerBase.
-# Though the functionality of Partners is actually fleshed out in PartnerBase, in HumanEssentails, we maintain a collection
+# Though the functionality of Partners is actually fleshed out in PartnerBase, in HumanEssentials, we maintain a collection
 # of which Partners are associated with which Diaperbanks.
 class PartnersController < ApplicationController
   include Importable

app/controllers/purchases_controller.rb

@@ -13,9 +13,16 @@ def index
     @paginated_purchases = @purchases.page(params[:page])
     # Are these going to be inefficient with large datasets?
     # Using the @purchases allows drilling down instead of always starting with the total dataset
+    # Purchase quantity
     @purchases_quantity = @purchases.collect(&:total_quantity).sum
     @paginated_purchases_quantity = @paginated_purchases.collect(&:total_quantity).sum
+    # Purchase value
     @total_value_all_purchases = @purchases.sum(&:amount_spent_in_cents)
+    @paginated_purchases_value = @paginated_purchases.collect(&:amount_spent_in_cents).sum
+    # Fair Market Values
+    @total_fair_market_values = @purchases.sum(&:value_per_itemizable)
+    @paginated_fair_market_values = @paginated_purchases.collect(&:value_per_itemizable).sum
+    # Storage and Vendor
     @storage_locations = current_organization.storage_locations.active_locations
     @selected_storage_location = filter_params[:at_storage_location]
     @vendors = current_organization.vendors.sort_by { |vendor| vendor.business_name.downcase }

app/controllers/requests_controller.rb

@@ -9,12 +9,12 @@ def index
                 .during(helpers.selected_range)
                 .class_filter(filter_params)
     @unfulfilled_requests_count = current_organization.requests.where(status: [:pending, :started]).count
-    @paginated_requests = @requests.page(params[:page])
+    @paginated_requests = @requests.includes(:partner).page(params[:page])
     @calculate_product_totals = RequestsTotalItemsService.new(requests: @requests).calculate
-    @items = current_organization.items.alphabetized
-    @partners = current_organization.partners.order(:name)
+    @items = current_organization.items.alphabetized.select(:id, :name)
+    @partners = current_organization.partners.alphabetized.select(:id, :name)
     @statuses = Request.statuses.transform_keys(&:humanize)
-    @partner_users = User.where(id: @paginated_requests.pluck(:partner_user_id))
+    @partner_users = User.where(id: @paginated_requests.map(&:partner_user_id)).select(:id, :name, :email)
     @request_types = Request.request_types.transform_keys(&:humanize)
     @selected_request_type = filter_params[:by_request_type]
     @selected_request_item = filter_params[:by_request_item_id]

app/controllers/transfers_controller.rb

@@ -49,7 +49,7 @@ def destroy
     results = transfer_destroy_service.call
 
     if results.success?
-      flash[:notice] = "Succesfully deleted Transfer ##{params[:id]}!"
+      flash[:notice] = "Successfully deleted Transfer ##{params[:id]}!"
     else
       flash[:error] = results.error.message
     end

app/javascript/controllers/checkbox_with_nested_element_controller.js

@@ -8,14 +8,14 @@ export default class extends Controller {
   static targets = [ "nestedElement", "checkbox" ]
 
   connect() {
-    this.toggleNestedElementVisiblity()
+    this.toggleNestedElementVisibility()
   }
 
   /**
    * Toggles the visibility of the nested element depending
    * on wither the checkbox is checked or not.
    */
-  toggleNestedElementVisiblity() {
+  toggleNestedElementVisibility() {
     this.nestedElementTarget.classList.toggle("d-none", !this.checkboxTarget.checked)
   }
 

app/javascript/controllers/item_units_controller.js

@@ -5,13 +5,17 @@ export default class extends Controller {
   static targets = ["itemSelect", "requestSelect"]
   static values = {
     // hash of (item ID => hash of (request unit name => request unit plural name))
-    "itemUnits": Object
+    "itemUnits": Object,
+    "selectedItemUnits": String
   }
 
   addOption(val, text) {
     let option = document.createElement("option");
     option.value = val;
     option.text = text;
+    if (this.selectedItemUnitsValue === val) {
+      option.selected = true;
+    }
     this.requestSelectTarget.appendChild(option);
   }
 

app/models/event.rb

@@ -40,11 +40,11 @@ class Event < ApplicationRecord
   end
   after_create :validate_inventory
 
-  # @return [Array<OpenStruct>]
+  # @return [Array<Option>]
   def self.types_for_select
-    descendants.map { |klass|
-      OpenStruct.new(name: klass.name.sub("Event", "").titleize, value: klass.name)
-    }.sort_by(&:name)
+    descendants.map do |klass|
+      Option.new(name: klass.name.sub("Event", "").titleize, id: klass.name)
+    end.sort_by(&:name)
   end
 
   # Returns the most recent "usable" snapshot. A snapshot is unusable if there is another event

app/models/item.rb

@@ -73,7 +73,7 @@ class Item < ApplicationRecord
   # 4/  Period supplies.  All things with 'menstrual in the category'
   # 5/  Other -- Miscellaneous, and wipes
   # Known holes and ambiguities as of 20240527.  Working on these with the business
-  # 1/  Liners.   We are adding a new item for AI liners,  and renaming the current liners to be specficially for periods,
+  # 1/  Liners.   We are adding a new item for AI liners,  and renaming the current liners to be specifically for periods,
   # having confirmed with the business that the majority of liners are for menstrual use.
   # However, there is a product which can be used for either, so we are still sussing out what to do about that.
 

app/models/option.rb

@@ -0,0 +1,3 @@
+# PORO used to create select options in views
+class Option < Data.define(:id, :name)
+end

app/models/partners/child.rb

@@ -33,8 +33,8 @@ class Child < Base
     scope :from_family, ->(family_id) {
       where(family_id: family_id)
     }
-    scope :from_children, ->(chidren_id) {
-      where(id: chidren_id)
+    scope :from_children, ->(children_id) {
+      where(id: children_id)
     }
     scope :active, -> { where(active: true) }
 

app/models/partners/item_request.rb

@@ -37,12 +37,10 @@ def request_unit_is_supported
     end
 
     def name_with_unit(quantity_override = nil)
-      if item
-        if Flipper.enabled?(:enable_packs) && request_unit.present?
-          "#{name} - #{request_unit.pluralize(quantity_override || quantity.to_i)}"
-        else
-          name
-        end
+      if Flipper.enabled?(:enable_packs) && request_unit.present?
+        "#{name} - #{request_unit.pluralize(quantity_override || quantity.to_i)}"
+      else
+        name
       end
     end
   end

app/models/purchase.rb

@@ -61,6 +61,16 @@ class Purchase < ApplicationRecord
   validates :amount_spent_in_cents, numericality: { greater_than: 0 }
   validate :total_equal_to_all_categories
 
+  SummaryByDates = Data.define(
+    :amount_spent,
+    :recent_purchases,
+    :period_supplies,
+    :diapers,
+    :adult_incontinence,
+    :other,
+    :total_items
+  )
+
   def storage_view
     storage_location.nil? ? "N/A" : storage_location.name
   end
@@ -84,7 +94,7 @@ def remove(item)
   def self.organization_summary_by_dates(organization, date_range)
     purchases = where(organization: organization).during(date_range)
 
-    OpenStruct.new(
+    SummaryByDates.new(
       amount_spent: purchases.sum(:amount_spent_in_cents),
       recent_purchases: purchases.recent.includes(:vendor),
       period_supplies: purchases.sum(:amount_spent_on_period_supplies_cents),

app/models/result.rb

@@ -0,0 +1,9 @@
+# Immutable result object for use in services
+class Result < Data.define(:value, :error)
+  # Set default attribute values
+  def initialize(value: nil, error: nil)
+    super
+  end
+
+  def success? = error.nil?
+end

app/models/storage_location.rb

@@ -64,13 +64,14 @@ class StorageLocation < ApplicationRecord
 
   # @param organization [Organization]
   # @param inventory [View::Inventory]
+  # @return [Array<Option>]
   def self.items_inventoried(organization, inventory = nil)
     inventory ||= View::Inventory.new(organization.id)
     inventory
       .all_items
       .uniq(&:item_id)
       .sort_by(&:name)
-      .map { |i| OpenStruct.new(name: i.name, id: i.item_id) }
+      .map { |i| Option.new(name: i.name, id: i.item_id) }
   end
 
   # @return [Array<Item>]

app/queries/low_inventory_query.rb

@@ -1,4 +1,6 @@
 class LowInventoryQuery
+  LowInventoryItem = Data.define(:id, :name, :on_hand_minimum_quantity, :on_hand_recommended_quantity, :total_quantity)
+
   def self.call(organization)
     inventory = View::Inventory.new(organization.id)
     items = inventory.all_items.uniq(&:item_id)
@@ -7,7 +9,7 @@ def self.call(organization)
     items.each do |item|
       quantity = inventory.quantity_for(item_id: item.id)
       if quantity < item.on_hand_minimum_quantity.to_i || quantity < item.on_hand_recommended_quantity.to_i
-        low_inventory_items.push(OpenStruct.new(
+        low_inventory_items.push(LowInventoryItem.new(
           id: item.id,
           name: item.name,
           on_hand_minimum_quantity: item.on_hand_minimum_quantity,
@@ -17,6 +19,6 @@ def self.call(organization)
       end
     end
 
-    low_inventory_items.sort_by { |item| item[:name] }
+    low_inventory_items.sort_by { |item| item.name }
   end
 end

app/services/distribution_totals_service.rb

@@ -45,7 +45,7 @@ def calculate_quantities(distributions)
       .to_h
   end
 
-  # Returns hash of total value of items per distribution WIHOUT item id/category filter
+  # Returns hash of total value of items per distribution WITHOUT item id/category filter
   # Value of entire distribution (not reduced by filtered items)
   #
   # @return [Hash<Integer, Integer>]

app/services/exports/export_request_service.rb

@@ -85,7 +85,7 @@ def compute_item_headers
       end
 
       # Adding this to handle cases in which a requested item
-      # has been deleted. Normally this wouldn't be neccessary,
+      # has been deleted. Normally this wouldn't be necessary,
       # but previous versions of the application would cause
       # this orphaned data
       item_names.sort.uniq << DELETED_ITEMS_COLUMN_HEADER
@@ -97,7 +97,7 @@ def build_row_data(request)
       row += Array.new(item_headers.size, 0)
 
       request.item_requests.each do |item_request|
-        item_name = item_request.name_with_unit(0) || DELETED_ITEMS_COLUMN_HEADER
+        item_name = item_request.item.present? ? item_request.name_with_unit(0) : DELETED_ITEMS_COLUMN_HEADER
         item_column_idx = headers_with_indexes[item_name]
         row[item_column_idx] ||= 0
         row[item_column_idx] += item_request.quantity.to_i

app/services/item_create_service.rb

@@ -12,9 +12,9 @@ def call
       new_item.sync_request_units!(@request_unit_ids)
     end
 
-    OpenStruct.new(success?: true, item: new_item)
+    Result.new(value: new_item)
   rescue StandardError => e
-    OpenStruct.new(success?: false, error: e)
+    Result.new(error: e)
   end
 
   private