🇩🇪Germany @geek-merlin

Ah, request context must be fixed too.

Here's the correct fix with comments.

This can and should be solved generically in the DB driver.

We can steal code from ye olde SQLite 999-limit issue: 📌 Make SQLite faster by combining multiple inserts and updates in a single query Needs work

Note that there have been several approaches in that issue. The current approach that sends multple values via JSON is really elegant, but likely too SQLite specific. But a former approach had a parameter chunker, that likely every DB driver can use. See #2031261-86: Make SQLite faster by combining multiple inserts and updates in a single query → ff.

Full error and trace:

Drupal\Core\Render\Component\Exception\ComponentNotFoundException: Unable to find component "c4c:state-transition-help--c4c_product" in the component repository. [The "c4c:state-transition-help--c4c_product" plugin does not exist. Valid plugin IDs for Drupal\Core\Theme\ComponentPluginManager are: navigation:toolbar-button, c4c:c4c-dash-stores, c4c:c4c-dash-products, c4c:c4c-dash-loan-limited, c4c:c4c-dash-deposits, c4c:c4c-dash-todo, c4c:c4c-dash-contacts, c4c:c4c-dash-wrapper, c4c:c4c-dash-submissions, c4c:state-transition-option, c4c:state-transition-formatter, c4c:state-transition-option-list, c4c:state-transition-help, c4c:state-transition-option-lists, c4c:disclose, c4c:disclose-help, c4c:disclose-edit, olivero:teaser] in Drupal\Core\Theme\ComponentPluginManager->createInstance() (line 118 of web/core/lib/Drupal/Core/Theme/ComponentPluginManager.php).


