🇨🇭Switzerland @tcrawford

tcrawford → created an issue.

Reviewed and the change looks safe. Thanks.

The issue was fixed here: Issue #3137498 by tBKoT: Unable to unpublish a product variation.

I think this issue is stale and can be closed as the product variation translation annotation now points to Drupal\commerce_product\ProductTranslationHandler instead of the original content translation handler.

tcrawford → created an issue.

Please note that the patch should be applied upstream to the code base, otherwise every customer who installs the module will encounter this error and need to search the queue and then apply the patch to their project. Please advise if you intend to do this. If you need a merge request for this, please let me know and I am happy to open one. I am reopening this issue as it does not appear that the fix has been applied to the code base.

I have rerolled #60 as #66 (against 10.2.x) and this applies for me properly. I hope that helps if someone else was having an issue with #60. I am sorry that I have not yet been able to otherwise move this issue forward.
→

The above issue with the TypeError is coming as the second diff in patch #60 did not apply properly on one of our projects (and weirdly failed silently both locally and in the pipeline). Applying the second diff manually works. There appears to be an issue with patch #60 in that case and it would be good if someone else can verify. I have never seen a patch partially apply and fail silently and therefore suspect there is an issue on our side.

diff --git a/core/modules/system/src/Controller/EntityAutocompleteController.php b/core/modules/system/src/Controller/EntityAutocompleteController.php
index a49da37340..fd597a3947 100644
--- a/core/modules/system/src/Controller/EntityAutocompleteController.php
+++ b/core/modules/system/src/Controller/EntityAutocompleteController.php
@@ -77,6 +77,7 @@ public static function create(ContainerInterface $container) {
    */
   public function handleAutocomplete(Request $request, $target_type, $selection_handler, $selection_settings_key) {
     $matches = [];
+    $target_types = explode(',', $target_type);

     // Get the typed string from the URL, if it exists.
     if ($input = $request->query->get('q')) {
       $tag_list = Tags::explode($input);
@@ -109,8 +110,12 @@ public function handleAutocomplete(Request $request, $target_type, $selection_ha
           }
         }
       }
-
-      $matches = $this->matcher->getMatches($target_type, $selection_handler, $selection_settings, $typed_string);
+      foreach ($target_types as $t_type) {
+        $target_matches = $this->matcher->getMatches($t_type, $selection_handler, $selection_settings, $typed_string);
+        if ($target_matches) {
+          $matches = array_merge($matches, $target_matches);
+        }
+      }
     }

     return new JsonResponse($matches);

I am seeing a type error in SelectionPluginManager as getSelectionGroups does not return an entry for the concatenated entity entity type ID of 'node,taxonomy_term'.

The website encountered an unexpected error. Try again later.

TypeError: uasort(): Argument #1 ($array) must be of type array, null given in uasort() (line 65 of core/lib/Drupal/Core/Entity/EntityReferenceSelection/SelectionPluginManager.php).

Drupal\Core\Entity\EntityReferenceSelection\SelectionPluginManager->getPluginId('node,taxonomy_term', 'default') (Line: 41)
Drupal\Core\Entity\EntityReferenceSelection\SelectionPluginManager->getInstance(Array) (Line: 41)
Drupal\Core\Entity\EntityAutocompleteMatcher->getMatches('node,taxonomy_term', 'default', Array, 're') (Line: 118)
Drupal\system\Controller\EntityAutocompleteController->handleAutocomplete(Object, 'node,taxonomy_term', 'default', '3kwQYPisvvlkGtu1zMaBQotUMioVXxI_qWM7KjOEK2w') (Line: 25)
Drupal\realname\Controller\RealnameAutocompleteController->handleAutocomplete(Object, 'node,taxonomy_term', 'default', '3kwQYPisvvlkGtu1zMaBQotUMioVXxI_qWM7KjOEK2w')
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 627)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
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: 58)
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: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

This is on Drupal 10.2.5 (PHP 8.1) with patch #60 applied.

Thank you for the review @czigor. I will work through your feedback as time permits. I am leaving the issue unassigned until I have capacity.

After fixing the issues I found, I have fixed the failing Kernel test and added some test coverage of the constraint validator. Therefore, I am moving this issue to 'Needs review'. Thanks in advance for a review.

Now that we correctly check whether the password has changed, the validation is not triggering when there is an existing password present. It is not clear why we should only trigger validation if the existing password is not correct valid.

    // password should not be changed, ignore empty password fields.
    if ($userUnchanged && !empty($pass)) {
      // Compare the values of the field this is being validated on.
      $triggerValidation = $pass != $userUnchanged->get('pass')->getValue() && !$user->checkExistingPassword($userUnchanged);
      $triggerValidation = $pass !== $userUnchanged->get('pass')->value && !$user->checkExistingPassword($userUnchanged);
    }

