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

Add support for manual settlements #48

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

visini
Copy link
Contributor

@visini visini commented Jan 3, 2024

Currently, transactions are auto-settled by default. However, in some cases, settlement of transactions should be delayed. This adds support for that, allowing setting auto_settle: false when authorizing a transaction.

transaction = datatrans.json_transaction(
  refno: 'ABCDEF',
  amount: 1000,
  # ...,
  auto_settle: false
)

Currently, transactions are auto-settled by default. However, in some
cases, settlement of transactions should be delayed. This adds support
for that, allowing setting `auto_settle: false` when authorizing
a transaction.
Copy link
Contributor

@andyundso andyundso 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!

I remember we talked about a second feature that you need in order to use the gem. Is this still the case or can I ship a release with only this change?

@visini
Copy link
Contributor Author

visini commented Jan 3, 2024

Thank you @andyundso! 😊

These changes are already useful, since it's now possible to set auto_settle: false and then later settle transactions, for example (manually) via the Datatrans UI.

However, I'm currently working on adding support for the settle endpoint of the JSON API, which is currently missing, via...

I want to validate if everything works as expected by further testing it in the context of my use case and in the sandbox environment. So far so good. I'd also like to add a section to the Readme (which should tie together these changes in a nice example).

As soon as everything is ready, I'll open these respective PRs against this repo. I think in the next couple of days. Hope that's okay, and thank you!

@andyundso
Copy link
Contributor

all good, then I wait with the release and looking forward to your contribution.

@andyundso andyundso merged commit b828d9e into simplificator:master Jan 3, 2024
12 checks passed
@visini visini deleted the support-manual-settlements branch January 6, 2024 16:18
@visini
Copy link
Contributor Author

visini commented Jan 6, 2024

@andyundso turns out I was on the wrong track after all... I found out that confusingly, JSON::Transaction::Authorize does not actually talk to the authorize endpoint (it is clearly stated in the class, but I didn't read it properly 😓).

Therefore I'm proposing to add the following "missing pieces":

Thank you for your feedback and for preparing a release, much appreciated! 🙏😊

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.

2 participants