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

Support CRUD + Charge for tokenised credit cards #22

Open
wants to merge 38 commits into
base: master
Choose a base branch
from

Conversation

Zensavona
Copy link

Basic quick + dirty support for creating, reading, updating, deleting and charging tokenised credit cards

Tests included + updated.

Note: I had to break your API for bill_to slightly to add extra fields because you didn't include postcode or state which are required here.

@speeddragon
Copy link
Contributor

Hi @Zensavona, this looks good. I have started in a different branch to provide tokenization for credit cards, but never end up finishing it.

@Zensavona
Copy link
Author

@speeddragon great - I'll just get those tests to pass on travis (works locally - I guess it's not using the saved VCR cassettes for some reason?)

@Zensavona
Copy link
Author

Hi @Zensavona, this looks good. I have started in a different branch to provide tokenization for credit cards, but never end up finishing it.

P.S. that's actually an entirely different service - TMS !== On-Demand Billing

@speeddragon
Copy link
Contributor

You're right. Looking better into the PR, maybe it could be better to move these methods into a new module, like CyberSourceSDK.RecurringSubscription ? I think this is growing beyond what was initial thought, so some new structure might be helpful for further improvements of this module.

@andreragsilva
Copy link

andreragsilva commented Aug 17, 2020

@speeddragon should we take a look into the PR or do you prefer to handle it?

@speeddragon
Copy link
Contributor

This PR needs a refactor since the Client module is getting huge, as mentioned in my last comment.

@andreragsilva
Copy link

@Zensavona can you evaluate the refactor suggestion made by @speeddragon and implement it?

@Zensavona
Copy link
Author

@rageofflames / @speeddragon since the Client module is getting huge, is it a problem from your guys' perspective to break the existing API and split it up? That's the reason I left it this way.

Just let me know about that and I'll get to it when I have a spare couple hours.

@speeddragon
Copy link
Contributor

We definitely should break it. If I have some time I will try to improve it also.

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.

4 participants