Thanks for writing this up!

We came to an agreement in the meeting — see details in the note below:
https://docs.google.com/document/d/1QiymgoXu2fUc_1NfgNI9NOio071z5H4FMjY1cklqLIc/edit?tab=t.0
the agreed payment flow could be (extendable for other payment methods, e.g. ideal/boleto)

[User submits payment]
          |
          v
[Payment Processor API returns "pending"]
          |
          v
+--------------------------+
| Show "Pending TY Page"   |
| (Explain payment under   |
| review; no email yet)    |
+--------------------------+
          |
          v
[Create Civi Contribution with status = Pending]
          |
          v
<-- Wait for Webhook (status update) -->
          |
   +--------------------+
   | Webhook arrives    |
   | (complete / fail)  |
   +--------------------+
          |
   +----------------------------+
   | If "complete":             |
   |  - Update Civi status -> Completed |
   |  - Send real Thank-You email      |
   |  - If recurring/MC: create recurring record |
   +----------------------------+
          |
   +----------------------------+
   | If "failed":               |
   |  - Update Civi -> Failed   |
   |  - Do NOT create recurring (Token invalid, etc.)   |
   +----------------------------+

@Eileenmcnaughton Do you know why we were not having pending status in Civi before, is that any potential risk here, thanks!

@AnnWF as to the why - that is probably more historical than meaningful

However, the main risk is that other code might not be checking the status appropriately - ie

• thank you mail
• triggers
• Acoustic export

(FWIW I'm pretty sure triggers DO check for completed status & Acoustic is probably driven by that so it's mostly thank you mail & the thing we haven't thought of)

I like your asci flow chart :-)

• At a code level you should use the Payment.create api when the payment arrives & make sure it passes the parameter (isSendNotification or similar) to prevent an email being triggered by that

Thanks Eileen for the historical explanation and juse as Elliott brings up that "we'd of course need to review all the triggers and the acoustic export."

Since you said triggers DO check for completed status could you help us to check if Acoustic is actually driven by that?

and for ty email, I see in Payment.Create is using notificationForCompleteOrder, there is a one as notificationForPayment which is false as default and notificationForCompleteOrder true as default, so should be fine, but we will test that when we actually adding 'pending' status.

Thanks again~

In T406663#11351205, @Eileenmcnaughton wrote:

@AnnWF as to the why - that is probably more historical than meaningful

However, the main risk is that other code might not be checking the status appropriately - ie

• thank you mail
• triggers
• Acoustic export

(FWIW I'm pretty sure triggers DO check for completed status & Acoustic is probably driven by that so it's mostly thank you mail & the thing we haven't thought of)

I like your asci flow chart :-)

• At a code level you should use the Payment.create api when the payment arrives & make sure it passes the parameter (isSendNotification or similar) to prevent an email being triggered by that.

I see a good smattering of things like this in the triggers file

  WHERE  c.contact_id = NEW.contact_id 
    AND c.contribution_status_id = 1
    AND (c.trxn_id NOT LIKE 'RFD %' OR c.trxn_id IS NULL)
) as totals

There are quite a few checks on contribution_status_id in the Acoustic export too.

I can't say there are no gaps but the code was definitely written on the assumption that we needed to only include Completed

Good to know, we will also use contribution_status_id:name check in our code then!

One more thing to review - contribution create hooks

In T406663#11351423, @Ejegg wrote:

One more thing to review - contribution create hooks

I added couple sub tasks, feel free to add more and the concern you and Eileen pointed out were added to that QA ticket