Problem/Motivation

The administration UI allows switching between Sandbox and Live credentials (radio), but you can only EITHER store the live credentials OR the sandbox credentials.

Instead of only having the two input fields for client ID and secret, there should be two for sandbox and two for live.
Switching could then show / hide them via form states API.

In all payment provider implementations I know allows to store both. For good reasons :)

Steps to reproduce

Proposed resolution

Remaining tasks

Check if 8.x-1.x has the same implementation

User interface changes

API changes

Data model changes

Issue fork commerce_paypal-3321915

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Does anyone know if the same issue exists for the Drupal 8 version? Then it should also be fixed there.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Version: 7.x-2.x-dev » 8.x-1.x-dev

Looks like 8.x-1.x has the same flaw, so perhaps we should implement it for 8.x-1.x first and backport it then, if needed.
See https://git.drupalcode.org/project/commerce_paypal/-/blob/8.x-1.x/src/Pl...

Anybody opened merge request !15

Anybody opened merge request !16

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Active » Needs review

Happy about first reviews, but please note that this is still WIP for both branches. Just some first implementations to show the idea behind.

jsacksick’s picture

jsacksick commented
Status: Needs review » Closed (won't fix)

This probably isn't going to happen. We usually don't recommend storing plain credentials in the database... Often times credentials are set from either settings.php or via the https://www.drupal.org/project/key module.

jsacksick’s picture

jsacksick commented
Status: Closed (won't fix) » Needs work

Well actually, having extra config keys would also allow to override both from config.

However, the form should be smart and just toggle the fields based on the selected mode so the sandbox fields are only shown when the sandbox mode is selected.

Also... You're missing changes to the schema.yml file and code in submitForm() to actually store the submitted values for the sandbox client ID and secret.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Hi @jsacksick, thank you very much for the review! I much appreciate your comment in #7 about storing the credentials in the settings.php or the key module. So I've opened a separate issue to add this important information to the configuration form: #3322047: In the UI inform the user about safely storing API credentials
Perhaps you'd like to help working on that then?

Also I very much appreciate your decision to reopen the issue and see the benefit in this. This makes the existing switch between "Live" and "Sandbox" even more useful (where ever credentials are stored). So thank you! :)

However, the form should be smart and just toggle the fields based on the selected mode so the sandbox fields are only shown when the sandbox mode is selected.

As written in the issue summary:

Switching could then show / hide them via form states API.

I agree and already started the implementation, but didn't have the time yet to finish and test that.

Also... You're missing changes to the schema.yml file and code in submitForm() to actually store the submitted values for the sandbox client ID and secret.

Thanks for the heads up, you're absolutely right. I didn't make it yesterday evening, but will proceed working on this.

Happy about first reviews, but please note that this is still WIP for both branches. Just some first implementations to show the idea behind.

#6

I'll keep you updated :) Thanks a lot!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Related issues: +#3322047: In the UI inform the user about safely storing API credentials
jsacksick’s picture

jsacksick commented

By the way, in the 8.x version there is also no need to add a "server" setting as there is a "mode" for that.

The mode has 2 possible values "test" or "live".

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

By the way, in the 8.x version there is also no need to add a "server" setting as there is a "mode" for that.

Can you explain that in detail? I didn't add something like that, see https://git.drupalcode.org/project/commerce_paypal/-/merge_requests/15/d...

In 7.x I just moved the setting above the inputs. It was below before. Nothing added.

jsacksick’s picture

jsacksick commented

So I think I was checking the 7.x by mistake... I think it's best to do this in 2 separate issues to avoid confusion. One for each version.

grevil’s picture

grevil commented
Title: Allow to store live and sandbox credentials for easy switching » [8.x] Allow to store live and sandbox credentials for easy switching
grevil’s picture

grevil commented

I opened a seperate 7.x issue here in #3327419: [7.x] Allow to store live and sandbox credentials for easy switching.

Anybody closed merge request !16

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Related issues: +#3327419: [7.x] Allow to store live and sandbox credentials for easy switching

Thanks @Grevil! :)

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » anybody

I'll further work on this asap.

grevil’s picture

grevil commented
Assigned: anybody » grevil
grevil’s picture

grevil commented
Assigned: grevil » Unassigned
Status: Needs work » Needs review

Ok, this is it! Done!

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Wouldn't it be better to clearly document how to override the credentials via settings.php?

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@damienmckenna thanks for your reply. In my eyes both is true.

Currently the issue is, that theres a "Live" vs. "Sandbox" switch, but you can only store public / private key for one of them. This combination doesn't make much sense in our eyes.
When switching the radio, you have to exchange the keys and enter the opposite ones and when switching back, same again...

The goal here is to add separate fields & config values for live and sandbox credentials, so you can switch at any time, if you need to, by just switching the radio.

Of course it should also be documented how to set these values in settings.php.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

So let's wait for final feedback how to finish this then, or any additions @damienmckenna?

rgpublic’s picture

rgpublic
Primary language German
Location Düsseldorf 🇩🇪 🇪🇺
commented

FWIW just some encouragement to go head with this: Overriding is certainly an alternative, but far less discoverable. The UI is already there and other payment modules use it correctly, so this is currently quite counter-intuitive. It should therefore be implemented IMHO. It's useful!