Backtrace	
#0 web/core/lib/Drupal/Core/Theme/ComponentPluginManager.php(139): Drupal\Core\Theme\ComponentPluginManager->createInstance()
#1 web/core/lib/Drupal/Core/Template/Loader/ComponentLoader.php(73): Drupal\Core\Theme\ComponentPluginManager->find()
#2 vendor/twig/twig/src/Loader/ChainLoader.php(87): Drupal\Core\Template\Loader\ComponentLoader->exists()
#3 vendor/twig/twig/src/Environment.php(480): Twig\Loader\ChainLoader->exists()
#4 vendor/twig/twig/src/Template.php(284): Twig\Environment->resolveTemplate()
#5 web/sites/default/files/php/twig/67378f97837bb_c4c:state-transition-form_MxcrkH4mxXQ-j3yNe7MvA5dcs/fwFCXY9fWd-nlDZd68es-21dTjo-Isaco2sckds-PRQ.php(182): Twig\Template->loadTemplate()
#6 vendor/twig/twig/src/Template.php(437): __TwigTemplate_9a5d9baeddf4125a0fced8b9cb5b502a___1869349367->block_content()
#7 vendor/twig/twig/src/Template.php(145): Twig\Template->yieldBlock()
#8 vendor/twig/twig/src/Template.php(206): Twig\Template->displayBlock()
#9 web/sites/default/files/php/twig/67378f97837bb_c4c:disclose-help_qGblSbwRYDcFnNy828ygtEOe1/sS0pDGOGxuk9-bo-gKRrne-OJHl1sVU_lK0aQsANW2o.php(51): Twig\Template->renderBlock()
#10 vendor/twig/twig/src/Extension/CoreExtension.php(1967): __TwigTemplate_98defebf1e93cb711dd6540015684b77->{closure}()
#11 web/sites/default/files/php/twig/67378f97837bb_c4c:disclose-help_qGblSbwRYDcFnNy828ygtEOe1/sS0pDGOGxuk9-bo-gKRrne-OJHl1sVU_lK0aQsANW2o.php(48): Twig\Extension\CoreExtension::captureOutput()
#12 vendor/twig/twig/src/Template.php(393): __TwigTemplate_98defebf1e93cb711dd6540015684b77->doDisplay()
#13 web/sites/default/files/php/twig/67378f97837bb_c4c:state-transition-form_MxcrkH4mxXQ-j3yNe7MvA5dcs/fwFCXY9fWd-nlDZd68es-21dTjo-Isaco2sckds-PRQ.php(169): Twig\Template->yield()
#14 vendor/twig/twig/src/Template.php(393): __TwigTemplate_9a5d9baeddf4125a0fced8b9cb5b502a___1869349367->doDisplay()
#15 web/sites/default/files/php/twig/67378f97837bb_c4c:state-transition-form_MxcrkH4mxXQ-j3yNe7MvA5dcs/fwFCXY9fWd-nlDZd68es-21dTjo-Isaco2sckds-PRQ.php(55): Twig\Template->yield()
#16 vendor/twig/twig/src/Extension/CoreExtension.php(1967): __TwigTemplate_9a5d9baeddf4125a0fced8b9cb5b502a->{closure}()
#17 web/sites/default/files/php/twig/67378f97837bb_c4c:state-transition-form_MxcrkH4mxXQ-j3yNe7MvA5dcs/fwFCXY9fWd-nlDZd68es-21dTjo-Isaco2sckds-PRQ.php(48): Twig\Extension\CoreExtension::captureOutput()
#18 vendor/twig/twig/src/Template.php(393): __TwigTemplate_9a5d9baeddf4125a0fced8b9cb5b502a->doDisplay()
#19 web/sites/default/files/php/twig/67378f97837bb___string_template__aacdbc_L9-lvlayo6eN8GigUwpfZV2IE/ZLOc-K5NEbq26FzUaFAaTMNppd7zuK4NYmRwLVhP5jk.php(130): Twig\Template->yield()
#20 vendor/twig/twig/src/Template.php(393): __TwigTemplate_4ba91fed2d9b6dd3a1d23364e8fabcda___64313199->doDisplay()
#21 web/sites/default/files/php/twig/67378f97837bb___string_template__aacdbc_L9-lvlayo6eN8GigUwpfZV2IE/ZLOc-K5NEbq26FzUaFAaTMNppd7zuK4NYmRwLVhP5jk.php(44): Twig\Template->yield()
#22 vendor/twig/twig/src/Template.php(393): __TwigTemplate_4ba91fed2d9b6dd3a1d23364e8fabcda->doDisplay()
#23 vendor/twig/twig/src/Template.php(349): Twig\Template->yield()
#24 vendor/twig/twig/src/Template.php(364): Twig\Template->display()
#25 vendor/twig/twig/src/TemplateWrapper.php(35): Twig\Template->render()
#26 web/core/lib/Drupal/Core/Template/TwigEnvironment.php(234): Twig\TemplateWrapper->render()
#27 web/modules/custom/twig_strict/src/TwigEnvironment/TwigStrictEnvironmentTrait.php(54): Drupal\Core\Template\TwigEnvironment->renderInline()
#28 web/core/lib/Drupal/Core/Render/Element/InlineTemplate.php(54): Drupal\Core\Template\TwigEnvironment\__use\Drupal\twig_strict\TwigEnvironment\TwigStrictEnvironmentTrait->renderInline()
#29 [internal function]: Drupal\Core\Render\Element\InlineTemplate::preRenderInlineTemplate()
#30 web/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php(113): call_user_func_array()
#31 web/core/lib/Drupal/Core/Render/Renderer.php(870): Drupal\Core\Render\Renderer->doTrustedCallback()
#32 web/core/lib/Drupal/Core/Render/Renderer.php(432): Drupal\Core\Render\Renderer->doCallback()
#33 web/core/lib/Drupal/Core/Render/Renderer.php(504): Drupal\Core\Render\Renderer->doRender()
#34 web/core/lib/Drupal/Core/Render/Renderer.php(248): Drupal\Core\Render\Renderer->doRender()
#35 web/core/lib/Drupal/Core/Template/TwigExtension.php(476): Drupal\Core\Render\Renderer->render()
#36 web/sites/default/files/php/twig/67378f97837bb_field.html.twig_951wjTAjxAQzDPd8iCs6h4TzC/T0J8I7T9jw9SG2hSY1zn66tN532aB5fNNAupJPX3ZG4.php(130): Drupal\Core\Template\TwigExtension->escapeFilter()
#37 vendor/twig/twig/src/Template.php(393): __TwigTemplate_593909c9f0ed964d0cacfa2423bbbe31->doDisplay()
#38 vendor/twig/twig/src/Template.php(349): Twig\Template->yield()
#39 vendor/twig/twig/src/Template.php(364): Twig\Template->display()
#40 vendor/twig/twig/src/TemplateWrapper.php(35): Twig\Template->render()
#41 web/core/themes/engines/twig/twig.engine(33): Twig\TemplateWrapper->render()
#42 web/core/lib/Drupal/Core/Theme/ThemeManager.php(348): twig_render_template()
#43 web/core/lib/Drupal/Core/Render/Renderer.php(491): Drupal\Core\Theme\ThemeManager->render()
#44 web/core/lib/Drupal/Core/Render/Renderer.php(504): Drupal\Core\Render\Renderer->doRender()
#45 web/core/lib/Drupal/Core/Render/Renderer.php(248): Drupal\Core\Render\Renderer->doRender()
#46 web/core/lib/Drupal/Core/Template/TwigExtension.php(476): Drupal\Core\Render\Renderer->render()
#47 web/sites/default/files/php/twig/67378f97837bb_commerce-product.html.twi_tOcY0iAUkU2l2fGSXjABAM7dg/LWE4gyDVD4X69M9JGRQqHbVHWfMTgGEmWyZVEt4T7Ts.php(55): Drupal\Core\Template\TwigExtension->escapeFilter()
#48 vendor/twig/twig/src/Template.php(393): __TwigTemplate_7a8da07bbb553fd7798ee89961aade03->doDisplay()
#49 vendor/twig/twig/src/Template.php(349): Twig\Template->yield()
#50 vendor/twig/twig/src/Template.php(364): Twig\Template->display()
#51 vendor/twig/twig/src/TemplateWrapper.php(35): Twig\Template->render()
#52 web/core/themes/engines/twig/twig.engine(33): Twig\TemplateWrapper->render()
#53 web/core/lib/Drupal/Core/Theme/ThemeManager.php(348): twig_render_template()
#54 web/core/lib/Drupal/Core/Render/Renderer.php(491): Drupal\Core\Theme\ThemeManager->render()
#55 web/core/lib/Drupal/Core/Render/Renderer.php(248): Drupal\Core\Render\Renderer->doRender()
#56 web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(238): Drupal\Core\Render\Renderer->render()
#57 web/core/lib/Drupal/Core/Render/Renderer.php(638): Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}()
#58 web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(231): Drupal\Core\Render\Renderer->executeInRenderContext()
#59 web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(128): Drupal\Core\Render\MainContent\HtmlRenderer->prepare()
#60 web/core/lib/Drupal/Core/EventSubscriber/MainContentViewSubscriber.php(90): Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse()
#61 [internal function]: Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray()
#62 web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(111): call_user_func()
#63 vendor/symfony/http-kernel/HttpKernel.php(186): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch()
#64 vendor/symfony/http-kernel/HttpKernel.php(76): Symfony\Component\HttpKernel\HttpKernel->handleRaw()
#65 web/core/lib/Drupal/Core/StackMiddleware/Session.php(53): Symfony\Component\HttpKernel\HttpKernel->handle()
#66 web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle()
#67 web/core/lib/Drupal/Core/StackMiddleware/ContentLength.php(28): Drupal\Core\StackMiddleware\KernelPreHandle->handle()
#68 web/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\ContentLength->handle()
#69 web/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass()
#70 web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\page_cache\StackMiddleware\PageCache->handle()
#71 web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle()
#72 web/core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php(36): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle()
#73 web/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\Core\StackMiddleware\AjaxPageState->handle()
#74 web/core/lib/Drupal/Core/DrupalKernel.php(741): Drupal\Core\StackMiddleware\StackedHttpKernel->handle()
#75 web/index.php(19): Drupal\Core\DrupalKernel->handle()
#76 web/.ht.router.php(71): require('...')
#77 {main}

