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

Feature: add revenue compatibility for v3 forms and donation model #7148

Merged
merged 4 commits into from
Jan 9, 2024

Conversation

jonwaldstein
Copy link
Contributor

@jonwaldstein jonwaldstein commented Dec 14, 2023

Resolves GIVE-178

Description

This is necessary for updating the revenue amount in v3 forms that use models like give-funds. Most notably for use with fee-recovery as the amount gets updated after initial donation creation. In v2 forms this was all processed during the initial instance of give_insert_payment - where in v3 it's more of an async request and we need to listen for udpates.

Basically, this will ensure the revenue table gets updated when a donation model is updated.

Affects

  • Updated an existing action to accept a donation model instead of donationId which is now used across both give_updated_edited_donation and givewp_donation_updated for the purpose of updating revenue amounts.

Visuals

Screenshot 2023-12-14 at 1 01 22 PM Screenshot 2023-12-14 at 1 03 07 PM

Testing Instructions

  • Make sure v2 forms still work properly with fee recovery and funds
  • v3 and funds compatibility is still in progress so would either need to test pending PR or programatically

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Includes unit tests
  • Reviewed by the designer (if follows a design)
  • Self Review of code and UX completed

@jonwaldstein jonwaldstein marked this pull request as ready for review December 14, 2023 18:07
@JasonTheAdams
Copy link
Contributor

JasonTheAdams commented Dec 14, 2023

@jonwaldstein I can't remember how our model hooks compatibility works. By switching to the model "updated" hook, will this still work sufficiently with v2 forms? Or is there a reason that's not necessary?

@jonwaldstein
Copy link
Contributor Author

@JasonTheAdams that's a great question!

We are using the models in both v2 and v3 forms - so the model hooks will fire for both forms. In fact, we have done our best to make sure the processing is as close as possible. However, there is still a difference in how some data is sent to the server and how we process custom fields in v2 forms that we need to account for.

For example, in v2 forms we send the sum amount of fee recovered to the server, where as in v3 forms, the two amounts are delivered separately and updated on the server via hooks.

So to answer your question, yes the "updated" hook will work for v2 forms - however we are not exactly switching to that hook - it's being added in addition to give_insert_payment and give_updated_edited_donation (admin). It's possible we can replace give_insert_payment with givewp_donation_created but i'm not ready to test that theory at the moment since we're still using the former throughout addons.

@jonwaldstein jonwaldstein requested review from glaubersilva and removed request for kjohnson December 20, 2023 16:11
@jonwaldstein
Copy link
Contributor Author

@glaubersilva can you please review this one

Copy link
Contributor

@glaubersilva glaubersilva left a comment

Choose a reason for hiding this comment

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

@jonwaldstein Nice work! Everything worked as expected for both v2 and v3 forms. I just left a couple of comments about a few minor changes related to the missing unreleased tag. After that, it's ready for QA. Thanks!

Copy link
Member

@rickalday rickalday left a comment

Choose a reason for hiding this comment

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

Passed manual QA tests.

@jonwaldstein jonwaldstein merged commit aa733cc into develop Jan 9, 2024
20 checks passed
@jonwaldstein jonwaldstein deleted the feature/revenue-donation-updated-GIVE-178 branch January 9, 2024 18:03
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