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: broken links in link checker #2981

Merged
merged 1 commit into from
Sep 23, 2024
Merged

fix: broken links in link checker #2981

merged 1 commit into from
Sep 23, 2024

Conversation

Ballonek
Copy link
Collaborator

Pull Request

Related issue

Fixes #NOT-78

PR: Fix Link Checker for Query Parameters and Anchor Links

Overview

This PR fixes the issue where the link checker incorrectly flags valid links containing query parameters or hash anchors as broken.

Changes

  1. Query Parameters Handling:

    • Strips query parameters (e.g., ?utm_campaign=...) from internal links before validation.
  2. Anchor Links Validation:

    • Ensures hash-based links (e.g., #heading) are correctly validated against document headings.
  3. No Changes to External or Static Asset Links:

    • External links and static assets continue to be ignored.

Why

Previously, valid links with query parameters or hash fragments were incorrectly reported as broken. This fix improves the accuracy of the link checker.

Testing

  • Links with query parameters are no longer flagged.
  • Hash-based links (anchors) are correctly validated.
  • External and static asset links are ignored as expected.

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

@meili-bot
Copy link
Collaborator

How to see the preview of this PR?

⚠️ Private link, only accessible to Meilisearch employees.

Go to this URL: https://website-git-deploy-preview-mei-16-meili.vercel.app/docs/branch:fix/NOT-78

Credentials to access the page are in the company's password manager as "Docs deploy preview".

@guimachiavelli
Copy link
Member

Looks good to mem thanks!

One question: does this cover issues related to the <NoticeTag>, as noted in the Linear issue?

@Ballonek
Copy link
Collaborator Author

Looks good to mem thanks!

One question: does this cover issues related to the <NoticeTag>, as noted in the Linear issue?

I tried that locally and any problems with <NoticeTag> did not appear.

Copy link
Member

@guimachiavelli guimachiavelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, then!

Merging!

@guimachiavelli guimachiavelli merged commit 88347e8 into main Sep 23, 2024
1 check passed
@guimachiavelli guimachiavelli deleted the fix/NOT-78 branch September 23, 2024 10:27
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.

3 participants