🇺🇸United States @kevinquillen

The additional saving of path alias in presave is also problematic if you are editing content for multiple Domains from one domain. At this level, I am not sure why the PathAlias handling is in a presave hook when it is explicitly being set otherwise.

I've updated the change to incorporate what was there previously, except without loading the node from the route parameter. Using the helper to get the domain ID seems to work 100% of the time, but grabbing the node from the request or otherwise loads the unsaved node with the previous value.

This now appears to work reliably on create/update of a node.

However, I now notice that node edit links are incorrect (like tasks). I will make a new issue for that.

I've added an initial change here that tries to do two things:

• - Eliminate the race condition in hook_entity_presave(). I tried using hook_entity_update, but I saw the same behavior(s) reported. With this change I cannot replicate the behavior.
• - Update the path entity from the Node level on save.
• - Move the other path entity update logic to a form submit on the path entity form.

It's not exactly perfect because the path entity form has no awareness of the domain id (perhaps it could be a hidden value) so on submit I have to call entity save, which is likely happening more than once and could introduce new problems. This part would need more work, I believe - especially to expand support beyond nodes to any content entity with a path.

So far removing the logic in presave that updates if the entity is a PathAlias resolves what I am seeing.

I think that the case here is when the user is either updating a node or updating a Path entity directly. But theres a few things I don't quite get:

    // Get domains configuration.
    $options = \Drupal::entityTypeManager()
      ->getStorage('domain')
      ->loadOptionsList();

    // Get domain_id from get request param.
    $helper = \Drupal::service('domain_unique_path_alias.helper');
    $domain_id = $helper->getDomainIdByRequest();

    // Get domain_id from field_domain_source field.
    if (empty($domain_id) || !in_array($domain_id, $options)) {
      // Get field_domain_source data if a new translation.
      $node = \Drupal::routeMatch()->getParameter('node');

If we are updating from the path form from admin/config/search/path/edit/{id} it seems like we should be getting the entity not from the route (because any content entity could have Path enabled) but try to extrapolate it from the path value, which is here:

      if (!$node instanceof NodeInterface) {
        $path = explode("/", $entity->getPath());
        if (
          isset($path[1], $path[2])
          && $path[1] === 'node'
          && is_numeric($path[2])
        ) {
          $node = Node::load($path[2]);
        }
      }

Which is likely fine as it is, but I think to prevent the race condition this may have to move to a submit hook on the path edit form.

As far as I can tell, since this happens in the presave hook that by the time the path alias logic runs the node it is attached to has not had its changes committed yet. So loading it from either the route or with the entityTypeManager is going to give you the previous Domain Access field value no matter what, until you save it again.

This is a real issue because its hard to track this kind of bug. Ultimately, it means changing the Domain Access value on an entity will lag behind one save and causing pathing issues as content resolves to potentially the wrong domain, causing a 404 on another domain (where it should be).

How can both cases effectively be handled without overwriting?

This also affects the Domain Redirect functionality as well. You have to patch in support for query string handling there, and particularly if you use Domain Unique Path Alias.

+1, looks good.

It almost seems like both the node and the path entity are updated and the latter overwrites the former. If I comment out the code in #4, it updates correctly on every save.

I think the issue may lie here:

      if ($node instanceof NodeInterface) {
        $langcode = \Drupal::languageManager()
          ->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)->getId();
        $langcode = $entity->get('langcode')->getString() ?? $langcode;
        $translation_languages = $node->getTranslationLanguages();
        if (in_array($langcode, array_keys($translation_languages))) {
          $domain_id = $helper->getDomainIdFromEntity($node);
        }
      }

The path was already updated before this, but the presave hook fires again, and this code is hit. It gets the value from the original node, which sets it back to the old value. On the next save, without making any changes, the correct value is inserted.

Cross posting the issue on ✨ Symfony Live Components Active

Not really, its in support of a legacy API outside of Drupal, embedded in a node in React (lots of identifiers and marketing related values in the query string).

