🇪🇸Spain @Eduardo Morales Alberti

Hi Alberto Paderno, any news of the module maintainer?

As work around it can be hidden by:

    $form['files_container'] = [
      '#type' => 'container',
      '#states' => [
        'visible' => [
          ':input[id="edit-subject"]' => [
            ['value' => $this->t('Question')],
            'or',
            ['value' => $this->t('Complaint')],
          ],
        ],
      ],
    ];

    $form['files_container']['images'] = [
      '#type' => 'managed_file',
      '#title' => $this->t('Choose an image (jpg/png)'),
      '#description' => $this->t('You may select up to 5 files.'),
      '#multiple' => TRUE,
      '#upload_validators' => [
        'file_validate_extensions' => ['png jpg jpeg'],
        'file_validate_size' => ['4194304'],
      ],
      '#cardinality' => 5,
    ];

After deploy the last version of Drupal 10.3.5 we got the following error:

Error: Class "Drupal\Core\EventSubscriber\RequestCloseSubscriber" not found in Drupal\Component\DependencyInjection\Container->createService() (line 259 of /mnt/www/html/drschar/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php) #0 /mnt/www/html/drschar/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php(177): Drupal\Component\DependencyInjection\Container->createService(Array, 'request_close_s...') #1 /mnt/www/html/drschar/docroot/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(105): Drupal\Component\DependencyInjection\Container->get('request_close_s...') #2 /mnt/www/html/drschar/vendor/symfony/http-kernel/HttpKernel.php(115): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object(Symfony\Component\HttpKernel\Event\TerminateEvent), 'kernel.terminat...') #3 /mnt/www/html/drschar/docroot/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(63): Symfony\Component\HttpKernel\HttpKernel->terminate(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Render\HtmlResponse)) #4 /mnt/www/html/drschar/docroot/core/lib/Drupal/Core/DrupalKernel.php(688): Drupal\Core\StackMiddleware\StackedHttpKernel->terminate(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Render\HtmlResponse)) #5 /mnt/www/html/drschar/docroot/index.php(22): Drupal\Core\DrupalKernel->terminate(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Render\HtmlResponse)) #6 @main.

After a long time, one month, we did not receive any response from the maintainer, so we move the issue to the Drupal.org project ownership.

Ready to review!!

Created MR https://git.drupalcode.org/project/translatable_menu_link_uri/-/merge_re... using menu tree manipulators strategy.

eduardo morales alberti → changed the visibility of the branch 3472291-override-the-menu to hidden.

Failed Javascript tests https://git.drupalcode.org/issue/drupal-3091246/-/pipelines/275582

Here came the regression, the references were removed on commit https://git.drupalcode.org/project/drupal/-/commit/552c484ad403a37e93056...

Tests are failing https://git.drupalcode.org/issue/drupal-3091246/-/jobs/2307611

The manipulator is not applying

    There was 1 failure:
    
    1) Drupal\KernelTests\Core\Menu\MenuLinkTreeTest::testMenuManipulatorAlter
    Failed asserting that an array contains 'menu-test-link'.
    
    /builds/issue/drupal-3091246/core/tests/Drupal/KernelTests/Core/Menu/MenuLinkTreeTest.php:220

The change proposed by @tunic works for as on Drupal 10.2.7

We are using 10.2.7, we are not sure, if Drupal 11 treats arrays in a different way, but the @tunic comment seems to be logical.

The event does not alters the manipulators array, help!

    $this->eventDispatcher->dispatch(new MenuLinkTreeManipulatorsAlterEvent($tree, $manipulators, $this), MenuLinkTreeEvents::ALTER_MANIPULATORS);
    foreach ($manipulators as $manipulator) {
      $callable = $this->callableResolver->getCallableFromDefinition($manipulator['callable']);

My subscriber:

  /**
   * React to manipulators alter event.
   */
  public function manipulatorsAlter(MenuLinkTreeManipulatorsAlterEvent $event) {
    $manipulators = $event->getManipulators();
    if (!empty($manipulators)) {
      array_unshift($manipulators, ['callable' => 'translatable_menu_link_uri.alter_links:alterMenuLinksManipulator']);
    }
    else {
      $manipulators = [['callable' => 'translatable_menu_link_uri.alter_links:alterMenuLinksManipulator']];
    }
    $event->setManipulators($manipulators);
  }

Maybe the array should be by reference?

We will create a new branch and MR with the manipulators approximation if the issue ✨ Allow MenuLinkTree manipulators to be altered Needs work is fixed.

After reviewing the Drupal Issue Queue the best short-term solution is to use the patch from ✨ Allow MenuLinkTree manipulators to be altered Needs work to alter the menu link tree.

+1 to this solution

Also fails because \Drupal\Core\Menu\DefaultMenuLinkTreeManipulators::checkAccess, if the menu link is not updated before BlockViewBuilder::preRender.

Previously applied the issue 🐛 Avoid extra DB query per menu item Needs review

eduardo morales alberti → created an issue.

It was from a bot and can not reproduce again, low priority

Same warning here!

Waiting for this change, thank you

Pending add test coverage.

@Shiraz Dindar you are right, we added a description to the processor to add the NID field to the index and check if the NID field exists to avoid breaking the search_api indexation.

Added commit to improve the processor

The QueueWorker queue each sitemap on \Drupal\simple_sitemap\Queue\QueueWorker::queue

Could be a good place to divide the sitemaps under conditions (languages, content types...).

public function queue($sitemaps = []): QueueWorker {
  $empty_variants = array_fill_keys(array_keys($sitemaps), TRUE);
  $all_data_sets = [];
 
  foreach ($sitemaps as $variant => $sitemap) {
    if ($sitemap->isEnabled()) {
      foreach ($sitemap->getType()->getUrlGenerators() as $url_generator_id => $url_generator) {
        // @todo Automatically set sitemap.
        foreach ($url_generator->setSitemap($sitemap)->getDataSets() as $data_set) {
          unset($empty_variants[$variant]);
          $all_data_sets[] = [
            'data' => $data_set,
            'sitemap' => $variant,
            'url_generator' => $url_generator_id,
          ];
 
          if (count($all_data_sets) === self::REBUILD_QUEUE_CHUNK_ITEM_SIZE) {
            $this->queueElements($all_data_sets);
            $all_data_sets = [];
          }
        }
      }
    }
  }
 
  if (!empty($all_data_sets)) {
    $this->queueElements($all_data_sets);
  }
  $this->getQueuedElementCount(TRUE);
 
  // Remove all sitemap content of variants which did not yield any queue
  // elements.
  foreach ($empty_variants as $variant => $is_empty) {
    $sitemaps[$variant]->deleteContent();
  }
 
  return $this;
} 

Example:

    $all_data_sets[] = [
      'data' => $data_set,
      'sitemap' => $variant,
      'variant' => [
        'type' => 'product',
        'language' => 'es-es',
      ],
      'url_generator' => $url_generator_id,
    ];

Eduardo Morales Alberti → created an issue.

Eduardo Morales Alberti → created an issue.

Thank you @saschaeggi

Hi, why the latest issues fixed were not included in the last release?
Some commits were from 3 months ago and the release is from 1st of June.

https://git.drupalcode.org/project/maillog/-/compare/8.x-1.1...8.x-1.x?f...

Eduardo Morales Alberti → created an issue.

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.