There is also an issue whereby the PasswordPolicyConstraintValidator does not trigger validation properly (where an existing password was not set) as getValue() returns an array.

    // Special case for the password, it being empty means that the existing
    // password should not be changed, ignore empty password fields.
    if ($userUnchanged && !empty($pass)) {
      // Compare the values of the field this is being validated on.
      $triggerValidation = $pass !== $userUnchanged->get('pass')->getValue() && !$user->checkExistingPassword($userUnchanged);
    }

Pushing a fix for this shortly.

I have pushed a fix for the above type error to MR !78, but I am leaving the issue in the status of 'needs work' as test coverage is needed.

I am also getting a type error when registering a new user as the password is null when registering a new user (until it is set subsequently upon email confirmation).

```

The website encountered an unexpected error. Try again later.

TypeError: Drupal\password_policy\PasswordPolicyValidator::validatePasswordPolicyConstraints(): Argument #1 ($password) must be of type string, null given, called in /app/docroot/modules/contrib/password_policy/src/Plugin/Validation/Constraint/PasswordPolicyConstraintValidator.php on line 88 in Drupal\password_policy\PasswordPolicyValidator->validatePasswordPolicyConstraints() (line 127 of modules/contrib/password_policy/src/PasswordPolicyValidator.php).

Drupal\password_policy\Plugin\Validation\Constraint\PasswordPolicyConstraintValidator->validate(Object, Object) (Line: 202)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateConstraints(Object, '00000000000014ef0000000000000000', Array) (Line: 154)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateNode(Object) (Line: 164)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateNode(Object, Array, 1) (Line: 106)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validate(Object, NULL, NULL) (Line: 93)
...

The PasswordPolicyConstraintValidator coming from patch #30 would appear to have an off by one error as it only adds violations if there are more than 1 rather than more than 0. I have just pushed a commit to fix this. Test coverage must need to be expanded though as this was not captured by testing. Therefore, I am setting this issue back to 'needs work'.

The tests are passing based on the prior patch (#30) now that I have applied it properly against 4.0.x. cspell/eslint/phpcs/phpstan are failing. However, if I am not mistaken, these appear mostly to be pre-existing code style issues and should therefore likely be fixed in another issue (e.g. Fix the issues reported by phpcs 📌 Fix the issues reported by phpcs Needs review ).

I am re-opening this as it is IMHO still relevant for 4.0.x. I will create an MR (based on the last patch #30) shortly.

The MR (!603) against 11.x is now mergeable and pipeline is now passing. Therefore, I am moving the status to 'needs review'.

I have merged 11.x. The blocking issue #3401988 seems to be resolved and the spellcheck has passed after merging 11.x into the issue fork.

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

I was suddenly seeing this issue on Drupal 10.2 where we have file_entity still installed.
The missing mime-type was "application/octet-stream" when uploading a pdf via a managed file widget.
To debug, place a stop point in the function file_entity_file_type(FileInterface $file) to determine what the mime type is that is not resolving.
Add the missing mime type to an existing (or new) file type at /admin/structure/file-types
A more sustainable solution would be to migrate away from file_entity.

czigor → credited tcrawford → .

Reassigning the ticket to myself to fix the failing tests.

I have added a proposed solution (MR 239) where I have split the permissions and modified the existing ProductAttributeForm. There are certainly places this approach could be improved. I would appreciate a code review. Thanks in advance.

I applied the patch, but this initially did fix the issue. I was however also seeing a TypeError in the console (L:64 of content_translation.admin.js) as $element[0] was undefined. I am not sure if this is related at all, but mention it in case others are seeing it. After adding a separate patch for the type error this patch addressed the issue. Thank you!

As previously mentioned by RgnYldz if using the symfony mailer it is possible already to use twig templates without the patch. Due to this, I am not sure this is really needed. The existing inline template approach also has advantages in some use cases. I can imagine though that if this patch is added it would make sense to consider making it configurable.

As an example of using symfony mailer with a specific hook_preprocess_HOOK() implementation for a specific mail type.

<?php
// mytheme.theme
/**
 * Implements hook_preprocess().
 */
function mytheme_preprocess_email__commerce__commerce_email_publication_fulfillment_mail(&$variables): void
{
  $email = $variables['email'];
  if (!$email instanceof Email) {
    return;
  }
  $entity = $email->getEntity();
  // Currently commerce_email does not set the entity on the email. We can get
  // this from the legacy_message param though.
  if ($entity === NULL) {
    $legacy_message = $email->getParam('legacy_message');
    // The context that is passed to the inline template.
    $context = $legacy_message['params']['body']['#context'] ?? NULL;
    if ($context) {
      $entity = $context[array_key_first($context)];
    }
  }
  if ($entity) {
    // Mail specific pre-processing.
    _mytheme_preprocess_publication_fulfillment_mail($variables, $entity);
  }
}

?>

You can then create a twig template for the specific mail, such as in my case: email--commerce--commerce-email-publication-fulfillment-mail.html.twig

tcrawford → created an issue.

@pvbergen. Thanks. With the changes made in 50ac3527 this issue should no longer occur as the webhook body is not used directly.

If you are using a monolog handler for PHP and you get this error it could well be that your handler service is creating a php error while instantiating. Only apply the handler to 'default' and not to 'php' in your services file. Then when it instantiates you will log the PHP error (rather than causing a recursion) so that you can fix the issue. For me the issue was that I was using the STDOUT constant when instantiating the stream handler. The constant is only available for the CLI SAPI so instantiating the logger caused the logger to instantiate itself again and Symfony detects this as a circular dependency and bails. (As a side note I tested my logger using drush scr and it worked perfectly, and it was not until I stopped using the logger for PHP that I could see the issue that caused the error when handling web requests).

@pvbergen Thanks for integrating this fix. Could you please apply the same to the 2.x branch, which is D10 compatible? Thanks in advance.

pvbergen → credited tcrawford → .

tcrawford → created an issue.

It was (is) a warning (deprecation) and not an error and so you won't see it unless your error reporting is showing warnings. If time permits I will add detailed test instructions. We saw this with 9.5.x. I have not yet tested with 10.1.x, but will do.

tcrawford → created an issue.

We encountered the same issue. Maybe we mis-configured the assignment of order item types. However, I think unless there is a important reason there is quite a bit of overhead to create different order items for licenses. Would there be any reason not to extend the guard / add a second guard to check if the purchased entity has a license type field? We are using this on a customer project with no adverse impacts to date.

```
// Skip purchasable entities that are not licenses.
if (!$order_item->getPurchasedEntity()->hasField('license_type') ||
$order_item->getPurchasedEntity()->get('license_type')->isEmpty()) {
return;
}