geek-merlin → created an issue.

Not sure when, but challenge taken.

The only change in 3.0.x was due to the nice simplification in 📌 Remove the variationType setting Fixed .

Added 3.0.x MR too.

@jsacksick See #6.

Patch is trivial and uncontroversial.

#17 has been addressed.
Tests are green.

So looks RTBC to me.

@amateescu: Great news! This was on my hot-list for long, but never found time.

From my mental notes:
> Add a boolean temporary base field to the workspace entity type
As "managed" workspace, it shloudl rather be a "Owner"/"Provider" base field, so modules can recognize their workspaces (which obeoletes the boolean field).

WDYT?

Trivial MR flying in.

Commit url applies cleanly on my site and fixes the issue for me.

And pipeline is green now.

Constructor parameter change is likely to need further ado, but i'm running out of time.

MR flying in.

Trivial MR flying in.

Commit url applies cleanly on my site and fixes the issue for me: https://git.drupalcode.org/project/drupal/-/commit/900e76dac8973176991a0...

geek-merlin → created an issue.

geek-merlin → created an issue.

#58: I don't see the other issue blocking this.

Good to hear. I won't be able to work on this any time soon, so the tests go to whoever it itches.

Yep, that works for me on a complex site.

Squashed: https://git.drupalcode.org/project/page_manager/-/commit/5a62749ad8bf170...

Let's handle 🐛 Requests are pushed onto the request stack twice, popped once Needs work then.

The other issue is fixed.

Thanks for the MR! Which has test failures, so NW for that.
https://git.drupalcode.org/issue/inline_entity_form-2683125/-/jobs/3297570

@bradjones1 Yes, very relevant questions.

