NH, USA
Account created on 25 January 2007, almost 18 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States DamienMcKenna NH, USA

Several other modules still use getStorage("node_type"), so I'm surprised this causes problems.

Is there a change notice related to it?

🇺🇸United States DamienMcKenna NH, USA

Thank you all for working through this bug and building a fix.

Committed.

🇺🇸United States DamienMcKenna NH, USA

This is a duplicate of 📌 Fix PHP 8.4 deprecations Active .

🇺🇸United States DamienMcKenna NH, USA

Doh! Let's include this in the next releases.

🇺🇸United States DamienMcKenna NH, USA

FYI I ended up handling this via an extra Drush command that runs after we run "drush deploy", with this code:

    $config_name = 'salesforce.salesforce_auth.legacy_oauth';
    $old_config = \Drupal::config($config_name);
    if ($old_config->hasOverrides()) {
      $config_factory = \Drupal::configFactory();
      $new_config = $config_factory->getEditable($config_name);
      $new_config->set('provider_settings.consumer_key', $old_config->get('provider_settings.consumer_key'));
      $new_config->set('provider_settings.consumer_secret', $old_config->get('provider_settings.consumer_secret'));
      $new_config->save();

      $this->io()->writeln(dt('Salesforce credentials updated.'));
    }
    else {
      $this->io()->writeln(dt('Did not update the Salesforce credentials'));
    }

It loads the configuration of the "legacy_oauth" authentication config, checks to see if it was overridden via settings.php, and if so re-saves the values into the config object.

🇺🇸United States DamienMcKenna NH, USA

Thank you for working on this.

In hindsight I think we should have transparently converted the name of the meta tag from a user entered string to something for the machine name, to avoid situations where meta tag names just don't match the limitations of the machine name logic.

That said, thank you for the improvements.

