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

Calculate redirects as relative paths when site is configured to use relative links. #335

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eddgrant
Copy link
Contributor

@eddgrant eddgrant commented Nov 1, 2023

What’s changed

v3.2.0 contained changes to support support sites which were configured to generated relative links. At the time I implemented these changes we were not making use of the old_paths page redirection functionality. We have since started using this which caused me to realise that the redirect page generation did not work correctly when a site is configured to use relative links.

This manifested in the redirect pages being create in the wrong location in the site hierarchy and their redirect URLs being wrong (pointing to paths which don't exist).

This PR updates the redirect functionality such that it now works correctly when sites are configured to use relative links . I have written some simple redirect unit tests as non existed previously.

Hope this all makes sense. Would be great if someone could give it some 👀 and consider it for inclusion. Very happy to discuss and fettle if required.

Cheers!

Identifying a user need

No UR done. User testing was what actually alerted us to this issue in the first place though, as users were not able to access redirect pages, because they were being generated incorrectly.

User story:

As an author of a tech-docs-gem based site which uses relative links
I want to be able to provide my users with redirect pages
So that I can confidently rename and relocate my pages, without degrading the experience of my users

…ed to use relative links.

(cherry picked from commit 177153a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant