🇪🇸Spain @Eduardo Morales Alberti

Why remove the recursive limit?
Before the MR the limit was 20, why remove it?
We should detect the real recursions but is there any way to render an element multiple times without recursions?

Differences between 4994 and 4050:

4994:
* The keys are the entity type, entity id and view mode:

 
    return $entity->getEntityTypeId()
      . $entity_id
      . $build['#view_mode'];

 
* The recursion protection is executed on the pre-render and the post-render:

    // Add callbacks to protect from recursive rendering.
    $build['#pre_render'] = [[$this, 'setRecursiveRenderProtection']];
    $build['#post_render'] = [[$this, 'unsetRecursiveRenderProtection']];

4050:
* The keys are a key on the render array and contain the revision and the language:

...
    $keys = [
      'entity_view',
      $this->entityTypeId,
      $entity->id(),
      $view_mode,
    ];
.... 
    // Add keys for the renderer to use to identify recursive rendering.
    $build['#recursion_keys'] = $keys;
    if ($entity instanceof RevisionableInterface) {
      $build['#recursion_keys'][] = $entity->getRevisionId();
    }
    if ($entity instanceof TranslatableDataInterface && count($entity->getTranslationLanguages()) > 1) {
      $build['#recursion_keys'][] = $entity->language()->getId();
    }

* The recursion protection is executed on the method do render:

protected function doRender(&$elements, $is_root_call = FALSE) {
....
    // Guard against recursive rendering now that all other early return
    // possibilities are exhausted and it's time to render children.
    $recursion_key = implode(':', $elements['#recursion_keys'] ?? []);
    if ($recursion_key) {
      if (isset($this->recursionKeys[$recursion_key])) {
        \Drupal::logger('render')
          ->error('Recursive rendering attempt aborted for %key. In progress: %guards', [
            '%key' => $recursion_key,
            '%guards' => print_r($this->recursionKeys, TRUE),
          ]);
        $elements['#printed'] = TRUE;
      }
      else {
        $this->recursionKeys[$recursion_key] = $recursion_key;
      }
    } 

So 4050 is more complete because it considers the revision IDs and the languages and allows adding more keys using #recursion_keys on the render array.

In our case with a multilingual website with more than 20 countries, the MR4994 does not fit because it does not take into account the language on the recursion keys, on our case the MR 4050 fits better.

The problem came from "mymodule_preprocess_node__article", it did not match with any template, the available templates were node__article__full and node__article__teaser, but it "confuses" the theme manager.
When the theme registry tries to get the suggestion before getting the node_search it gets the node__article.

The list is:
{code:java}Array
(
    [0] => node__search
    [1] => node__article
    [2] => node__article__search
    [3] => node__679
    [4] => node__679__search
)
{code}

docroot/core/lib/Drupal/Core/Theme/ThemeManager::render()
{code:java}foreach (array_reverse($suggestions) as $suggestion) {
  if ($theme_registry->has($suggestion)) {
    $info = $theme_registry->get($suggestion);
    break;
  }
} {code}
 
 

Created new branch 2.x https://git.drupalcode.org/issue/yotpo-3450844/-/tree/2.x?ref_type=heads

Eduardo Morales Alberti → created an issue.

Probably related with change record CKEditor 5 plugin definitions must explicitly indicate which tags they can create →

Same problem with "ckeditor5_language" https://www.drupal.org/node/3283526#comment-15391076 →

It seems that the core ckeditor5_language plugin should be updated thusly:

elements:
-
-
Right now if you want to enable the language plugin but do not want to enable source editing, you have no options. The following error appears:

"The Language plugin needs another plugin to create , for it to be able to create the following attributes: . Enable a plugin that supports creating this tag. If none exists, you can configure the Source Editing plugin to support it."

This is confusing. See https://www.drupal.org/project/drupal/issues/3388978 🐛 Eliminate or improve error upon adding the language plug-in Active

It seems natural to allow the language plugin to also create the tag it may modify.

Is there any problem with this?

Eduardo Morales Alberti → created an issue.

Eduardo Morales Alberti → created an issue.

Moved colorbox to new submodule, ready to review.

Thank you @Luke.Leber

Pending to move the Javascript to a submodule.

Eduardo Morales Alberti → created an issue.

After applying the last patch #79 the CKEditor appearance is not right.

Eduardo Morales Alberti → created an issue.

Eduardo Morales Alberti → created an issue.

@omarlopesino The module name is "watchdog_statistics", the admin user may need to know the statistics of the last month or the last week.

It is not the same 2000 entries of a log error in the last year than 2000 entries in the last week, with the filter of the date you can measure the log-entries/time.

The error can be tested manually, and you can check that the error is gone, now you can not measure which error appeared more in the last week.

In my opinion, it is an interesting feature and not a custom one.

Added related issue

Patch #2 is not right because it mascarade the problem and it does not solve it.
After some debugging, we saw that the theme function did not have the variables as array.
Before:

function mymodule_theme($existing, $type, $theme, $path) {
  return [
    'mymodule_test' => [
      'variables' => NULL,
    ],
  ];
}

ThemeManager.php sets the 'render element' with the same value as the variables but its expects an array.

    if (isset($variables['#theme']) || isset($variables['#theme_wrappers'])) {
      $element = $variables;
      $variables = [];
      if (isset($info['variables'])) {
        foreach (array_keys($info['variables']) as $name) {
          if (\array_key_exists("#$name", $element)) {
            $variables[$name] = $element["#$name"];
          }
        }
      }
      else {
        $variables[$info['render element']] = $element;
        // Give a hint to render engines to prevent infinite recursion.
        $variables[$info['render element']]['#render_children'] = TRUE;
      }
    }

Solution:

function mymodule_theme($existing, $type, $theme, $path) {
  return [
    'mymodule_test' => [
      'variables' => [],
    ],
  ];
}

Added related issue

Eduardo Morales Alberti → created an issue.

Eduardo Morales Alberti → created an issue.

Eduardo Morales Alberti → created an issue.

Eduardo Morales Alberti → made their first commit to this issue’s fork.

We created a new branch "3282547-drush-delete-content-reimport" to delete all entities from default_content based on all the uuids defined on the default_content module.
Also we add a reimport command (deletes and import again the content).

Ready to review

Eduardo Morales Alberti → created an issue.

In our case, we tried to find all the UUIDs definitions on the default_content and remove them, it will remove all entities defined by default_content.
We tried the drush command and the paragraphs were not removed for example.

$entity_types = [
  'node',
  'paragraph',
  'taxonomy_term',
  'media',
  'file',
  'menu_link_content',
];

$uuids = [];
$module_folder = \Drupal::moduleHandler()
      ->getModule($module)
      ->getPath() . '/content';
$dirs = array_filter(glob($module_folder . '/content/*'), 'is_dir');

$uuids = [];
$pattern = '/uuid: (\w.*-\w.*-\w.*-\w.*-\w.*)/m';
foreach ($dirs as $dir) {
  $entity_type = basename($dir);
  if (!in_array($entity_type, $entity_types)) {
    continue;
  }
  $files = glob($dir . '/*.yml');

  foreach ($files as $file) {
    $contents = file_get_contents($file);

    if (preg_match_all($pattern, $contents, $matches, PREG_SET_ORDER, 0)) {
      foreach ($matches as $match) {
        $uuids[] = $match[1];
      }
    }
    $uuids = array_unique($uuids);

  }

}

foreach ($entity_types as $entity_type) {
  $entities = \Drupal::entityTypeManager()->getStorage($entity_type)->loadByProperties(['uuid' => $uuids]);
  \Drupal::entityTypeManager()->getStorage($entity_type)->delete($entities);
}

@caesius It is difficult to determine the default value on the update, in 2.0.3 was impossible to "disable the consent mode", so some users probably enabled it by omission and then it is not possible to know what it should be.
Also, the consent mode is a breaking change, from a user that does not want it, should we force to enable it?

As you said, the best option is leave it clear on the release notes.

Added gitlab-ci

The consent_mode is a boolean https://git.drupalcode.org/project/google_tag/-/blob/2.0.x/config/schema...

google_tag.advanced_settings.consent_mode:
  type: boolean
  label: Consent mode

So we should add a hook update to add the property to FALSE, and then leave the users to choose if enable it or not, and save it as a boolean.

Eduardo Morales Alberti → made their first commit to this issue’s fork.

We should update PHPUnit as https://github.com/sebastianbergmann/resource-operations is also an archived library and is required only on the last version supported by Drupal 9.6.17.

+1

Ready to review.
Button after patch:

Added tests, and do not log the submission deletion on batch.
The failed test is because placekitten website does not respond, nothing related to the changes.
Ready to review.

Eduardo Morales Alberti → created an issue.

If we want to delete all submissions of a form, why log it?
If there are a lot of submissions and has dblog enabled, it will add too many entries.

Added also array filter on HtmlResponseAttachmentsProcessor to avoid setting already loaded libraries when the array is empty.

[0 = ""]

Also covered the core/lib/Drupal/Core/Asset/LibraryDependencyResolver.php class, and continue if the extension or the name are not defined.

@harika gujjula We refactor the default value code to check the config saved, sorry we missed it, thank you.
Could you test it again and confirm?

Commit https://git.drupalcode.org/project/dblog_filter/-/merge_requests/9/diffs...

We should try to add the test coverage to it.

Same error here.

In our case, this happens when the library does not exist on Drupal because was removed.

Example of our case:

A search Ajax callback with a library that was removed:
https://example.com/search?ajax_page_state[theme]=radix&ajax_page_state[theme_token]=&ajax_page_state[libraries]=addtoany/addtoany.front&f[0]=type:article
When the library does not exist or is not a valid name, then it gives a warning.
Should be it fixed? Maybe not or maybe it should log which library is not available to leave to the admin decide what to do.

@harika gujjula

1. The form values does not appear to be saved for Db log Severity levels and syslog severity levels. This is because the type in Schema file is defined as boolean where it is expecting a string. Also accordingly update Schema and install files if necessary.

Did you execute the module update?
There is an update for this point, it takes the actual config on string format and transform it to booleans.
If you have any problem with the update, please let me know.
https://git.drupalcode.org/project/dblog_filter/-/merge_requests/9/diffs...

    switch ($key) {
      case 'severity_levels':
      case 'syslog_severity_levels':
        $severity_levels = [
          'critical' => FALSE,
          'notice' => FALSE,
          'emergency' => FALSE,
          'alert' => FALSE,
          'error' => FALSE,
          'warning' => FALSE,
          'info' => FALSE,
          'debug' => FALSE,
        ];
        $levels = (isset($config_raw_data[$key]) && is_array($config_raw_data[$key])) ? $config_raw_data[$key] : [];
        foreach ($levels as $level => $value) {
          if ($value && isset($severity_levels[$level])) {
            $severity_levels[$level] = TRUE;
          }
        }
        $config->set($key, $severity_levels);
        break;

2. Minor Warnings noticed on Trait when log values are empty.

It is also possible that is related with the update, could you give us more information of the warnings? We could not reproduce it.

It would be good to include few readability changes alongside.
Update description text for "Enter DB Log type and Severity Levels" for both DBlog and Syslog to "Enter Log type and Severity Levels" on the Form.
Update help text for "Enter one value per line, in the format log_type|level1,level2,level3..|message to restrict the log messages stored, the message and the level are optionals." to "Enter one value per line, in the format log_type|level1,level2,level3..|message to limit the log messages stored, the message and the level are optionals."

The text was updated on commit https://git.drupalcode.org/project/dblog_filter/-/merge_requests/9/diffs...

Same error here on 10.2.3, the patch https://www.drupal.org/files/issues/2024-01-11/3413508-22.patch → solves our issue.

Thank you @artem_sylchuk

We also have a large site with malicious crawlers, but we took another path.
Use JavaScript to show the messages instead of the classic Drupal message that sets an anonymous session to show it.

https://www.drupal.org/project/r4032login/issues/3425396 ✨ Do not set anonymous session to avoid lost cacheability on redirection Needs review

We changed to a hash in the URL instead of a query param, because in some cases the cache is based on the query params.

Eduardo Morales Alberti → created an issue.

Those changes where applied on issue ✨ Complete Refactor - Filter on log message - Possible new version 4.x RTBC

If this issue is merged, we recommend releasing a new version. 4.x

Set the GitlabCI file

We also added the filtering by message on issue ✨ Complete Refactor - Filter on log message - Possible new version 4.x RTBC and PHPUnit coverage.

Added PHPUnit test coverage.

Message filtered based on issue ✨ Get log values (type and level) for syslog like the input box for dblog Needs review

We refactored the module to follow the coding and Drupal standards and added the option mentioned in this issue.

• Completed the Schema
• Added a config install with default values
• Refactor settings form to allow the user to config dblog and syslog at the same time
• Refactor the log filter to avoid code duplications
• Removed dblog dependency from the info

Eduardo Morales Alberti → made their first commit to this issue’s fork.

Any news?

Tested on drupal 10.2.3 RTBC!

Any news?