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

Fix for homepage preview URLs on proper domains #6856

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

lukasbestle
Copy link
Member

@lukasbestle lukasbestle commented Dec 9, 2024

Description

Summary of changes

  • Clean up today's preview token quickfix
  • Add unit tests to prevent this bug from reappearing

Reasoning

Sites on real domains have their homepage on https://example.com and not https://example.com/. The unit tests did not catch this because they all used / as site index URL, which naturally does not have this trailing slash problem.

Additional context

Today's quickfix by Basti passes all those tests as well. 🎉

So I think releasing a beta 2 with this PR isn't urgent at all. I decided to change the code mainly for readability and future robustness (Basti's fix only works because the Uri class normalizes the trailing slash as well, but we can't rely on that here).

Changelog

Refactoring (since beta 1, not for final release notes)

  • Update preview token logic to reliably fix tokens for the homepage

Docs

None

Ready?

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and CI checks all pass

For review team

  • Add lab and/or sandbox examples (wherever helpful)
  • Add changes & docs to release notes draft in Notion

@lukasbestle lukasbestle added the type: bug 🐛 Is a bug; fixes a bug label Dec 9, 2024
@lukasbestle lukasbestle added this to the 5.0.0-beta.2 milestone Dec 9, 2024
@lukasbestle lukasbestle self-assigned this Dec 9, 2024
@bastianallgeier bastianallgeier merged commit e352c68 into v5/develop Dec 12, 2024
12 checks passed
@bastianallgeier bastianallgeier deleted the v5/fix/preview-tokens-home branch December 12, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Is a bug; fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants