undefined - Contrib.social

🇧🇪Belgium herved

I just stubled on this, the current starterkit setup on radix 6 includes the main css file, which is rather bad as it will leak outside ckeditor.
Also I don't understand how #4 B and #16 could work because laravelmix executes everything in parallel. If I delete my build folder, and try to mix.postCss a file that is generated via mix.sass, laravelmix complains that the source file doesn't exist (because it has not been compiled yet).
Where is assets/css/ckeditor5.style.css coming from there?
Processing the same entry twice also is not an option because mix complains about that.

📌 | Redirect | Document that URL redirects work from node/{nid} but not alias

🇧🇪Belgium herved

Ok my main question here relates to what should happen when we actually delete a node that has a redirect.

From #2879648-103:

For the node deletion problem:
- when a path alias is deleted,
- and a redirect for the source of the path alias exists
- if the existing redirect language matches the path alias language being deleted, we update the redirect source to the path alias url
- else if the existing redirect language == "und", we duplicate the redirect and set the redirect source and language according to the path alias being deleted.

Say we have an EN /node/123 with alias /foo
And a user created a redirect from /node/123 to /bar. At this point both /node/123 and /bar redirect to /foo.
Shouldn't we do anything when the node (and so path alias) gets deleted? so we maintain /foo > /bar redirection?

📌 | Redirect | Document that URL redirects work from node/{nid} but not alias

🇧🇪Belgium herved

Snapshot of current MR for composer

📌 | Redirect | Document that URL redirects work from node/{nid} but not alias

📌 | Translation Centre For the Bodies of the EU - Plugin for TMGMT | Automated Drupal 11 compatibility fixes for tmgmt_cdt

🇧🇪Belgium herved

Coming from 🐛 Redirects from aliased paths aren't triggered Needs review
I did some thinking back then on that issue (see comment #103 / #108) and I ended up with a similar conclusion to @berdir, which is that we should only inform/guide the user when they input aliases.
Till now I was using the patch from #104, but the approach here seems like an it improves on it. So thank you for moving it forward.

🇧🇪Belgium herved

Hello @cdteu,
We are preparing for the upgrade from Drupal 10 to 11, and tmgmt_cdt is one of the last modules blocking us.
Would you be able to take a look and prepare a release for D11 when possible?
Thanks in advance!

📌 | Media entity Soundcloud | Fix deprecation since core 10.3

🇧🇪Belgium herved

herved → changed the visibility of the branch 3509415-fix-deprecation to hidden.

✨ | Organic groups | Leverage the new access policy API ?

🇧🇪Belgium herved

Thank you both, this is still a rough POC.
Indeed maybe using a "validated-member" OG role could be a good compromize for now.

✨ | Organic groups | Leverage the new access policy API ?

🇧🇪Belgium herved

POC branch added, functional and kernel tests are passing, I only skipped unit tests.
I left a few todos.

My only major concern on this approach is that although the AccessPolicyProcessor caches things (statically and persistently) it can get quite expensive to load all memberships and groups to compute all permissions for a user, especially if he belongs in thousands of groups. Although that seems to be what drupal/group does...

✨ | Organic groups | Leverage the new access policy API ?

🇧🇪Belgium herved

herved → created an issue.

🐛 | Default Content | BaseFieldOverride cause inconsistencies during export

🇧🇪Belgium herved

The IS is no longer accurate since 🐛 Make BaseFieldOverride inherit internal property from the base field Fixed fixed isInternal for BaseFieldOverride and path aliases are now exported.
But maybe we could use this issue to discard fields that are marked as read-only?
I created a MR, anyway.

📌 | Media entity Soundcloud | Fix deprecation since core 10.3

🇧🇪Belgium herved

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

✨ | Drupal core | [PP-1] Add thread depth configuration to comments

🇧🇪Belgium herved

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

📌 | Drupal core | Use comment access handler instead of hardcoding permissions

🇧🇪Belgium herved

🐛 | Drupal core | Creating comment field w/o a comment type is allowed

🇧🇪Belgium herved

I discovered an issue in CommentLinksTest.php revealed by #2879087-148: Use comment access handler instead of hardcoding permissions →
There are other cases in tests where the comment type is not created.

📌 | Drupal core | Use comment access handler instead of hardcoding permissions

🇧🇪Belgium herved

📌 | Drupal core | Use comment access handler instead of hardcoding permissions

🇧🇪Belgium herved

I pushed an attempt now to address #118 to #125 (reply to {$pid}).
By calling $parent_comment->access('reply') in CommentController::replyFormAccess and doing the actual check in CommentAccessControlHandler::checkAccess

Also, I had a look at the other issue ✨ [PP-1] Add thread depth configuration to comments Postponed which requires this but it seems to rely only on CommentAccessControlHandler::checkCreateAccess at the moment. Since we now have the reply access operation, I'm not even sure we need all the changes to $context (parent_comment, commented_entity).
I'll sync with @claudiu.cristea on Monday.

---

PS: still the same 2 (less concerning) test failures which I left out for now since I don't quite understand them yet.

📌 | Drupal core | Use comment access handler instead of hardcoding permissions

🇧🇪Belgium herved

Important fix for cacheability:
The CommentDefaultFormatterCacheTagsTest was failing because the CommentDefaultFormatter was not applying cacheability to the render array.

---

On that note, we have a need in our project where the create comment permission will have a very high cache variance (per user).
This means that currently, it would kill the Dynamic Page Cache when CommentDefaultFormatter checks for the create permission and adds the lazy_builder and cacheability to the render array.
I propose to move the access check inside the lazy_builder callback, in CommentLazyBuilders::renderForm.

I'd like to keep the scope to a minimum but this touches on the same lines and lots of projects may be in a similar scenario so we should try to preserve the page cache as much as possible. Viewing comment is less likely to have such requirements.
The change is very small, but this could also be reverted and a separate issue or follow up instead if anyone objects.

📌 | Drupal core | Use comment access handler instead of hardcoding permissions

🇧🇪Belgium herved

For now here are patches of the current MR23 commit f2ac23e6.

📌 | Drupal core | Use comment access handler instead of hardcoding permissions

🇧🇪Belgium herved

Update: I recreated the history from previous/2879087-comment-access-handler/2022-01-31 branch
https://git.drupalcode.org/issue/drupal-2879087/-/tags/previous%2F287908...
Merged 11.x, fixed conflicts, phpcs, phpstan and the tests touched by the patch.

5 tests are failing at the moment, among those these are concerning to me and relate to the changes in CommentController::replyFormAccess:
- core/modules/comment/tests/src/Functional/CommentNonNodeTest.php
- core/modules/comment/tests/src/Functional/CommentAnonymousTest.php

Also #137 may be right, I'll try to investigate those issues.

📌 | Drupal core | Use comment access handler instead of hardcoding permissions

🐛 | TMGMT Translator for eTranslation | Consider using password form element in plugin configuration form

🇧🇪Belgium herved

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

🐛 | TMGMT Translator for eTranslation | Consider using password form element in plugin configuration form

🇧🇪Belgium herved

Thanks @dieterholvoet,
I realize now that there might be an issue also with this part as it shows the user/pass in clear (placeholder) when we use config override:

if ($value !== $original) {
  $form[$child]['#placeholder'] = $value;
  $form[$child]['#disabled'] = TRUE;
  $form[$child]['#description'] = $this->t('This config cannot be changed because it is overridden.');
}

🐛 | Drupal core | Error: cannot call methods on dialog prior to initialization; attempted to call method 'option'

🇧🇪Belgium herved

herved → created an issue.