From 42edea8e604066933b9635f064957c91387f6f21 Mon Sep 17 00:00:00 2001 From: Casey Helbling Date: Tue, 17 Jun 2025 12:05:45 -0500 Subject: [PATCH 1/5] Attempt to get local event query performance to return results --- .../20250617142915_add_indexes_events.rb | 18 ++++++++++++++++++ db/schema.rb | 7 ++++++- docker-compose.yml | 11 +++++++++-- 3 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20250617142915_add_indexes_events.rb diff --git a/db/migrate/20250617142915_add_indexes_events.rb b/db/migrate/20250617142915_add_indexes_events.rb new file mode 100644 index 000000000..e757d96ca --- /dev/null +++ b/db/migrate/20250617142915_add_indexes_events.rb @@ -0,0 +1,18 @@ +class AddIndexesEvents < ActiveRecord::Migration[7.1] + def change + add_index :events, [:nonprofit_id, :end_datetime, :published, :deleted], + name: 'idx_events_listings_query' + + add_index :tickets, [:event_id, :quantity, :checked_in], + name: 'idx_tickets_event_metrics' + + add_index :payments, [:donation_id, :gross_amount], + name: 'idx_payments_donations' + + add_index :tickets, [:payment_id, :event_id], + name: 'idx_tickets_payments' + + add_index :payments, [:id, :gross_amount], + name: 'idx_payments_gross_amount' + end +end diff --git a/db/schema.rb b/db/schema.rb index 34cc1a1e5..1f9e53a85 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2025_06_12_221922) do +ActiveRecord::Schema[7.1].define(version: 2025_06_17_142915) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" enable_extension "plpgsql" @@ -408,6 +408,7 @@ t.datetime "end_datetime", precision: nil t.index ["nonprofit_id", "deleted", "published", "end_datetime"], name: "events_nonprofit_id_not_deleted_and_published_endtime" t.index ["nonprofit_id", "deleted", "published"], name: "index_events_on_nonprofit_id_and_deleted_and_published" + t.index ["nonprofit_id", "end_datetime", "published", "deleted"], name: "idx_events_listings_query" t.index ["nonprofit_id"], name: "index_events_on_nonprofit_id" end @@ -821,8 +822,10 @@ t.integer "donation_id" t.datetime "date", precision: nil t.index ["date"], name: "payments_date" + t.index ["donation_id", "gross_amount"], name: "idx_payments_donations" t.index ["donation_id"], name: "payments_donation_id" t.index ["gross_amount"], name: "payments_gross_amount" + t.index ["id", "gross_amount"], name: "idx_payments_gross_amount" t.index ["kind"], name: "payments_kind" t.index ["nonprofit_id"], name: "payments_nonprofit_id" t.index ["supporter_id"], name: "payments_supporter_id" @@ -1238,7 +1241,9 @@ t.boolean "deleted", default: false t.uuid "source_token_id" t.integer "ticket_purchase_id" + t.index ["event_id", "quantity", "checked_in"], name: "idx_tickets_event_metrics" t.index ["event_id"], name: "index_tickets_on_event_id" + t.index ["payment_id", "event_id"], name: "idx_tickets_payments" t.index ["payment_id"], name: "index_tickets_on_payment_id" t.index ["supporter_id"], name: "index_tickets_on_supporter_id" t.index ["ticket_purchase_id"], name: "index_tickets_on_ticket_purchase_id" diff --git a/docker-compose.yml b/docker-compose.yml index cd86c4e83..fc582e2f1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,4 +1,3 @@ -version: "3.9" services: db: image: postgres:16 @@ -8,6 +7,14 @@ services: volumes: - ./tmp/db:/var/lib/postgresql/data:delegated - ./tmp/shared:/tmp/shared:cached + command: > + postgres + -c shared_buffers=1GB + -c max_connections=100 + -c max_parallel_workers=8 + -c max_parallel_workers_per_gather=4 + -c work_mem=64MB + -c maintenance_work_mem=512MB environment: - POSTGRES_USER=admin - POSTGRES_PASSWORD=password @@ -32,4 +39,4 @@ services: - BUILD_DATABASE_URL=postgres://admin:password@db/commitchange_development_legacy depends_on: - db - env_file: ".env" \ No newline at end of file + env_file: ".env" From 6119bfd1d49ad72eb45d8815d25afa6cef515bf4 Mon Sep 17 00:00:00 2001 From: Casey Helbling Date: Tue, 17 Jun 2025 12:06:38 -0500 Subject: [PATCH 2/5] Tweak UI to add number of each event and whitespace between each block --- app/views/events/index.html.erb | 8 ++++---- client/js/events/listing-item/index.js | 26 +++++++++++++++--------- client/js/events/listings/index.js | 28 +++++++++++++++----------- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/app/views/events/index.html.erb b/app/views/events/index.html.erb index 53ea81b25..2167ce586 100644 --- a/app/views/events/index.html.erb +++ b/app/views/events/index.html.erb @@ -13,10 +13,10 @@ <% end %> <%= render 'components/header', -icon_class: 'icon-ticket-2', -title: 'Events', -profile: @nonprofit, -has_mosaic: true %> + icon_class: 'icon-ticket-2', + title: 'Events', + profile: @nonprofit, + has_mosaic: true %>
<% if current_user %> diff --git a/client/js/events/listing-item/index.js b/client/js/events/listing-item/index.js index 91beb2b5d..e12e0040e 100644 --- a/client/js/events/listing-item/index.js +++ b/client/js/events/listing-item/index.js @@ -19,25 +19,30 @@ const dateTime = (startTime, endTime) => { ] } -const metric = (label, val) => +const metric = (label, val) => h('span.u-inlineBlock.u-marginRight--20', [h('strong', `${label}: `), val || '0']) -const row = (icon, content) => +const checkedInPercent = (checkedIn = 0 , total = 0) => { + if (total === 0) return 0; + return Math.round((checkedIn * 100) / total); +} + +const row = (icon, content) => h('tr', [ h('td.u-centered', [h(`i.fa.${icon}`)]) , h('td.u-padding--10', content) ]) -module.exports = e => { +module.exports = (e, key) => { const path = `/nonprofits/${app.nonprofit_id}/events/${e.id}` const location = [ - h('p.strong.u-margin--0', e.venue_name) + h('p.strong.u-margin--0', e.venue_name) , h('p.u-margin--0', commaJoin([e.address, e.city, e.state_code, e.zip_code])) ] const attendeesMetrics = [ - metric('Attendees', e.total_attendees) - , metric('Checked In', e.checked_in_count) - , metric('Percent Checked In', Math.round((e.checked_in_count || 0) * 100 / (e.total_attendees || 0)) + '%') + metric('Attendees', e.total_attendees) + , metric('Checked In', e.checked_in_count) + , metric('Percent Checked In', checkedInPercent(e.checked_in_count, e.total_attendees) + '%') ] const moneyMetrics = [ metric('Ticket Payments', '$' + format.centsToDollars(e.tickets_total_paid)) @@ -48,9 +53,10 @@ module.exports = e => { h('a.u-marginRight--20', {props: {href: path, target: '_blank'}}, 'Event Page') , h('a', {props: {href: path + '/tickets', target: '_blank'}}, 'Attendees Page') ] - return h('div.u-paddingTop--10.u-marginBottom--20', [ - h('h5.u-paddingX--20', e.name) - , h('table.table--striped.u-margin--0', [ + + return h('div.u-paddingTop--10.u-paddingBottom--10', [ + h(`h5.u-padding--20.u-margin--0.fundraiser--${key}`, e.name) + , h(`table.table--striped.u-margin--0.fundraiser--${key}`, [ row('fa-clock-o', dateTime(e.start_datetime, e.end_datetime)) , row('fa-map-marker', location) , row('fa-users', attendeesMetrics) diff --git a/client/js/events/listings/index.js b/client/js/events/listings/index.js index 4a31e1326..b7e1ea4a7 100644 --- a/client/js/events/listings/index.js +++ b/client/js/events/listings/index.js @@ -16,27 +16,31 @@ module.exports = pathPrefix => { const init = _ => { return { active: get('active') - , past: get('past') - , unpublished: get('unpublished') - , deleted: get('deleted') + , past: get('past') + , unpublished: get('unpublished') + , deleted: get('deleted') } } const listings = (key, state) => { const resp$ = state[key] - const mixin = content => + const mixin = (content, count) => h('section.u-marginBottom--30', [ - h('h5.u-centered.u-marginBottom--20', key.charAt(0).toUpperCase() + key.slice(1) + ' Events') - , h(`div.fundraiser--${key}`, content) + h('h5.u-centered.u-marginBottom--20', count + ' ' + key.charAt(0).toUpperCase() + key.slice(1) + ' Events') + , h(`div`, content) ]) - if(!resp$()) - return mixin([h('p.u-padding--15', 'Loading...')]) - if(!resp$().body.length) - return mixin([h('p.u-padding--15', `No ${key} events`)]) - return mixin(resp$().body.map(listing)); + + if(!resp$()) + return mixin([h(`p.u-padding--15.fundraiser--${key}`, 'Loading...')], 0) + + const numberElems = resp$().body.length + if(!numberElems) + return mixin([h(`p.u-padding--15.fundraiser--${key}`, `No ${key} events`)], 0) + + return mixin(resp$().body.map(item => listing(item, key)), numberElems); } - const view = state => + const view = state => h('div', [ listings('active', state) , listings('past', state) From 529b88be08bba0de0b1f172c3cf769607cb3291e Mon Sep 17 00:00:00 2001 From: Casey Helbling Date: Tue, 17 Jun 2025 12:07:40 -0500 Subject: [PATCH 3/5] Rewrite event metrics query so it actually returns --- app/legacy_lib/query_event_metrics.rb | 209 +++++++++++++------- app/models/donation.rb | 1 + app/models/event.rb | 2 +- app/models/payment.rb | 3 +- client/js/events/listings/index.js | 4 +- spec/factories/events.rb | 6 +- spec/factories/import.rb | 6 + spec/factories/nonprofits.rb | 6 +- spec/factories/ticket_levels.rb | 3 + spec/factories/tickets.rb | 10 + spec/lib/insert/insert_source_token_spec.rb | 3 + spec/lib/query/query_event_metrics_spec.rb | 112 +++++++++++ spec/lib/update/update_tickets_spec.rb | 11 +- 13 files changed, 292 insertions(+), 84 deletions(-) create mode 100644 spec/factories/import.rb create mode 100644 spec/lib/query/query_event_metrics_spec.rb diff --git a/app/legacy_lib/query_event_metrics.rb b/app/legacy_lib/query_event_metrics.rb index f4da6aabd..eeb4f49c2 100644 --- a/app/legacy_lib/query_event_metrics.rb +++ b/app/legacy_lib/query_event_metrics.rb @@ -1,86 +1,151 @@ # License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later + module QueryEventMetrics - def self.expression(additional_selects = []) - selects = [ - "coalesce(tickets.total, 0) AS total_attendees", - "coalesce(tickets.checked_in_count, 0) AS checked_in_count", - "coalesce(ticket_payments.total_paid, 0) AS tickets_total_paid", - "coalesce(donations.payment_total, 0) AS donations_total_paid", - "coalesce(ticket_payments.total_paid, 0) + coalesce(donations.payment_total, 0) AS total_paid" - ] - - tickets_sub = Qx.select("event_id", "SUM(quantity) AS total", "SUM(tickets.checked_in::int) AS checked_in_count") - .from("tickets") - .group_by("event_id") - .as("tickets") - - ticket_payments_subquery = Qx.select("payment_id", "MAX(event_id) AS event_id").from("tickets").group_by("payment_id").as("tickets") - - ticket_payments_sub = Qx.select("SUM(payments.gross_amount) AS total_paid", "tickets.event_id") - .from(:payments) - .join(ticket_payments_subquery, "payments.id=tickets.payment_id") - .group_by("tickets.event_id") - .as("ticket_payments") - - donations_sub = Qx.select("event_id", "SUM(payments.gross_amount) as payment_total") - .from("donations") - .group_by("event_id") - .left_join("payments", "donations.id=payments.donation_id") - .as("donations") - - selects = selects.concat(additional_selects) - Qx.select(*selects) - .from("events") - .left_join( - [tickets_sub, "tickets.event_id = events.id"], - [donations_sub, "donations.event_id = events.id"], - [ticket_payments_sub, "ticket_payments.event_id=events.id"] - ) - end + # For now limit to 1000 + QUERY_MAX_LIMIT = 1_000 def self.with_event_ids(event_ids) - return [] if event_ids.empty? - QueryEventMetrics.expression.where("events.id in ($ids)", ids: event_ids).execute + return [] if event_ids.blank? + + events = Event.select(:id, :name, :venue_name, :address, :city, :state_code, + :zip_code, :start_datetime, :end_datetime, :organizer_email) + .where(id: event_ids) + .limit(QUERY_MAX_LIMIT) + + add_metrics_to_events(events) end - def self.for_listings(id_type, id, params) - selects = [ - "events.id", - "events.name", - "events.venue_name", - "events.address", - "events.city", - "events.state_code", - "events.zip_code", - "events.start_datetime", - "events.end_datetime", - "events.organizer_email" - ] - - exp = QueryEventMetrics.expression(selects) - - if id_type == "profile" - exp = exp.and_where(["events.profile_id = $id", id: id]) - end - if id_type == "nonprofit" - exp = exp.and_where(["events.nonprofit_id = $id", id: id]) + def self.for_listings(id_type, profile_or_nonprofit_id, params) + events = base_events(id_type, profile_or_nonprofit_id, params) + return [] if events.blank? + + # Add metrics to the events + add_metrics_to_events(events) + end + + private + + def self.base_events(id_type, id, params) + query = Event.select(:id, :name, :venue_name, :address, :city, :state_code, + :zip_code, :start_datetime, :end_datetime, :organizer_email) + + case id_type + when "profile" + query = query.where(profile_id: id) + when "nonprofit" + query = query.where(nonprofit_id: id) + else + raise "Unknown id_type #{id_type}" end + if params["active"].present? - exp = exp - .and_where(["events.end_datetime >= $date", date: Time.now]) - .and_where(["events.published = TRUE AND coalesce(events.deleted, FALSE) = FALSE"]) + query = query + .where("end_datetime >= ?", Time.current) + .where(published: true) + .where("COALESCE(deleted, false) = false") + elsif params["past"].present? + query = query + .where("end_datetime < ?", Time.current) + .where(published: true) + .where("COALESCE(deleted, false) = false") + elsif params["unpublished"].present? + query = query + .where("COALESCE(published, false) = false") + .where("COALESCE(deleted, false) = false") + elsif params["deleted"].present? + query = query.where(deleted: true) + end + + query = query.order(end_datetime: :desc) + query = query.limit(QUERY_MAX_LIMIT) + + query + end + + def self.add_metrics_to_events(events) + return [] if events.blank? + + event_ids = events.pluck(:id) + + ticket_metrics = ticket_metrics(event_ids) + donation_metrics = donation_metrics(event_ids) + payment_metrics = payment_metrics(event_ids) + + events.map do |event| + build_event_result(event, + ticket_metrics[event.id], + donation_metrics[event.id], + payment_metrics[event.id]) end - if params["past"].present? - exp = exp - .and_where(["events.end_datetime < $date", date: Time.now]) - .and_where(["events.published = TRUE AND coalesce(events.deleted, FALSE) = FALSE"]) + end + + def self.build_event_result(event, tickets, donations, payments) + tickets ||= { total: 0, checked_in_count: 0 } + donations ||= { donation_total: 0 } + payments ||= { payment_total: 0 } + + { + 'id' => event.id, + 'name' => event.name, + 'venue_name' => event.venue_name, + 'address' => event.address, + 'city' => event.city, + 'state_code' => event.state_code, + 'zip_code' => event.zip_code, + 'start_datetime' => event.start_datetime, + 'end_datetime' => event.end_datetime, + 'organizer_email' => event.organizer_email, + 'total_attendees' => tickets[:total], + 'checked_in_count' => tickets[:checked_in_count], + 'tickets_total_paid' => payments[:payment_total], + 'donations_total_paid' => donations[:donation_total], + 'total_paid' => payments[:payment_total] + donations[:donation_total] + } + end + + def self.ticket_metrics(event_ids) + return {} if event_ids.blank? + + ticket_data = Ticket.where(event_id: event_ids) + .group(:event_id) + .pluck(:event_id, + Arel.sql('COALESCE(SUM(quantity), 0) as total'), + Arel.sql('COALESCE(SUM(CASE WHEN checked_in THEN 1 ELSE 0 END), 0) as checked_in_count') + ) + + ticket_data.to_h do |event_id, total, checked_in_count| + [event_id, {total:, checked_in_count:}] end - if params["unpublished"].present? - exp = exp.and_where(["coalesce(events.published, FALSE) = FALSE AND coalesce(events.deleted, FALSE) = FALSE"]) + end + + def self.donation_metrics(event_ids) + return {} if event_ids.blank? + + donation_data = Donation.left_joins(:payments) + .where(event_id: event_ids) + .group(:event_id) + .pluck(:event_id, Arel.sql('COALESCE(SUM(payments.gross_amount), 0) as donation_total')) + + donation_data.to_h do |event_id, donation_total| + [event_id, {donation_total:}] end - if params["deleted"].present? - exp = exp.and_where(["events.deleted = TRUE"]) + end + + def self.payment_metrics(event_ids) + return {} if event_ids.blank? + + # TODO: consider deleted tickets too + payment_data = Payment.joins(:tickets) + .where(tickets: { event_id: event_ids }) + .group('tickets.event_id') + .pluck('tickets.event_id', Arel.sql('COALESCE(SUM(payments.gross_amount), 0) as payment_total')) + + payment_data.to_h do |event_id, payment_total| + [event_id, {payment_total:}] end - exp.execute + end + + def self.execute(query) + Event.connection.execute query end end diff --git a/app/models/donation.rb b/app/models/donation.rb index 1827c916c..943556c9d 100644 --- a/app/models/donation.rb +++ b/app/models/donation.rb @@ -41,6 +41,7 @@ class Donation < ApplicationRecord has_one :offsite_payment has_one :tracking has_many :modern_donations + belongs_to :supporter belongs_to :card belongs_to :direct_debit_detail diff --git a/app/models/event.rb b/app/models/event.rb index a979d8156..62ea27431 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -73,7 +73,7 @@ class Event < ApplicationRecord scope :past, -> { where("end_datetime < ?", Date.today).published } scope :unpublished, -> { where.not(published: true) } - validates :slug, uniqueness: {scope: :nonprofit_id, message: "You already have a campaign with that name."} + validates :slug, uniqueness: {scope: :nonprofit_id, message: "You already have an event with that name."} before_validation(on: :create) do unless slug diff --git a/app/models/payment.rb b/app/models/payment.rb index d912fef9a..a248edb18 100644 --- a/app/models/payment.rb +++ b/app/models/payment.rb @@ -19,14 +19,15 @@ class Payment < ApplicationRecord :supporter_id, :supporter + belongs_to :donation belongs_to :supporter belongs_to :nonprofit + has_one :charge has_one :offsite_payment has_one :refund has_one :dispute_transaction has_many :disputes, through: :charge - belongs_to :donation has_many :tickets has_one :campaign, through: :donation has_many :campaign_gifts, through: :donation diff --git a/client/js/events/listings/index.js b/client/js/events/listings/index.js index b7e1ea4a7..e86cd4e14 100644 --- a/client/js/events/listings/index.js +++ b/client/js/events/listings/index.js @@ -25,8 +25,8 @@ module.exports = pathPrefix => { const listings = (key, state) => { const resp$ = state[key] const mixin = (content, count) => - h('section.u-marginBottom--30', [ - h('h5.u-centered.u-marginBottom--20', count + ' ' + key.charAt(0).toUpperCase() + key.slice(1) + ' Events') + h('section.u-marginBottom--20.u-marginTop--30', [ + h('h4.u-marginBottom--0.u-paddingX--20', count + ' ' + key.charAt(0).toUpperCase() + key.slice(1) + ' Events') , h(`div`, content) ]) diff --git a/spec/factories/events.rb b/spec/factories/events.rb index 0f116618c..82d7ef5e2 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -7,9 +7,13 @@ address { "100 N Appleton St" } city { "Appleton" } state_code { "WI" } - slug { "event-of-wonders" } + slug { SecureRandom.uuid } nonprofit profile + + before(:create) do |event, context| + Event.any_instance.stub(:geocode).and_return([1, 1]) + end end factory :event_base, class: "Event" do diff --git a/spec/factories/import.rb b/spec/factories/import.rb new file mode 100644 index 000000000..b08c736b6 --- /dev/null +++ b/spec/factories/import.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :import do + nonprofit + user + end +end diff --git a/spec/factories/nonprofits.rb b/spec/factories/nonprofits.rb index c2c709b41..7e278ebdb 100644 --- a/spec/factories/nonprofits.rb +++ b/spec/factories/nonprofits.rb @@ -6,10 +6,14 @@ state_code { "NM" } zip_code { 55555 } email { "example@email.com" } - slug { "sluggy-sluggo" } + slug { SecureRandom.uuid } billing_subscription { build(:billing_subscription, billing_plan: build(:billing_plan_percentage_fee_of_2_5_percent_and_5_cents_flat)) } vetted { true } + before(:create) do |np, context| + Nonprofit.any_instance.stub(:geocode).and_return([1, 1]) + end + factory :nonprofit_with_cards do after(:create) { |nonprofit, evaluator| create(:active_card_1, holder: nonprofit) diff --git a/spec/factories/ticket_levels.rb b/spec/factories/ticket_levels.rb index 6727564db..bc7bb4318 100644 --- a/spec/factories/ticket_levels.rb +++ b/spec/factories/ticket_levels.rb @@ -1,6 +1,9 @@ # License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later FactoryBot.define do factory :ticket_level do + name { Faker::Lorem.words(number: 3) } + event + trait :has_tickets do end end diff --git a/spec/factories/tickets.rb b/spec/factories/tickets.rb index 99e045e11..16a7ce011 100644 --- a/spec/factories/tickets.rb +++ b/spec/factories/tickets.rb @@ -1,6 +1,16 @@ # License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later FactoryBot.define do factory :ticket do + event_discount + supporter + profile + ticket_level + charge + payment + source_token + ticket_purchase + note { Faker::Lorem.sentence } + trait :has_event do event end diff --git a/spec/lib/insert/insert_source_token_spec.rb b/spec/lib/insert/insert_source_token_spec.rb index 3791d8aec..08c614838 100644 --- a/spec/lib/insert/insert_source_token_spec.rb +++ b/spec/lib/insert/insert_source_token_spec.rb @@ -58,9 +58,11 @@ end it "with event" do + Timecop.freeze(2020, 4, 5) do ouruuid = nil + event # to make sure SecureRandom.uuid from slug doesnt interfere with InsertSourceToken tokenizable = Card.create! expect(SecureRandom).to receive(:uuid).and_wrap_original { |m| ouruuid = m.call @@ -119,6 +121,7 @@ Timecop.freeze(2020, 4, 5) do ouruuid = nil + event # to make sure SecureRandom.uuid from slug doesnt interfere with InsertSourceToken tokenizable = Card.create! expect(SecureRandom).to receive(:uuid).and_wrap_original { |m| ouruuid = m.call diff --git a/spec/lib/query/query_event_metrics_spec.rb b/spec/lib/query/query_event_metrics_spec.rb new file mode 100644 index 000000000..5a015b1ae --- /dev/null +++ b/spec/lib/query/query_event_metrics_spec.rb @@ -0,0 +1,112 @@ +# License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later +require "rails_helper" + +describe QueryEventMetrics do + let(:nonprofit) { create(:nonprofit, slug: SecureRandom.uuid) } + let(:user) { create(:user) } + let(:profile) { create(:profile, user:) } + let(:supporter) { create(:supporter, nonprofit:, profile:, import: create(:import, user:, nonprofit:)) } + let(:supporter2) { create(:supporter, nonprofit:, import: create(:import, nonprofit:)) } + + let!(:event_active1) do + event = create(:event, nonprofit:, + name: Faker::FunnyName.four_word_name, + organizer_email: Faker::Internet.email, + end_datetime: 10.days.since.at_beginning_of_day, start_datetime: 10.days.ago.at_beginning_of_day, published: true, slug: SecureRandom.uuid, + address: Faker::Address.street_address, city: Faker::Address.city, state_code: Faker::Address.state_abbr, zip_code: Faker::Address.zip) + end + + let!(:event_active2) do + create(:event, nonprofit:, + name: Faker::FunnyName.four_word_name, + organizer_email: Faker::Internet.email, + end_datetime: 20.days.since.at_beginning_of_day, start_datetime: 10.days.ago.at_beginning_of_day, published: true, slug: SecureRandom.uuid, + address: Faker::Address.street_address, city: Faker::Address.city, state_code: Faker::Address.state_abbr, zip_code: Faker::Address.zip) + end + + let!(:tl1) { create(:ticket_level, event: event_active2) } + let!(:payment1) { create(:payment, supporter:, nonprofit:, gross_amount: 2137, fee_total: -159, net_amount: 1978) } + let!(:ticket1) { create(:ticket, event: event_active2, supporter:, profile:, payment: payment1, ticket_level: tl1, checked_in: true, quantity: 4) } + + let!(:payment2) { create(:payment, supporter:, nonprofit:, gross_amount: 1085, fee_total: -85, net_amount: 1000) } + let!(:ticket2) { create(:ticket, event: event_active2, supporter: supporter2, payment: payment2, ticket_level: tl1, checked_in: nil, quantity: 3) } + # Two tickets, two payments (21.37 + 10.85 = $32.22) + + let!(:donation_payment1) { create(:payment, supporter:, nonprofit:, gross_amount: 2664, fee_total: -164, net_amount: 2500) } + let!(:donation1) { create(:donation, supporter:, nonprofit:, event: event_active2, card: create(:card), amount: 2664, payment: donation_payment1) } + + let!(:donation_payment2) { create(:payment, supporter:, nonprofit:, gross_amount: 5295, fee_total: -295, net_amount: 5000) } + let!(:donation2) { create(:donation, supporter:, nonprofit:, event: event_active2, card: create(:card), amount: 5295, payment: donation_payment2) } + # Two donations 2664 + 5295 = $79.59 + + let!(:event_past) { create(:event, nonprofit:, end_datetime: 20.days.ago.at_beginning_of_day, published: true, slug: SecureRandom.uuid) } + let!(:event_deleted) { create(:event, nonprofit:, deleted: true, slug: SecureRandom.uuid) } + let!(:event_unpublished) { create(:event, nonprofit:, slug: SecureRandom.uuid) } + + let!(:nonprofit2) { create(:nonprofit, slug: SecureRandom.uuid) } + let!(:other_event_active2) { create(:event, nonprofit: nonprofit2, end_datetime: 10.days.since, published: true, slug: SecureRandom.uuid) } + let!(:other_event_active3) { create(:event, nonprofit: nonprofit2, end_datetime: 20.days.since, published: true, slug: SecureRandom.uuid) } + + describe ".for_listings" do + describe "for nonprofit" do + describe "active" do + subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"active" => true})} + + it "query result has correct attributes" do + expect(results.count).to eq 2 + + # first element is event_active2 because of order desc + expect(results.first["id"]).to eq event_active2.id + expect(results.first["name"]).to eq event_active2.name + expect(results.first["venue_name"]).to eq event_active2.venue_name + expect(results.first["address"]).to eq event_active2.address + expect(results.first["city"]).to eq event_active2.city + expect(results.first["state_code"]).to eq event_active2.state_code + expect(results.first["zip_code"]).to eq event_active2.zip_code + expect(results.first["start_datetime"]).to eq 10.days.ago.at_beginning_of_day + expect(results.first["end_datetime"]).to eq 20.days.since.at_beginning_of_day + expect(results.first["organizer_email"]).to eq event_active2.organizer_email + expect(results.first["checked_in_count"]).to eq 1 + expect(results.first["total_attendees"]).to eq 7 + expect(results.first["tickets_total_paid"]).to eq 3_222 + expect(results.first["donations_total_paid"]).to eq 7_959 + expect(results.first["total_paid"]).to eq (3_222 + 7_959) + + expect(results.last["id"]).to eq event_active1.id + end + end + + describe "active" do + subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"past" => true})} + it "query result has correct element" do + expect(results.count).to eq 1 + expect(results.first["id"]).to eq event_past.id + end + end + + describe "unpublished" do + subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"unpublished" => true})} + it "query result has correct element" do + expect(results.count).to eq 1 + expect(results.first["id"]).to eq event_unpublished.id + end + end + + describe "deleted" do + subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"deleted" => true})} + it "query result has correct element" do + expect(results.count).to eq 1 + expect(results.first["id"]).to eq event_deleted.id + end + end + end + + describe "for profile" do + skip "TODO" + end + end + + describe ".with_event_ids" do + skip "TODO" + end +end diff --git a/spec/lib/update/update_tickets_spec.rb b/spec/lib/update/update_tickets_spec.rb index 7abf3ca81..b77675f00 100644 --- a/spec/lib/update/update_tickets_spec.rb +++ b/spec/lib/update/update_tickets_spec.rb @@ -36,8 +36,9 @@ profile_id: nil, note: nil, deleted: false, - source_token_id: nil, - ticket_level_id: nil + source_token: nil, + ticket_level: nil, + ticket_purchase: nil } } @@ -68,13 +69,11 @@ let(:charge) { force_create(:charge) } let(:ticket) { - force_create(:ticket, - general_ticket.merge(event: event)) + force_create(:ticket, general_ticket.merge(event: event)) } let(:other_ticket) { - force_create(:ticket, - general_ticket.merge(event: other_event)) + force_create(:ticket, general_ticket.merge(event: other_event)) } it "basic validation" do From 6e1d88f2ff4534594df781f3a5e3b20a17da71fc Mon Sep 17 00:00:00 2001 From: Casey Helbling Date: Tue, 17 Jun 2025 22:47:40 -0500 Subject: [PATCH 4/5] Linter --- app/legacy_lib/query_event_metrics.rb | 77 +++++++++++----------- spec/lib/query/query_event_metrics_spec.rb | 28 ++++---- 2 files changed, 51 insertions(+), 54 deletions(-) diff --git a/app/legacy_lib/query_event_metrics.rb b/app/legacy_lib/query_event_metrics.rb index eeb4f49c2..4da38b6ec 100644 --- a/app/legacy_lib/query_event_metrics.rb +++ b/app/legacy_lib/query_event_metrics.rb @@ -8,9 +8,9 @@ def self.with_event_ids(event_ids) return [] if event_ids.blank? events = Event.select(:id, :name, :venue_name, :address, :city, :state_code, - :zip_code, :start_datetime, :end_datetime, :organizer_email) - .where(id: event_ids) - .limit(QUERY_MAX_LIMIT) + :zip_code, :start_datetime, :end_datetime, :organizer_email) + .where(id: event_ids) + .limit(QUERY_MAX_LIMIT) add_metrics_to_events(events) end @@ -27,7 +27,7 @@ def self.for_listings(id_type, profile_or_nonprofit_id, params) def self.base_events(id_type, id, params) query = Event.select(:id, :name, :venue_name, :address, :city, :state_code, - :zip_code, :start_datetime, :end_datetime, :organizer_email) + :zip_code, :start_datetime, :end_datetime, :organizer_email) case id_type when "profile" @@ -57,9 +57,7 @@ def self.base_events(id_type, id, params) end query = query.order(end_datetime: :desc) - query = query.limit(QUERY_MAX_LIMIT) - - query + query.limit(QUERY_MAX_LIMIT) end def self.add_metrics_to_events(events) @@ -73,33 +71,33 @@ def self.add_metrics_to_events(events) events.map do |event| build_event_result(event, - ticket_metrics[event.id], - donation_metrics[event.id], - payment_metrics[event.id]) + ticket_metrics[event.id], + donation_metrics[event.id], + payment_metrics[event.id]) end end def self.build_event_result(event, tickets, donations, payments) - tickets ||= { total: 0, checked_in_count: 0 } - donations ||= { donation_total: 0 } - payments ||= { payment_total: 0 } + tickets ||= {total: 0, checked_in_count: 0} + donations ||= {donation_total: 0} + payments ||= {payment_total: 0} { - 'id' => event.id, - 'name' => event.name, - 'venue_name' => event.venue_name, - 'address' => event.address, - 'city' => event.city, - 'state_code' => event.state_code, - 'zip_code' => event.zip_code, - 'start_datetime' => event.start_datetime, - 'end_datetime' => event.end_datetime, - 'organizer_email' => event.organizer_email, - 'total_attendees' => tickets[:total], - 'checked_in_count' => tickets[:checked_in_count], - 'tickets_total_paid' => payments[:payment_total], - 'donations_total_paid' => donations[:donation_total], - 'total_paid' => payments[:payment_total] + donations[:donation_total] + "id" => event.id, + "name" => event.name, + "venue_name" => event.venue_name, + "address" => event.address, + "city" => event.city, + "state_code" => event.state_code, + "zip_code" => event.zip_code, + "start_datetime" => event.start_datetime, + "end_datetime" => event.end_datetime, + "organizer_email" => event.organizer_email, + "total_attendees" => tickets[:total], + "checked_in_count" => tickets[:checked_in_count], + "tickets_total_paid" => payments[:payment_total], + "donations_total_paid" => donations[:donation_total], + "total_paid" => payments[:payment_total] + donations[:donation_total] } end @@ -107,14 +105,13 @@ def self.ticket_metrics(event_ids) return {} if event_ids.blank? ticket_data = Ticket.where(event_id: event_ids) - .group(:event_id) - .pluck(:event_id, - Arel.sql('COALESCE(SUM(quantity), 0) as total'), - Arel.sql('COALESCE(SUM(CASE WHEN checked_in THEN 1 ELSE 0 END), 0) as checked_in_count') - ) + .group(:event_id) + .pluck(:event_id, + Arel.sql("COALESCE(SUM(quantity), 0) as total"), + Arel.sql("COALESCE(SUM(CASE WHEN checked_in THEN 1 ELSE 0 END), 0) as checked_in_count")) ticket_data.to_h do |event_id, total, checked_in_count| - [event_id, {total:, checked_in_count:}] + [event_id, {total:, checked_in_count:}] end end @@ -122,9 +119,9 @@ def self.donation_metrics(event_ids) return {} if event_ids.blank? donation_data = Donation.left_joins(:payments) - .where(event_id: event_ids) - .group(:event_id) - .pluck(:event_id, Arel.sql('COALESCE(SUM(payments.gross_amount), 0) as donation_total')) + .where(event_id: event_ids) + .group(:event_id) + .pluck(:event_id, Arel.sql("COALESCE(SUM(payments.gross_amount), 0) as donation_total")) donation_data.to_h do |event_id, donation_total| [event_id, {donation_total:}] @@ -136,9 +133,9 @@ def self.payment_metrics(event_ids) # TODO: consider deleted tickets too payment_data = Payment.joins(:tickets) - .where(tickets: { event_id: event_ids }) - .group('tickets.event_id') - .pluck('tickets.event_id', Arel.sql('COALESCE(SUM(payments.gross_amount), 0) as payment_total')) + .where(tickets: {event_id: event_ids}) + .group("tickets.event_id") + .pluck("tickets.event_id", Arel.sql("COALESCE(SUM(payments.gross_amount), 0) as payment_total")) payment_data.to_h do |event_id, payment_total| [event_id, {payment_total:}] diff --git a/spec/lib/query/query_event_metrics_spec.rb b/spec/lib/query/query_event_metrics_spec.rb index 5a015b1ae..301797df9 100644 --- a/spec/lib/query/query_event_metrics_spec.rb +++ b/spec/lib/query/query_event_metrics_spec.rb @@ -9,19 +9,19 @@ let(:supporter2) { create(:supporter, nonprofit:, import: create(:import, nonprofit:)) } let!(:event_active1) do - event = create(:event, nonprofit:, - name: Faker::FunnyName.four_word_name, - organizer_email: Faker::Internet.email, - end_datetime: 10.days.since.at_beginning_of_day, start_datetime: 10.days.ago.at_beginning_of_day, published: true, slug: SecureRandom.uuid, - address: Faker::Address.street_address, city: Faker::Address.city, state_code: Faker::Address.state_abbr, zip_code: Faker::Address.zip) + create(:event, nonprofit:, + name: Faker::FunnyName.four_word_name, + organizer_email: Faker::Internet.email, + end_datetime: 10.days.since.at_beginning_of_day, start_datetime: 10.days.ago.at_beginning_of_day, published: true, slug: SecureRandom.uuid, + address: Faker::Address.street_address, city: Faker::Address.city, state_code: Faker::Address.state_abbr, zip_code: Faker::Address.zip) end let!(:event_active2) do create(:event, nonprofit:, - name: Faker::FunnyName.four_word_name, - organizer_email: Faker::Internet.email, - end_datetime: 20.days.since.at_beginning_of_day, start_datetime: 10.days.ago.at_beginning_of_day, published: true, slug: SecureRandom.uuid, - address: Faker::Address.street_address, city: Faker::Address.city, state_code: Faker::Address.state_abbr, zip_code: Faker::Address.zip) + name: Faker::FunnyName.four_word_name, + organizer_email: Faker::Internet.email, + end_datetime: 20.days.since.at_beginning_of_day, start_datetime: 10.days.ago.at_beginning_of_day, published: true, slug: SecureRandom.uuid, + address: Faker::Address.street_address, city: Faker::Address.city, state_code: Faker::Address.state_abbr, zip_code: Faker::Address.zip) end let!(:tl1) { create(:ticket_level, event: event_active2) } @@ -50,7 +50,7 @@ describe ".for_listings" do describe "for nonprofit" do describe "active" do - subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"active" => true})} + subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"active" => true}) } it "query result has correct attributes" do expect(results.count).to eq 2 @@ -70,14 +70,14 @@ expect(results.first["total_attendees"]).to eq 7 expect(results.first["tickets_total_paid"]).to eq 3_222 expect(results.first["donations_total_paid"]).to eq 7_959 - expect(results.first["total_paid"]).to eq (3_222 + 7_959) + expect(results.first["total_paid"]).to eq(3_222 + 7_959) expect(results.last["id"]).to eq event_active1.id end end describe "active" do - subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"past" => true})} + subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"past" => true}) } it "query result has correct element" do expect(results.count).to eq 1 expect(results.first["id"]).to eq event_past.id @@ -85,7 +85,7 @@ end describe "unpublished" do - subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"unpublished" => true})} + subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"unpublished" => true}) } it "query result has correct element" do expect(results.count).to eq 1 expect(results.first["id"]).to eq event_unpublished.id @@ -93,7 +93,7 @@ end describe "deleted" do - subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"deleted" => true})} + subject(:results) { QueryEventMetrics.for_listings("nonprofit", nonprofit.id, {"deleted" => true}) } it "query result has correct element" do expect(results.count).to eq 1 expect(results.first["id"]).to eq event_deleted.id From 3acd120d05c8dfd27763093f0291836c61a629b8 Mon Sep 17 00:00:00 2001 From: Casey Helbling Date: Tue, 17 Jun 2025 23:04:15 -0500 Subject: [PATCH 5/5] Remove redundant index --- db/migrate/20250617142915_add_indexes_events.rb | 3 --- db/schema.rb | 1 - 2 files changed, 4 deletions(-) diff --git a/db/migrate/20250617142915_add_indexes_events.rb b/db/migrate/20250617142915_add_indexes_events.rb index e757d96ca..918f14164 100644 --- a/db/migrate/20250617142915_add_indexes_events.rb +++ b/db/migrate/20250617142915_add_indexes_events.rb @@ -1,8 +1,5 @@ class AddIndexesEvents < ActiveRecord::Migration[7.1] def change - add_index :events, [:nonprofit_id, :end_datetime, :published, :deleted], - name: 'idx_events_listings_query' - add_index :tickets, [:event_id, :quantity, :checked_in], name: 'idx_tickets_event_metrics' diff --git a/db/schema.rb b/db/schema.rb index 1f9e53a85..e40fc4de5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -408,7 +408,6 @@ t.datetime "end_datetime", precision: nil t.index ["nonprofit_id", "deleted", "published", "end_datetime"], name: "events_nonprofit_id_not_deleted_and_published_endtime" t.index ["nonprofit_id", "deleted", "published"], name: "index_events_on_nonprofit_id_and_deleted_and_published" - t.index ["nonprofit_id", "end_datetime", "published", "deleted"], name: "idx_events_listings_query" t.index ["nonprofit_id"], name: "index_events_on_nonprofit_id" end