Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.0][FIX] API call to payments #130

Closed

Conversation

robinkeunen
Copy link

@robinkeunen robinkeunen commented Aug 11, 2022

The API nows only returns a single data dictionary instead of a list.
Also, the transaction id must be passed in the url.

Previously, data argument was overwritten in the function body. I'm tempted to either

  • keep overwriting the data dict in the case some other function relies on the content of the data dict
  • avoid overwriting the data dict as a best practice to avoid side effects.

I chose the latter option.


if "status" in data:
status = data["status"]
status = mollie_response.get("status", "undefined")
Copy link

Choose a reason for hiding this comment

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

We may need to check that the result returned by mollie does match the transaction ID we sended. Like it's done on line 153 (at the end). Else we should raise an error and report it in the log.

Copy link

Choose a reason for hiding this comment

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

Resolved.


if status == "paid":
paidDatetime = mollie_response["paidDatetime"].replace(".0Z", "")
Copy link

Choose a reason for hiding this comment

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

standard naming for variable ? paid_datetime ? :)

Copy link

Choose a reason for hiding this comment

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

Resolved

The API nows only returns a single data dictionary
instead of a list.
Also, the transaction id must be passed in the url.
@robinkeunen
Copy link
Author

@odoo-mvds can you have a look at this ? We've been using it in production for 3 months now.

@odoo-mvds
Copy link
Collaborator

Hello,

we do not support old versions anymore, however we will have a look at this fix as it's easy to merge.
We'll give you an update soon.

br,

@odoo-mvds
Copy link
Collaborator

Hi,

Thank you for your contribution and for submitting the pull request (PR).

As we have informed you, we are no longer maintaining the older version of the project to which your PR applies. We recommend upgrading to the latest version. In case we receive more such complains for the same issue, we will probably reopen the issue. Consequently, we will be closing this PR.

We truly appreciate your effort and interest in improving the project and look forward to any future contributions you may have.

Best regards,

@odoo-mvds odoo-mvds closed this May 27, 2024
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