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

Refactor: Update goal/stats to use Donation Query #7376

Conversation

kjohnson
Copy link
Member

@kjohnson kjohnson commented Apr 29, 2024

Resolves GIVE-677

Description

This PR updates the donation goal and the donation stats for the Donation Form to use a new DonationQuery, which supports a date range and accounts for recovered fees.

Affects

  • Adds a new DonationQuery class to account for recovered fees.
  • Adds new goal settings to the DonationFormGoalData DTO.
  • Updates the getTotalRevenue method with the DonationQuery.
  • Adds a new getTotalRevenueForDateRange method to the repository.
  • Updates the getTotalNumberOfDonations method with the DonationQuery.
  • Adds a new getTotalNumberOfDonationsForDateRange method to the repository.
  • Updates the formStatsData array to use the DonationQuery.

@kjohnson kjohnson changed the title wip Refactor: Update goal/stats to use Donation Query Apr 30, 2024
@kjohnson kjohnson requested a review from alaca April 30, 2024 16:38
@kjohnson kjohnson marked this pull request as ready for review April 30, 2024 16:40
public function between($startDate, $endDate)
{
$this->joinMeta('_give_completed_date', 'completed');
$this->whereBetween('completed.meta_value', $startDate, $endDate);
Copy link
Member

@alaca alaca Apr 30, 2024

Choose a reason for hiding this comment

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

I didn't test it on my end, but I'm very surprised that this works because the meta_value type is longtext and not datetime 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Tested this on my end and it works 😅
Interesting

$this->joinMeta('_give_payment_total', 'amount');
$this->joinMeta('_give_fee_donation_amount', 'intendedAmount');
return $this->sum(
'COALESCE(intendedAmount.meta_value, amount.meta_value)'
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm, I'm not sure I fully understand this.
_give_payment_total has an amount that includes fees, and _give_fee_donation_amount doesn't include fees.
_give_fee_donation_amount is set only if fee recovery is checked. Correct me if I'm wrong, but COALESCE will return both _give_payment_total and _give_fee_donation_amount if the values are not null. So, if both are set, then the calculation is wrong, or I'm missing something?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, it works as expected.

Copy link
Member

@alaca alaca left a comment

Choose a reason for hiding this comment

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

I had some concerns regarding dates, but it looks like it works as expected. Also, COALESCE is doing its magic.

public function between($startDate, $endDate)
{
$this->joinMeta('_give_completed_date', 'completed');
$this->whereBetween('completed.meta_value', $startDate, $endDate);
Copy link
Member

Choose a reason for hiding this comment

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

Tested this on my end and it works 😅
Interesting

$this->joinMeta('_give_payment_total', 'amount');
$this->joinMeta('_give_fee_donation_amount', 'intendedAmount');
return $this->sum(
'COALESCE(intendedAmount.meta_value, amount.meta_value)'
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, it works as expected.

@alaca alaca merged commit e3e86ef into epic/time-based-form-goals-GIVE-565 May 2, 2024
20 checks passed
@alaca alaca deleted the feature/time-based-goal-queries-GIVE-677 branch May 2, 2024 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants