Skip to content

feat: add payment lifecycle hooks#15

Open
brendanjryan wants to merge 2 commits into
stripe:mainfrom
brendanjryan:brendanjryan/mpp-rb-event-hooks-upstream
Open

feat: add payment lifecycle hooks#15
brendanjryan wants to merge 2 commits into
stripe:mainfrom
brendanjryan:brendanjryan/mpp-rb-event-hooks-upstream

Conversation

@brendanjryan
Copy link
Copy Markdown
Contributor

@brendanjryan brendanjryan commented May 18, 2026

Summary

Add client and server payment lifecycle hooks for observing challenges, credentials, payment responses, successes, and failures.

This mirror similar functionality in other SDKs and documented at: tempoxyz/mpp#649

@brendanjryan brendanjryan marked this pull request as ready for review May 18, 2026 19:05
Copy link
Copy Markdown
Contributor

@raubrey-stripe raubrey-stripe left a comment

Choose a reason for hiding this comment

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

Couple q's but overall looking good!

Comment on lines +138 to +146
if payment_response.code.to_i.between?(200, 299) && @events.has_handlers?(Mpp::Events::PAYMENT_RESPONSE)
@events.emit(Mpp::Events::PAYMENT_RESPONSE, {
challenge: challenge,
credential: auth_header,
input: url,
method: matched_method,
response: payment_response
})
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we emit payment failed events when it's non-2xx?

begin
event_credential = nil
if @events.has_handlers?(Mpp::Events::CHALLENGE_RECEIVED)
event_credential = @events.emit_first(Mpp::Events::CHALLENGE_RECEIVED, {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it significant that this event is the only one to use emit_first?

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