- 🇺🇸United States neclimdul Houston, TX
Here's an updated patch. It addresses my review but also makes this patch D10 compatible.
Also this restores the test that was removed in #28
Probably still needs real tests though.
- 🇮🇳India Rinku Jacob 13 Kerala
Hi @neclimdul ,Reviewed the patch on drupal version 9.5.3-dev. I think Sub-pathauto (Sub-path URL Aliases) Version: 8.x-1.x-dev Works with druapl versions 8 and 9. it is not compatible with D10. So i have tested the patch with drupal9. Patch works perfectly for me. Need RTBC+1
- Status changed to Needs work
over 1 year ago 2:30pm 13 February 2023 - Status changed to Needs review
over 1 year ago 3:17pm 15 February 2023 - Status changed to Needs work
over 1 year ago 6:21pm 15 February 2023 - 🇨🇦Canada NickDickinsonWilde Victoria, BC (T'So-uke lands)
Testing concern:
- The default value of `process_admin_routes` is TRUE. However, in the `/tests/src/Kernel/SubPathautoKernelTest.php b/tests/src/Kernel/SubPathautoKernelTest.php` we have `\Drupal::configFactory()->getEditable('subpathauto.settings')->set('process_admin_routes', TRUE)->save();` Instead of replicating the default setting, in the test it should start with default settings and then toggle the setting and confirm it is working as expected. - Issue was unassigned.
- Status changed to Needs review
about 1 year ago 7:07pm 1 May 2023 - last update
about 1 year ago Patch Failed to Apply - 🇺🇸United States neclimdul Houston, TX
Improved testing. Does what Nick suggested plus a bit more because it doesn't look like there was any _actual_ testing of the new feature provided here.
- last update
about 1 year ago 21 pass - 🇺🇸United States neclimdul Houston, TX
woops... diff'd the wrong commit. This should be correct.
Also noticed there was a dead property on the path processor. @see interdiff.
- 🇧🇪Belgium LOBsTerr
Maybe we should use a generic solution ✨ Ignore specific paths RTBC
Like this we can ignore not only admin, but any path we want. It would be more flexible