Remove column from articles URL #6594
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Toflar
previously approved these changes
Dec 4, 2023
3 tasks
leofeyer
approved these changes
Dec 5, 2023
|
Thank you @aschempp. |
leofeyer
pushed a commit
that referenced
this pull request
Jan 17, 2024
Description ----------- Finally! This is a first step towards the _Content URL Generator_. I was able to get this wrapped up so that **generating the URL** is separated from **matching a URL (with parameters)**. This is only the part about generating URLs for any content, since the other part is way more complex and not finished yet. Be aware that this deprecates the `PageModel::getFrontendUrl`/`getAbsoluteUrl` methods, but it does not replace it in all places. Probably not everywhere in core-bundle, and currently in non of the add-on bundles. This does not yet implement content URL generator for any of our add-on bundles (like news and events), because that alone are dozens of file to adjust and will be easier to review in separate PRs (I do have some of that code available locally though, so I can quickly add them once this is merged). - [x] Replaces #6547 - [x] (Probably) requires #6595 and #6594 - [x] Needs tests on the new services Commits ------- ba2bd8e Implement the content URL generator 83d3f27 Various improvements, CS and tests 9636820 Use the ContentUrlGenerator in the SERP widget ad1e137 Correctly mark URL result as redirect cc27676 Cache exceptions thrown when generating a content URL 279308a Merge remote-tracking branch 'upstream/5.x' into feature/content-url-… 71c1abd Remove unnecessary optional parameters 53f4cda Re-add the reference type bbbb67a Fix existing code for absolute URLs b8177b8 Fixed tests 873e033 Skip cache if content or parameters are not serializable (e.g. contai… 1903333 Tests for ContentUrlGenerator 8fd38de Fixes from code review ba10d11 Tests for resolvers 74267e8 Fixed argument of AuthenticationSuccessHandler 9a13b4e Fixed phpstan error 419360d Fixed phpstan error 5487c57 Drop the obsolete SerpPreview url_callback for tl_page 03b5b56 Replace PageModel::getFrontendUrl and getAbsoluteUrl 2ea63d7 Added review comments 23b8f7b Fix CI 223644d Replace PageModel::getFrontendUrl and getAbsoluteUrl 4303856 Correctly make URLs absolute 34cf962 Fixed tests a941ccd Rename the ContentUrlResult ba24a4a Revert "Rename the ContentUrlResult" 9b529db Use NULL instead of ContentUrlResult::abstain() f5fe023 Remove the null option
leofeyer
pushed a commit
to contao/core-bundle
that referenced
this pull request
Jan 17, 2024
Description ----------- Finally! This is a first step towards the _Content URL Generator_. I was able to get this wrapped up so that **generating the URL** is separated from **matching a URL (with parameters)**. This is only the part about generating URLs for any content, since the other part is way more complex and not finished yet. Be aware that this deprecates the `PageModel::getFrontendUrl`/`getAbsoluteUrl` methods, but it does not replace it in all places. Probably not everywhere in core-bundle, and currently in non of the add-on bundles. This does not yet implement content URL generator for any of our add-on bundles (like news and events), because that alone are dozens of file to adjust and will be easier to review in separate PRs (I do have some of that code available locally though, so I can quickly add them once this is merged). - [x] Replaces contao/contao#6547 - [x] (Probably) requires contao/contao#6595 and contao/contao#6594 - [x] Needs tests on the new services Commits ------- ba2bd8ef Implement the content URL generator 83d3f277 Various improvements, CS and tests 96368207 Use the ContentUrlGenerator in the SERP widget ad1e1378 Correctly mark URL result as redirect cc276765 Cache exceptions thrown when generating a content URL 279308ad Merge remote-tracking branch 'upstream/5.x' into feature/content-url-… 71c1abd1 Remove unnecessary optional parameters 53f4cdaa Re-add the reference type bbbb67a3 Fix existing code for absolute URLs b8177b81 Fixed tests 873e033f Skip cache if content or parameters are not serializable (e.g. contai… 1903333f Tests for ContentUrlGenerator 8fd38de8 Fixes from code review ba10d117 Tests for resolvers 74267e8c Fixed argument of AuthenticationSuccessHandler 9a13b4e2 Fixed phpstan error 419360dc Fixed phpstan error 5487c57a Drop the obsolete SerpPreview url_callback for tl_page 03b5b566 Replace PageModel::getFrontendUrl and getAbsoluteUrl 2ea63d7c Added review comments 23b8f7b0 Fix CI 223644dd Replace PageModel::getFrontendUrl and getAbsoluteUrl 4303856b Correctly make URLs absolute 34cf9623 Fixed tests a941ccd5 Rename the ContentUrlResult ba24a4a3 Revert "Rename the ContentUrlResult" 9b529db7 Use NULL instead of ContentUrlResult::abstain() f5fe0239 Remove the null option
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While preparing for the Content URL Generator, I noticed this: did you know that (sometimes) Contao generates the URL to an article as
.../articles/section:aliasand not just.../articles/alias?The reason behind this is, that
Controller::getFrontendModulewill ignore the articles parameter if it does not belong the the given column (section). However, this wasa) not applied in all place
b) totally unnecessary because we can achieve the same correct thing without having it in the URL (see PR code)
c) allowed for URL manipulation, I could theoretically generate an article in the wrong section (not saying that would be a real problem, but … )
This PR removes the need for passing the section in the article alias, because it was irrelevant to most places anyway. This makes generating the URL of an article easier and probably also fixes bugs because no third-party developer I know would currently have generated a correct URL 😅