Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve product drive index page performance #4881

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
9 changes: 7 additions & 2 deletions app/controllers/product_drives_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def index
setup_date_range_picker
@product_drives = current_organization
.product_drives
.includes(:tags, donations: {line_items: :item})
.includes(:tags)
.class_filter(filter_params)
.within_date_range(@selected_date_range)
.order(start_date: :desc)
Expand All @@ -17,12 +17,17 @@ def index
@selected_name_filter = filter_params[:by_name]
@selected_item_category = filter_params[:by_item_category_id]
@selected_tags = filter_params[:by_tags]
@summary = ProductDriveSummaryService.call(
product_drives: @product_drives,
within_date_range: helpers.selected_interval,
item_category_id: @selected_item_category
)

respond_to do |format|
format.html
format.csv do
send_data Exports::ExportProductDrivesCSVService.new(
@product_drives,
@product_drives.unscope(:includes).includes(donations: {line_items: :item}),
current_organization,
helpers.selected_range
).generate_csv, filename: "Product-Drives-#{Time.zone.today}.csv"
Expand Down
2 changes: 1 addition & 1 deletion app/models/product_drive.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class ProductDrive < ApplicationRecord

scope :by_name, ->(name_filter) { where(name: name_filter) }
scope :by_item_category_id, ->(item_category_id) {
joins(donations: {line_items: :item})
includes(donations: {line_items: :item})
.where(item: { item_category_id: item_category_id })
}

Expand Down
40 changes: 40 additions & 0 deletions app/services/product_drive_summary_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Calculates the quantity, unique item count, and value for product
# drives from donations (filtered by items donated in a given date range
# and within a given item category if provided).
class ProductDriveSummaryService
class << self
ProductDriveSummary = Data.define(:quantity, :unique_item_count, :value)
NULL_PRODUCT_DRIVE_TOTALS = ProductDriveSummary.new(quantity: 0, unique_item_count: 0, value: 0)

def call(product_drives:, within_date_range:, item_category_id: nil)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a YARD comment mentioning the types of the inputs and outputs?

calculate_totals(product_drives, within_date_range, item_category_id)
end

private

# @return [Hash<Integer, ProductDriveSummary>]
def calculate_totals(product_drives, within_date_range, item_category_id)
query = ProductDrive
.left_joins(donations: {line_items: [:item]})
.where(id: product_drives.ids)
.where(donations: {issued_at: within_date_range[0]..within_date_range[1]})
.group("product_drives.id")
.distinct

query = query.where(items: {item_category_id: item_category_id}) if item_category_id.present?

totals = query.pluck(Arel.sql(
"product_drives.id AS id,
COALESCE(SUM(line_items.quantity), 0) AS quantity,
COUNT(DISTINCT line_items.item_id) AS unique_item_count,
COALESCE(SUM(COALESCE(items.value_in_cents, 0) * line_items.quantity), 0) AS value"
)).to_h do |(id, quantity, unique_item_count, value)|
[id, ProductDriveSummary.new(quantity:, unique_item_count:, value:)]
end

# If a product drive has no matching donations/items, set totals to
# default values (zeroes).
product_drives.ids.to_h { |id| [id, NULL_PRODUCT_DRIVE_TOTALS] }.merge(totals)
end
end
end
6 changes: 3 additions & 3 deletions app/views/product_drives/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@
<td><%= product_drive.end_date&.strftime("%m-%d-%Y") %></td>
<td><%= product_drive.tags_for_display %></td>
<td><%= is_virtual(product_drive: product_drive) %></td>
<td class="text-right"><%= product_drive.donation_quantity_by_date(selected_range, @selected_item_category) %></td>
<td class="text-right"><%= product_drive.distinct_items_count_by_date(selected_range, @selected_item_category) %></td>
<td class="text-right"><strong><%= dollar_value(product_drive.in_kind_value) %></strong></td>
<td class="text-right"><%= @summary[product_drive.id].quantity %></td>
<td class="text-right"><%= @summary[product_drive.id].unique_item_count %></td>
<td class="text-right"><strong><%= dollar_value(@summary[product_drive.id].value) %></strong></td>
<td class="text-right"><%= view_button_to product_drive_path(product_drive.id) %></td>
</tr>
<% end %>
Expand Down