Skip to content

Conversation

@AlexandrosMor
Copy link
Contributor

Description

Tested scenarios

Fixed issue:

@coveralls
Copy link

coveralls commented Jan 8, 2019

Coverage Status

Coverage increased (+2.4%) to 39.319% when pulling a66d3cc on feature/checkoutv40 into 72c24d8 on develop.

Adyen/client.py Outdated
service (str): API service to place request through.
action (str): the API action to perform.
"""
if action == "paymentDetails":
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if action == "paymentDetails":
if action == "paymentsDetails":

Adyen/client.py Outdated
action (str): the API action to perform.
"""
if action == "paymentDetails":
action = "payment/details"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
action = "payment/details"
action = "payments/details"

Adyen/client.py Outdated
"""
if action == "paymentDetails":
action = "payment/details"
if action == "paymentResult":
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if action == "paymentResult":
if action == "paymentsResult":

Adyen/client.py Outdated
if action == "paymentDetails":
action = "payment/details"
if action == "paymentResult":
action = "payment/result"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
action = "payment/result"
action = "payments/result"

Adyen/client.py Outdated
action = "payment/details"
if action == "paymentResult":
action = "payment/result"
if action == "originKey":
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if action == "originKey":
if action == "originKeys":

Adyen/client.py Outdated
action = "v1/originKeys"

base_uri = settings.BASE_CHECKOUT_URL.format(platform)
api_version = settings.CHECKOUT_API_VERSION
Copy link
Contributor

Choose a reason for hiding this comment

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

for checkoutUtility (originKeys) the versioning is different(v1 only at the moment),
I would put api_version at the beginning and replace it in case the action is originKeys

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is fixed now

int: HTTP status code, eg 200,404,401
dict: Key/Value pairs of the headers received.
"""
""" % self.pr
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
""" % self.pr
"""

xapikey="",
headers=None,
timeout=30):
self.pr = """pr"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.pr = """pr"""

action, **kwargs)

def payment_result(self, request="", **kwargs):
action = "paymentResult"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
action = "paymentResult"
action = "paymentsResult"

"channel", "returnUrl", "countryCode",
"shopperLocale", "sessionValidity",
"merchantAccount"]
actions['paymentResult'] = ["payload"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
actions['paymentResult'] = ["payload"]
actions['paymentsResult'] = ["payload"]

ENDPOINT_PROTOCOL = "https://"
BASE_CHECKOUT_URL = "https://checkout-{}.adyen.com"
CHECKOUT_URL_LIVE_SUFFIX = "-checkout-live.adyenpayments.com/checkout"
CHECKOUT_API_VERSION = "v40"
Copy link
Contributor

Choose a reason for hiding this comment

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

these 4 static variables looks very similar and a bit difficult to understand. Would it be an idea to keep it easy like:
const ENDPOINT_PROTOCOL = "https://";
ENDPOINT_CHECKOUT_TEST = "https://checkout-test.adyen.com/checkout";
ENDPOINT_CHECKOUT_LIVE_SUFFIX = "-checkout-live.adyenpayments.com/checkout";
like we do in php ?

self.username = username
self.password = password
self.xapikey = xapikey
self.review_payout_username = review_payout_username
Copy link
Contributor

Choose a reason for hiding this comment

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

should we not add live_endpoint_prefix as property ?

"""This function will POST to the url endpoint using requests.
Returning an AdyenResult object on 200 HTTP response.
Either json or data has to be provided.
Either json or data has to be %sovided.
Copy link
Contributor

Choose a reason for hiding this comment

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

what is %sovided ?

Adyen/client.py Outdated
# xapikey at self object has highest priority.
# fallback to root module
# and ensure that it is set.
if self.xapikey:

Choose a reason for hiding this comment

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

the api key can become the first to check and use instead of the username password, since all our APIs are working fine with the API key only.


# Adding basic auth if username and password provided.
auth = None
if username and password:

Choose a reason for hiding this comment

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

Also here the api key can be the default and not the fallback plan

Copy link
Contributor Author

@AlexandrosMor AlexandrosMor Jan 10, 2019

Choose a reason for hiding this comment

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

I am not sure what you mean default. Do you suggest to remove basic authentication? If the xapi become first then it is default, correct?

Choose a reason for hiding this comment

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

I agree with Attilla ApiKey has preference over username and password.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is fixed by adding the xapikey in high priority

BASE_PAL_URL = "https://pal-{}.adyen.com/pal/servlet"
BASE_HPP_URL = "https://{}.adyen.com/hpp"
API_VERSION = "v30"
ENDPOINT_LIVE_SUFFIX = "-pal-live.adyenpayments.com"
Copy link
Contributor

Choose a reason for hiding this comment

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

is this being used ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct, it is not used so it can be removed


BASE_PAL_URL = "https://pal-{}.adyen.com/pal/servlet"
BASE_HPP_URL = "https://{}.adyen.com/hpp"
API_VERSION = "v30"

Choose a reason for hiding this comment

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

Depricate the API_VERSION instead of removing it to follow semver

@AlexandrosMor AlexandrosMor merged commit 1d94500 into develop Jan 16, 2019
@AlexandrosMor AlexandrosMor deleted the feature/checkoutv40 branch February 14, 2019 13:32
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.

7 participants