Account created on 10 January 2007, about 18 years ago
#

Merge Requests

More

Recent comments

🇦🇺Australia nterbogt

Available in next release.

🇦🇺Australia nterbogt

Initial code is complete and awaiting review.

🇦🇺Australia nterbogt

This has been outdated by Drush improvements Active . While it doesn't specifically address the issue; the entire architecture has changed and if this is still required we can start again on the changes.

🇦🇺Australia nterbogt

I feel this is a self-created issue. While I agree that taking over the namespace of an existing package is a supply chain issue... the package probably shouldn't have existed in the first place. The underlying problem is that pseudo packages were created (that actually work) rather than a "This is not the package you seek" type response.

I think it's worth discussing changing the way it works now, rather than letting the problem get worse over time.

🇦🇺Australia nterbogt

Cross posting a possible solution from another issue discussing the same problem.

After doing some research into the composer schema, would it not be better to use "abandoned": "drupal/[submodule]" in submodules, suggesting the main module? This would allow a user to find submodules through composer, but also guide them to installing the package correctly. And when a submodule is moved out, it can become a 'real' package and override the abandoned definition.

🇦🇺Australia nterbogt

I do find this counter intuitive that Drupal.org allows me to create a top level project that conflicts with the namespace of a submodule in composer.

The current solution is that the new project gets the namespace drupal/data_pipelines_elasticsearch-data_pipelines_elasticsearch in my example.

Would it not be more intuitive if the submodule got drupal/data_pipelines-data_pipelines_elasticsearch in my example? I assume there is an underlying package manager reason why we need it in the first place; but I can't see it magically causing an issue on an existing site without the developer actually running composer require [package].

We needed to split out the module to support two different APIs concurrently, depending on the site, so there are valid use cases for doing what I did to the modules.

🇦🇺Australia nterbogt

Will be available in the next release.

🇦🇺Australia nterbogt

This is now fixed. It will be rolled out in the next release.

🇦🇺Australia nterbogt

nterbogt made their first commit to this issue’s fork.

🇦🇺Australia nterbogt

The original changes were merged. What is in the MR now is a duplicate of another issue project bot created.

🇦🇺Australia nterbogt

This conversion is complete. Should unblock the submodule creation for elasticsearch and opensearch... or at least reduce the amount of rework we have to do in the future.

🇦🇺Australia nterbogt

This is ready for review.

🇦🇺Australia nterbogt

The module supports views. You can override the path with a view of your creation, with your own filters. That's how we use it.

🇦🇺Australia nterbogt

I'm happy with the changes. The remaining phpstan issues are fixed by 📌 Automated Drupal 11 compatibility fixes for data_pipelines Active .

🇦🇺Australia nterbogt

This ticket is best placed on the admin_toolbar module rather than core.

That said, you can't currently override a deriver as far as I'm aware. The code to do that doesn't exist, see Allow base plugin definition to be altered before derived Active .

You can however override the menu links by using a module hook. Here is an example from our codebase.

/**
 * Implements hook_menu_links_discovered_alter().
 */
function MY_MODULE_menu_links_discovered_alter(&$links) {
  foreach ($links as $route_name => $link) {
    // Remove all the individual views from the menu.
    if (\str_starts_with($route_name, 'admin_toolbar_tools.extra_links:views_ui') && !\in_array($route_name, ['admin_toolbar_tools.extra_links:views_ui.add', 'admin_toolbar_tools.extra_links:views_ui.field_list'])) {
      unset($links[$route_name]);
    }
  }
}
🇦🇺Australia nterbogt

Finished the changes. Not sure I have permission to create a merge request.

🇦🇺Australia nterbogt

Deployed in release 1.1.0-rc1.

🇦🇺Australia nterbogt

Merged to 1.1.x. Release to follow.

🇦🇺Australia nterbogt

Can someone have a look over this. Should be no functional changes, just making the whole build pipeline green.

🇦🇺Australia nterbogt

Tests seem to be running as expected on current versions of Drupal. Can re-open / create a new ticket if this comes back.

🇦🇺Australia nterbogt

Agree. Created a 1.1.x branch for 10.3+ Drupal version support.

🇦🇺Australia nterbogt

I've merged to 1.1.x for now... have to make a few other changes before I can roll a release. So will confirm if composer restrictions are working as expected.

🇦🇺Australia nterbogt

I ran this on a site with 31 entities and 1039 fields and it took 1.9 seconds on my local machine. I'm not sure this qualifies it as slow; it's certainly not slow compared to rebuilding node access as an example.

🇦🇺Australia nterbogt

This has been merged and will be in the next release.

🇦🇺Australia nterbogt

Merged. Sorry, forgot the commit message.
Will go out with the next release.

🇦🇺Australia nterbogt

I agree with the sentiment behind this ticket; but not necessarily the implementation.

I'd like to see the code automatically leverage the cache `url.query_args` bubbleable metadata, so that implementing it correctly in a block or similar, automatically enables that argument in the page cache ignore.

I haven't quite worked out how to do that yet.

But both options have merit and it would be worth doing an analysis over which route should be taken (pun intended) :)

🇦🇺Australia nterbogt

Merged and will be available in the next release.

🇦🇺Australia nterbogt

Merged for next release.

🇦🇺Australia nterbogt

This has been merged and will be available in the next release.

🇦🇺Australia nterbogt

This has been merged and will be available in the next release.

🇦🇺Australia nterbogt

nterbogt made their first commit to this issue’s fork.

🇦🇺Australia nterbogt

This has been merged and will be included in the next release.

🇦🇺Australia nterbogt

nterbogt made their first commit to this issue’s fork.

🇦🇺Australia nterbogt

Is this still an issue? Can you confirm whether the fix recommended by pookmish solves the problem?

🇦🇺Australia nterbogt

Ready for review.

🇦🇺Australia nterbogt

Ready for review.

🇦🇺Australia nterbogt

Can we update the target branch of the merge request to 2.x please? I'm not sure how to do it or I don't have permission.

🇦🇺Australia nterbogt

Local testing had the number of fields go from 253 to 73 and now doesn't contain items that have empty bundles.

🇦🇺Australia nterbogt

This patch caused significant issues for us for non admin users. I'm yet to look into what is different in the HTML structure between admin and non admin users that might be affecting it.

🇦🇺Australia nterbogt

I'm just going to bump this one to RTBC because it's a filename change and I ran it locally.

🇦🇺Australia nterbogt

Thank you for you submission. Converted patch to merge request.

Now all this needs is tests.

Production build 0.71.5 2024