Conversation
This changes attempts to consolidate Payment initiation in on central location PaymentInitialization. It also introduces another payment method with sold appliance.
| * @param string $message Transaction routing key (device serial or entity ID) | ||
| * @param string $type Transaction type (e.g. 'deferred_payment', 'energy') | ||
| * @param int $customerId Person ID of the customer | ||
| * @param int $creatorId Admin user ID (used for cash transactions) |
There was a problem hiding this comment.
$creatorId is specific to cash. I don't think we need to expose this on the high level interface. Could we find a way to pass implementation specific parameters?
| ) {} | ||
|
|
||
| /** | ||
| * Initialize a payment with any enabled provider. |
There was a problem hiding this comment.
I'm struggling a bit to understand what our understanding of Initialize is. Could you explains this more in the PHPDocs here?
I.e. does this have to create a transaction entry with a certain status etc?
There was a problem hiding this comment.
This diagram mentions Provider Validation. Where in the code does this happen?
There was a problem hiding this comment.
I was referencing to validations done on initializePayment for example with Paystack provider which goes through a call chain that ends up in InitializeTransactionResource which does validation on the structure of the data.
dmohns
left a comment
dmohns
left a comment
There was a problem hiding this comment.
I'm still struggling a little bit to follow the payment processing flow. Both, with and without your changes 😅
After coming back to this multiple times, I think one of the confusions for me lies in the redundancy between Transaction Providers (Providers here mean Laravel Providers, not Payment Providers) and Transaction Services
For Providers we already have a Contract ITransactionProvider. For Services we have AbstractPaymentAggregatorTransactionService
Now, looking at ITransactionProvider it seems like it was intended to already cover a lot of aspects that you are trying to add here.
For example it has
public function init(BasePaymentProviderTransaction $transaction): void;Do you think it within the scope of this PR to clean up this redundancy?
If I understand your comment correctly
Yes, this is very much part of the chain of refactor we are ultimately aiming to achieve.
To be fair, this only PR tries to make it possible to bring all payment initiation steps in one place. My hope is that this will open up better abstractions that could simplify the entire flow, which is still rather complex. |
2 participants
This changes attempts to consolidate Payment initiation in on central location PaymentInitialization. It also introduces another payment method with sold appliance.
Makes progress on #1243
Brief summary of the change made
Are there any other side effects of this change that we should be aware of?
Describe how you tested your changes?
Pull Request checklist
Please confirm you have completed any of the necessary steps below.