Catching up on this issue - I just ran into a need to support a case in a Drupal site where the path isn't changing, but its query strings are (which triggers different application parts on the page from React, etc) and is used for marketing purposes. The Drupal node just houses the components for this part.

i.e.

foo-page?param_1=123&param_2=456

needs to redirect to:

foo-page?param_1=789&param_2=456

theres no other way I can think of, in the UI, to support such a case.

In my case, I adapted parts of this patch to do the following:

• Implement a new event subscriber in a custom module, extending the one in Redirect
• Gave it a higher priortity
• If request query is empty, pass it off to the parent event
• Take the path and lookup any alias for it
• If the alias is empty, pass it off to the parent event
• Pass the alias into findMatchingRedirect instead of the $path
• If no $rid was found, let the parent class try to find a redirect the way it currently does
• Add url.query_args cache context to the redirect, and set the response

This appeared to work in my case - I only want this behavior when there is a query string detected and an existing alias is found. Are there any drawbacks to this?

I have not seen this myself, but colleagues will show me a screenshot of a form when they are editing other domain record values or third party settings, and the hostname has changed to reflect the one in the URL (they match). I am trying to replicate it.

kevinquillen → created an issue.

I needed a quick off the shelf theme, client liked the layout/colors vs all the others on drupal.org. I was able to edit the right pieces to get it to work for me (header/footer/nav).

kevinquillen → created an issue.

A few notes:

I suggest leveraging the AI module as an optional dependency for Search API Solr to facilitate embeddings and/or some generic tagged service that can collect more. But being that the AI module is popular, its a good place to start.

The Typesense module uses the AI provider manager to get a list of providers to offer as embedding options:

https://git.drupalcode.org/project/search_api_typesense/-/blob/1.0.x/src...

That gives you a list of models to use, which would expand support beyond HuggingFace (which AI also supports too → ).

Then, the field type vectorDimension should be configurable, as each one has a different limit. OpenAI for example is 1534 (something like that), and others vary quite a bit. This should open up options for people using different providers that have different embedding capabilities.

I am not sure where Transformers come into play, but its possible not everyone would have access to edit the ini conf to enable it. Is it possible to swap that out to use the AI provider capability of embedding on the fly to query with?

One minor point is that it is a little hard to determine (for non technical or novice users) which version they should be using. For example:

https://www.drupal.org/project/entity_clone →

Both are the same color and at a glance look basically the same, 2.1 is D11 compatible though. But there are projects out there that may have two major version branches running in parallel, but one is the 'recommended' one to use. Maybe some visual cues or improvements can be done to help distinguish beyond blue tinted release boxes.

Yeah. At the time it was the simplest way to do model filtering, but they keep adding more and a regex will only get more complex. It was a way to keep people from selecting a model that wasn't going to work. Perhaps we should change that now.

Test failure appears to be in the base Domain module.

I found an area where OpenAI\Client is used directly:

https://git.drupalcode.org/project/ai_provider_openai/-/blob/1.1.x/src/F...

kevinquillen → created an issue.

So if we are using guzzle directly and not the drupal client your issue might indeed be true.

If I read this right, specifically with OpenAI, the OpenAI client library uses Guzzle and not the one from \Drupal::httpClient, which would preconfigure it properly.

https://github.com/openai-php/client/blob/main/src/Factory.php#L180

