Read the provider settings from config instead of state

Created on 20 April 2022, almost 3 years ago
Updated 19 September 2024, 7 months ago

Problem/Motivation

The module currently uses \Drupal::state() in many places but should IMO use config instead.

There are several issues by using Drupal::state() for the provider settings:
1. When a DB is copied from PROD to another environment (local, acceptance, etc), the key_value table holding the data for \Drupal::state() is retained.
So the whole provider config including api_mode (simulation/real), and even api_url, api_username and api_password (if not using the settings.php way) is retained. Which means one can easily end up on a PROD setup while being on another environment and break/delete/complete any translation jobs.

2. Storing this in state duplicates what is already stored in the TMGMT provider config (in tmgmt.translator.tmgmt_cdt.yml via \Drupal\tmgmt\Form\TranslatorForm::save).
The config stored there is actually ignored since values are actually retrieved from Drupal:state() throughout tmgmt_cdt.

3. Because of this, it is not possible to use config overrides or config split and have different settings per environment.
Right now api_url, api_username and api_password can be retrieved from settings.php (if given) but api_mode which is essential as well is not included. A quick fix could be to read it from settings.php but a better way IMO would be to use config override.
Config override would also allow for example, different proxy settings, etc...

One example of what could be a PROD config override:

$config['tmgmt.translator.tmgmt_cdt']['conf_mode']['api_mode'] = \Drupal\tmgmt_cdt\TmgmtCdtHelper::PRODUCTION;
$config['tmgmt.translator.tmgmt_cdt']['conf_credentials']['api_url'] = ...
$config['tmgmt.translator.tmgmt_cdt']['conf_credentials']['api_username'] = ...
$config['tmgmt.translator.tmgmt_cdt']['conf_credentials']['api_password'] = ...

$config['tmgmt.translator.tmgmt_cdt']['conf_advanced']['debug'] = 0;

$config['tmgmt.translator.tmgmt_cdt']['conf_network']['curl_proxy_host'] = ...
$config['tmgmt.translator.tmgmt_cdt']['conf_network']['curl_proxy_port'] = ...
$config['tmgmt.translator.tmgmt_cdt']['conf_network']['curl_proxy_user'] = ...
$config['tmgmt.translator.tmgmt_cdt']['conf_network']['curl_proxy_pass'] = ...
...

4. It prevents updating CDT settings via code: if we want to update/change the CDT settings on PROD, the only way is to go there and do the changes manually via UI. Any changes that happened in config (code) doesn't have any effect and is misleading for developers.
Normally the config:import part during the deployment should take care of that.

Proposed resolution

Read and store the provider settings from/to config instead of state
=> replace all \Drupal::state() calls, TMGMT already does the saving part for us.

For the credentials (api_url, api_username, api_password), probably the most secure is to enforce the use of config override to set them and validate it if api_mode = real.

For the rest (e.g: tmgmt_cdt.api_access_token, tmgmt_cdt.api_access_token_expire, tmgmt_cdt.ready,...), decide on the appropriate storage (cache seems appropriate?).

📌 Task
Status

Active

Version

1.0

Component

Code

Created by

🇧🇪Belgium herved

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇧🇪Belgium herved

    Also related, config schema validation is failing on this module:

      tmgmt.translator.tmgmt_cdt:settings.conf_mode       missing schema
      tmgmt.translator.tmgmt_cdt:settings.conf_advanced   missing schema
      tmgmt.translator.tmgmt_cdt:settings.conf_network    missing schema
    
Production build 0.71.5 2024