Account created on 7 May 2013, over 11 years ago
#

Merge Requests

More

Recent comments

🇬🇧United Kingdom andy_w

So this is the original route I was looking at here: https://git.drupalcode.org/project/drupal/-/merge_requests/10144 (from https://www.drupal.org/project/drupal/issues/3486604#comment-15852875 🐛 Node edit form re-saves menu items even if no change has occurred Active )

🇬🇧United Kingdom andy_w

Two things, we are trialling the approach above and it seems to be working in our scenario, but obviously couldn't account for all possible options.

Secondly I totally agree that fixing this in menu_ui would certainly be the best approach, I failed to achieve this last time (it was my first area of interrogation, but I'll take another look).

🇬🇧United Kingdom andy_w

This is not an issue with purge but an issue with drupal core cache invalidation with menus:
https://www.drupal.org/project/drupal/issues/3486604 🐛 Node edit form re-saves menu items even if no change has occurred Active and https://www.drupal.org/project/drupal/issues/3485030 📌 MenuTreeStorage shouldn't invalidate cache tags if the menu didn't change Active

🇬🇧United Kingdom andy_w

Just following up on the comment by @berdir
The thread was here: https://drupal.slack.com/archives/C079NQPQUEN/p1731584309641329
And the related issue is here: https://www.drupal.org/project/drupal/issues/3486604 🐛 Node edit form re-saves menu items even if no change has occurred Active (now closed as a duplicate of this)

🇬🇧United Kingdom andy_w

I agree @dmitry.korhov

Closed as duplicate of https://www.drupal.org/project/drupal/issues/3485030 📌 MenuTreeStorage shouldn't invalidate cache tags if the menu didn't change Active

🇬🇧United Kingdom andy_w

My assumption is that the failing tests (in unrelated aspects of the system) are based on the fact that they require a cache clear and are relying on an incidental cache clear to occur based on this change.

🇬🇧United Kingdom andy_w

It would appear that this was flagged as a follow up action many years https://www.drupal.org/project/drupal/issues/2302137#comment-12070910 and further discussed here: https://www.drupal.org/project/drupal/issues/2876343

However this has a significant impact upon a sites performance if every time an editor saves a page in the menu system (without making a change to the menu) the whole site is essentially cleared from the cache.

🇬🇧United Kingdom andy_w

andy_w changed the visibility of the branch 3486604 to hidden.

🇬🇧United Kingdom andy_w

The actual issue here appears to stem from the fact that the doSave method on menu tree storage still returns the current menu even if no changes were detected - causing an unnecessary cache clear.

The code suggests an exit early state but it still returns with the current menu, causing follow on actions.

🇬🇧United Kingdom andy_w

andy_w changed the visibility of the branch 3486604-node-edit-form to hidden.

🇬🇧United Kingdom andy_w

As I understand it $entity->hasTranslationChanges() will only give us the output we are after if the menu is setup to use translation, which is why I wrote it off in the first place (though I could be wrong, and hope I am, as it would be a much nicer solution).

🇬🇧United Kingdom andy_w

Its worth mentioning that menu link weight has a similar patch for a very similar observation,

🇬🇧United Kingdom andy_w

We found the use of fromUserInput and getPathByAlias to be the source of the performance issue, and wondered whether there would be a benefit in making use of some of the other data available in the options, i.e. potentially the entity itself or the href as the first line of defence.

🇬🇧United Kingdom andy_w

Currently this module works fine if workflows module is not used, or if the content notify date is set manually in the field.

However the default date is ignored currently due to the erroneous return (as highlighted above) if the workflows module is enabled but transitions are not used as part of the module configuration.

This patch does not go back and rectify old data, if you had been using this module with existing data this patch will not fix existing data, but will allow the default to be populated on new and updated content going forward.

🇬🇧United Kingdom andy_w

andy_w created an issue.

🇬🇧United Kingdom andy_w

Assuming this is no longer an issue, given the timeframe.

🇬🇧United Kingdom andy_w

I suspect that you would need https://www.drupal.org/project/role_delegation (but closing this ticket now, as I suspect the user has solved there issue).

🇬🇧United Kingdom andy_w

I would like to apply to be a maintainer of this project, we make use of it, and have the amends to make a drupal 10 ready version.

🇬🇧United Kingdom andy_w

Added a sub-module, currently only works with the media gallery image upload.

🇬🇧United Kingdom andy_w

Apologies, I missed this, I agree, and have just added a few extra comments.

🇬🇧United Kingdom andy_w

In order to resolve the issue we need to use the BookManagerInterface as drupal 10.2 changed to use a proxy class for the book manager (see https://www.drupal.org/node/3397515 )

🇬🇧United Kingdom andy_w

The mysql Connection now resides in its own module, so I've attached an amend to the patch.

🇬🇧United Kingdom andy_w

As it stands at the moment the suggestion mode simply populates the alt text field on upload, ready for the user to either overwrite or accept (i.e. not overwrite) the suggestion.

🇬🇧United Kingdom andy_w

It strikes me that I must have missed something, but it seems like extracting the other associated issues we are left with the fact that the edit / delete links do not currently work if the field is not a multi value element (i.e. it has no delta). Would an adequate solution to this be the assignment of a delta of 0, as this appears to allow the editing/deleting of the scheduled instance.

Apologies if I have misunderstood the ticket.

🇬🇧United Kingdom andy_w

I began looking at this issue, and could see that the forward revisions were not included, the attached patch will cause future revisions to be included in the dynamic view. There is still an issue around moderation state and datetime that requires further work.

So in short the patch will show all valid rows, it will however be missing the future publishing state and execution time.

🇬🇧United Kingdom andy_w

Added a patch for this behaviour.

🇬🇧United Kingdom andy_w

Thanks @jeroen_vreuls I missed that, and it turned out that caused issues for us as well, and low and behold you had already fixed it!

🇬🇧United Kingdom andy_w

I've re-factored the patch to work on 1.1 and switched to using a checkbox as it does make sense to either redirect to the front page or to the setup form.

🇬🇧United Kingdom andy_w

Sorry about that, a very good reason not to copy & paste a solution into another file :facepalm

🇬🇧United Kingdom andy_w

One possible solution in light of the inline JS being used to prevent this problem in the toolbar (admittedly not a nice solution) would be to use a similarly unpleasant inline css solution to prevent that initial flash of menu.

I offer this merely to see if it sparks an alternative solution, I will continue to try to find a better solution when I can find a bit more time.

🇬🇧United Kingdom andy_w

Thanks for identifying this issue jerrac, I have updated the patch now that the node_type condition has been removed (see https://www.drupal.org/project/drupal/issues/1932810 )

🇬🇧United Kingdom andy_w

I have made a start on the process to update this drush command to work with drush 11, this is not yet fully tested but I'm aiming to find some more time to work on this soon.

🇬🇧United Kingdom andy_w

As this issue is still ongoing, I'm not sure if this is helpful but here is the updated patch from 34 to work with the 2.x branch.

🇬🇧United Kingdom andy_w

Additional to this Drupal 10 also requires all queries explicitly set accessCheck as part of the query (see https://www.drupal.org/node/3201242 ) therefore the request to fetch all view_mode_page_patterns requires ->accessCheck(TRUE) to support Drupal 10.

🇬🇧United Kingdom andy_w

Potentially a new suggestion plugin for handling media absolute url's.

🇬🇧United Kingdom andy_w

We had a similar use case, and I'm afraid I'm not sure if this is helpful for either this use case or the module in general (apologies if not).

We needed to extend the module to utilise the source entity (i.e. when displaying a form on a node page) and then display that source entity dynamically in a given view mode.

Attached is the patch that we used for this (great module btw, thanks).

🇬🇧United Kingdom andy_w

Really impressive find, but as you mentioned it does seem to be throughout the drupal core codebase. Including the tests themselves. So I've added the update to the the tests in BrowserTestBaseTest (which failed above).

Production build 0.71.5 2024