Updated: Comment #0
Problem/Motivation
We currently have ConfigMapperInterface::get(Add|Edit|Delete)Path() only to support #type operations (which uses theme_links()). Now that theme_links() supports routes, this is no longer necessary.
Proposed resolution
Remove ConfigMapperInterface::getAddPath(), ConfigMapperInterface::getEditPath(), ConfigMapperInterface::getDeletePath().
This currently does not yet remove ConfigMapperInterface::getOverviewPath().
We could also make ConfigNamesMapper::getBasePath() protected, then we would be path-free in the interface.
Remaining tasks
API changes
Remove ConfigMapperInterface::getAddPath(), ConfigMapperInterface::getEditPath(), ConfigMapperInterface::getDeletePath().
Comment | File | Size | Author |
---|---|---|---|
#10 | interdiff.txt | 628 bytes | Gábor Hojtsy |
#8 | interdiff.txt | 1.57 KB | Gábor Hojtsy |
#8 | 2136551-4.patch | 9.46 KB | Gábor Hojtsy |
#7 | 2136551-3.patch | 7.89 KB | Gábor Hojtsy |
#7 | interdiff.txt | 1.15 KB | Gábor Hojtsy |
Comments
Comment #1
tstoecklerHere we go.
We still use getBasePath() for the access check, I'll look into doing that route-based now.
We still use getOverviewPath() for redirecting pack to the translation overview with ?destination=. I guess we should have route-based ?destination=, or whatever, but either way we should be able to use something like $request->requestUri() or something.
Anyway, if this is green, it should be good to go.
Comment #2
Gábor HojtsyWell, the @todo from getPathFromRoute() should also be removed then. I can do that while committing :) Awaiting green-ness.
Comment #4
tstoecklerSo we need the getBasePath() for the getRequestForPath() dance. I think there is no equivalent to do that route-based because of the _system_path request attribute, which that relies on. So leaving that for now. Also not removing getOverviewPath() then for now...
Found a cooler way to do the access checking, though.
Comment #5
tstoecklerYes, #2 is correct.
Comment #7
Gábor HojtsyThere is no reason to change that access checking code for the sake of changing it :) Let's keep it as-is and minimise unrelated fail potential :)
Comment #8
Gábor HojtsyAlso this will never pass without an entity annotations route fix as well.
Comment #10
Gábor HojtsyCommitted that with this little additional interdiff as discussed in #2.