I'm rerolling the patch against 2.x release. I'm including a fix regarding the BOM issue, because is a blocker in order to take the unique_identifier in account if that field is the first column in the csv. I'm taking this solution https://www.php.net/manual/en/function.fgetcsv.php#122696
akalam → changed the visibility of the branch 3487820-layout-context-in to hidden.
The fix makes totally sense, thanks for reporting and fixing!
Created a MR fixing the issue. Ready to review
akalam → created an issue.
akalam → created an issue.
I've found an issue when trying to add a entity field block on layout builder. Seems like the best solution is to filter for the contexts that are both required and empty, so non required contexts are still available.
I'm adding a MR instead of a static patch.
We've found a similar issue when working with the groups module. We've created a custom group_permission_calculator service, similar to the new Access Policy API but for groups. In that service, we are trying to use the \Drupal::routeMatch()->getParameter('group_content'), and that method is returning null when accessing to group_content pages. While debuging, we've found that the call to the access check comes from the following function:
\Drupal\user\Plugin\LanguageNegotiation\LanguageNegotiationUserAdmin::isAdminPath
This is the problematic code:
$route_match = $this->stackedRouteMatch->getRouteMatchFromRequest($request);
if ($route_match && !$route_object = $route_match->getRouteObject()) {
try {
// Some inbound path processors make changes to the request. Make a
// copy as we're not actually routing the request so we do not want to
// make changes.
$cloned_request = clone $request;
// Process the path as an inbound path. This will remove any language
// prefixes and other path components that inbound processing would
// clear out, so we can attempt to load the route clearly.
$path = $this->pathProcessorManager->processInbound(urldecode(rtrim($cloned_request->getPathInfo(), '/')), $cloned_request);
$attributes = $this->router->match($path);
Replacing the "router" service by "router.no_access_checks" is solving the issue, and seems ok in order to check if the current route is an admin route or not. The change can provide also a beneficial side-effect in terms of performance.
I'm adding a patch with that change and linking a couple of issues that might be related.
The previous patches had a side effect: When you are trying to add a block on a node, the node fields for all content types are available which is not nice. On the other hand, according to the name of the method "getPopulatedContexts()", make sense how it works, using the runtime context instead of the available contexts.
Therefore I've changed the approach, and I'm getting the available context only on the \Drupal\layout_builder\Form\ConfigureBlockFormBase::doBuildForm
Even when the previous patch was avoiding layout builder to filter some contexts, many context definitions are not complete (for example they don't have label). For this reason when having different contexts for the same entity type, they use the entity type name instead of the context label.
Calling the method getAvailableContexts() instead is doing the trick, and now the labels are properly setup. Among that, is more performative to call just the available context and not the runtime contexts if we are not going to take care if the context is populated or not.
I come into this issue in the following scenario using the "group" module.
We are using the group content entity to render the full view mode of the node, and we are using layout builder on the nodes. The problem is with the blocks that have group context, because the run time contexts are not selectable on the block config form because we don't have the context of the group populated (since we are on the layout builder of a node), but the context of the group will be populated properly when viewing the node because it's gonna be rendered within the group content entity, and therefore accessed within a route with the group context.
The general idea is that not having the context populated on layout builder doesn't mean that the context won't be populated when viewing that entity. Filtering the contexts based on that precept will make layout builder less functional compared to the block layout, where no contexts are populated and they are not filtered.
I'm uploading a simple patch just removing that filtering so all contexts will be available to select.
Patch #32 is using "this" instead of "that" in a context where this is the windows object. Here's a patch fixing it
I can reproduce it after upgrading to 10.3.1
The patch solved the issue
As reported in the previous comment, patches #50 and #52 were causing a fatal error, so I was creating a new one based on the patch #49. However, after testing, seems like the patch #49 was incorrect too, because is not loading the entity from uuid on the getEntity method as it's being done in the MR, and therefore the patch #53 is incorrect as well.
I'm uploading a direct reroll from the MR against core 10.3.1 in case anyone needs it.
Patches #50 and #52 are not including the function ViewsConfigUpdater::needsTidFilterWithMultipleVocabulariesUpdate which is being used on the hook post update.
I'm uploading a reroll against 10.3.1 based on patch #49
@monkk the constructor is declared as final on ActionBase on ECA v2, so it can't be overridden. We have removed that override and used an alternative method for dependency injection.
This should be fine in beta5.
Thanks for reporting
akalam → created an issue.
Patch 73 was introducing a regression: In the context of an Ajax call, there are situations when the wrapper id is in the response, but we don't want to replace just the partial but the whole response. This is the case of the media library after adding a new media, because the div containing the media add form, has the same id as the form with the file upload at the top of the media list.
As consequence, with the patch applied, when you add a new media through the media library, the dialog gets replaced with just the file upload form, so the auto-insert feature doesn't work (because the media list doesn't get replaced).
As a solution, I'm proposing the replacement of partial response to be an opt-in (disabled by default). This setting can be activated through a data attribute (data-replace-partial="true") on specific links, so it can be used keeping the default behavior untouched.
That's our case, we have 2 different plugins for nodes, the default provided by groups gnode sub-module, and a custom one.
In order to mitigate the issue I'm adding here a patch as starting point, adding a new setting to the widgets to select the plugin id, to use the chosen plugin id instead of the first one. If the plugin id setting is empty, will use the first one for backward compatibility.
The widget seems to work fine on creating the group content entity, but on listing the existing group contents, is still listing all of them, and not only the group contents of the selected plugin id.
However, I'm sharing the work in progress just in case it helps to somebody else.
amateescu → credited akalam → .
Provided a MR and uploaded a static patch implementing the countable interface in the Attribute object
The wrapper ajax setting is not being considered when replacing the content. That can causes issues when the wrapper is set to something different than the content region. Here is a patch fixing it. Basically if the wrapper exist on the content inside the ajax response, we reduce the content to just the "wrapper" element. If the wrapper element is not found, we keep the content as it is, replacing the wrapper with the content from the ajax response.
Created a MR implementing a fix using a form alter. I had to enable the static_caching on the facets config entity to ensure the active facet values are still available when loading the facet on a later phase (in our case the form alter). The general activation of static caching of config_entities is being discussed here #1885830: Enable static caching for config entities →
akalam → created an issue.
This issue might be related? 🐛 Exposed filters get values from url when ajax is on Needs work
Patch on #28 didn't work for me using facets v3.
I'm uploading a new patch using a simpler solution, skip url querystring on the settings object being passed to the ajax method on javascript. By this way, all the input from the form is being sent, but the querystring is not, so no override on parameters is raising the server.
Uploaded a new version of the patch to use EntityContextDefinition instead of ContextDefinition, otherwise there's an error:
AssertionError: assert(!str_starts_with($data_type, 'entity:') || $this instanceof EntityContextDefinition) in core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
I can't reproduce the bug anymore following the steps from #46 on drupal 10.2
Rerolled against 10.2 and introduced a change to make it compatible with the change introduced on 🐛 Prevent empty block_content info fields from causing php deprecation notices when placing blocks with no label. Fixed
This one is for 10.2.5
Rerolled #93 against core 10.2
This looks like a duplicate of 🐛 Cache tags from Computed fields do not bubble up to Entity render array Fixed , which has been committed on 10.2
Rerolled #69 for 10.2
Re-rolled against core 10.2
Re-rolled against core 10.2
I've created a MR rerolling the patch from #33 to the latest Drupal 11.x release. I'm attaching also a static patch to apply safely from composer
This module is intended to be used until the patch on 📌 Allow synced Layout override Translations: translating labels and inline blocks Needs work gets committed to core, so I wonder this issue can be closed, as the incompatibility seems natural.
The patch that conflicts with this module is the one on 📌 Allow synced Layout override Translations: translating labels and inline blocks Needs work
I'm adding a MR and a static patch with the missing methods
Here's a patch adding back the counter depending on the facet configuration.
This is great! the patch is displaying the facets in a hierarchy manner. However the counter disappear when the patch is applied for both hierarchical and non hierarchical facets.
I will take a look on and try to provide a fix.
I've found when you have a menu link derivative, and you are using an TranslatableEntityLabelMarkup as title, the title gets overridden by the static_menu_link_overrides, transformed into a string and not displaying the proper translated text.
I'm adding a patch with a fix so the title override skips in case it has been defined as a TranslatableEntityLabelMarkup.
Note: I've found the patch doens't apply anymore for the 11.x branch, so I'm uploading it as it is, just introducing the change mentioned above. The patch is applying on 10.1.x
This issue exists on 1.x branch as well. The patch is applying and solving the issue on that version too
Added a new commit to fix an error because the entity.entity_subqueue.add_form doesn't have the entity_subqueue as route parameter.
Hi @artemboiko the problem with patches from MR is that they are dynamic, they are generated automatically from the MR and the MR can change at anytime. It can lead on your site being deployed introducing untested code, or even the patch not applying without prior notification. There's a big discussion on this topic on the following issues: 🐛 GitLab Merge Requests Unable to Generate Incremental Patch Files Active and #2488266: [META] Improve Git workflow on Drupal.org by implementing issue workspaces →
Static patch updated with latest changes from MR
I'm uploading a static patch to apply from composer
Thanks for the MR @artemboiko! I'm still seeing the links for the operations which the user don't have access on the "Entity Queue" tab on entities, so I'm adding a new commit to fix it.
In the commit I'm checking the access to the url instead of checking the access to the entity operation. That's because the access could had been extended by other modules (like the group_entityqueue for example or custom access requirements) and by checking the access to the url will work properly on all scenarios.
Uploading a patch to apply easily from composer
Thanks for reviewing and applying the fix!
Thanks!
I've created a MR with a fix. Not sure if it's the optimal solution but it works.
Created a MR with the initial implementation of the new "Combine Ldap attributes" view filter
akalam → created an issue.
I'm uploading a patch based on the MR to apply safety from composer
Patch on #5 is also fixing the integration with other modules (I'm working with layout_builder_lock for example), because it's adding third_party_settings to the Section as well, already supported in core since 8.7: #2942661: Sections should have third-party settings →
This patch is compatible with version 8.x-2.0-alpha6
The MR !5263 worked for me in Drupal 10.1. Uploaded a static patch to apply safety from composer.
Created a MR and uploaded a static patch to apply easily from composer.
akalam → created an issue.
The MR 165 wasn't solving the issue. I created a new MR with a different approach. The problem seems to be during the creation of the view, not during the creation of the facet. Seems like the depends on the view (this is correct) but view is having the facets as exposed filters so the facet source is being initialized at that time. The new approach is to skip the initialization during the config import.
akalam → changed the visibility of the branch 3413405-fatal-error-when to hidden.
Created a MR against 3.0.x branch, and uploading a plain patch version to apply safely via composer.
akalam → created an issue.
Might this issue be deprecated?
I'm uploading a patch based on the latest version of the MR to ensure it can be applied safety via composer. Tested and working fine in Drupal 10.1.7. Thanks!
Uploading a patch based on the MR 19
The MR looks good to me. Thanks! I'm uploading a static patch based on the MR 14 to use it easily with composer.
The patch on #3 is working fine in case there's a single exposed form block. In case there are many instances it will replace only one of them. I'm adding here a modified version using data-drupal-selector instead of the id as selector for the replacement.
I have tested some of the patches attached in this issue (#84, #86 and #93) and all of them work with ajax disabled. Having 2 exposed forms with ajax enabled avoid the exposed form block from being replaced (both don't get replaced). Without the patch at least one of the blocks get replaced but with the patch none of them.
Finally, our issue was coming from the following patch: https://www.drupal.org/project/drupal/issues/2605218#comment-15002530 🐛 Views Block Display skips preBlockBuild() call on ajax rebuild Needs work
After applying the latest version of the MR on 🐛 Views Block Display skips preBlockBuild() call on ajax rebuild Needs work the issue went away!
We have personally found this issue when trying to update a view to use facets v3 as exposed filters and ajax enabled. The view have 7 exposed filters (6 of them are facets) and 1 exposed sorting criteria. The error happens after changing the filters a few times (not too many), for example, applying the filters one by one.
I can confirm the commit on the issue 📌 Compress ajax_page_state Fixed helps by reducing the size of the request uri, but doesn't solve the duplication when consecutive ajax calls get triggered on a Drupal 10.1 instance.
The MR doesn't solve the issue and is changing the indentation wrongly. There's a working patch with a proper solution: 🐛 Library detection warnings when using contrib version of CKEditor 4 Needs review
The MR is correct because the $hex variable is a string so should use "." to concat. I fixed a wrong format with the if closing bracket.
akalam → made their first commit to this issue’s fork.
Thanks @tonibarbera