Skip to content

fix: resolve broken external links and missing target handling in learning resources (#245) - #266

Closed
mithilP007 wants to merge 2 commits into
komalharshita:mainfrom
mithilP007:fix/issue-245
Closed

fix: resolve broken external links and missing target handling in learning resources (#245)#266
mithilP007 wants to merge 2 commits into
komalharshita:mainfrom
mithilP007:fix/issue-245

Conversation

@mithilP007

Copy link
Copy Markdown

This PR resolves #245 by introducing a robust link helper utility to safely parse and standardize external link handling, registering a custom Jinja filter 'parse_resource' to handle formatting, and updating 'templates/project.html' to render resource links with appropriate target and rel attributes. Additionally, unit tests were added to verify the helper logic.

Copilot AI review requested due to automatic review settings May 18, 2026 05:28
@vercel

vercel Bot commented May 18, 2026

Copy link
Copy Markdown

@mithilP007 is attempting to deploy a commit to the komalsony234-1530's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you for submitting your first pull request to DevPath.

Before review:

  • Complete the PR template fully
  • Ensure all tests pass
  • Link your PR to an issue
  • Keep changes scoped to the issue

A maintainer will review your contribution soon.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR standardizes how project learning-resource links are parsed and rendered so external resources open safely in a new tab with appropriate security attributes.

Changes:

  • Added a reusable parse_external_link helper for extracting labels and URLs.
  • Registered a parse_resource Jinja filter through the main routes blueprint.
  • Updated the project detail template and added helper unit tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
utils/link_helper.py Adds link parsing utility for resource strings.
routes/main_routes.py Registers the helper as a Jinja template filter.
templates/project.html Uses parsed link metadata to render resource links safely.
tests/test_basic.py Adds unit coverage for the new link helper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mithilP007

Copy link
Copy Markdown
Author

Solved under Gssoc2026
you can checkout the change

@komalharshita

Copy link
Copy Markdown
Owner

@mithilP007 resolve the merge conflicts

@github-actions github-actions Bot added gssoc-2026 type:bug Something isn't working labels May 20, 2026
@mithilP007

Copy link
Copy Markdown
Author

Now you can check i have fixed it

@komalharshita komalharshita left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the contribution — improving broken learning-resource links and standardizing external-link handling is a valuable quality improvement for DevPath.

The overall direction of the PR is good, especially because learning resources are a core part of the user experience and inconsistent/broken external links reduce trust in the platform.

What was done well

  • Focuses on a real usability issue
  • Improves consistency of external-link behavior
  • Keeps the scope reasonably targeted
  • Helps improve learning-resource reliability
  • Aligns well with existing external-link security practices

Important things to verify before merge

  • Any use of:

    target="_blank"

    should also consistently include:

    rel="noopener noreferrer"

    for security reasons.

  • Please confirm that:

    • all modified links were manually tested
    • no valid existing URLs were accidentally changed
    • internal navigation behavior remains unaffected
  • If dataset/resource entries were updated, maintainers should also verify:

    • project pages render correctly
    • recommendation cards still function properly
    • malformed URLs are not introduced
  • If not already included, automated validation/tests for external-link handling would be a good addition.

Overall, this is a useful quality-focused fix, but the above verification points should be completed before merge.

@komalharshita komalharshita added type:docs Improvements or additions to documentation need review Further information is requested level:beginner and removed type:accessibility type:performance ui type:testing type:security labels May 25, 2026
@komalharshita

Copy link
Copy Markdown
Owner

@mithilP007 kindly resolve the merge conflicts

@komalharshita

Copy link
Copy Markdown
Owner

Closing this PR for inactivity. Please open a fresh PR if possible.

@komalharshita komalharshita removed type:bug Something isn't working type:docs Improvements or additions to documentation need review Further information is requested level:beginner quality:clean documentation labels Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Broken External Links and Missing Target Handling Cause Navigation Disruption

3 participants