But, the AiProviderClientBase class is injecting an instance of a client from http_client_factory with options from the provider configuration:

  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
    $client_options = $configuration['http_client_options'] ?? [];

    return new static(
      $plugin_id,
      $plugin_definition,
      $container->get('http_client_factory')->fromOptions($client_options + [
        'timeout' => 60,
      ]),

Which is then passed to the OpenAI wrapper to use as the Client:

https://git.drupalcode.org/project/ai_provider_openai/-/blob/1.1.x/src/P...

So I think the AI module(s) are doing the right thing here. I suppose one question is, here, where is this configuration set, or does more need to be added to the provider to pass along?

$client_options = $configuration['http_client_options'] ?? [];

I'm not sure if 1024 is the limit anymore. I remember the docs used to mention that, but it looks like this was fixed in 9.3:

https://issues.apache.org/jira/browse/SOLR-16836

Rebuilding node permissions seems to fix this issue, but I am not sure how it got into that state to begin with.

I could be wrong but this seems like a big issue. The module appears to assign a domain_id record - but I think in order to adequately have this enforced and checked, you need to override the core AliasManager and AliasRepository to have their lookups respect the domain_id key. Otherwise it seems like you can enter any path for a Domain and if it matches a path assigned to another domain, Drupal will return that and route it to the user.

I am kind of stumped at how to get the desired behavior.

This is a bit more challenging than I thought. So far, I came up with

    $global_config = $this->configFactory->get('system.site');
    $domain_config = $this->configFactory->get('domain.config.' . $domain_id . '.system.site');

    if ($domain_config) {
      $path = $domain_config->get('page.404');

      if (!empty($path)) {
        return $this->getAliasByPath($path, $langcode);
      }
    }

    if ($global_config) {
      $path = $global_config->get('page.404');

      if (!empty($path)) {
        return $this->getAliasByPath($path, $langcode);
      }
    }

    return $this->inner->getPathByAlias($alias, $langcode);

But this breaks valid, non-content paths (like /admin paths) and returns a 404. Attempts to load the route by path or alias seems to result in recursion errors.

kevinquillen → created an issue.

Thats what I mean, I don't think this takes over normal paste, because of its behavior. That is why its set to the CTRL SHIFT V shortcut.

So it did work? Just clarifying because I did not install it yet (due to this thread) but checked through the CK5 docs first.

kevinquillen → created an issue. See original summary → .

Still getting this in 2.1.x dev:

TypeError: Cannot assign null to property CommerceGuys\Addressing\Validator\Constraints\AddressFormatConstraint::$validatePostalCode of type bool in Symfony\Component\Validator\Constraint->__construct() (line 120 of /var/www/html/vendor/symfony/validator/Constraint.php).
CommerceGuys\Addressing\Validator\Constraints\AddressFormatConstraint->__construct() (Line: 31)
Drupal\Core\Validation\ConstraintFactory->createInstance() (Line: 83)
Drupal\Component\Plugin\PluginManagerBase->createInstance() (Line: 86)
Drupal\Core\Validation\ConstraintManager->create() (Line: 394)

Would it be possible to get this fix out into a stable release?

Interesting, do you have an error trace beyond that? I only see one instance of max_tokens mentioned in the provider code:

https://git.drupalcode.org/project/ai_provider_openai/-/blob/1.0.x/src/P...

#24 was this of any help? 🐛 Shield middleware invokes hooks before modules are loaded, corrupting module_implements cache Needs work https://www.drupal.org/project/shield/issues/3277210#comment-15560180 🐛 Shield middleware invokes hooks before modules are loaded, corrupting module_implements cache Needs work

For Acquia Platform Email, reply-to header is required or you will get this error (Amazon SES service). The patch made this work, although I do not understand why you have to uncheck 'Disable site email notification' to get reply-to to be set. Can't it just be set by default to the site mail that is sending the message out and not include the site mail as a (main) recipient?

       $recipient = '';
      $reply_to = NULL;
       if (!$notification->disableSiteMail()) {
         $recipient = \Drupal::config('system.site')->get('mail');
        $reply_to = $recipient;
    }

Why not simply $reply_to = \Drupal::config('system.site')->get('mail');?

The CKEditor docs say:

With the plain text pasting feature, text pasted using the Ctrl/Cmd + Shift + V keystroke will match the formatting of the content you paste it into.

Are folks using CTRL SHIFT V, or just CTRL V (CMD V)?

When I try the demo page here:

https://ckeditor.com/docs/ckeditor5/latest/features/pasting/paste-plain-...

CTRL V pastes as-is from Clipboard, but CTRL SHIFT V pastes as plain text.

MR opened.

kevinquillen → created an issue.

I reconfigured the test to not rely on a custom module that also required Domain (for the domain.negotiator service in one of its classes) and this issue went away. It must be something in Domain module and similar modules that rely on core/lib/Drupal/Core/Path/PathMatcher::matchPath.

I was able to trigger this in a simple kernel test where I want to check that some custom entity routes are marked as admin.

/**
 * Tests the EntityAdminHtmlRouteProvider.
 */
class EntityAdminHtmlRouteProviderTest extends KernelTestBase {

  /**
   * {@inheritdoc}
   */
  protected static $modules = [
    'cci_api',
    'cci_card',
    'domain',
    'node',
    'text',
    'user',
    'system',
  ];

  /**
   * The entity type manager service.
   *
   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
   */
  protected EntityTypeManagerInterface $entityTypeManager;

  /**
   * @var \Drupal\Core\Routing\AccessAwareRouter
   */
  protected AccessAwareRouter $router;

  /**
   * @var \Drupal\Core\ProxyClass\Routing\RouteBuilder
   */
  protected RouteBuilder $routerBuilder;

  /**
   * {@inheritdoc}
   */
  protected function setUp(): void {
    parent::setUp();
    $this->installEntitySchema('mymodule');
    $this->installSchema('system', ['sequences']);
    $this->entityTypeManager = $this->container->get('entity_type.manager');
    $this->router = $this->container->get('router');
    $this->routerBuilder = $this->container->get('router.builder');
    $this->routerBuilder->rebuild();
  }

  /**
   * Tests if the canonical route is correctly configured.
   */
  public function testCanonicalRoute(): void {
    $collection = $this->router->getRouteCollection();
    $route = $collection->get('entity.mymodule_entity.canonical');
    $this->assertTrue($route->getOption('_admin_route'), 'The route is marked as an admin route.');
  }

}

The test passes, but is marked with a deprecation. Not sure how to get around it.

The proposed change seems to have no impact on Chrome 135 (Linux) in Drupal 11.1 for node forms. On my custom entity forms, the behavior in Chrome is not present (multiple clicks does not result in multiple entities created) using Gin for the admin theme. I tried Claro too, same result.

Interestingly enough, it does not happen at all in Firefox or Waterfox. It does happen in Brave.

kevinquillen → created an issue.

Yes... this would be great for seeing the relationships and all entity fields.

Like others, just chiming in to say I ran into this exact issue in 11.1. In my case, I have a half dozen custom content entities that I recently added content moderation for (states of data, don't think of them like Nodes).

In any event, I made them using Drush 13. One of the things Drush generates is an entity form with boilerplate like this:

final class ResultForm extends ContentEntityForm {

  /**
   * {@inheritdoc}
   */
  public function save(array $form, FormStateInterface $form_state): int {
    $result = parent::save($form, $form_state);

    $message_args = ['%label' => $this->entity->toLink()->toString()];
    $logger_args = [
      '%label' => $this->entity->label(),
      'link' => $this->entity->toLink($this->t('View'))->toString(),
    ];

    switch ($result) {
      case SAVED_NEW:
        $this->messenger()->addStatus($this->t('New result %label has been created.', $message_args));
        $this->logger('cci_result')->notice('New result %label has been created.', $logger_args);
        break;

      case SAVED_UPDATED:
        $this->messenger()->addStatus($this->t('The result %label has been updated.', $message_args));
        $this->logger('cci_result')->notice('The result %label has been updated.', $logger_args);
        break;

      default:
        throw new \LogicException('Could not save the entity.');
    }

    $form_state->setRedirectUrl($this->entity->toUrl('collection'));

    return $result;
  }

}

Upon adding content moderation and workflow against them, they started failing when testing "Published" -> "Draft" state. Its because parent::save returns false (which core modules consider okay) and that causes the exception to be thrown here.

Updating my save method to this, mimicing core Media/Node, made it work:

  /**
   * {@inheritdoc}
   */
  public function save(array $form, FormStateInterface $form_state) {
    $result = parent::save($form, $form_state);

    $message_args = ['%label' => $this->entity->toLink()->toString()];
    $logger_args = [
      '%label' => $this->entity->label(),
      'link' => $this->entity->toLink($this->t('View'))->toString(),
    ];

    switch ($result) {
      case SAVED_NEW:
        $this->messenger()->addStatus($this->t('New result %label has been created.', $message_args));
        $this->logger('cci_messages')->notice('New result %label has been created.', $logger_args);
        break;

      default:
        $this->messenger()->addStatus($this->t('The result %label has been updated.', $message_args));
        $this->logger('cci_messages')->notice('The result %label has been updated.', $logger_args);
        break;
    }

    $form_state->setRedirectUrl($this->entity->toUrl());

    return $result;
  }

It was disorienting because the false value returned made me think the save failed - but it doesn't fail. Its just not the default revision. I think this should be addressed as soon as possible, it seems like the code comments are incorrect as mentioned.

And yes, core and specifically entity forms are doing a very bad job at that.

Given that Drupal can make an excellent API platform for controlling and managing data beyond the "Node" type, there must be something that can be done here to improve clarity. The diff looks pretty simple - does it really matter if the saved entity was the default revision or not?

kevinquillen → created an issue.

I thought I could get away with this by creating a link template for my custom entity and specifying that in the matcher. While that was working, I wanted the link element to also have additional attributes added automatically (because this specific entity match should open in a modal window) - i.e. let JS take over. Without requiring any additional action from the editor. I could not figure out how to modify the element before it was added into CKEditor to have that.

I guess my only option because I am crunched for time is to add in https://www.drupal.org/project/editor_advanced_link → and add a custom checkbox or two and work the rest from JS.

kevinquillen → created an issue.

Test failure seems related to parent Domain module. But the change resolved the fatal error we were seeing on save.

kevinquillen → created an issue.

Just came across this issue when trying to figure out how to resolve this for frontend developers on a project where invalid libraries syntax was not caught.

I located a schema here: https://json.schemastore.org/drupal-libraries.json

After hooking it to my IDE, it started noting incorrect syntax or keys in library files.

It would be nice if Drupal could also catch this and gracefully degrade (in our case, a missing 'css' key threw an exception on the site).

kevinquillen → created an issue.

According to the README, UUID should not be copied into any domain.config* file. This seems like it could be a simple addition to the Domain module to prevent that from happening with:

declare(strict_types=1);

namespace Drupal\mymodule\EventSubscriber;

use Drupal\Core\Config\ConfigEvents;
use Drupal\Core\Config\StorageInterface;
use Drupal\Core\Config\StorageTransformEvent;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
 * Listen for configuration events and remove UUIDs from domain.config entities.
 */
final class ConfigExportSubscriber implements EventSubscriberInterface {

  /**
   * Handle the storage transform export event.
   *
   * @param \Drupal\Core\Config\StorageTransformEvent $event
   *   The event.
   */
  public function onStorageTransformExport(StorageTransformEvent $event): void {
    $this->removeDomainConfigUuid($event->getStorage());
  }

  /**
   * Handle the storage transform import event.
   *
   * @param \Drupal\Core\Config\StorageTransformEvent $event
   *   The event.
   */
  public function onStorageTransformImport(StorageTransformEvent $event): void {
    $this->removeDomainConfigUuid($event->getStorage());
  }

  /**
   * Remove UUIDs from any domain.config configuration entity.
   *
   * @param \Drupal\Core\Config\StorageInterface $storage
   *   The storage interface.
   */
  protected function removeDomainConfigUuid(StorageInterface $storage): void {
    $domain_configs = $storage->listAll('domain.config');

    foreach ($domain_configs as $domain_config) {
      $config = $storage->read($domain_config);

      if (isset($config['uuid'])) {
        unset($config['uuid']);
      }

      $storage->write($domain_config, $config);
    }
  }

  /**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents(): array {
    return [
      ConfigEvents::STORAGE_TRANSFORM_IMPORT => ['onStorageTransformImport', -99],
      ConfigEvents::STORAGE_TRANSFORM_EXPORT => ['onStorageTransformExport', -99],
    ];
  }

}

is there any reason why this does not exist, or why I should not do this? The domain_config_test module includes several YAML files that also have no UUID with them.

#14 so you are saying you have to delete the UUID manually every time from a domain.config.* file? The issue you linked to goes to a 404.

The proposed change would add filter-ability similar to what Node and Users get in Views along with "Current Domain" options. This makes it easier OOTB for those who have or implement custom entities to work with Domain.

kevinquillen → created an issue.

Did you try the dev branch?

What was the module?

kevinquillen → created an issue.

greggles → credited kevinquillen → .

I tried looking at other examples of outbound processors in core and contrib, but I could not locate any that themselves called Url::fromUserInput. The only route I can think of is to at least try/catch here so it prevents the OP from occurring and offers some way of recovering and fixing the data.

This was really hard to replicate - somehow my issue (and assume OP issue) was that $path was passed as NULL (or ''). Which should not happen, and the how it happened... not quite sure. But a try/catch won't harm anything here.

I ran into this with a misconfigured Site Studio component and a Link field that was returning "" because it could not look up the user input value, it did not resolve to any entity in the database.

The problem was here:

  /**
   * {@inheritdoc}
   */
  public function processOutbound($path, &$options = [], Request $request = NULL, BubbleableMetadata $bubbleable_metadata = NULL) {
    // Load the active domain if not set.
    $active_domain = $options['active_domain'] ?? $this->getActiveDomain();

    // Only act on valid internal paths and when a domain loads.
    $external = $options['external'] ?? FALSE;
    if (is_null($active_domain) || $path === '' || $external === TRUE) {
      return $path;
    }

    // Set the default source information.
    $source = NULL;
    $options['active_domain'] = $active_domain;
    $entity = NULL;

    // Get the current language.
    $langcode = NULL;
    if (isset($options['language'])) {
      $langcode = $options['language']->getId();
    }

    // Get the URL object for this request.
    $alias = $this->aliasManager->getPathByAlias($path, $langcode);
    $url = Url::fromUserInput($alias, $options);

I think at a minimum the lines:

    // Get the URL object for this request.
    $alias = $this->aliasManager->getPathByAlias($path, $langcode);
    $url = Url::fromUserInput($alias, $options);

should be wrapped in a try catch. Log the path, request, options, alias and return $path. That gives someone a chance to trace back where the input came from, potentially, and lets pages load.

I could not access the Content admin, URL Alias page, URL Redirect page, or the Admin Content view or areas like that until I resolved this.

To get back up and going to debug the problem for your specific local database, you can do:

if ($path == '') {
  return $path;
}

    // Get the URL object for this request.
    $alias = $this->aliasManager->getPathByAlias($path, $langcode);
    $url = Url::fromUserInput($alias, $options);

in DomainSourcePathProcessor.php. That will prevent the pages from crashing so you can trace back the error. In my case, I snagged a database, fixed the content issue, and pushed the database back under maintenance to get the site back online.

Updated patch, removes newline.

Without the change(s) the module version is not usable in Drupal 11.

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

kevinquillen → created an issue.

kevinquillen → created an issue.

kevinquillen → created an issue.

kevinquillen → created an issue.

Merged to 2.0.x.

kevinquillen → created an issue.

kevinquillen → created an issue.

kevinquillen → created an issue.

kevinquillen → created an issue.

Looks good to me.

kevinquillen → created an issue.

kevinquillen → created an issue.

kevinquillen → created an issue.

kevinquillen → created an issue.