🇫🇷France @vbouchet

vbouchet → created an issue.

The suggested commit is not reworking the global implementation, just fixing the way total is counted in the Recipe sources so it does count only after filtering is applied.

As per my investigation, this is because the Recipes source is not updating the number of result.

Object.entries(data).forEach((item) => {
  const [source, result] = item;
  if (result.totalResults !== 0) {
    $activeTab = source;
    [project] = result.list;
    projectExists = true;
  }
});

The JS behind /admin/modules/browse/{module_name} still invokes all the source plugins but expect only one source to return results.

Globally, I think the JS should be aware of the active source tab so other sources can act appropriately:
- In case it is the /browser, run a lightweight query as the goal is only to know the number of projects matching the criteria.
- In case it is the /browser/{project_name}, don't do anything

vbouchet → created an issue.

vbouchet → created an issue.

vbouchet → created an issue.

I updated the merge request mainly to replace the + operator by a array_merge. I replaced annotation by attributes and tried to implement some tests.

Investigation shows that using Condition API will drastically degrade the way policies are configured.

vbouchet → made their first commit to this issue’s fork.

Basic logo has been added.

vbouchet → created an issue.

Thanks @thejimbirch. Config is not complaining during recipe install but accessing the module settings page is causing a fatal as form_ids is supposed to be an array.
I also tried

config:
  actions:
    antibot.settings:
      simple_config_update:
        form_ids:
          - 'webform_*'

but as expected, it simply replace the entire config and only webform are protected. For now my solution is to duplicate the original config and add mine:

config:
  actions:
    antibot.settings:
      simple_config_update:
        form_ids:
          - 'comment_*'
          - user_login_form
          - user_pass
          - user_register_form
          - 'contact_message_*'
          - 'webform_*'

In my case, it is not a big issue as my recipe is about setting some security things. But I can imagine a "form" recipe which wants to conditionnaly integrate with the antibot module and append the config without rewritting everything.

I think the feature request is still very valid.

I am very new in using Recipes and config actions. My use case is to append (or prepend, the order is not very important for me) a value in the antibot.settings.form_ids to add webform_*. Right now, I have not find any other solution than copying the default config in my recipe config action.

vbouchet → created an issue.

I did some more testing and I am not able to replicate anymore after the last change (removal of ctools). It is merged in 2.x.

Please reopen if you still face the issue.

I updated the schema so the config is slightly more validatable.

We need to dive into the existing way of validating specific config likes routes, roles, etc. String is too much generic and is not really validating anything.

Roles are now properly listed as dependencies.

vbouchet → created an issue.

Method calculateDependencies() has been overridden in the MemoryLimitPolicy ConfigEntity type.

vbouchet → created an issue.

I faced the same issue on my local. I am using demo_umami profile and very few contrib modules.

When running drush config:inspect, I got the following error:

In ValidKeysConstraintValidator.php line 23:
                                                                   
  [Symfony\Component\Validator\Exception\UnexpectedTypeException]  
  Expected argument of type "array", "null" given                  
                                                                   

Exception trace:
  at /var/www/html/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ValidKeysConstraintValidator.php:23
 Drupal\Core\Validation\Plugin\Validation\Constraint\ValidKeysConstraintValidator->validate() at /var/www/html/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:202
 Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateConstraints() at /var/www/html/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:154
 Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateNode() at /var/www/html/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:164
 Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateNode() at /var/www/html/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:106
 Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validate() at /var/www/html/core/lib/Drupal/Core/TypedData/Validation/RecursiveValidator.php:93
 Drupal\Core\TypedData\Validation\RecursiveValidator->validate() at /var/www/html/core/lib/Drupal/Core/TypedData/TypedData.php:132
 Drupal\Core\TypedData\TypedData->validate() at /var/www/html/modules/contrib/config_inspector/src/ConfigInspectorManager.php:355
 Drupal\config_inspector\ConfigInspectorManager->validateValues() at /var/www/html/modules/contrib/config_inspector/src/Commands/InspectorCommands.php:222
 Drupal\config_inspector\Commands\InspectorCommands->inspect() at n/a:n/a
 call_user_func_array() at /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php:276
 Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback() at /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php:212
 Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter() at /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php:176
 Consolidation\AnnotatedCommand\CommandProcessor->process() at /var/www/html/vendor/consolidation/annotated-command/src/AnnotatedCommand.php:391
 Consolidation\AnnotatedCommand\AnnotatedCommand->execute() at /var/www/html/vendor/symfony/console/Command/Command.php:326
 Symfony\Component\Console\Command\Command->run() at /var/www/html/vendor/symfony/console/Application.php:1096
 Symfony\Component\Console\Application->doRunCommand() at /var/www/html/vendor/symfony/console/Application.php:324
 Symfony\Component\Console\Application->doRun() at /var/www/html/vendor/symfony/console/Application.php:175
 Symfony\Component\Console\Application->run() at /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php:110
 Drush\Runtime\Runtime->doRun() at /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php:40
 Drush\Runtime\Runtime->run() at /var/www/html/vendor/drush/drush/drush.php:139
 require() at /var/www/html/vendor/drush/drush/drush:4
 include() at /var/www/html/vendor/bin/drush:119

