Merge pull request #116 from houdiniproject/fix_campaign_export_calculations

Campaign export calculations are fixed and specs

db/migrate/20181128221143_add_indexes_to_recurring_donations.rb

@@ -0,0 +1,5 @@
+class AddIndexesToRecurringDonations < ActiveRecord::Migration
+  def change
+    add_index :recurring_donations, :donation_id
+  end
+end

db/migrate/20181129205652_add_donation_id_index_to_campaign_gifts.rb

@@ -0,0 +1,5 @@
+class AddDonationIdIndexToCampaignGifts < ActiveRecord::Migration
+  def change
+    add_index :campaign_gifts, :donation_id
+  end
+end

db/migrate/20181129224030_add_index_to_payments_created_at.rb

@@ -0,0 +1,5 @@
+class AddIndexToPaymentsCreatedAt < ActiveRecord::Migration
+  def change
+    add_index :payments, :created_at
+  end
+end

db/structure.sql

@@ -3147,6 +3147,13 @@ CREATE INDEX index_activities_on_supporter_id ON public.activities USING btree (
 CREATE INDEX index_campaign_gifts_on_campaign_gift_option_id ON public.campaign_gifts USING btree (campaign_gift_option_id);
 
 
+--
+-- Name: index_campaign_gifts_on_donation_id; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_campaign_gifts_on_donation_id ON public.campaign_gifts USING btree (donation_id);
+
+
 --
 -- Name: index_campaigns_on_parent_campaign_id; Type: INDEX; Schema: public; Owner: -
 --
@@ -3189,6 +3196,20 @@ CREATE INDEX index_exports_on_nonprofit_id ON public.exports USING btree (nonpro
 CREATE INDEX index_exports_on_user_id ON public.exports USING btree (user_id);
 
 
+--
+-- Name: index_payments_on_created_at; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_payments_on_created_at ON public.payments USING btree (created_at);
+
+
+--
+-- Name: index_recurring_donations_on_donation_id; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_recurring_donations_on_donation_id ON public.recurring_donations USING btree (donation_id);
+
+
 --
 -- Name: index_refunds_on_charge_id; Type: INDEX; Schema: public; Owner: -
 --
@@ -4372,3 +4393,9 @@ INSERT INTO schema_migrations (version) VALUES ('20181003212559');
 
 INSERT INTO schema_migrations (version) VALUES ('20181120182105');
 
+INSERT INTO schema_migrations (version) VALUES ('20181128221143');
+
+INSERT INTO schema_migrations (version) VALUES ('20181129205652');
+
+INSERT INTO schema_migrations (version) VALUES ('20181129224030');
+

lib/qexpr.rb

@@ -159,6 +159,12 @@ class Qexpr
       .put(:joins, (@tree[:joins] || Hamster::Vector[]).add("\nLEFT OUTER JOIN".bold.light_blue + " #{table_name}\n  ".blue + "ON".bold.light_blue + " #{on_expr}".blue))
   end
 
+  def join_lateral(join_name, select_statement, success_condition=true, data={})
+    select_statement = Qexpr.interpolate_expr(select_statement, data)
+    return Qexpr.new @tree
+             .put(:joins, (@tree[:joins] || Hamster::Vector[]).add("\n JOIN LATERAL".bold.light_blue + " (#{select_statement})\n  #{join_name} ".blue + "ON".bold.light_blue + " #{success_condition}".blue))
+  end
+
 
   def as(name)
     return Qexpr.new @tree.put(:as, name)

lib/query/query_campaign_gifts.rb

@@ -15,25 +15,46 @@ module QueryCampaignGifts
 	# Includes the overall sum as well as the donations without gift options
 	def self.report_metrics(campaign_id)
 
+
 		data = Psql.execute(%Q(
 			SELECT campaign_gift_options.name
 				, COUNT(*) AS total_donations
 				, SUM(ds_one_time.amount) AS total_one_time
 				, SUM(ds_recurring.amount) AS total_recurring
-			FROM donations AS ds
+			FROM (#{donations_for_campaign(campaign_id).parse}) AS ds
-			LEFT OUTER JOIN donations ds_one_time
+			LEFT OUTER JOIN (#{get_corresponding_payments(campaign_id, %Q(LEFT OUTER JOIN recurring_donations ON recurring_donations.donation_id = donations.id
-			ON ds_one_time.id=ds.id AND (ds.recurring IS NULL OR ds.recurring='f')
+                          ), %Q(WHERE recurring_donations.id IS NULL))}) ds_one_time
-			LEFT OUTER JOIN donations ds_recurring
+			ON ds_one_time.id = ds.id
-			ON ds_recurring.id=ds.id AND ds.recurring='t'
+			LEFT OUTER JOIN (#{get_corresponding_payments(campaign_id, %Q(INNER JOIN recurring_donations ON recurring_donations.donation_id = donations.id))}) ds_recurring
+					ON ds_recurring.id = ds.id
 			LEFT OUTER JOIN campaign_gifts
 			ON campaign_gifts.donation_id=ds.id
 			LEFT OUTER JOIN campaign_gift_options
 			ON campaign_gifts.campaign_gift_option_id=campaign_gift_options.id
-			WHERE ds.campaign_id=#{Qexpr.quote(campaign_id)}
 			GROUP BY campaign_gift_options.id
 			ORDER BY total_donations DESC
 		))
 
 		return Hamster::Hash[data: data]
 	end
+
+	def self.donations_for_campaign(campaign_id)
+		Qx.select('donations.id, donations.amount').from(:donations).where("campaign_id IN ($ids)", {ids:QueryCampaigns.get_campaign_and_children(campaign_id)
+																																											 })
 	end
+
+	def self.get_corresponding_payments(campaign_id, recurring_clauses, where_clauses="")
+		%Q(SELECT donations.id, payments.gross_amount AS amount
+			FROM (#{donations_for_campaign(campaign_id).parse}) donations
+				#{recurring_clauses}
+			JOIN LATERAL (
+				SELECT payments.id, payments.gross_amount, payments.donation_id, payments.created_at FROM payments
+				WHERE payments.donation_id = donations.id
+				ORDER BY payments.created_at ASC
+				LIMIT 1
+			) payments ON true
+			#{where_clauses}
+		)
+	end
+end
+

lib/query/query_campaign_metrics.rb

@@ -4,21 +4,19 @@ module QueryCampaignMetrics
   def self.on_donations(campaign_id)
     campaign = Campaign.find(campaign_id)
 
-    result = Qx.select(
+    result = Psql.execute(
-        "COALESCE(COUNT(DISTINCT donations.id), 0) AS supporters_count",
+      Qexpr.new.select("COALESCE(COUNT(DISTINCT donations.id), 0) AS supporters_count",
-        "COALESCE(SUM(payments.gross_amount), 0) AS total_raised"
+                       "COALESCE(SUM(payments.gross_amount), 0) AS total_raised")
-         )
         .from("campaigns")
         .join(
-        ["donations", "donations.campaign_id=campaigns.id"],
+          "donations", "donations.campaign_id=campaigns.id"
-        ["payments", "payments.donation_id=donations.id"]
         )
+        .join_lateral("payments", QueryDonations.get_first_payment_for_donation.parse, true)
         .where("campaigns.id IN (#{QueryCampaigns
                                        .get_campaign_and_children(campaign_id)
                                        .parse
         })")
-      .execute
+    ).last
-      .last
 
     return {
         'supporters_count' => result['supporters_count'],

lib/query/query_donations.rb

@@ -8,8 +8,8 @@ module QueryDonations
     Psql.execute_vectors(
       Qexpr.new.select([
         'donations.created_at',
-				'(donations.amount/100.00)::money::text AS amount',
+				'(payments.gross_amount/100.00)::money::text AS amount',
-				"COALESCE(donations.recurring, FALSE) AS recurring",
+				"COUNT(recurring_donations.id) > 0 AS recurring",
 				"STRING_AGG(campaign_gift_options.name, ',') AS campaign_gift_names"
       ].concat(QuerySupporters.supporter_export_selections)
        .concat([
@@ -19,10 +19,13 @@ module QueryDonations
      .join(:supporters, "supporters.id=donations.supporter_id")
      .left_outer_join(:campaign_gifts, "campaign_gifts.donation_id=donations.id")
      .left_outer_join(:campaign_gift_options, "campaign_gift_options.id=campaign_gifts.campaign_gift_option_id")
+     .left_outer_join(:recurring_donations, "recurring_donations.donation_id = donations.id")
+     .join_lateral(:payments,
+                   get_first_payment_for_donation.parse, true)
      .where("donations.campaign_id IN (#{QueryCampaigns
                                             .get_campaign_and_children(campaign_id)
                                             .parse})")
-     .group_by("donations.id", "supporters.id")
+     .group_by("donations.id", "supporters.id", "payments.id", "payments.gross_amount")
      .order_by("donations.date")
     )
 	end
@@ -69,4 +72,11 @@ module QueryDonations
     )[1..-1].map(&:flatten)
   end
 
+  def self.get_first_payment_for_donation(table_selector='donations')
+    Qx.select('payments.id, payments.gross_amount').from(:payments)
+        .where("payments.donation_id = #{table_selector}.id")
+        .order_by('payments.created_at ASC')
+        .limit(1)
+  end
+
 end

lib/query/query_supporters.rb

@@ -13,6 +13,11 @@ module QuerySupporters
      .left_outer_join('donations', 'donations.supporter_id=supporters.id')
      .left_outer_join('campaign_gifts', 'donations.id=campaign_gifts.donation_id')
      .left_outer_join('campaign_gift_options', 'campaign_gifts.campaign_gift_option_id=campaign_gift_options.id')
+    .join_lateral(:payments,  Qx
+                                  .select('payments.id, payments.gross_amount').from(:payments)
+                                  .where('payments.donation_id = donations.id')
+                                  .order_by('payments.created_at ASC')
+                                  .limit(1).parse, true)
      .where("supporters.nonprofit_id=$id", id: np_id)
      .where("donations.campaign_id IN (#{QueryCampaigns
                                              .get_campaign_and_children(campaign_id)
@@ -41,7 +46,7 @@ module QuerySupporters
         'supporters.id',
         'supporters.name',
         'supporters.email',
-        'SUM(donations.amount) AS total_raised',
+        'SUM(payments.gross_amount) AS total_raised',
         'ARRAY_AGG(DISTINCT campaign_gift_options.name) AS campaign_gift_names',
         'DATE(MAX(donations.created_at)) AS latest_gift',
       ).limit(limit).offset(offset)

spec/lib/query/query_campaign_gifts_spec.rb

@@ -0,0 +1,61 @@
+# License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later
+require 'rails_helper'
+
+describe QueryCampaignGifts do
+  GIFT_LEVEL_ONE_TIME = 1111
+  GIFT_LEVEL_RECURRING = 5585
+  GIFT_LEVEL_CHANGED_RECURRING = 5512
+  CAMPAIGN_GIFT_OPTION_NAME = "theowthoinv"
+  let(:np) { force_create(:nonprofit)}
+  let(:supporter1) { force_create(:supporter, nonprofit: np)}
+  let(:supporter2) { force_create(:supporter, nonprofit: np)}
+  let(:campaign) { force_create(:campaign, nonprofit: np, slug: "slug stuff")}
+  let(:campaign_gift_option) { force_create(:campaign_gift_option, campaign: campaign, name: CAMPAIGN_GIFT_OPTION_NAME, amount_one_time: GIFT_LEVEL_ONE_TIME, amount_recurring: GIFT_LEVEL_RECURRING)}
+  let(:campaign_gift1) { force_create(:campaign_gift, campaign_gift_option: campaign_gift_option, donation: donation1)}
+  let(:donation1) { force_create(:donation, amount: GIFT_LEVEL_ONE_TIME, campaign: campaign, supporter:supporter1)}
+
+  let(:payment1) {force_create(:payment, gross_amount: GIFT_LEVEL_ONE_TIME, donation: donation1)}
+
+  let(:donation2)  {force_create(:donation, amount: GIFT_LEVEL_CHANGED_RECURRING, campaign: campaign, supporter:supporter2)}
+  let(:payment2) {force_create(:payment, gross_amount: GIFT_LEVEL_RECURRING, donation: donation2)}
+  let(:payment3) {force_create(:payment, gross_amount: GIFT_LEVEL_CHANGED_RECURRING, donation: donation2)}
+  let(:campaign_gift2) { force_create(:campaign_gift, campaign_gift_option: campaign_gift_option, donation: donation2)}
+  let(:recurring) {force_create(:recurring_donation, donation: donation2, amount: GIFT_LEVEL_CHANGED_RECURRING)}
+
+
+  let(:init_all) {
+    np
+    supporter1
+    supporter2
+    campaign_gift1
+    campaign_gift2
+    recurring
+    payment1
+    payment2
+    payment3
+    gift_level_match
+  }
+  let(:gift_level_match) {
+
+    QueryCampaignGifts.report_metrics(campaign.id)
+  }
+
+  before(:each) {
+    init_all
+  }
+
+  it 'counts gift donations properly' do
+    glm = gift_level_match
+
+    data = glm[:data]
+
+    expect(data).to eq([
+        {
+            "name" =>  CAMPAIGN_GIFT_OPTION_NAME,
+            'total_donations' => 2,
+            'total_one_time'=> GIFT_LEVEL_ONE_TIME,
+            'total_recurring' => GIFT_LEVEL_RECURRING
+        }])
+
+  end
+end

spec/lib/query/query_campaign_metrics_spec.rb

@@ -1,9 +1,7 @@
 # License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later
 require 'rails_helper'
 
-
 describe QueryCampaignMetrics do
-
   describe 'calculates your metrics plus children' do
     let(:nonprofit) {force_create(:nonprofit)}
     let(:campaign) {force_create(:campaign, nonprofit:nonprofit, show_total_count:false, show_total_raised: false, goal_amount: 16000)}
@@ -17,28 +15,24 @@ describe QueryCampaignMetrics do
     let(:donation2) { force_create(:donation, campaign: campaign, amount: 2000)}
     let(:payment2) { force_create(:payment, donation: donation2, gross_amount:2000)}
 
-
+    let(:donation3) { force_create(:donation, campaign: campaign_child, amount: 2000)}
-    let(:donation3) { force_create(:donation, campaign: campaign_child, amount: 4000)}
+    let(:payment3) { force_create(:payment, donation: donation3, gross_amount:4000, kind:'RecurringPayment')}
-    let(:payment3) { force_create(:payment, donation: donation3, gross_amount:4000)}
+    let(:payment3_1) { force_create(:payment, donation: donation3, gross_amount:2000, kind:'RecurringPayment')}
 
     let(:donation4) { force_create(:donation, campaign: campaign_child_2, amount: 8000)}
     let(:payment4) { force_create(:payment, donation: donation4, gross_amount:8000)}
 
-
-
     let(:payments) do
       payment
       payment2
       payment3
+      payment3_1
       payment4
     end
+
     let (:campaign_metric) do
-
-
       payments
-
       QueryCampaignMetrics.on_donations(campaign.id)
-
     end
 
     let(:campaign_child_metric) do
@@ -75,7 +69,5 @@ describe QueryCampaignMetrics do
       expect(campaign_child_2_metric['show_total_count']).to eq true
       expect(campaign_child_2_metric['show_total_raised']).to eq true
     end
-
   end
-
 end

spec/lib/query/query_donations_spec.rb

@@ -0,0 +1,57 @@
+# License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later
+require 'rails_helper'
+
+describe QueryDonations do
+
+  describe :campaign_export do
+    let(:nonprofit) {force_create(:nonprofit)}
+    let(:supporter) {force_create(:supporter)}
+    let(:campaign) {force_create(:campaign, nonprofit:nonprofit, show_total_count:false, show_total_raised: false, goal_amount: 16000)}
+    let(:campaign_child) {force_create(:campaign, nonprofit:nonprofit, parent_campaign:campaign, show_total_count:true, show_total_raised: true, goal_amount: 8000)}
+
+    let(:campaign_child_2) {force_create(:campaign, nonprofit:nonprofit, parent_campaign:campaign, show_total_count:true, show_total_raised: true, goal_amount: 4000 )}
+
+    let(:donation) { force_create(:donation, campaign: campaign, amount: 1000, supporter:supporter)}
+    let(:payment) { force_create(:payment, donation: donation, gross_amount:1000, supporter:supporter)}
+
+    let(:donation2) { force_create(:donation, campaign: campaign, amount: 2000, supporter:supporter)}
+    let(:payment2) { force_create(:payment, donation: donation2, gross_amount:2000, supporter:supporter)}
+
+    let(:donation3) { force_create(:donation, campaign: campaign_child, amount: 2000, supporter:supporter)}
+    let(:payment3) { force_create(:payment, donation: donation3, gross_amount:4000, kind:'RecurringPayment', supporter:supporter)}
+    let(:payment3_1) { force_create(:payment, donation: donation3, gross_amount:2000, kind:'RecurringPayment', supporter:supporter)}
+    let(:recurring) {force_create(:recurring_donation, donation: donation3, amount: 2000, supporter:supporter)}
+
+    let(:donation4) { force_create(:donation, campaign: campaign_child_2, amount: 8000, supporter:supporter)}
+    let(:payment4) { force_create(:payment, donation: donation4, gross_amount:8000, supporter:supporter)}
+
+    let(:payments) do
+      payment
+      payment2
+      payment3
+      payment3_1
+      recurring
+      payment4
+    end
+
+    let (:campaign_export) do
+      payments
+      QueryDonations.campaign_export(campaign.id)
+
+    end
+
+    it 'payment amounts get the first payment, not additional ones' do
+      export = vector_to_hash(campaign_export)
+
+      expect(export.map{|i| i['Amount']}).to match_array(['$10.00', '$20.00', '$40.00', '$80.00'])
+    end
+  end
+
+  ## move to common area
+  def vector_to_hash(vecs)
+    keys = vecs.first.to_a.map{|k| k.to_s.titleize}
+
+    vecs.drop(1).map{|v| keys.zip(v).to_h}
+  end
+
+end

spec/lib/query/query_supporters_spec.rb

@@ -0,0 +1,55 @@
+# License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later
+require 'rails_helper'
+
+describe QueryCampaignGifts do
+  GIFT_LEVEL_ONE_TIME = 1111
+  GIFT_LEVEL_RECURRING = 5585
+  GIFT_LEVEL_CHANGED_RECURRING = 5512
+  CAMPAIGN_GIFT_OPTION_NAME = "theowthoinv"
+  let(:np) { force_create(:nonprofit)}
+  let(:supporter1) { force_create(:supporter, nonprofit: np)}
+  let(:supporter2) { force_create(:supporter, nonprofit: np)}
+  let(:campaign) { force_create(:campaign, nonprofit: np, slug: "slug stuff")}
+  let(:campaign_gift_option) { force_create(:campaign_gift_option, campaign: campaign, name: CAMPAIGN_GIFT_OPTION_NAME, amount_one_time: GIFT_LEVEL_ONE_TIME, amount_recurring: GIFT_LEVEL_RECURRING)}
+  let(:campaign_gift1) { force_create(:campaign_gift, campaign_gift_option: campaign_gift_option, donation: donation1)}
+  let(:donation1) { force_create(:donation, amount: GIFT_LEVEL_ONE_TIME, campaign: campaign, supporter:supporter1)}
+
+  let(:payment1) {force_create(:payment, gross_amount: GIFT_LEVEL_ONE_TIME, donation: donation1)}
+
+  let(:donation2)  {force_create(:donation, amount: GIFT_LEVEL_CHANGED_RECURRING, campaign: campaign, supporter:supporter2)}
+  let(:payment2) {force_create(:payment, gross_amount: GIFT_LEVEL_RECURRING, donation: donation2)}
+  let(:payment3) {force_create(:payment, gross_amount: GIFT_LEVEL_CHANGED_RECURRING, donation: donation2)}
+  let(:campaign_gift2) { force_create(:campaign_gift, campaign_gift_option: campaign_gift_option, donation: donation2)}
+  let(:recurring) {force_create(:recurring_donation, donation: donation2, amount: GIFT_LEVEL_CHANGED_RECURRING)}
+
+
+  let(:init_all) {
+    np
+    supporter1
+    supporter2
+    campaign_gift1
+    campaign_gift2
+    recurring
+    payment1
+    payment2
+    payment3
+  }
+
+  let(:campaign_list) {
+
+    QuerySupporters.campaign_list(np.id, campaign.id, {page: 0})
+  }
+
+  before(:each) {
+    init_all
+  }
+
+  it 'counts gift donations properly' do
+    glm = campaign_list
+
+    data = glm[:data]
+
+    expect(data.map{|i| i['total_raised']}).to match_array([GIFT_LEVEL_ONE_TIME, GIFT_LEVEL_RECURRING])
+
+  end
+end