For me it was not possible to run drush cr and drush updb as the recurring_period plugin was missing. See issue #2920481.

I confirm also the issue.

The simplest workaround I could find (until the issue is resolved) is to install recurring_period module, so that the cache clear can occur and the update hooks in commerce_file can run to convert the recurring_period to commerce_license_periods.

I first tried to install the module with a composer alias to side-step the conflicts constraint added to commerce_file, but this was unsuccessful, so I just copied the module into the docroot/custom folder. Then I could run drush updb to update the config.

I think the issue could be resolved by re-adding the dependency at present. It is always a challenge to cleanly remove dependencies in Drupal as it requires multiple deployments.

Patch #6 allowed me to import field storage config where I had added an index. Thanks.

Patch attached.

ParagraphsWidget::errorElement() attempts to get the sub element when the path is valid. It currently checks for empty($this->arrayPropertyPath) where arrayPropertyPath is got via magic on Drupal\Core\Field\InternalViolation, but InternalViolation does not implement __isset(). Therefore the code on errorElement never returns $sub_element, even if there is a valid sub element found by NestedArray::getValue(). We could open an issue in core for InternalViolation to implement __isset(), but likely that won't move forward as accessing arrayPropertyPath via magic is deprecated. Therefore, I would propose the following solution (patch to follow):

?>
/**
   * {@inheritdoc}
   */
  public function errorElement(array $element, ConstraintViolationInterface $error, array $form, FormStateInterface $form_state) {
    // Validation errors might be a about a specific (behavior) form element
    // attempt to find a matching element.
    // A temporary variable needed as arrayPropertyPath is accessed via magic
    // and Drupal\Core\Field\InternalViolation does not implement __isset.
    // Therefore, empty($error->arrayPropertyPath) returns true even when the
    // magic getter returns a value.
    $arrayPropertyPath = $error->arrayPropertyPath;
    if (!empty($arrayPropertyPath) && $sub_element = NestedArray::getValue($element, $error->arrayPropertyPath)) {
      return $sub_element;
    }
    return $element;
  }

We are using #51 on a project. I have re-rolled patch #51 so that it applies on 2.0.x, which is required for an upgrade to D10.0.x. As per previous concerns, there is no test coverage for the patch, but I hope it helps someone who is upgrading.

I am currently upgrading a project to Drupal 10 and so I have re-rolled #84 for D10.

The patch (2923634-92.patch) is identical as #84, except for removal of the changes to /core/modules/aggregator/src/Plugin/views/row/Rss.php, due to the removal of the aggregator module from core. I have not created an interdiff as the first patch no longer applies on D10.

I hope this assists if you are currently upgrading to D10. As the issue is postponed I have not created a patch against D11.

The attached patch (add-support-for-remote-video-media-D10-3021913-43.patch) should address the PHP linting issue with #42.

@Lukas I will update of commerce_shipping (and likely core as it needs to be done soon) and see if that resolves the issue.