So we have existing code that relies on carrying request-specific state, in a world where that assumption does not hold anymore.
Whar are our minimally invasive solutions?

Build a fresh container, and give each request a deep copy of it, to throw away in the end. Saves us at least container-building, but the deep copy is the expensive part.

Then add a blessed stateless_service tag, and only swap out the remaining services for a new request.

Something like $kernel->rebuildContainerForNewRequest(), as minimal API for now. And maybe in the end we can live with throwing away a handful of services.

Yes, this would help a lot. Also, the restriction to the Hooks namespace is problematic even in modules:

I implemented hundreds of hooks in the hux POC, where there was the choice to use either the Hooks namespace with auto-service registration, or ANY service. In >90% of the cases, from a code organization perspective, the hook was best to live where it belonged semantically, not in the Hooks namespace.

Or stated it differently: For sane code organization, restrikting hooks to Drupal\my_module\Hooks is the same PITA as to have to put them in the module file.

@kristiaanvandeneynde I'd like to have this and ported your POC from #7 to the new API.

At least i think so, because it looks like new permisisons are in the UI, but it seems they do not save.

Can you review?

geek-merlin → changed the visibility of the branch 3207064-prevent-group-admins to active.

geek-merlin → changed the visibility of the branch 3207064-prevent-group-admins to hidden.

#240 @kristiaanvandeneynde

This lacks one key feature of entity module approach:
IMHO the entity module approach is the right direction in adding a declarative field condition that can be applied to single-entity AND entity-query access. I think it is NOT expressive enough though.

And a much bigger objection: The MR contains @dawehner's quich shot from #58 from 7 years ago, and is a strong Drupalism.

Let's instead leverage the kernel.environment container parameter. It is symfony standard and already heavily supported in our Drupal kernel:
- \Drupal\Core\DrupalKernel::getKernelParameters
- \Drupal\Core\DrupalKernel::getContainerCacheKey

Drupalorg does not test patches anymore. Can someone create a MR of it?

One objection to a setting: IIRC, settings jump in quite late. A container parameter would be better in this regard.

I am finding this in dblog on a site with no trace of umami. I read alexpott's #37 as "not related to umami" too, so adjusting issue title.
Setting to "field system" for now.

Hmm, #54 not only has BC issues, it also runs quite late.

I think a new kernel parameter can solve both issues.

Thanks for the research! Since #2, where i was open to this, i learnt a lot about translations.
Setting to wontfix for reasons:
- A t() string should be hard to mis-contextualize, because translators on localize.drupal.org do not see the UI.
- String changes should only be made if absolutely necessary, as they invalidate existing community translations.
- Anyone can translate the string as they like, even to english.

@mably: >200 lines of code where 5 are enough. Plus added complexity. A good developer does not burden himself and others with that...

Hmm, strange, but i now just did set it. Thanks for enlightening me.

@mably: Have you tried clicking "Sign in", top right?

Add warning about Access checking in the controller content method

geek-merlin → created an issue.

+1 for this, it makes the module more useful.

Code review: Looks solid, after all it's just moving code around and making it pluggable.

Did not test it (and the module has not moved its tests to gitlab ci).

@mariia: You may want to learn adding MRs, it has lots of benefits (maintainers often like it, gitlab ci, and more :-).

Everything is possible.
Invitations are fieldable, can contain a secret, and some code can be triggered that compares entered data to the secret and take action.
The limitation is time and competence to develop it.

@mably: Please look what the string is in the original widget, and use the same one (apart from the format). That way, no re-translation is necessary.

Yes you can: https://git.drupalcode.org/project/datetimehideseconds/-/settings/reposi...

If you ask me, we got these options:
- 1) magically recognizing compatible widgets
- 2) adding UI or developer settings
- 3) adding a whitelist of allowed widgets
- 4) adding a blacklist of disallowed widgets

Noone knows if 1 is possible or worth the effort. 2 is useless complexity. So it's 3 or 4.

Checking fieldType seems like the wrong thing though, a check on widgetType is needed.

So i'm proposing: Factor out sth like

function _datetimehideseconds_widget_applies(WidgetInterface $widget): bool {
  return $widget instanceof DateTimeWidgetBase && !(
    // Blacklist of widget classes not playing well with this module.
    $widget instanceof FooWidget
    || $widget instanceof BarWidget
  );
}

And every time s.o complains, add another class to the exclusion list.

Nothing to discuss.

@mably Yep, sounds reasonable.

Yes, i agree.

Postponed on the discussion in this issue.

Postponing on the discussion in the other issue.

Yes, this one bit me too. Bit me hard...

