Skip to content

Conversation

@amihaiemil
Copy link
Member

Wallet.pay(...) now returns the registered (successful) Payment.

Exceptions from the payment process are going to be caught in a decorator and registered as failed Payment.
We will also write a decorator to pull out all the payment pre-checks.

@zoeself
Copy link
Collaborator

zoeself commented Mar 5, 2021

@amihaiemil thank you for your Pull Request. I'll assign someone to review it soon.

@amihaiemil
Copy link
Member Author

@criske @fellahi-ali FYI, this is quite a big change.

An Invoice's successful Payment is created by Invoices.registerAsPaid(...) and returned by Wallet.pay(...).
Failed payments will be registered based on caught exceptions in a decorator. :D

@zoeself
Copy link
Collaborator

zoeself commented Mar 5, 2021

@amihaiemil please review this Pull Request. Deadline (when it should be merged or closed) is 2021-03-08T16:07:17.837977.

You should check if the requirements have been implemented (partially or in full), if there are unit tests covering the changes and if the CI build passes. Feel free to reject the PR or ask for changes if it's too big or not clear enough.

Estimation here is 30 minutes, that's how much you will be paid. You will be paid even if this PR gets rejected.

@criske
Copy link
Contributor

criske commented Mar 5, 2021

@amihaiemil Wouldn't be better though to split the Payment interface into two subinterfaces?

interface Payment {
  BigDecimal value();
  LocalDateTime paymentTime();
}
interface SuccessPayment extends Payment {
  String transactionId();
  Invoice invoice();
  PlatformInvoice platformInvoice();
}
interface FailedPayment extends Payment {
  String failReason();
}

This way we can get rid of of status.

@amihaiemil
Copy link
Member Author

amihaiemil commented Mar 6, 2021

@criske Turns out we need the status. I just realized we will have at least 3: Successful, Failed (not enough cash, authentication failed etc) and Error (communication error with Stripe).

Errored Payments should never occur, it means there's something wrong with the system or with Stripe. We will save these as well, if they happen and we will have a job in self-pm, to make the PM report Issues in self-core, for each errored Payment (which will probably need investigation).

Then later maybe we introduce other stati, such as Retry (a payment which can be retried, based on Stripe's PaymentIntent ID).

@amihaiemil amihaiemil merged commit 4b42c94 into self-xdsd:master Mar 6, 2021
@amihaiemil amihaiemil deleted the 1034 branch March 6, 2021 13:43
@zoeself
Copy link
Collaborator

zoeself commented Mar 6, 2021

@amihaiemil thank you for resolving this ticket. I've just added it to your active invoice. You can always check all your invoices and more on the Contributor Dashboard.

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.

3 participants