For the project with the issue we are on
PHP 8.0
drupal/core 9.4.11
drupal/comerce 2.33, and
drupal/commerce_shipping 2.4.

I have resolved this issue and created a new release (2.1.0).

The issue reported primarily manifests when the module is used together with a custom commerce condition that allows a promotion to be applied using a promotion link only (not applied by customer manually in the checkout). The temp store was used to check if the promotion was a applied by a link and as the temp store was lost on login the promotion was invalidated and removed. Therefore, I have changed this ticket from a bug report to a feature request.

There remains an issue that if the promo link is used and the user logs in before adding an item to their cart, the promotion will not be applied due to loss of the temp store on login (#3308135). However, this issue should be resolved with the aforementioned issue.

I have applied patch #79 and tested. I am seeing warnings and errors though in core (and in paragraphs) when using the paragraphs module together with this patch.

If I re-save the node in the original language before saving the translation, I see the following warning:

```
Warning: Undefined array key "original_deltas" in Drupal\Core\Field\WidgetBase->flagErrors() (line 474 of core/lib/Drupal/Core/Field/WidgetBase.php).
Drupal\Core\Field\WidgetBase->flagErrors(Object, Object, Array, Object) (Line: 2210)
Drupal\paragraphs\Plugin\Field\FieldWidget\ParagraphsWidget->flagErrors(Object, Object, Array, Object) (Line: 270)
Drupal\Core\Entity\Entity\EntityFormDisplay->flagWidgetsErrorsFromViolations(Object, Array, Object) (Line: 268)
Drupal\Core\Entity\ContentEntityForm->flagViolations(Object, Array, Object) (Line: 214)
Drupal\Core\Entity\ContentEntityForm->validateForm(Array, Object)
call_user_func_array(Array, Array) (Line: 82)
Drupal\Core\Form\FormValidator->executeValidateHandlers(Array, Object) (Line: 275)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object, 'node_article_form') (Line: 118)
```
If I don't resave the node in the original language first, but add a translation, I get the following TypeError.
```
The website encountered an unexpected error. Please try again later.
TypeError: Drupal\paragraphs\Plugin\Field\FieldWidget\ParagraphsWidget::errorElement(): Argument #1 ($element) must be of type array, null given, called in /app/docroot/core/lib/Drupal/Core/Field/WidgetBase.php on line 478 in Drupal\paragraphs\Plugin\Field\FieldWidget\ParagraphsWidget->errorElement() (line 2217 of modules/contrib/paragraphs/src/Plugin/Field/FieldWidget/ParagraphsWidget.php).
Drupal\paragraphs\Plugin\Field\FieldWidget\ParagraphsWidget->errorElement(NULL, Object, Array, Object) (Line: 478)
Drupal\Core\Field\WidgetBase->flagErrors(Object, Object, Array, Object) (Line: 2210)
Drupal\paragraphs\Plugin\Field\FieldWidget\ParagraphsWidget->flagErrors(Object, Object, Array, Object) (Line: 270)
Drupal\Core\Entity\Entity\EntityFormDisplay->flagWidgetsErrorsFromViolations(Object, Array, Object) (Line: 268)
Drupal\Core\Entity\ContentEntityForm->flagViolations(Object, Array, Object) (Line: 214)
Drupal\Core\Entity\ContentEntityForm->validateForm(Array, Object)
call_user_func_array(Array, Array) (Line: 82)
Drupal\Core\Form\FormValidator->executeValidateHandlers(Array, Object) (Line: 275)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object, 'node_article_form') (Line: 118)
Drupal\Core\Form\FormValidator->validateForm('node_article_form', Array, Object) (Line: 591)
Drupal\Core\Form\FormBuilder->processForm('node_article_form', Array, Object) (Line: 323)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 48)
Drupal\Core\Entity\EntityFormBuilder->getForm(Object, 'default', Array) (Line: 394)
Drupal\content_translation\Controller\ContentTranslationController->add(Object, Object, Object, 'node')
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 580)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 169)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 81)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 68)
Drupal\simple_oauth\HttpMiddleware\BasicAuthSwap->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->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: 49)
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: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 713)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

As we are working on the same project, I have reviewed and tested the patch. It works. Thanks for the contribution.

Moving to RTBC due to the positive feedback.

Patch #42 works great for me, also (tested 9.4.x). Thank you very much.

Thanks. I will re-roll this against 9.5.x-dev.

tcrawford → created an issue.

@tim.plunket Thank you for your feedback. I apologize .The issue fork was created unintentionally. I only meant to leave a comment to provide our (positive) feedback on the patch. Thank you again.

We applied the patch (#92) to a D8 for a project and it worked extremely well. We could use it with no functional changes. We had some UI issues, but I think most of these related to layout builder UI customization we have. Thank you to everyone who has contributed.

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