Skip to content

Conversation

@mrsaicharan1
Copy link
Member

@mrsaicharan1 mrsaicharan1 commented Jun 5, 2019

omise-final

Frontend: fossasia/open-event-frontend#3073**
Fixes #5823

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.
  • The unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All the functions created/modified in this PR contain relevant docstrings.

Short description of what this resolves:

This creates a charge route for Omise which handles the details from FE, creates a charge object and authorises the payment. This gateway does not require redirection, therefore, can easily be integrated with the android app.

Changes proposed in this pull request:

  • Create charge route
  • Handled all possible exceptions and errors with logging

except TypeError:
return jsonify(status=False, error='Source object status error')

@order_misc_routes.route('/orders/<string:order_identifier>/omise-checkout', methods=['POST', 'GET'])

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

@mrsaicharan1 mrsaicharan1 force-pushed the omise-backend branch 2 times, most recently from c723181 to a971509 Compare June 5, 2019 20:15
@mrsaicharan1 mrsaicharan1 force-pushed the omise-backend branch 3 times, most recently from 8efb0ca to e198957 Compare June 5, 2019 20:20
@codecov
Copy link

codecov bot commented Jun 5, 2019

Codecov Report

Merging #6004 into development will decrease coverage by 0.1%.
The diff coverage is 21.87%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #6004      +/-   ##
===============================================
- Coverage        66.26%   66.16%   -0.11%     
===============================================
  Files              285      285              
  Lines            14007    14038      +31     
===============================================
+ Hits              9282     9288       +6     
- Misses            4725     4750      +25
Impacted Files Coverage Δ
app/api/orders.py 31.48% <19.04%> (-1.32%) ⬇️
app/api/helpers/payment.py 28.03% <27.27%> (-0.09%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40c2400...5fbc0ac. Read the comment docs.

@mrsaicharan1 mrsaicharan1 changed the title [wip]feat: Omise charge route and error handling feat: Omise charge route and error handling Jun 5, 2019
@auto-label auto-label bot added the feature label Jun 5, 2019
@mrsaicharan1 mrsaicharan1 changed the title feat: Omise charge route and error handling feat: Omise charge route and payment flow Jun 5, 2019
token = request.form.get("omiseToken")
try:
charge = OmisePaymentsManager.charge_payment(order_identifier, token)
except omise.errors.BaseError as e:

Choose a reason for hiding this comment

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

undefined name 'omise'

)
return charge

class OmisePaymentsManager(object):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

@mrsaicharan1 mrsaicharan1 force-pushed the omise-backend branch 3 times, most recently from 29607d4 to 8ec6f20 Compare June 5, 2019 21:25
try:
charge = OmisePaymentsManager.charge_payment(order_identifier, token)
except omise.errors.BaseError as e:
logging.error(f"""OmiseError: {repr(e)}. See https://www.omise.co/api-errors""")
Copy link
Contributor

Choose a reason for hiding this comment

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

please make the syntax python 3.6 compatible

Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Member

Choose a reason for hiding this comment

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

Upgrade your python please

@mrsaicharan1 mrsaicharan1 force-pushed the omise-backend branch 2 times, most recently from e85220b to bd16931 Compare June 6, 2019 08:30
@iamareebjamal
Copy link
Member

Omise docs needs the amount to be in integer so we should make it
amount=int(order.amount)

Umm, so if the order amount is 4.99 dollars, I only charge 4 dollars?

Makes no sense actually

@mrsaicharan1 mrsaicharan1 force-pushed the omise-backend branch 2 times, most recently from 8344c80 to 014da3b Compare June 6, 2019 19:00
@mrsaicharan1 mrsaicharan1 force-pushed the omise-backend branch 2 times, most recently from b26aa65 to 159d70d Compare June 7, 2019 15:13
@mrsaicharan1
Copy link
Member Author

@iamareebjamal Please check.

@fossasia fossasia deleted a comment Jun 7, 2019
@fossasia fossasia deleted a comment Jun 7, 2019
@iamareebjamal
Copy link
Member

Peer Review First

@mrsaicharan1
Copy link
Member Author

@uds5501 @shreyanshdwivedi Please review this.

uds5501
uds5501 previously approved these changes Jun 8, 2019
Copy link
Contributor

@uds5501 uds5501 left a comment

Choose a reason for hiding this comment

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

Looks good. just take down hound errors please

Copy link
Member

@shreyanshdwivedi shreyanshdwivedi left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mrsaicharan1
Copy link
Member Author

Looks good. just take down hound errors please

There are no hound errors! :D

iamareebjamal
iamareebjamal previously approved these changes Jun 9, 2019
@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented Jun 14, 2019

@iamareebjamal Updated with dev branch rebase. Please merge this.

Omise Addition requirements

Shifted to payments module

Final omise

Round off amount

Omise final
@iamareebjamal iamareebjamal merged commit a6b7ae0 into fossasia:development Jun 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement Omise Payment Gateway

5 participants