@kristiaanvandeneynde said:
> An alternative would be to grab the target entity's cache tags and invalidate those. Now that the entity storage is using a proper cache internally, we could do that.
> [But] That would be a behavioral change

What about the legacy-settings pattern:
- Add a setting "Re-save entity on group content create (instead of only invalidating caches)"
- Default it to falsej for new projects.
- Set it to true for legacy projects.

Great how this is moving forward!!

No doubt of this.

@mably Great you ask. As main maintainer up to now i took the liberty to elaborate our previous maintenance policy here:
🌱 Maintenance policy Active

Feel free to add there!

geek-merlin → created an issue.

@mably Welcome from metoo!

I have one additional wish: Currently the stability of the core functionality we need was ensured by not adding features (for simply no time).
Now, if more features are to come (which is a good thing), it would be very helpful to add automated tests so to prevent regressions.

That said, rock on!

@andypost #31
> Looks great by skimming!

Nice you looked into it and like it too! I think the approach has quite some potential.
Can you give a code review (make all corrections you think fit) and help move this forward?

#32: Looks enigmatic to me... wrong issue?

Squashed commit for local application: https://git.drupalcode.org/project/page_manager/-/commit/d3764a67c50e69e...

Finally i found some time to look into this thoroughly.
@berdir is right that the former fake-request approach is at least limited and possibly FUBAR.

So...
- created a SubRequestAccessChecker and fixed some core limitations.
- added tests, which are green.
- also test-only changes are red (nice we have that now!), which proves that the MR fixes the issue.

Nice.

Hi @finnsky and all,

this is a great step forward, and this is why i cherry-picked this early for use in one project. (And i see my initial feedback was helpful, nice!).

Here is some more feedback:
- I like the '#help' approach, it makes altering existing elements possible (without additional wrapper).
- In the current approach, the trigger button is added with javascript, leveraging a library that does the positioning. This gave us some pain in our project.
- (1) First, js-added stuff is a PITA for theming (it's like select box improvements, where we switched to sumoselect, as it responds to CSS).
- (2) But the killer was, the javascript totally breaks when the element is not in the viewort initially, like with tabs and other disclosure elements.
- I doubt that (2) can be healed robustly, and tbh. in face of (1) we don't think putting effort in it is worth it.
- The result was, that we had to abandon this approach, and roll our own.

You can find our approach here and pick whatever you like: https://gitlab.com/geeks4change/modules/h4d_help
I suggest using this approach, because:
- It builds the markup server-side, so it works without JS (at least soon-ish, when the polyfills are obsoleted)
- It is stylable via CSS
- It works with tabs and other disclosure elements
- Apart from the popover polyfill, it uses anchor positioning for the toggle and help (native in chromium-based browsers, polyfilled in others) in its anchor-position variant. (The anchor-positioning Insert-Area variant is even nicer "soon" but not polyfilled yet.)

So needs-work for that imho.

@hchonov: Nice to meet you again here. We face a similar problem and your approach pointed me in the right direction.

Made an overview 🌱 Overview of Pathauto language problems and approaches Active with some more pointers and what core does ("und if entity OR path field is untranslatable"). HTH!

No additional info needed. Patches can mature parallel.

geek-merlin → created an issue.

Archaeology...

Archaeology...

Thanks for the quick commit!

Ah OK, this needs a rebase.
The related issue changed the code here a lot.

Maybe it solved this issue too? Let's see.

Made a simplification, then squashed.
Patch url: https://git.drupalcode.org/project/page_manager/-/commit/02e263051d80646...

Hey Jonathan,
> Are you sure this is the only place that is problematic to support this? There's probably plenty of code assuming variation types are returned by $product_type->getVariationTypeIds().

Short: Yes.

Long version: I'm running this on a critical site and don't want to shoot me in the foot. So i checked all usages of said method (only 10-ish, most in foreach), and robustified the 3 places where it coughs.

Now i rebased and simplified the MR, re-applied it on my site, and verified all integration tests are still green.

ME applies cleanly on my site and fixes the issue for me.

geek-merlin → created an issue.

So CWAD...

geek-merlin → created an issue.

Right.

I reviewed the code, backported it for a 10.x project, and played with it, and noticed one bug:
- The #toggletip code in theme.inc does NOT attach the library. It should.
- The demo form "manually" attaches the library. It should not.

All in all: Hot stuff!

Note that according to function TranslatableMarkup::__construct | Drupal API the existing behavior of doing a

t($value)

• opens security risks
• prevents template extraction

To fix #13, the for loop should be replaced with sth like:

$entities = $this->entityTypeManager->getStorage($entity_type_id)->loadByProperties([$uuid_key => $uuids]);

geek-merlin → created an issue.