Thank you very much.
Currently blocked by Issue #3531714 - Drupal 11.2.0 incompatibility (sebastian/diff incompatiblity due to config_patch).
tcrawford → created an issue.
tcrawford → created an issue.
I have updated the patch file for the alternative approach to handling system files.
It turns out that it is better to process system files with the base scheme instead of the internal scheme. Otherwise URLs will be ouput as /system/files?file=...... I will fix this and push to the MR shortly and update the patch.
Added a test for the fix for system files.
Here is the patch to the current latest commit on MR 12:
pathologic-language-prefix-2418369-84.diff →
I tested this and it worked for me in the case of linking from a German node with an anchor to a node in another language (e.g. French link →
However, for the case of system files (e.g. some.pdf the patch results in the URL being mangled. I debugged this and found the cause.
System private files are now processed with the internal: scheme, whereas without the patch they are processed with the base: scheme. This (internal:) seems semantically correct, because they are routed. However, this has a nasty side-effect.
The _pathologic_replace callback passes options to fromUri(), with options that contain an empty query param.
$url = Url::fromUri($path, $url_params['options'])->toString();
fromUri() then calls from static::fromInternalUri(). Here the options with an empty query param array are passed to the URL.
This overrides the file query param that is added by the PathProcessorFiles (inbound path processor), resulting in the files query param being lost and the URL being rendered as
$url->setOptions($options + $url->getOptions());
We likely need to treat system files as base: (does not seem semantically correct, but works) or unset the the query key from the options before calling Url:fromUri. I tested both approaches and they work. I will push this change to the MR and try to add a test for this case.
This looks to be a duplicate, so I am closing this.
tcrawford → created an issue.
I suggest to work only on the MR (and provide patches on this, as kensae has done) rather than also providing new patches. It is difficult to compare #44 against #42 without creating an interdiff.
I installed patch #42 (based on MR!8) and I see type errors in the console. A colleague will test the functionality further, and we will provide feedback when available.
gtag.js?svsmmn:22 Uncaught TypeError: Cannot read properties of null (reading 'length')
at gtag.js?svsmmn:22:20
I am working on a project where this issue has suddenly surfaced after a Drupal 11 update. I am also not sure why we have also redirect_fallback_action of NULL on most node types. To resolve, I have just opened the node type config and resaved.
Thanks for the feedback.
In best case, a redirect to the login page of another Drupal domain will result in the destination parameter being lost.
The domain access (and not just the domain source) module might be required to first trigger the 403.
That said, we also have other custom access and tac lite (
https://www.drupal.org/project/tac_lite →
) installed on the project, and I am not 100% sure they are also not required to reproduce (although looking at the stack trace suggests not).
I would have to test the scenario on a clean install, but have not yet had the possibility to do that.
tcrawford → created an issue.
We have the same symptom, but a slightly different cause and therefore MR87 does not work for us.
In our case, we have a custom form mode and $form['account']['pass'] is not set at all. Therefore, the check falsely identifies the password as being mutable.
$password_mutable = $form['account']['pass']['#access'] ?? TRUE;
I would suggest the check could be changed to the following, where we check if the pass field is even present and if so check the access to that. I would also reverse the order of the and as it is less expensive that way.
$password_mutable = (isset($form['account']['pass']) && ($form['account']['pass']['#access'] ?? TRUE));
// Load form if relevant.
if ($password_mutable && \Drupal::service('password_policy.validation_manager')->tableShouldBeVisible()) {
The check also needs to be added to check if validation should run (otherwise we just don't show the table rows, but validation will still catch us). That possibly should be though in the PasswordPolicyValidationManager.
if ($password_mutable && \Drupal::service('password_policy.validation_manager')->validationShouldRun()) {
$form['#validate'][] = '_password_policy_user_profile_form_validate';
$form['#after_build'][] = '_password_policy_user_profile_form_after_build';
}
I have created an MR. We are testing this (by using the issue fork as a composer repository, as drupal lenient can't be used due to the composer dependency). Therefore, I am moving this on to 'needs review'.
"commerce_payrexx_integration": {
"type": "package",
"package": {
"name": "drupal/commerce_payrexx_integration",
"version": "2.0.1",
"type": "drupal-module",
"source": {
"url": "https://git.drupalcode.org/issue/commerce_payrexx_integration-3513451.git",
"type": "git",
"reference": "685b6c29d9946151be20e16355e674b3de2cde70"
}
}
},
tcrawford → created an issue.
We had the same issue on a project. The above hint was very helpful. Thank you.
I wonder if the menu item recursion should not be checked during a cache clear to prevent such configurations getting into production.
I am re-assigning this as there are some minor issues with the view.
I have added the view for commerce product variations, adapted the derivative plugin and updated the view access tests. I am therefore assigning this one to 'needs review'. Thanks in advance.
@Jonathan1055: Sorry. I missed your prior message. I now have tests passing now. I just need to add the view and integrate this and consider whether to do the rules integration.
Thanks for the feedback on where to include the plugin.
But is there any benefit in putting it within a submodule of Scheduler which only gets enabled when required?
I would tend to suggest to leave it in the main module as it is consistent with the commerce product plugin.
Is this why you added 'language', 'locale' and 'config' to SchedulerBrowserTestBase here ?
Yes. I have removed these though as they were not required.
Hi @jonathan1055. Thanks for the feedback. I had not, but I just read the documentation.
If you would prefer the plugin for product variations to be stored in a third party module or is it a commonly used enough entity to be included here (perhaps due to its relation to the commerce product entity that it included)?
Unfortunately, I had some issues running these tests locally (the runner is complaining about missing dependencies that don't seem to be an issue on the server). I reached out to pfrenssen, who is a colleague, for some assistance and so if I can get the tests running locally I will save some resources on the server and be a bit faster.
Great idea, I will comment out data types (now that the prior tests are passing) to get feedback on the new entity type only.
tcrawford → made their first commit to this issue’s fork.
The test pipeline failures appear to predate this change.
The automated patch that was merged in MR 3 unfortunately missed the composer dependency. I am fixing this now.
I created MR 16 (against 2.x) that also contains the changes to the info file. This needs a review.
This issue was created against 2.x and MR 15 does not apply against 2.x (only 1.x). MR 12 does not adapt the core version requirement. Therefore, I am placing this issue back to needs work. I suggest to create a separate issue for Drupal 11 compatibility for 1.x.
tcrawford → created an issue.
Thanks Milan. Much appreciated. I confirm the patch worked. We probably need to consider a sustainable fix here, such as making this optional.
I bumped in to the issue.
In our case we have a form display mode that does not have the name field present, which is a valid use case.
We have the MR from
https://www.drupal.org/project/password_policy/issues/2451159
✨
Password policy doesn't work when updating the user
Needs work
installed as a patch, but the issue can be reproduced without this and the patch resolves the issue regardless.
The name is actually already persisted on the entity.
Error:
```
Warning: Undefined array key "account" in /app/docroot/modules/contrib/password_policy/password_policy.module on line 243
Warning: Trying to access array offset on value of type null in /app/docroot/modules/contrib/password_policy/password_policy.module on line 243
Warning: Trying to access array offset on value of type null in /app/docroot/modules/contrib/password_policy/password_policy.module on line 243
Warning: Undefined array key "#default_value" in /app/docroot/modules/contrib/password_policy/password_policy.module on line 243
The website encountered an unexpected error. Try again later.
Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'name' cannot be null: INSERT INTO "users_field_data" ("uid", "langcode", "preferred_langcode", "preferred_admin_langcode", "name", "pass", "mail", "timezone", "status", "created", "changed", "access", "login", "init", "default_langcode") VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10, :db_insert_placeholder_11, :db_insert_placeholder_12, :db_insert_placeholder_13, :db_insert_placeholder_14); Array ( [:db_insert_placeholder_0] => 52200 [:db_insert_placeholder_1] => de [:db_insert_placeholder_2] => de [:db_insert_placeholder_3] => fr [:db_insert_placeholder_4] => [:db_insert_placeholder_5] => $2y$10$O0rUbJ7ueExLy7vEmBfVp.oOv5Vzl1xnQM52SOyp8rsy2JK5RcD0W [:db_insert_placeholder_6] => sanitized@example.com [:db_insert_placeholder_7] => Europe/Zurich [:db_insert_placeholder_8] => 1 [:db_insert_placeholder_9] => 1737379851 [:db_insert_placeholder_10] => 1737473718 [:db_insert_placeholder_11] => 1737380061 [:db_insert_placeholder_12] => 1737380061 [:db_insert_placeholder_13] => sanitized@example.com [:db_insert_placeholder_14] => 1 ) in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 817 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).
Drupal\Core\Database\StatementWrapperIterator->execute(Array, Array) (Line: 44)
Drupal\mysql\Driver\Database\mysql\Insert->execute() (Line: 1022)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->saveToSharedTables(Object) (Line: 935)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->doSaveFieldItems(Object, Array) (Line: 33)
Drupal\user\UserStorage->doSaveFieldItems(Object) (Line: 718)
Drupal\Core\Entity\ContentEntityStorageBase->doSave('52200', Object) (Line: 486)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 806)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object) (Line: 354)
Drupal\Core\Entity\EntityBase->save() (Line: 293)
Drupal\Core\Entity\EntityForm->save(Array, Object)
call_user_func_array(Array, Array) (Line: 129)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 67)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 597)
Drupal\Core\Form\FormBuilder->processForm('user_pharmacies_form', Array, Object) (Line: 326)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 68)
Drupal\simple_oauth\HttpMiddleware\BasicAuthSwap->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 53)
Asm89\Stack\Cors->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 742)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
```
Cause:
Function _password_policy_user_profile_form_validate(&$form, FormStateInterface $form_state) sets the name to an empty value if not found in the form.
Proposed solution:
As above. I am moving this back to needs review. Please consider again the prior proposed solution.
https://www.drupal.org/files/issues/2023-01-12/3332884-integrity-constra... →
// set username if form_state name value is empty
if (empty($form_state->getValue('name'))) {
$user->setUsername($form_state->getValue('name', $user->name->value));
}
I have created a separate module with an event subscriber to set the checkout language on the order. The rational for a separate module is that it can be activated or de-activated, thereby avoiding the need for configuration. I use both the add to cart and checkout completion events (see the prior comment). I can create the module if you agree, otherwise, I can update the MR for core (with tests) if agreed.
<?php
declare(strict_types=1);
namespace Drupal\commerce_checkout_language\EventSubscriber;
use Drupal\commerce_cart\Event\CartEntityAddEvent;
use Drupal\commerce_cart\Event\CartEvents;
use Drupal\commerce_checkout\Event\CheckoutEvents;
use Drupal\commerce_order\Entity\OrderInterface;
use Drupal\commerce_order\Event\OrderEvent;
use Drupal\Core\Language\LanguageManagerInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
/**
* Sets the checkout language on the order.
*/
final class CommerceOrderLanguageSubscriber implements EventSubscriberInterface {
/**
* Constructs a CommerceCheckoutLanguageSubscriber object.
*/
public function __construct(
private readonly LanguageManagerInterface $languageManager,
) {}
/**
* {@inheritdoc}
*/
public static function getSubscribedEvents(): array {
return [
CheckoutEvents::COMPLETION => ['onCheckoutCompletion'],
CartEvents::CART_ENTITY_ADD => ['onCartEntityAdd'],
];
}
/**
* Respond to the checkout completion event.
*
* @param \Drupal\commerce_order\Event\OrderEvent $event
* The order event.
*/
public function onCheckoutCompletion(OrderEvent $event): void {
// The checkout completion event requires the user to return to the checkout
// after any offsite gateway.
$this->setOrderLanguage($event->getOrder());
}
/**
* Respond to the cart entity add event.
*
* @param \Drupal\commerce_cart\Event\CartEntityAddEvent $event
* The cart entity add event.
*/
public function onCartEntityAdd(CartEntityAddEvent $event): void {
// The cart entity add event is fired when a purchasable entity is added to
// the cart. This is used as a default, which may be overridden by the
// checkout completion event. We do this to ensure that the order language
// is set even if the order is completed in the IPN (and the user does not
// return to the checkout).
$this->setOrderLanguage($event->getCart());
}
/**
* Sets the order language to the current language.
*
* @param \Drupal\commerce_order\Entity\OrderInterface $order
* The order.
*/
protected function setOrderLanguage(OrderInterface $order): void {
$order->setData('checkout_language', $this->languageManager->getCurrentLanguage()->getId());
}
}
I will also open an issue on the Drupal Symfony Mailer module (which includes the CommerceOrderEmailBuilder), which will include the following patch:
From 6cd4e1fbf60ad99d6673996b50fd0a5566fa5cb0 Mon Sep 17 00:00:00 2001
From: Trent Crawford <trent.crawford@liip.ch>
Date: Mon, 6 Jan 2025 14:47:35 +0100
Subject: [PATCH] Set the language based on the checkout language.
---
.../CommerceOrderEmailBuilder.php | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/src/Plugin/EmailBuilder/CommerceOrderEmailBuilder.php b/src/Plugin/EmailBuilder/CommerceOrderEmailBuilder.php
index 601cc8b..656833b 100644
--- a/src/Plugin/EmailBuilder/CommerceOrderEmailBuilder.php
+++ b/src/Plugin/EmailBuilder/CommerceOrderEmailBuilder.php
@@ -5,6 +5,7 @@ namespace Drupal\symfony_mailer\Plugin\EmailBuilder;
use Drupal\commerce_order\Entity\OrderInterface;
use Drupal\commerce_order\Entity\OrderType;
use Drupal\symfony_mailer\Address;
+use Drupal\symfony_mailer\AddressInterface;
use Drupal\symfony_mailer\EmailFactoryInterface;
use Drupal\symfony_mailer\EmailInterface;
use Drupal\symfony_mailer\Entity\MailerPolicy;
@@ -92,6 +93,27 @@ class CommerceOrderEmailBuilder extends EmailBuilderBase {
->setVariable('order_number', $order->getOrderNumber())
->setVariable('store', $store->getName());
+
+ // This is a workaround to set the email language based on the
+ // order checkout language.
+ if ($checkout_language = $order->getData('checkout_language')) {
+ // Calling setTo() above is still required to ensure we get the
+ // list of addresses.
+ $to_addresses = $email->getTo();
+ // In this case we likely only have one to address.
+ foreach ($to_addresses as $index => $to_address) {
+ if ($to_address instanceof AddressInterface) {
+ $to_addresses[$index] = new Address(
+ $to_address->getEmail(),
+ $to_address->getDisplayName(),
+ $checkout_language,
+ $to_address->getAccount()
+ );
+ }
+ }
+ $email->setTo($to_addresses);
+ }
+
// Get the actual email value without picking up the default from the site
// mail. Instead we prefer to default from Mailer policy.
if ($store_email = $store->get('mail')->value) {
--
2.47.1
I will open an issue on commerce core with an MR:
From 344ef06b869b9535b2b558df380deb1146a0a5c7 Mon Sep 17 00:00:00 2001
From: Trent Crawford <trent.crawford@liip.ch>
Date: Mon, 6 Jan 2025 16:29:52 +0100
Subject: [PATCH] Send the email in the checkout language if available.
---
modules/order/src/Mail/OrderReceiptMail.php | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/modules/order/src/Mail/OrderReceiptMail.php b/modules/order/src/Mail/OrderReceiptMail.php
index 67affa1e..ff0db9c7 100644
--- a/modules/order/src/Mail/OrderReceiptMail.php
+++ b/modules/order/src/Mail/OrderReceiptMail.php
@@ -43,6 +43,8 @@ class OrderReceiptMail implements OrderReceiptMailInterface {
$order_type = $order_type_storage->load($order->bundle());
$subject = $order_type->getReceiptSubject();
if (!empty($subject)) {
+ // @todo Support token replacement in the checkout language.
+ // @see Drupal\commerce\MailHandler::changeActiveLanguage().
$subject = $this->token->replace($subject, [
'commerce_order' => $order,
]);
@@ -73,6 +75,11 @@ class OrderReceiptMail implements OrderReceiptMailInterface {
$params['langcode'] = $customer->getPreferredLangcode();
}
+ // Use the checkout language if available.
+ if ($checkout_language = $order->getData('checkout_language')) {
+ $params['langcode'] = $checkout_language;
+ }
+
return $this->mailHandler->sendMail($to, $subject, $body, $params);
}
--
2.47.1
I am welcome to any feedback on the approach.
The commerce email module will potentially also need an update for this.
I am not sure the checkout completion event is a reliable point in time to set the language, as order completion can occur in the IPN/webhoook and the user may not return to the site.
Order completion is also not reliable as the completion may occur in the webhook, where the language context is not available (as not all payment gateways provide a way to set the callback URL per order).
tcrawford → created an issue.
tcrawford → created an issue.
I am also seeing the issue periodically. Looking in the logs, I am currently not seeing "PluginNotFoundException" (although we have in the past in connection with this issue). What I am seeing is the out of memory issue. Therefore, I am linking this to the existing issue #3303674. I am not 100% positive they are related, but it provides context.
I seem to be having the same symptom. I am using symfony mailer lite. I suspect this is related. I will update if I find the cause.
tcrawford → created an issue.
The issue was caused by another module 'responsive_preview'.
Disabling this module resulted in the module working as expected.
Closing as 'cannot reproduce'.
tcrawford → created an issue.
tcrawford → created an issue.
tcrawford → created an issue.
tcrawford → created an issue.
This same issue occurs at other places. I will add some commits to the above MR.
I am just following up again on this issue, are you willing to work with me so I can help improve the module?
I am looking at a cleaner way to implement this, so re-assigning the issue to me.
I have added test coverage. Thanks in advance for a code review and community testing.
You can also user massageFormvalues() to set empty strings to null, which is the approach I took in a custom widget.
tcrawford → created an issue.
@mondrake You are right. The issue only occurs for me in projects using the drupal/rokka module, which uses its own RokkaToolkit that extends the GDToolkit. Rokka supports the svg extension, but there is no related type and so we see the type error.
/**
* Get the mime type.
*
* @return string
* Mime type.
*/
public function getMimeType(): string {
$mime_type = $this->getType() ? image_type_to_mime_type($this->getType()) : '';
// GD library does not support SVG, but Rokka does. So we will trick the
// toolkit and detect SVG as a mime time the first time the file is uploaded
// to the tmp folder. If we detect an SVG image, we allow the upload.
if (empty($mime_type)) {
$uri = $this->getSource();
if ($uri && file_exists($uri) && strpos($uri, 'rokka://') === FALSE) {
$rokka_additional_allowed_mime_types = ['image/svg', 'image/svg+xml'];
$mime_type_guessed = mime_content_type($uri);
if (in_array($mime_type_guessed, $rokka_additional_allowed_mime_types)) {
$mime_type = $mime_type_guessed;
}
}
}
return $mime_type;
}
I guess we need to get have the Rokka Toolkit modified to override the load() method also and check that getType() is set.
I am closing this issue as 'cannot reproduce' as it only occurs in combination with the RokkaToolkit.
tcrawford → created an issue.
tcrawford → created an issue.
tcrawford → created an issue.
@jon.oltman. I have added some test coverage now, so assigning back to you. Thank you.
Thanks John. I am not sure we need the test to use the CRON. The CRON just runs a services to queue expired carts for deletion and the queue worker just uses the storage to delete them, so we could test by just deleting the carts programmatically. We can update the "steps to reproduce" to include "programmatically delete a cart" if you would like. I can write a test for that as time permits.