ContentCreatorVisibleXbConfigEntityAccessControlHandler's `view` access must refuse access to disabled config entities

wim leers → credited penyaskito → .

Tests still needed.

This has new tests, removes xb_visible_when_disabled, and behaves the same way as HEAD.
The only issue is that JavaScriptComponent is singled-out:

    $require_status_true = match ($xb_config_entity_type->getHandlerClass('access')) {
      ContentCreatorVisibleXbConfigEntityAccessControlHandler::class => FALSE,
      XbConfigEntityAccessControlHandler::class => ($xb_config_entity_type->id() !== JavaScriptComponent::ENTITY_TYPE_ID),
      default => throw new \LogicException(),
    };

This means that Pattern probably should have had the xb_visible_when_disabled flag too.
NR for confirmation.

wim leers → credited larowlan → .

mglaman → changed the visibility of the branch 0.x to hidden.

RTBC, rerolled @penyaskito work. Called out a few items to discuss which look OK.

This lost multiple performance optimizations for the crucial /xb/api/v0/config/component response — which is what is a critical factor in booting the XB UI! This is a big, and expensive response, and so the existing optimizations must be kept if at all possible.

Posted comments on the MR to explain this and pushed commits to bring those back while staying true to the spirit of the issue. There's at least one failure in need of debugging still.

Approved on the assumption that the remaining failure will be fixed while keeping both optimizations (the config entity query optimization as well as the cache tag response header optimization).

CI failing but Wim made good fixes, I'll address

Dug deeper, because this can't land without passing e2e tests that this caused to fail.

The root cause for all the painful bits that were surfaced here is type: field.value.datetime. Introduced in 2014. Zero issues mention it! 🤯

1. It dates back to #1989468-4: Weird messing with 'default_value_function' in date widgets ? → (June 5, 2013 — aka 12 years and 1 day ago today), which I apparently reviewed (zero recollection)
2. It has been causing confusion for years: 💬 The configuration schema for datetime doesn't describe the field properties the way other field types do. Closed: works as designed .
3. It's actually still actively being refined: 🐛 Better handling of timezones for relative default date + times Active .

Wow! 🤯

I first wanted to open an upstream (core) issue titled Allow creating a default field value from a valid field value — but then I realized that literally defeats the point of #1989468-4: Weird messing with 'default_value_function' in date widgets ? → — the very point is that no hardcoded value makes sense for datetime — only: "now" and "X time before/after now".

So, instead opening an XB follow-up. Because this is the first time the shape matching logic and infrastructure ends up … not working. 😬

Attempted to fix it here, by special-casing the datetime and daterange field types in ::computeDefaultFieldValue(). 🤞

Need to shift to other things now…

Manual testing suggests that I succeeded in adding ::processDefaultValue() support? 🤞

FYI: this is wildly out of scope here, but it happens to have been surfaced here 😬😭

Fortunately, all the access control changes are basically trivial, which is why this MR's size is still very manageable. So if #20 passes, then I'm gonna go ahead and merge, so we can all move on and pretend this was a bad nightmare 🧟‍♀️

I hope all remaining failures are solved by 🐛 Deterministic Component version hash should depend not only on source-specific settings, but also slots + explicit input schema Active . Because they are AFAICT all like this:
[versioned_properties] The version 6c1061657e221db7 does not match the hash of the settings for this version, expected 0273dc5c85818057.

This is postponed because we added lookup keys, right? Can we un-postpone this ticket is adding lookup keys for performance was spun out and blocked on that other ticket? That way we land the new access handlers albeit with a performance hit until lookup keys added?

Edit: we can still keep condition check but just not check lookup keys, so same as it was before

Excellent call, @mglaman. Will make it so.

Note that lookup keys already exist (PageRegionuses them too). But it's the combination of

• ApiConfigControllers::list() being modified here
• restoring the Component config entity's lookup keys, to keep the intended optimization, even though it's absent in HEAD
• a very old bug obscured through multiple layers of abstraction

… that caused it to go unnoticed.

Splitting out is worth it here to A) allow progress, B) improve archeology. Will do so 👍

Done.

I'm calling it a day — 🐛 Deterministic Component version hash should depend not only on source-specific settings, but also slots + explicit input schema Active was incredibly tricky too. Braindead now.

I'll need to file a follow-up next week to bring back https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

Merged!

Happy weekend everyone.

Created 📌 Implement `lookup_keys` in Component for optimized queries Active

wim leers → closed merge request !918

Automatically closed - issue fixed for 2 weeks with no activity.