Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
#3140535: Automated Drupal Rector fixes made a lot of progress getting this module D9-ready. However, the bot couldn't solve everything, and tests are currently failing when you try to run this on a 9.2.x core test site.
Steps to reproduce
- Install Drupal core 9.2.x.
- Install the latest 8.x-1.x of fieldable_path. Apply latest patch from #3140535: Automated Drupal Rector fixes
./vendor/bin/phpunit -c core/phpunit.xml modules/contrib/fieldable_path/tests/src/
Proposed resolution
Get the tests passing in D9 by finishing the port.
Remaining tasks
"The deprecated hook hook_path_insert() is implemented in these functions: fieldable_path_path_insert(). It will be removed before Drupal 9.0.0. Use hook_ENTITY_TYPE_insert() for the 'path_alias' entity type instead. See https://www.drupal.org/node/3013865.""The deprecated hook hook_path_update() is implemented in these functions: fieldable_path_path_update(). It will be removed before Drupal 9.0.0. Use hook_ENTITY_TYPE_update() for the 'path_alias' entity type instead. See https://www.drupal.org/node/3013865.""The deprecated hook hook_path_delete() is implemented in these functions: fieldable_path_path_delete(). It will be removed before Drupal 9.0.0. Use hook_ENTITY_TYPE_delete() for the 'path_alias' entity type instead. See https://www.drupal.org/node/3013865."Make sure the existing tests pass on D9.- Maybe add some additional test coverage to have a bit more confidence in the results.
Reviews / refinements.Commit.Tag a new stable release (8.x-1.0-rc6)Rejoice
User interface changes
Hopefully not.
API changes
- Deprecate the legacy
FieldablePathController::checkEntityAndSyncPath()method. - Add a new
FieldablePathController::syncEntityPath()method that takes a PathAliasInterface parameter.
Data model changes
TBD, probably not.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 3217038-9.patch | 5.12 KB | dww |
Comments
Comment #2
dwwFilled in the D9 porting info on the project page to point here.
Comment #3
dwwEeek. https://www.drupal.org/node/3013865 changed a lot of the core API around path aliases.
I don't know if anyone's actually using/extending our
FieldablePathControllerclass, but I didn't want to go radically changing the methods in there to match how core now deals with aliases.So here's a work-in-progress patch that deprecates the existing
FieldablePathController::checkEntityAndSyncPath()method with the old interface, and adds a newsyncEntityPath()method that takes aPathAliasInterfaceargument, not a string and an array.The tests are getting further now, on both D8 and D9. But they're failing when we're trying to delete aliases. Still digging on that front, but I wanted to post what I've got so far, especially to get feedback from @Spleshka on this approach.
Comment #5
dwwOnce again, yay for tests! ;) I completely borked the API reshuffle for the delete case, which is exactly what the tests were telling me.
Here's a much better pair of patches. ;) Both of them work on D8. Only one of them works on D9:
FieldablePathControllerclass, but leaves the legacy hook implementations. So this still works on D8 due to core's BC layer to invoke the deprecated hooks. It fails on D9 since without implementing the new hooks, nothing's getting triggered and the tests fail.FieldablePathController, but also changes to fieldable_path.module to implement the correct core hooks.Comment #6
dwwHere's a slightly better version of the full patch. I was using
\Drupal::entityTypeManager()in a spot, but we're now extendingControllerBasewhich has its ownentityTypeManager()method we should use, instead.Comment #7
dwwI had to requeue the 9.1.x tests for #5 and #6 since I forgot core bumped the minimum mysql version. So the test was failing trying to install Drupal with mysql 5.5.
All the test results are now as expected:
I believe this is now ready, but I'm going to leave this open and assign to @Spleshka to get the other maintainer to confirm they like this approach before I commit it.
Stay tuned...
Comment #8
spleshkaI looked through the code of the patch from #6 and the changes look completely okay to me. Unfortunately I don't have a real project to test the upgraded version of the module in the real life, but if others are happy with how it works then I'm all up for getting it committed. Shall I mark it as RTBC at this stage or let's wait for others to test it first?
Comment #9
dwwBased on a Slack chat, here's an update to the version strings in the deprecation message to be more accurate and hopefully less confusing.
Also, I added a CR about the API change, so the @see comment immediately after @deprecated can follow the conventions and point to the CR about the change.
I think if you and I are both happy, we should just commit it. ;) But I'll leave this open for the next ~12 hours while I sleep and deal with the start of my work day tomorrow, in case anyone else appears who can review / test this.
Thanks again,
-Derek
Comment #10
spleshkaAll good, let's get it in! :)
Comment #12
dwwThanks! Committed and pushed to 8.x-1.x branch.
Also tagged and published the 8.x-1.0-rc6 release.