Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
After path aliases are converted to entities, we should be able to convert their custom admin overview to an entity list builder.
Proposed resolution
Do it.
Remaining tasks
See above.
User interface changes
Nope.
API changes
Nope.
Data model changes
Nope.
Comment | File | Size | Author |
---|---|---|---|
#28 | interdiff-28.txt | 812 bytes | amateescu |
#28 | 3011807-28.patch | 17.68 KB | amateescu |
#25 | interdiff-25.txt | 4.64 KB | amateescu |
#25 | 3011807-25.patch | 16.88 KB | amateescu |
#22 | Path_aliases___Drupal_8_x.jpg | 165.96 KB | plach |
Comments
Comment #2
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedThis patch applies on top of #3007832-5: Convert custom path alias forms to entity forms, not posting a combined patch yet because there's some work to be done in the other issues.
Comment #3
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedOk, this should apply and work properly on top of #3007832-7: Convert custom path alias forms to entity forms.
Comment #4
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedThe dependent patch has been fixed, there's no change to the "for review" one here so just posting a combined one to get a green light from the testbot :)
Comment #5
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedRerolled on top of #3007832-10: Convert custom path alias forms to entity forms.
Comment #6
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedRerolled the combined patch again, the review one doesn't need an update.
Comment #7
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedRerolled the combined patch again.
Comment #9
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedRerolled on top of #3007832-15: Convert custom path alias forms to entity forms.
Comment #10
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedRerolled the combined patch.
Comment #11
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedYet another reroll :)
Comment #12
jibran#2336597: Convert path aliases to full featured entities is in.
Comment #13
Chris Matthews CreditAttribution: Chris Matthews commentedComment #14
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedThis is still postponed on #3007832: Convert custom path alias forms to entity forms.
Comment #15
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedBased on #3007832-30: Convert custom path alias forms to entity forms, we need to add a BC layer for the route that we're removing here.
Comment #16
BerdirInteresting, so we're going to keep the custom exposed filters instead of replacing it with a view.
Specific reason for that or was it just easier to keep that working instead of adding views and upgrade path for it?
Could use PathFilterForm::class.
And we even have a feature here that still doesn't work in views. I guess that was another argument for keeping it hardcoded?
I never even knew that feature existed, doesn't seem very intuitive :)
Comment #17
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedFixed the second point :) Not uploading a combined patch because there's no functional change since #15.
Comment #19
amateescu CreditAttribution: amateescu for Pfizer, Inc. commented#3007832: Convert custom path alias forms to entity forms is in, so the patch from #17 should be good to go now :)
Comment #20
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedComment #21
BerdirThis looks ready as well, given that testbot agrees.
We're not actually saving any LOC here according to the diffstatt, also because the path alias list has a few special features that we keep, but it removes important usages of AliasStorage methods that we need to remove to be able to deprecate that properly.
It is a bit uncommon to have exposed filters and sorting in a list builder, but adding views integration, specifically for some of the extra features (see #16), would be a lot more work.
Comment #22
plachLooks great, just a couple of non-blocking nits!
However, the change in the page title introduced a visual inconsistency that might affect UX:
I'll ask Angie or Gábor about this.
Would it make sense to inject the current request directly?
+=
:)Comment #23
plachJust spoke with Gábor about this: the inconsistency should be fixed before this lands. We should probably change all user facing labels to "URL alias(es)" to keep the current UX. It may be worth doing that in a separate issue, since that wouldn't be specifically about this page, but not feeling strongly about this.
Comment #24
plachJust spoke with @Berdir: I meant that we should change the entity type definition user-facing labels, nothing else. Sorry for the confusion.
Comment #25
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedFixed #22 / #23 :)
Comment #26
plachLooks great, thanks! I will wait for the testbot and then commit, unless someone beats me to it ;)
Comment #28
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedUpdated that test.
Comment #32
plachCommitted 55a2b40 and pushed to 9.0.x and friends. Thanks!
Let's not forget to update the relevant CRs :)
Comment #33
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedUpdated the CR (https://www.drupal.org/node/3013865) to mention the route that has been deprecated here.