The latest line related to config_inspector in the trace is Drupal\Core\TypedData\TypedData->validate() at /var/www/html/modules/contrib/config_inspector/src/ConfigInspectorManager.php:355.

I simply added var_dump($config_name) prior line 355 to check what config was being validated. In my case, it was search_api_solr.solr_cache.cache_document_default_7_0_0. I deleted my search index/server and disabled the search_api_solr module. After that, the drush command was working as expected.

We are probably all reporting the same issue but on a different config. Given the trace, it is very likely to be a core issue.

vbouchet → created an issue.

vbouchet → created an issue.

I have raised a Draft MR in #3444342 📌 Remove ctools dependency Active which remove Ctools.

I can't reproduce the issue anymore and I can see the "Devel" operation in the policy dropdown button.

I need to perform some more tests before merging the MR and creating a new release.

I raised a draft MR to give some visibility but I need to some more tests before merging.

vbouchet → created an issue.

I feel this is some incompatibility between Devel and EntityWizard from Ctools which is used to generate the multistep form.

I don't think the multistep form add so much value and I was already wondering about removing Ctools dependency to ease the maintenance.

I will create a specific ticket to remove Ctools dependency and leave this bug open to remember to check it is fixed after Ctools removal.

Hi @joncjordan,

Thanks for reporting. I can reproduce a similar issue.

The website encountered an unexpected error. Try again later.

Symfony\Component\Routing\Exception\MissingMandatoryParametersException: Some mandatory parameters are missing ("machine_name", "step") to generate a URL for route "entity.memory_limit_policy.devel_load". in Drupal\Core\Routing\UrlGenerator->doGenerate() (line 187 of core/lib/Drupal/Core/Routing/UrlGenerator.php).
Drupal\Core\Routing\UrlGenerator->getInternalPathFromRoute('entity.memory_limit_policy.devel_load', Object, Array, Array) (Line: 300)
Drupal\Core\Routing\UrlGenerator->generateFromRoute('entity.memory_limit_policy.devel_load', Array, Array, 1) (Line: 108)
Drupal\Core\Render\MetadataBubblingUrlGenerator->generateFromRoute('entity.memory_limit_policy.devel_load', Array, Array, 1) (Line: 765)
Drupal\Core\Url->toString(1) (Line: 183)
Drupal\Core\Utility\LinkGenerator->generate(Object, Object) (Line: 102)
Drupal\Core\Render\Element\Link::preRenderLink(Array)
call_user_func_array(Array, Array) (Line: 111)

It seems devel is trying to add its own route to debug the entity. I need to check why it fails as I do expect this to be very generic and applied to all kind of entity types.

Thank you for reporting and proposing a fix. However, I think it is on purpose than basic auth service is not injected. The ShieldMiddleware does not depend on Basic Auth module to work. Before the service is used, the basic_auth module's status is checked.
The code you introduce does not check anything and will fail (missing/unknown service) on any site without basic_auth enabled.

vbouchet → created an issue.

vbouchet → created an issue.

It has been done with commit #a490344. There is now a memory_limit_policy_env_variable module.

vbouchet → created an issue.

vbouchet → created an issue.

vbouchet → created an issue.

Actually reading at the exact title and the workaround, it seems more a visual indication which is requested here.

If we decide to so, it should be consistent with other places we list entities with "Status" column.

I am closing as it seems a duplicate of #3359159 ✨ Add Status to OverviewTerms.php form Fixed which was solved some time back.

I partially reproduced the issue. I am not able to reproduce with select. However, I am able to reproduce with radios.

