Skip to content

Conversation

LucasWerey
Copy link
Member

@LucasWerey LucasWerey commented Aug 28, 2025

βœ… Checklist

  • npx changeset was attached.
  • Covered by automatic tests.
  • Impact of the changes:
    • error banner

πŸ“ Description

This PR introduces a fix to be able to render link received from status.ledger.com/api/v2. The issue is that they send the anchor tag directly in the body of the incident. We need to handle this.
For security reasons, we need to whitelist the domain before making the link clickable. If it's not clickable, we will just display text and no link.

Also, I leverage MSW to mock incidents from the status page so it's easier to debug

Donjon has checked this method, and the risk is sufficiently mitigated βœ…

Before After
image image
 image

❓ Context


🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.
  • Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

Copy link

vercel bot commented Aug 28, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

4 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
ledger-live-github-bot Ignored Ignored Preview Aug 29, 2025 8:58am
native-ui-storybook Ignored Ignored Preview Aug 29, 2025 8:58am
react-ui-storybook Ignored Ignored Preview Aug 29, 2025 8:58am
web-tools Ignored Ignored Preview Aug 29, 2025 8:58am

@live-github-bot live-github-bot bot added the desktop Has changes in LLD label Aug 28, 2025
@LucasWerey LucasWerey force-pushed the fix/LIVE-21101 branch 3 times, most recently from 553a595 to 9488816 Compare August 28, 2025 13:03
@LucasWerey LucasWerey requested a review from Copilot August 29, 2025 08:18
Copilot

This comment was marked as outdated.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Fixes the rendering of translated error messages that contain HTML anchor tags from the Ledger Status API. Previously, anchor tags were not properly rendered as clickable links, and debug console logs were left in production code.

  • Added a new utility function to parse and render HTML anchor tags as React Link components
  • Implemented security validation to only allow clicks on trusted Ledger domains
  • Removed debug console.log statements from production code

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
TranslatedError.tsx Added conditional rendering for error messages containing anchor tags
ErrorWithAnchor.tsx New utility for parsing HTML anchors and rendering secure clickable links
ErrorWithAnchor.test.tsx Comprehensive test coverage for anchor parsing and security validation
ServiceStatusPanel.tsx Updated to use new anchor rendering utility and removed debug logs
ledgerStatus.ts Added mock data for testing status API responses with anchor tags
browser.ts Added MSW handler for status API mocking
proud-bugs-care.md Changeset documentation for the fix

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@LucasWerey LucasWerey marked this pull request as ready for review August 29, 2025 08:55
@LucasWerey LucasWerey requested a review from a team as a code owner August 29, 2025 08:55
@LucasWerey LucasWerey merged commit e3894a7 into develop Aug 29, 2025
48 of 51 checks passed
@LucasWerey LucasWerey deleted the fix/LIVE-21101 branch August 29, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop Has changes in LLD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants