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

The redirect url is required! #29

Closed
LaKing opened this issue Apr 2, 2020 · 3 comments · Fixed by #30
Closed

The redirect url is required! #29

LaKing opened this issue Apr 2, 2020 · 3 comments · Fixed by #30

Comments

@LaKing
Copy link

LaKing commented Apr 2, 2020

Probably a documentation bug. The parameter is not optional.

When using the default example:

errors:
[ { Title: 'Model Validation Error',
Description: 'The redirect url is required!',
ErrorCode: 'ModelValidationError',
HappenedAt: '2020-04-02T01:54:16.0574019Z',
EndPoint: 'https://api.test.barion.com/v2/Payment/Start' } ] }
@aron123
Copy link
Owner

aron123 commented Apr 2, 2020

This is a modification of the Barion API, that I forgot to implement and document. I'll fix the schema and the README in 1-2 days.

Thanks for the report.

@aron123 aron123 added bug Something isn't working and removed bug Something isn't working labels Apr 2, 2020
@aron123
Copy link
Owner

aron123 commented Apr 2, 2020

I've investigated a bit and there is the explanation for this inconsistency with Barion Docs:

Earlier the default redirect URL can be specified in Barion account's admin panel. Later Barion removed this setting from the admin panel, and started to get the redirect URL only in the "payment start" API call. But for backward compatibility, they already store the former default redirect URLs that are set in admin panel and use it if the API caller does not provide any redirect URL in their request.

So from node-barion module's view, the 'RedirectUrl' is still optional. If I change it, it can break implementations that are currently deal with the redirect URL set in the old admin panel.
Otherwise, I'll update the code example and integration tests to show the best practice.

@LaKing
Copy link
Author

LaKing commented Apr 3, 2020

Thank you.

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 a pull request may close this issue.

2 participants