griffynh → credited acbramley → .
Happy to close, I think splitting the update hooks now may be more disruptive. It's easy enough to remove the new attribute manually.
@anybody nope, I upgraded from 1.4. But that's exactly my point, I don't want the attribute so why is it being added to existing installs when menu_link_attributes_update_8002 is just about correcting labels and descriptions? Perhaps there should've been 2 upgrade hooks.
Just for my understanding: For what reasons are you removing it? I guess you don't need it and it fills up the UI?
I don't need it, it has no purpose in my site.
I don't think we need dedicated permissions.
I imagine this will end up being a meta with conversions being split up in some logical format, perhaps by module or by hook?
Echoing what I said back in #37 this is awesome to see this issue get so close, I really hope we can get to a state with the documentation that we can move some things to follow ups (that could block next minor) and get this thing in. Kudos to @nicxvan for the incredible effort recently!
I would love to test this on some of our projects that heavily use Hux and see what the migration path would look like. I'm hoping it's as simple as a namespace change on the attributes (and probably a class name change). I don't think that'll be possible though until the sites are on Drupal 11.
The upgrade path here added the container_class attribute even if it wasn't configured. This results in adding the attribute to sites that have removed it. Opened
🐛
menu_link_attributes_update_8002 adds container_class attribute if disabled
Active
acbramley → created an issue.
Thanks @theemstra, can we get a new stable release with this included?
Rebased and removed the cache contexts. From manual testing I don't see a local task on non parentable pages, let's see if anything fails.
This is still an issue https://git.drupalcode.org/project/entity_hierarchy/-/blob/3.x/src/Routi...
This effectively makes every node page uncacheable by DPC for users with access to the Children tab.
@smustgrave yeah I think that's a good idea, or perhaps just 2.1.x, since we're also dropping core versions here. That'd also mean we could ditch the upgrade path tests which are failing on next major
After a lot of faffing about trying to get previous major to work I realised that I'm dropping D9 support here anyway so it's going to fail... This is ready for a review.
acbramley → created an issue.
MR is green, let's get 'er in!
acbramley → created an issue.
acbramley → changed the visibility of the branch project-update-bot-only to hidden.
Opened https://git.drupalcode.org/project/views_data_export/-/merge_requests/45 and resolved issues with tests.
Rebase MR once 📌 Tests failing on 8.x-1.x-dev RTBC is merged.
acbramley → made their first commit to this issue’s fork.
acbramley → created an issue.
acbramley → changed the visibility of the branch 11.x to hidden.
acbramley → changed the visibility of the branch 1452100-private-file-download to hidden.
Tests are passing!
acbramley → created an issue.
acbramley → made their first commit to this issue’s fork.
Available in 1.1.0
acbramley → made their first commit to this issue’s fork.
Available in 1.1.0
acbramley → created an issue.
One failure was related, the other didn't seem it.
acbramley → made their first commit to this issue’s fork.
Thanks a lot @japerry!
Hey @japerry I've noticed this on a few issues of yours now but you seem to be forgetting to give credit for all the people that worked on these issues. Many people on this issue were not credited for their work. Hopefully this is just an oversight, would appreciate you fixing it up. Thanks!
Big +1 on this and agree with #12. The longer we wait to do this the larger the potential baseline as more and more code gets added, I know we're pretty good at asking for return types but this would enforce it.
The MR has some conflicts
Still needs more access based testing.
kim.pepper → credited acbramley → .
Generally in support of the feature but I think a bit more thought needs to be put into the access side.
The route requires edit access to the file, which is only allowed (with core) by the owner of the file. I also don't see much test coverage around access either?
Do we want to focus efforts on 🐛 File entity update is allowed only for user who has uploaded the file Active first?
You're right, I've dropped unsupported versions.
Re #97 - any chance you could remove that HTML and add it as an attachment instead to reduce the noise? It's quite hard to parse as is anyway
acbramley → created an issue.
acbramley → made their first commit to this issue’s fork.
acbramley → created an issue.
Nice catch!
This property is defined in UiHelperTrait, this may cause issues if the 2 traits don't exist on a test class right? (Although I don't know if that situation is realistic)
I've added a gitlab ci file to run tests, it seems like the expected values were for a much older version of the module. I've fixed them all up now.
acbramley → made their first commit to this issue’s fork.
acbramley → made their first commit to this issue’s fork.
I've just run into this with a client as well, this is on Drupal 10.3.2 (also reproducible on 10.2.6). It only happens on AWS environments and only when editing a node, not adding. Exposed filters work fine as well as exposed sorts.
The difference between editing and adding a node is down to query param length
When editing a node the query param is 2162 characters long
When adding a node the query param is 1938 characters long
This is referring to the number of characters appearing after /views/ajax? when clicking a pagination link in the media library on the above mentioned screens
Re #65 that happens currently (as in without the patch) and is intended functionality.
📌 Add Gitlab CI file Needs review should be committed first so we can start running tests against D11.
acbramley → created an issue.
I am not seeing a lot of support for the changes here, and I disagree with #91 that it is analogous to Json::encode. With the Json::encode example we are passing preferred options to the PHP function.
With setTimeLimit we are also doing that (in its current state).
With the changes in the MR we are changing the behaviour by adding to the php ini time limit - this could be a huge behaviour change for people calling this function.
I think our best bet is deprecate + new function.
acbramley → created an issue.
Pulled CI file from
✨
Add Gitlab CI file
Active
and added OPT_IN_TEST_NEXT_MAJOR
acbramley → made their first commit to this issue’s fork.
acbramley → created an issue.
acbramley → changed the visibility of the branch project-update-bot-only to hidden.
Got it green!
Working on these tests today, there's a huge amount to fix in there.
@greenhorn you do not need the patch, you need to clear cache.
This looks fine, but do we really need a test for this? Surely the maxlength property is tested elsewhere?
Came up in slack and the advice from @berdir was to deprecate instead. I agree as it's minimal code to get a list of revision IDs using an entity query.
I've also just found some very strange behaviour where the new JS themed messages are not being used for anonymous users. Both for webform submissions and even password reset submissions they're unthemed.
We're using the same method as Olivero so I'm not sure what's going on here.
When logged in everything works fine.
acbramley → changed the visibility of the branch 3428529-automated-drupal-11 to hidden.
I've gone back through https://git.drupalcode.org/project/redis/-/commits/8.x-1.x/src/Controlle... and added all changes back to the ReportController instead of trying to fix the insane conflicts (due to the refactors to that controller on this branch).
If we can get this in without having to reroll again it would make my year 😅
That sounds likely, since config is owned by the site you can change the label in the view mode freely.
That is the label of a view mode that does not exist as part of this projects config. You can modify this in your own project, I'm not sure where it came from.
This looks outdated, feel free to reopen if needed.
griffynh → credited acbramley → .
acbramley → made their first commit to this issue’s fork.
Blocked on https://www.drupal.org/project/past → D11 compat I believe.
This needs to be peeled back to just the relevant changes to be D11 compatible.
acbramley → changed the visibility of the branch project-update-bot-only to hidden.
acbramley → made their first commit to this issue’s fork.
The 2 failures were from the 2 lines, essentially testing some functionality which I can not see implemented anywhere.
The tests were testing that the existing alias based redirect was updated when the node had its alias changed. I personally can't see any code that would add that functionality, and IMO is not necessary. The whole idea of redirecting from a node alias would mostly around redirecting away from stale content easily (by using the alias). If we want to add automatic updates to existing alias redirects then we could do that in a follow up, especially seeing how long this issue has taken to get resolved.
Therefore, I've removed the assertions.
acbramley → made their first commit to this issue’s fork.
Not sure why this was closed, but I'd like this feature too.