Loenhout
Account created on 21 May 2010, almost 15 years ago
#

Merge Requests

More

Recent comments

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

Added this in the MR.

* This adds an And / or filter to the block condition plugin.
* This adds the same to paragraphs.
* This also adds a negate filter to paragraphs.
* Also added update hooks to install and populate the paragraph fields.

Note: Negate for block condition plugin is already available (through the base condition).

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

mallezie β†’ made their first commit to this issue’s fork.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

And done that, now that there is a new release there.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

Patch looks good to me.

In standard this is not needed, since there the options module is already installed, but on minimal, or where you don't have this, this enforces that options are installed.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

MR looks good to me, nothing more to change here.
We should mention this in the release notes however, for projects using IEF, so they can include it in their root composer.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

Seems i've patched the patch from https://www.drupal.org/project/facets/issues/3013702#comment-14588342 ✨ multiple facets with one apply button Needs review

Closing this one as a duplicate. Sorry for the noise.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

Reading up a bit on the symfony unpack feature, they even made it the default (auto) behavior now to unpack composer packages when installing symfony packs (which are really similar to what recipes are).
Was wondering if we could just pull in symfony/flex and then use the symfony unpack command.
https://github.com/symfony/flex/tree/2.x/src/Unpack

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

Just nothing that this approach does not work for mails sent by other mail builders then the LegacyEmailBuilder.

That's the only plugin setting the needed parameter for reroute_email_mailer_build()

/**
 * Implements hook_mailer_build().
 */
function reroute_email_mailer_build(EmailInterface $email) {
  // Prevent calling reroute for symfony legacy email builder.
  // For the mode an email reroute is handled by symfony_mailer itself by invoke 'mail' hook.
  // @see \Drupal\symfony_mailer\Plugin\EmailBuilde\LegacyEmailBuilderr::createParams()
  if (!empty($email->getParam('legacy_message'))) {
    return;
  }

If using the symfony_mailer_bc submodule and provided user mails (for example contact mails, this breaks).

Error: [] operator not supported for strings in user_mail() (line 795 of core/modules/user/user.module).

This because the body is predefined to a string in this patch, while it should be an array for the user_module. However this does then conflict again in reroute_email.

The website encountered an unexpected error. Please try again later.
TypeError: nl2br(): Argument #1 ($string) must be of type string, array given in nl2br() (line 500 of modules/contrib/reroute_email/reroute_email.module).
nl2br(Array) (Line: 500)
reroute_email_mailer_post_render(Object)

No solution yet IMO, just posting here for people to get a heads up.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

This was merged and included in the latest release.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

It might be not that bad as thought.

https://github.com/mglaman/phpstan-drupal/issues/479

The issue is already fixed in phpstan-druoal, just waiting for a new release should be enough.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

This looks perfect! Only removal of things, without changing code, thus only removal of false positives.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

For me the changes look clean and good!

Only adding of access checks, and removing them from the baseline.

2 things wondering here.

There are still some entity query access checks left (in the baseline). Those might be fixed as well by a new update. Currently on mglaman/phpstan-drupal 1.1.27 (but 1.128 and 1.129 should fix some more?).
https://github.com/mglaman/phpstan-drupal/releases/tag/1.1.28
https://github.com/mglaman/phpstan-drupal/releases/tag/1.1.29

There are still some updated deps failures caused by entity access checks, which might also be fixed if we do the other update first?
See https://www.drupal.org/node/3060/qa β†’ and https://www.drupal.org/pift-ci-job/2592021 β†’

So not sure if we should do another update first here?

If not this is RTBC for me.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

Just FYI

B) The phpstan errors. These are actually caused because the phpstan library and phpstan-drupal library are updated in this MR. This seems out of scope here. Also some other libraries seem updated, which seems also out of scope.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

Thanks sdstyles. This looks much better to me.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

Thanks! Totally forgot about this one.

Just checked the new MR, and seems the same to the original. (Which i did, so probably i'm not the one allowed to put it in RTBC).

Closed the old merge request.

πŸ‡§πŸ‡ͺBelgium mallezie Loenhout

Would this also include a deprecation of the magic __get method on FieldItemList.

I don't have a preferred approach, but i do agree here, that this would simplify (and greatly improve DX) if we could remove a lot of the ways of accessing field data, and go to one standardized approach.

This would indeed be a disruptive change for contrib and custom code, but it can be cleanly done through deprecating.

Production build 0.71.5 2024