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

[BUG] Update documentation after merchantAccount got removed from Config for NodeJS implementation #609

Closed
smafre opened this issue Nov 14, 2020 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@smafre
Copy link
Contributor

smafre commented Nov 14, 2020

The documentation needs to be updated after merchantAccount got removed from Config in PR #590. (https://docs.adyen.com/checkout/drop-in-web and maybe other places I don't know about).

@smafre smafre added the bug Something isn't working label Nov 14, 2020
@KadoBOT
Copy link
Contributor

KadoBOT commented Nov 16, 2020

Hi @smafre,

I've just checked and looks like the config merchantAccount is not obsolete as I first thought. It's being used to store the merchantAccount initially, and then it's referenced when you need to specify the merchantAccount in the requests. E.g.:

config.merchantAccount = "someMerchantAccount"

and later on:

checkout.payment({ ..., merchantAccount: config.merchantAccount })

And this is helpful if you need to switch between merchant accounts because you just need to change the value in one place.

With that said, I'll have to revert the changes to the config.merchantAccount

@KadoBOT
Copy link
Contributor

KadoBOT commented Nov 16, 2020

Hi @smafre,

The changes are reverted (see d459f00). I'll close this issue, but let me know if there's anything else.

Best regards,

@KadoBOT KadoBOT closed this as completed Nov 16, 2020
@smafre
Copy link
Contributor Author

smafre commented Nov 16, 2020

But why do we even have to specify the merchantAccount in the requests, when we've already set it up in the config? Thats the part I don't quite understand.

If we've set the merchantAccount in the Config, which is used in Client, which is used in the CheckoutAPI, we should have access to it in the implementation of the .payment()-function. So we still specify the merchantAccount twice (kind of).

@KadoBOT
Copy link
Contributor

KadoBOT commented Nov 16, 2020

But why do we even have to specify the merchantAccount in the requests, when we've already set it up in the config? Thats the part I don't quite understand.

We believe it's better to leave this choice to whoever is implementing the request than checking which requests contains the merchantAccount and using the value from the config.

Again, the merchantAccount from the config is just used to store your merchantAccount. You can use it to read the merchantAccount from an environment file and save it in the config, instead of referencing to the process.env.MERCHANT_ACCOUNT every time you want to use it for example.

If we've set the merchantAccount in the Config, which is used in Client, which is used in the CheckoutAPI, we should have access to it in the implementation of the .payment()-function. So we still specify the merchantAccount twice (kind of).

The merchantAccount from the config is optional, so you don't need to use it if you don't want. But if you do, instead of using merchantAccount = "merchantAccountName" you can use merchantAccount = config.merchantAccount, and if later on you decide to use another merchant account name, you don't need to go through your code and check where you use the merchantAccount to change the value.

I understand it can be a bit frustrating to repeat this information in some places, but it's a trade-off which we believe is worthy.

@smafre
Copy link
Contributor Author

smafre commented Nov 16, 2020

Ok, got it :) So I can choose to only specify the merchantAccount in my requests, and leave it empty in the config if I choose to do so.
I see the advantages of setting it in the config though, so I'll do that. I'm reading it from an environment variable and setting it in my config like you also mentioned.

Thanks for the info and for getting back to me so fast 👍

@KadoBOT
Copy link
Contributor

KadoBOT commented Nov 16, 2020

Anytime, @smafre!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants