I believe this change exists in 10.2 already and can be closed.
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/misc/ajax.js?...
PR looks good to me :)
fubarhouse → credited 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.
nterbogt → created an issue.
nterbogt → created an issue.
I'm just going to bump this one to RTBC because it's a filename change and I ran it locally.
Can we merge this to 5.x too when it's released?
Thank you for you submission. Converted patch to merge request.
Now all this needs is tests.
Updating to 2.x-dev.
Fixed. Will be rolled out in 2.3.0.
nterbogt → created an issue.
Fixed. 2.3.0 release to follow.
nterbogt → made their first commit to this issue’s fork.
This is a simple change, but will conflict with the other issue I put in review. I'll update whichever afterwards.
I decided to run all the tests again with the new entity_test_parent (no parent field) and entity_test (with parent field) as children.
Let me know if this is over testing.
nterbogt → created an issue.
Released in 2.0.0-beta2
Deployed in 2.0.0-beta1.
nterbogt → made their first commit to this issue’s fork.
I'll roll a release shortly.
It's one and the same... I've been working on the code for the Children tab in 5.x quite a bit :)
Is this opt in? Slightly concerned it could break our 'Children' tab when we have a `5 / 70 / 10 / 6` type hierarchy. (format of child summary in views).
Ok, misunderstood the code. Back to passing now. (Note to self) Next time, just run the module in a sandbox with working tests... makes it easier :)
Tests failed. In theory there was no functional change from previous code. I'll look into it.
nterbogt → created an issue.
Sorry, forgot status change.
Updated with new method to get the list builder and tested locally.
The local task deriver changes were because I originally had another change in mind, but rejected it. The change only removes unused variables and uses constructor promotion, bringing it in line with other modernised code in EH 5.x. So decided to include it.
I think this is ready for a review. At least in terms of the functionality it provides. Happy to discuss specific implementation.
nterbogt → created an issue.
nterbogt → created an issue.
jibran → credited nterbogt → .
nterbogt → created an issue.
larowlan → credited nterbogt → .
Confirm this works for me too.
I believe this is the same issue that I'm seeing.
URL that comes in is /sites/default/files/2022-05/Vivid.jpg
Proxy request is stage_file_proxy.ERROR: Stage File Proxy encountered an error when retrieving file https://my.host///2022-05/Vivid.jpg
I gave this some thought and decided to just remove the reference to 'Test'. 'entity' is globally used and the other options considered were:
* Add the current content entity to the ChildEntityWarningBuilder and then pass it through to the ChildEntityWarning. And then render the current entity type into the message.
* Assume entities in the ChildEntityWarning->relatedEntities were always going to be the same type, so grab one and render the entity type into the message.
In both cases, does that added complexity actually add value to the user. Probably not... they are already on the edit page of what they want to edit.
nterbogt → created an issue.
nterbogt → made their first commit to this issue’s fork.
This has been fixed and rolling release 1.0.0-rc1.
Can I please get a review of this. Phpstan throws only a couple of warnings on 10.0, but tests currently set to 10,1 which throws a Database::tablePrefix() issue.
nterbogt → created an issue.
nterbogt → made their first commit to this issue’s fork.
nterbogt → created an issue.
nterbogt → created an issue.
Added a functional test for the settings form. Has a phpstan warning, but it's the same warning in a lot of other projects.
https://git.drupalcode.org/project/token/-/jobs/275660
The warning is documented here.
https://www.drupal.org/docs/develop/development-tools/phpstan/handling-u... →
I agree with the complex logic in the form, I'm also not really sure about a decent approach to test that. Do you have any recommendations?
A unit test that just checks the default values and possible options could work, but might not be what you're after.
This is causing significant user complaints for our site too. There are many people confused about what is happening and why.
I'm trying to get this to RTBC, but will have to come back a bit later.
I don't believe this needs to be optional / opt-in. I think it's clear it should be an admin route, next to 'View', 'Edit', 'Delete', 'Revisions', etc. I'm not sure there is any UX benefit to using the page theme and having it inconsistent with everything else.
It's also a duplicate of 🐛 The generate-preview-link page renders in the site's theme RTBC , which I know was created after. But might be easier to push the individual changes through rather than globbing them together.
I agree that this is a poor user experience. Run the change locally, working as expected.
This is ready for a review please.
I've redone the permission form configuration. Also moved it to a merge request.
I still need to implement the bundle configuration, but the rest is done, as far as I know.
nterbogt → made their first commit to this issue’s fork.
Looks like this is actually fixed in latest release.
nterbogt → created an issue.
This appears to be an own goal. Sorry. Closing.
I'll investigate further before anyone should look at this. Might be a PEBKAC problem given my current week.
nterbogt → created an issue.
I ran into the same issue. Test above RTBC, but needs tests updated.
Re-rolling patch for 10.1.x.
Re-rolling patch for 10.1.x. Haven't checked if this is working for 10.0.
This change is supported at a functional level. But I don't think we should merge it until the bug in core is fixed... or 📌 Prevent selection of unsupported toolkits Active is implemented and merged and we can detect that you have the required patch.
For ease of finding, core patch listed above is ✨ Let GDToolkit support AVIF image format Needs work
Removed code that wasn't used, and if was used could break your site. Can add a similar function back in with appropriate tests and a use case (like hook_uninstall()).
This has been merged. Another DDoS risk mitigation!
This has been updated. Thanks everyone for getting this over the line.
Sorry, that was me. Merged and marking fixed.
Closing ticket as it has been fixed in another PR.
Also merged. No longer as required, but a nice tidy up none the less.
This has been merged. I won't roll a new release just yet because there appears to be a bunch of other changes stacked on this one.
If no progress has been made by the end of the week, I'll roll a new release.
It's good. Just rechecked locally.
Just checking the patch again.
Unable to test as a Kernel test. Route wasn't registering properly.
A functional test in the meantime.
I think this is ready for a review.
Small logic fix. Tests don't cover the case where there is a recursive reference to self.
Improving patch with early exit for no descendants and adding accessCheck() for query.
I've had a go at a patch for this and confirmed no difference in the $children array for my test case.
Limitation of my patch is that it assumes all children will be the same entity type as the one being validated, which based on the field definitions, I believe is a safe assumption. Could alter the query to include the type otherwise.
This patch means we don't run out of memory with 28k children nodes (don't ask).
On a page with 6 children... the entity query appears marginally slower. 4.1 seconds (entity query) vs 3.55 seconds (loadMultiple).
nterbogt → created an issue.
Just wanted to add a comment to say that the upstream epic that was being tracked appears to now be complete and was marked for release in v38.0.0 of CKEditor 5.
My thoughts are different for community vs NSW use cases.
For community, I think the record should be deleted from the index if it becomes invalid. This gives you data integrity.
But based on HA strategy for NSW, we would normally prefer to provide stale content than a 'page not found' while the problem is resolved.
I think we go with the community solution in this case, and if we want to override it, we can look at that later as a hook or otherwise on our end.
Just to confirm... I don't think it should be a sub module, but another project on drupal.org. My personal feeling is that we need to move away from including everything in one mega module... that was good before we had composer. Now we can do better :)
Don't we actually want ->accessCheck(FALSE) for this one? It doesn't really matter who does the rebuild task, if they have access, they should rebuild for everything?
I'm happy to make you a co-maintainer, I think you've done good things for the project. I just want to get a commitment not to include code to help 'clean up duplicates'... the module is about identifying and restricting them. I think clean up code is a one run problem... and complicates the code and maintenance. It's also very installation specific.
We could look at a `media_duplicates_cleanup` module.
An initial implementation based on entity_hierarchy. Tested in context of nodes, but not wider than that. Also tested above pseudo code for route permissions.
nterbogt → created an issue.
I think it comes down to whether we support NULL or "" in other yml config. If we do, this is a bug... if not, then it's just identifying a misconfiguration that it could cope with before.
Actually, I think I might have solved a different issue. More thinking required.
Setting this to needs review. The risk of an empty label is gone, and the bug is clearer, imo.
I debugged this further and the error appears to be caused by the strict 'string' argument to `strnatcasecmp`.
Drupal is passing in strings or `Drupal\Core\StringTranslation\TranslatableMarkup` objects... which used to work when typecast to string.
Here is a rebuilt patch to apply cleanly to 9.2.x.