feat(payment-service): adds payment-service feature #257
Conversation
| }) | ||
| chargeResponse: DataObject<{}>, | ||
| ): Promise<unknown> { | ||
| console.log(this.req.query, 'query'); |
| ): Promise<any> { | ||
| console.log(id); | ||
| const Order = await this.ordersRepository.findById(id); | ||
| return this.res.send(await this.gatewayHelper.create(Order)); |
There was a problem hiding this comment.
Why using this.res.send ?
|
@sumiter92 please look into the sonar issues as well. |
sumit-tuteja
force-pushed
the
feature/payment-service
branch
from
793814c to
1429002
Compare
| const newOrder = await this.ordersRepository.create(orderEntity); | ||
| return this.res.redirect( | ||
| redirectStatusCode, | ||
| `http://localhost:3000/transactions/orderid/${newOrder.id}`, |
There was a problem hiding this comment.
why do we need a hardcoded redirect here?
There was a problem hiding this comment.
@sumiter92 this needs to come from DB or env
There was a problem hiding this comment.
Needs lots of improvements, summary
- DB need to be revisited, needs lots of improvement to caputre data
- Use DB transactions on all payment operations. and rollback on errors
- Exception handling not implemented anywhere
- Returning HTML in api responses
- Classes need to be restructured. it should not have gateway. name checks in code like if stripe do this if razor do ths.. this is not extendable. This can be improved using passing gateway object in constructer and calling functions from parent class ..
- striepe and razor classes should only have gateway related operations. All DB operations shoudl be writen seperatly and can be called seperatly, as same data need to be stored with any payment gateway.
| ) {} | ||
|
|
||
| async create(payorder: Orders) { | ||
| if (payorder.paymentmethod === 'razorpay') { |
There was a problem hiding this comment.
This code is not generic, static checks for payment gateways.. insted of this, payment gateway should be provided argument in constructer and the generic method of this provider gateway would call the payment gateway's method via that constructer refrence.. This makes this class generic to be used with any payment gatewy. This gateway class would be initialized with specific payment gateway class when it is to be initilized
There was a problem hiding this comment.
refactored done
| gatewayId, | ||
| ); | ||
| const gatewayType = paymentGateway.gatewayType; | ||
| if (gatewayType === 'stripe') { |
There was a problem hiding this comment.
Same,, static check of gateway should not be there.. not extandable
| }); | ||
| const transRes = transactions[0]?.res; | ||
| if (payorder?.status === 'paid') { | ||
| return `<html> |
There was a problem hiding this comment.
yes in one case we are returning, because card number was handled from payment gateway side and we are not storing card information , in frontend request , we are sending 302 temporary redirect to backend for payment and after successful payment , it can be redirected to frontend again with successurl through env and yes by default template is set but can be changed according to requirement and comes from db now
| } | ||
| const razorPayOptions = { | ||
| amount: payorder.totalAmount, // amount in the smallest currency unit | ||
| currency: 'INR', |
There was a problem hiding this comment.
changed , now its dynamic
| currency: 'INR', | ||
| }; | ||
| if (transactions.length === 0) { | ||
| await instance.orders.create( |
There was a problem hiding this comment.
- Payment method class should only have functions related to payment gateway intraction
- All db related operations should be handeled seperatly, as same wouldbe used in other payment gateways also.
- While dealing with payment and database.. always use transactions.
There was a problem hiding this comment.
this instance is of razorpay and create order on razorpay database , our order is created in transactions controller
|
|
||
| refund: async (transactionId: string) => { | ||
| const transaction = await this.transactionsRepository.findById( | ||
| transactionId, |
| ); | ||
| const paymentId = transaction?.res?.chargeResponse?.id; | ||
| const refund = await instance.payments.refund(paymentId); | ||
| transaction.res.refundDetails = refund; |
| await this.transactionsRepository.create(transactionData); | ||
| } | ||
| return `<html> | ||
| <title>Stripe Payment Demo</title> |
| const transaction = await this.transactionsRepository.findById( | ||
| transactionId, | ||
| ); | ||
| const paymentId = await transaction?.res?.chargeResponse?.id; |
There was a problem hiding this comment.
Same comments as in razorpay
- DB operations should be seperate from payment, as same operations are happenign ere
- use DB transacions and rollback on failover
- no exception handling
- dont return HTML
| @@ -0,0 +1,31 @@ | |||
| CREATE SCHEMA IF NOT EXISTS main; | |||
|
|
|||
| CREATE TABLE main.orders ( | |||
There was a problem hiding this comment.
DB need to be revisited if this is to be made generic, many important fields are missing
There was a problem hiding this comment.
we are adding only payment related fields and storing different gateway response in json format in one column , if we want to add more column according to business needs we can extend these models
| @@ -0,0 +1,81 @@ | |||
| { | |||
| "name": "payment-service", | |||
There was a problem hiding this comment.
name should be @sourceloop/payment-service
| "@loopback/rest-explorer": "^3.2.1", | ||
| "@loopback/service-proxy": "^3.1.1", | ||
| "@sourceloop/core": "^1.0.0", | ||
| "@types/uuid": "^8.3.1", |
There was a problem hiding this comment.
types should be in dev dependencies
| "@loopback/eslint-config": "^10.1.1", | ||
| "eslint": "^7.23.0", | ||
| "typescript": "~4.2.3" | ||
| } |
There was a problem hiding this comment.
add commitizen, husky hooks and commit lint.
|
@sumiter92 also look into the sonar issues. |
…,repo,model in export
sumit-tuteja
force-pushed
the
feature/payment-service
branch
from
1831e52 to
e4cf971
Compare
…e to fix version issue
|
Kudos, SonarCloud Quality Gate passed!
|
Description
Adds payment-service that supports configurable multiple payment gateways.
Fixes #256 #212 #275 #285
Type of change
Checklist: