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: Default form #7616

Merged
merged 26 commits into from
Dec 4, 2024
Merged

Conversation

alaca
Copy link
Member

@alaca alaca commented Nov 8, 2024

Description

This PR refactors the logic of getting and setting the default form. The default form id is now property of a campaign. The existing form_id column is used as a default form id column - this way we don't have to refactor P2P. Also, Campaign's forms and defaultForm methods are refactored. The forms method now returns both V2 and V3 forms.

Affects

Campaign migrations
Campaign model
Campaign repository

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Self Review of code and UX completed

$defaultForm = $this->forms()
->where('campaign_forms.is_default', true)
->get();

if (is_null($defaultForm)) {
$defaultForm = $this->legacyForms()
->where('campaign_forms.is_default', true)
->get();
}

return $defaultForm;
return give(DonationFormsRepository::class)->getById($this->defaultFormId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Now we're talkin! 🙌

@JasonTheAdams
Copy link
Contributor

This is exactly what I was hoping for. This cleaned things up so much. Always good to see a PR that results in less, faster code. 💪

@kjohnson
Copy link
Member

There is a new test failure that we need to address:

WordPress database error Column 'form_id' in field list is ambiguous

See https://github.com/impress-org/givewp/actions/runs/11936509340/job/33270320353?pr=7616

@alaca alaca requested a review from kjohnson November 27, 2024 12:14
@alaca alaca merged commit 9db67ba into epic/campaigns Dec 4, 2024
20 checks passed
@alaca alaca deleted the refactor/default-form-GIVE-1949 branch December 4, 2024 08:31
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.

4 participants