A few minor nitpicks:

     $form['id'] = [
       '#type' => 'machine_name',
+      '#maxlength' => 64,
+      '#description' => $this->t('A unique name for this custom tag. Must be alpha-numeric, hyphen, or underscore separated.'),
+  protected function createCustomMetaTag($id, $label, $description, $htmlElement, $htmlNameAttribute, $htmlValueAttribute) {
+    // Access custom meta add page.
+    $this->drupalGet('admin/config/search/metatag/custom-tags/add');
+    $this->assertSession()->statusCodeEquals(200);
+    $edit = [];
+    $edit['id'] = $id;
+    $edit['label'] = $label;
+    $edit['description'] = $description;
+    $edit['htmlElement'] = $htmlElement;
+    $edit['htmlNameAttribute'] = $htmlNameAttribute;
+    $edit['htmlValueAttribute'] = $htmlValueAttribute;
+    $this->submitForm($edit, 'Save');
+    $this->assertSession()->addressEquals('/admin/config/search/metatag/custom-tags');
+    $this->assertSession()->pageTextContains('Created Format Detection Custom tag.');
+  }

The pageTextContains() check has the name of one meta tag hardcoded, it should check for the $label.

🇺🇸United States DamienMcKenna NH, USA

In my case we have content like this:

<p class="text-align-center"><drupal-media data-entity-type="media" data-entity-uuid="00517131-5632-4716-a835-3e30f3200517" data-view-mode="default"></drupal-media></p>

When that's rendered it turns into this:

<p class="text-align-center">
</p>
<figure class="media--image media--default media--image--default image.jpg">
    <img loading="lazy" src="/sites/default/files/styles/large/public/image.jpg?itok=w3VwAQsO" width="320" height="400" alt=" " class="image-style-large">
    <figcaption class="field field--name-field-caption field--type-string-long field--label-hidden field__item"></figcaption>
</figure>

This is migrated content, so I wonder if I need to modify the migrated content to avoid the problem?

🇺🇸United States DamienMcKenna NH, USA

The MR changes the "#token_types" value from "all" to use the field's entity type, and it works in my local testing.

🇺🇸United States DamienMcKenna NH, USA
🇺🇸United States DamienMcKenna NH, USA

I logged it against 10.3.x while I was first diagnosing the problem.

🇺🇸United States DamienMcKenna NH, USA

aurora-norris: FYI your comments weren't visible because your account wasn't approved and the automated security systems on the site don't let you post patches until your account is confirmed. I've confirmed your account and have published your two comments (though the second one is a dupe).

🇺🇸United States DamienMcKenna NH, USA

FYI there's a related issue on the base entity query where it doesn't add a condition on the "langcode" field, I opened a separate issue for it and included a prototype patch to start from: 🐛 Views list of terms results in duplicate records because JOIN does not include langcode Active

🇺🇸United States DamienMcKenna NH, USA

This needs to be turned into a merge request and needs test coverage.

🇺🇸United States DamienMcKenna NH, USA

This seems to cover it:

    if ($data_table) {
      $data[$base_table]['table']['join'][$data_table] = [
        'left_field' => $entity_id_key,
        'field' => $entity_id_key,
        'type' => 'INNER',
      ];
      // If the entity type is translatable another JOIN condition is required
      // on the langocde column to avoid duplicate records.
      if ($this->entityType->isTranslatable()) {
        $data[$base_table]['table']['join'][$data_table]['extra'][] = [
          'field' => $entity_keys['langcode'],
          'left_field' => $entity_keys['langcode'],
        ];
      }
🇺🇸United States DamienMcKenna NH, USA

The code in EntityViewsData::getViewsData() should look like this:

    if ($data_table) {
      $data[$base_table]['table']['join'][$data_table] = [
        [
          'left_field' => $entity_id_key,
          'field' => $entity_id_key,
          'type' => 'INNER',
        ],
        [
          'left_field' => 'langcode',
          'field' => 'langcode',
          'type' => 'INNER',
        ],
      ];

Unfortunately that doesn't work.

🇺🇸United States DamienMcKenna NH, USA

I wonder if the issue is in EntityViewsData? It might be a generic issue with all entities, not just terms.

🇺🇸United States DamienMcKenna NH, USA

I'm also throwing my hat in for #26 and #29. The beginners who don't know the ins and outs of composer issues will most likely want guard rails to help them towards a stable, more reliable path, while the more experienced developers who just want to save time building a new site and intend to customize the site after installation will be able to customize the composer file as needed.

🇺🇸United States DamienMcKenna NH, USA

luisnicg: Please open a new issue for that, it seems like a distinct bug that we could resolve.

🇺🇸United States DamienMcKenna NH, USA

damienmckenna created an issue.

🇺🇸United States DamienMcKenna NH, USA

We definitely want to get this in the next release.

Can someone please update the MR to include the latest changes in the 8.x-3.x branch?

Also, this needs test coverage to make sure the change isn't accidentally broken later on.

Thank you.

🇺🇸United States DamienMcKenna NH, USA

8.x-3.4 has been superseded by 8.x-3.5 and 8.x-3.6, if the problem persists please let us know otherwise we'll assume this issue has been resolved.

🇺🇸United States DamienMcKenna NH, USA

Please see if the current dev release fixes this, some minor changes were committed in 📌 Change Testing to GitLab CI Active that might affect this.

🇺🇸United States DamienMcKenna NH, USA

gitlab support has been added to the repo, can someone please update the MR to pull in that change so we can see what the tests say? Thank you.

🇺🇸United States DamienMcKenna NH, USA

gitlab support has been added to the repo, can someone please update the MR to pull in that change so we can see what the tests say? Thank you.

🇺🇸United States DamienMcKenna NH, USA

Committed. Thank you.

🇺🇸United States DamienMcKenna NH, USA

Thank you for offering to comaintain the module, bobi-mel. As avpaderno points out, in order to become a comaintainer on a project that opted into security advisory coverage you need to go through the security review process with a module you wrote yourself. That said, in the meantime we maintainers would be very grateful for any assistance you could provide triaging the issue queue, as you have done already. Thank you.

🇺🇸United States DamienMcKenna NH, USA

One interesting detail is that if you override $config['salesforce.settings']['salesforce_auth_provider'] in settings.php the admin/config/salesforce/authorize/list page shows the change immediately.

There's a bug with the proposed change when multiple configurations are defined - the connection details from the default auth provider is shown on the authorize/list page for all connections, not just the one it affects.

This shows the list page without the patch or any changes to settings.php:

This shows the list page without the patch and with the $config['salesforce.settings']['salesforce_auth_provider'] and $config['salesforce.salesforce_auth.legacy_oauth']['provider_settings'] defined in settings.php:

This shows the list page with patch and without changes to the settings.php file:

This shows the list page with the patch and with the settings.php changes:

As you can see, with multiple configurations it retains the default settings when it shows all settings items.

Another issue is that the state value named "salesforce.auth_tokens.IDENTID" is an object, which will be hard to override.

🇺🇸United States DamienMcKenna NH, USA

This would be a great feature improvement for 2.2.x

🇺🇸United States DamienMcKenna NH, USA

These tags were removed at some point, see 📌 Remove deprecated Twitter Card plugins (D9) Fixed for more details.

🇺🇸United States DamienMcKenna NH, USA

Drupal 7 hits its EOL today, this won't be resolved.

🇺🇸United States DamienMcKenna NH, USA

Drupal 7 hits its EOL today, this won't be resolved.

🇺🇸United States DamienMcKenna NH, USA

Drupal 7 hits its EOL today, this won't be resolved.

🇺🇸United States DamienMcKenna NH, USA

Drupal 7 hits its EOL today, this won't be resolved.

🇺🇸United States DamienMcKenna NH, USA

Drupal 7 hits its EOL today, hopefully you've been able to resolve the problem.

🇺🇸United States DamienMcKenna NH, USA

Drupal 7 hits its EOL today, so this change will not be done.

🇺🇸United States DamienMcKenna NH, USA

Drupal 7 hits its EOL today, so this change will not be done.

🇺🇸United States DamienMcKenna NH, USA

Drupal 7 hits its EOL today, so this feature request will not be done.

🇺🇸United States DamienMcKenna NH, USA

Drupal 7 hits its EOL today, so this feature request will not be done.

🇺🇸United States DamienMcKenna NH, USA

Drupal 7 hits its EOL today, so this feature request will not be done.

🇺🇸United States DamienMcKenna NH, USA

Drupal 7 is deprecated, so this feature request will not be done.

🇺🇸United States DamienMcKenna NH, USA

Committed. Thank you Kul!

🇺🇸United States DamienMcKenna NH, USA

Committed.

🇺🇸United States DamienMcKenna NH, USA

I've committed the tests, with some D10 fixes, but as with #5 the tests pass as-is. This means that we still need to build a test that fails with the current logic,

🇺🇸United States DamienMcKenna NH, USA

This also needs tests to make sure it works as expected.

🇺🇸United States DamienMcKenna NH, USA

File added, let's see what it says.

Production build 0.71.5 2024