Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make URLs end with a trailing slash + replace some hardcoded URLs with lookups #807

Closed

Conversation

ddabble
Copy link
Member

@ddabble ddabble commented Apr 7, 2024

Made most of the project's URLs end with a trailing slash (0ba27aa).

Reasons:

  • Many/most of the website's views use a trailing slash in their URLs, so this will make it consistent and (hopefully) establish an internal convention
  • To my knowledge, it's conventional for (Django) websites to use a trailing slash in URLs (except for URLs to files, of course)
  • Django redirects URLs missing a trailing slash to the same URL with a trailing slash by default, but not vice versa (see the docs on the APPEND_SLASH setting)
  • This fixes 404 on news URLs with trailing slash #806

Also:

  • Replaced hardcoded success_urls with lookups (8b20b52)
  • Replaced hardcoded hrefs with lookups (aa22ea0)

Note that I've not taken the time to test the code after making these changes (other than running the tests, which don't seem to cover most of the URLs), but I have been thorough in checking the whole codebase for references to the old URLs. Nonetheless, I'd appreciate if one of the maintainers could help me catch any URLs I might have missed :)

ddabble added 3 commits April 8, 2024 00:03
Reasons:
* Many/most of the website's views use a trailing slash in their URLs,
  so this will make it consistent and (hopefully) establish an internal
  convention
* To my knowledge, it's conventional for (Django) websites to use a
  trailing slash in URLs (except for URLs to files, of course)
* Django redirects URLs *missing* a trailing slash to the same URL
  *with* a trailing slash by default, but not vice versa (see
  https://docs.djangoproject.com/en/stable/ref/settings/#append-slash)
* This fixes a 404 bug introduced in
  85c78ca - which removed the trailing
  slash from the "news:details" path
@michaelbrusegard
Copy link
Member

Fixed the merge conflict in a new branch, because I can not modify a branch outside of this repo, see #810. Good work!

@ddabble ddabble deleted the urls-with-trailing-slashes branch April 10, 2024 16:36
@ddabble
Copy link
Member Author

ddabble commented Apr 10, 2024

Fixed the merge conflict in a new branch

Nice, thanks 😊

because I can not modify a branch outside of this repo

Thought I'd let you know that, actually, you can, since I (and most people when creating a PR from a fork) left the "Allow edits and access to secrets by maintainers" checkbox checked when creating the PR, which allows maintainers to e.g. rebase and push to that specific branch 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

404 on news URLs with trailing slash
2 participants