I debugged the FormBuilder::doBuildForm() method. At some point, it does Element::children($element) to process the children. For select, it does not do anything because the options are simply attributes of the element. However, for the radios, the options are individual elements (see Radios.php::processRadios($element)). The Element::children($element) method has a condition to process or not a children: if (is_int($key) || $key === '' || $key[0] !== '#') {. Because of the last condition, the options starting with # are not returned by the method and then are not processed. I confirmed by hijacking the condition to if (is_int($key) || $key === '' || $key === '#FFFFFF' || $key === '#000000' || $key[0] !== '#') { and the options are properly displayed.

I see 2 options to fix this issue:

• Update the Radios::processRadios($element) method to update the few places the $key (which is the option value) is used by something else (for exemple 'option-' . $key) so it is not stripped later as it does not start with # anymore.
• Update the Element::children($elements) method to compare the $key with the keys in $options. Something like if (is_int($key) || $key === '' || $key[0] !== '#' || (!empty($elements['#options']) && isset($elements['#options'][$key]))) {

Contextual link added, tests updated.

vbouchet → made their first commit to this issue’s fork.

I raised a MR which:

• Move the media source from the cloudflare_stream_hosted_video submodule to the main cloudflare_stream module
• Rename the media source from HostedVideo to Cloudflare Stream
• Rename the submodule cloudflare_stream_hosted_video to cloudflare_stream_bundle
• Adapt the code in cloudflare_stream_bundle to use the new bundle name and media source name
• Adapt the code in cloudflare_stream_sync

I adapted the cloudflare_stream_sync module as-is but I think it should be adapted to be more generic and not only check the cloudflare_stream media bundle provided by cloudflare_stream_bundle but instead should check for all the bundles using the cloudflare_stream media source. We can probably have a checkbox added to cloudflare_stream media source when cloudflare_sync module is enable to decide if the bundle should be synced or not.

Please note that I have not been able to test my change (other than enabling the module and checking the possibility to create a new media type, etc) as the Cloudflare test account I was using for my PoC expired.

tim-diels → credited vbouchet → .

Returning back to the D7 version, it was using hook_boot() in place of the http_middleware. Looking at the change records ( https://www.drupal.org/node/1909596 → ), it seems using an event_subscriber on KernelEvents::REQUEST.

I did a small PoC on my local and it seems working.

What I am not really clear yet is the potential impacts with the various http_middleware that are currently coming after ShieldMiddleware (mainly PageCache) but which will come before the ShieldSubscriber once moved to event_subscriber.

I am interested in getting some feedback to find a proper solution to it because the plan for the 2.x branch of Shield is to move the Auth method and Exceptions into plugins (so it is easier for each project to extend if needed). However, having plugins will very likely make the corrupted cache issue to arise more often with the current implementation. That is why the 2.x branch is blocked from some time now.

vbouchet → created an issue.

as @Sivaji_Ganesh_Jojodae, I think we should remove any mention to Drupal 8 in composer.yml or info.yml files. Also, all the sub-modules should be updated.

vbouchet → created an issue.

I fixed it in the #3408236 MR which is also about the schema.

I reviewed the code which looks ok. I fetched the branch and tried the basic feature to embed or delete a video with success.

I know you marked it as Novice but I had to fix it to progress on something else so I used the opportunity to contribute it back.

vbouchet → made their first commit to this issue’s fork.

I think the current approach is OK with overwriting the config from settings. There is no difference with other modules which the config may be managed via settings.

On my local I have remove the token from the config (using drush cset), I added a fake token via $config['cloudflare_stream.settings']['api_token'] = 'dummy'; in the settings file. The API calls are failing as expected. I then changed it to a real token and the API calls are working as expected.

I suspect it works ok in 99% of the cases but I am wondering in case we use $config['cloudflare_stream.settings']['api_token'] = 'xxxxxxx' in a secrets.settings.php (to avoid exposing this in config), it may make impossible to submit the form. I should probably give a check at other modules exposing form for settings that we tend to exclude from config for security reason.

Hi tim-diels. I have reviewed the few places we are using the API getHeaders method and it seems all working well with the new auth method. I created a task (and a MR) to validate the token on form submit so we don't discover later the token is not valid.

vbouchet → created an issue.

I will check and maybe reopen the ticket. I confirm that I used the Bearer approach with Postman on my local to test and struggled a little bit to setup the module on my local with 2.x branch. I think today the module is using API key which I think may give too much permission, whereas API token can be restricted to Stream read/write only.

In the meantime, switching this issue to 3.x-dev.

Switching to 3.0.x-dev as it seems to be the default branch.

Is 2.x-dev the proper branch for this?

vbouchet → created an issue.

vbouchet → created an issue.

Thank you for the updated MR @shybhangi1995. I just merged it and will create a new release now.

I think it is blocked until the parent issue is completed.

Thank you. I merged the PR, will create a new release.

I applied the same change in a MR instead of a patch.

Can't promise any timing but I am always interested to follow my patches when I get reply and clear direction 😉.
Will try to have a look.

As you mentioned in other issue, seems the tests are not executed automatically anymore. At least I had to manually queues these for the few patches I submited today and I also noticed the "reqs" things. I will try to turn it into a MR layer.

Thanks for the headsup @smustgrave. Will try to setup my local for easier work with issue fork. 😉

Please find a patch doing so. New tests need to be created.

I am suggesting to close this plan ticket as outdated. There is already a stable release and an independent ticket to track adding tests.

I only noticed the issue's fork when uploading the patch. Seems the fork is empty anyway.

Mark as "Reviewed & tested by the community" as the test is now passing again.

From my point of view, the others form submit look legit as they serve the purpose of the tests (testing the moderation form).

I will give a check. As the ticket title mentioned "to setup language", I restricted my check to this only.