Conversation
Signed-off-by: ragnep <ragneinfo@gmail.com>
WalkthroughAdds URL normalization and base-origin validation for Seize links: introduces Changes
Sequence Diagram(s)sequenceDiagram
participant Handler as Link Handler
participant SeizeLP as SeizeLinkParser
participant Cache as Base URL Cache
participant URL as URL constructor
Handler->>SeizeLP: ensureStableSeizeLink(href, currentHref?)
SeizeLP->>Cache: getSeizeBaseOrigin()
Cache-->>SeizeLP: baseOrigin or null
alt baseOrigin available
SeizeLP->>URL: new URL(href, baseOrigin)
URL-->>SeizeLP: parsed URL (origin, pathname, searchParams)
SeizeLP->>SeizeLP: preserve/drop query params, align with currentHref if provided
SeizeLP-->>Handler: stableHref (absolute, baseOrigin)
else baseOrigin missing or parse fail
SeizeLP-->>Handler: fallback href (original/current)
end
Handler->>Handler: use stableHref for matching (seize/external/internal) and rendering (anchor/open-graph/preview)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
__tests__/helpers/SeizeLinkParser.test.ts (1)
85-152: Good coverage for URL normalization; add a hash caseAdd a test asserting that URL fragments (hash) are preserved when rewriting (e.g., current https://site.com/messages#top + incoming /?drop=abc ⇒ ...?drop=abc#top).
components/drops/view/part/dropPartMarkdown/handlers/seize.tsx (1)
95-109: Safer dropId extraction with origin check — LGTMSwitch to URL parsing with getSeizeBaseOrigin() is correct and resilient. Optional follow-up: avoid double extraction by caching extract(href) between match/render if this becomes hot.
helpers/SeizeLinkParser.ts (1)
40-49: Align parseSeizeQuoteLink with the public type; parse via URLThe interface exposes optional dropId, but regex parsing never captures it. Prefer URL-based parsing with optional drop extraction for consistency and resilience.
Apply this diff:
-export function parseSeizeQuoteLink(href: string): SeizeQuoteLinkInfo | null { - const regex = - /\/waves\?wave=([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})&serialNo=(\d+)$/; - - const match = href.match(regex); - if (!match) return null; - - const [, waveId, serialNo, dropId] = match; - return { waveId, serialNo, dropId }; -} +export function parseSeizeQuoteLink(href: string): SeizeQuoteLinkInfo | null { + try { + const baseUrl = getBaseUrl(); + if (!baseUrl) return null; + let url: URL; + try { + url = new URL(href); + } catch { + url = new URL(href, baseUrl.origin); + } + if (url.origin !== baseUrl.origin) return null; + if (url.pathname !== "/waves") return null; + const waveId = url.searchParams.get("wave"); + const serialNo = url.searchParams.get("serialNo"); + if (!waveId || !serialNo) return null; + const dropId = url.searchParams.get("drop") || undefined; + return { waveId, serialNo, dropId }; + } catch { + return null; + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx(5 hunks)__tests__/helpers/SeizeLinkParser.test.ts(3 hunks)components/drops/view/part/dropPartMarkdown/handlers/seize.tsx(2 hunks)components/drops/view/part/dropPartMarkdown/linkHandlers.tsx(5 hunks)helpers/SeizeLinkParser.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before props
**/*.{ts,tsx}: Use TypeScript for source code and follow existing code style and naming conventions
Adhere to clean code standards as measured by SonarQube
Files:
__tests__/helpers/SeizeLinkParser.test.tscomponents/drops/view/part/dropPartMarkdown/handlers/seize.tsxcomponents/drops/view/part/dropPartMarkdown/linkHandlers.tsxhelpers/SeizeLinkParser.ts__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
__tests__/**
📄 CodeRabbit inference engine (tests/AGENTS.md)
Place Jest test suites under the
__tests__directory mirroring source folders (e.g., components, contexts, hooks, utils)
Files:
__tests__/helpers/SeizeLinkParser.test.ts__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
__tests__/**/{fixtures,helpers}/**
📄 CodeRabbit inference engine (tests/AGENTS.md)
Put shared test fixtures and helpers in dedicated
fixturesandhelperssubfolders within tests
Files:
__tests__/helpers/SeizeLinkParser.test.ts
{**/__tests__/**,**/*.test.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place tests in tests directories or alongside components as ComponentName.test.tsx
Files:
__tests__/helpers/SeizeLinkParser.test.ts__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
{**/__tests__/**,**/*.test.{ts,tsx}}
📄 CodeRabbit inference engine (AGENTS.md)
Mock external dependencies and APIs in tests
Files:
__tests__/helpers/SeizeLinkParser.test.ts__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingImplement React components as functional components using hooks (no class components)
Files:
components/drops/view/part/dropPartMarkdown/handlers/seize.tsxcomponents/drops/view/part/dropPartMarkdown/linkHandlers.tsx__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
__tests__/components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use
@testing-library/reactand@testing-library/user-eventfor React component tests
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
🧬 Code graph analysis (5)
__tests__/helpers/SeizeLinkParser.test.ts (1)
helpers/SeizeLinkParser.ts (1)
ensureStableSeizeLink(100-155)
components/drops/view/part/dropPartMarkdown/handlers/seize.tsx (1)
helpers/SeizeLinkParser.ts (1)
getSeizeBaseOrigin(29-32)
components/drops/view/part/dropPartMarkdown/linkHandlers.tsx (2)
helpers/SeizeLinkParser.ts (1)
ensureStableSeizeLink(100-155)components/waves/LinkPreviewCard.tsx (1)
LinkPreviewCard(43-113)
helpers/SeizeLinkParser.ts (1)
next.config.mjs (2)
publicEnv(177-177)publicEnv(235-235)
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx (2)
helpers/SeizeLinkParser.ts (1)
ensureStableSeizeLink(100-155)components/drops/view/part/dropPartMarkdown/linkHandlers.tsx (1)
createLinkRenderer(56-195)
🪛 Biome (2.1.2)
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
[error] 14-14: expected , but instead found jest
Remove jest
(parse)
[error] 14-14: expected , but instead found .
Remove .
(parse)
[error] 14-14: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '('.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 14-14: Expected a function body but instead found '=>'.
Expected a function body here.
(parse)
[error] 15-15: expected , but instead found __esModule
Remove __esModule
(parse)
[error] 15-16: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '-'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 16-16: Expected an expression but instead found 'default'.
Expected an expression here.
(parse)
[error] 16-16: expected , but instead found (
Remove (
(parse)
[error] 17-17: expected , but instead found default
Remove default
(parse)
[error] 17-17: expected , but instead found (
Remove (
(parse)
[error] 21-21: Expected an expression but instead found ')'.
Expected an expression here.
(parse)
[error] 10-10: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx (1)
224-237: Stable href behavior — tests read wellThe normalization tests using ensureStableSeizeLink with an overridable current location correctly validate root rebasing and cross-path sharing. Nice.
Also applies to: 239-254
components/drops/view/part/dropPartMarkdown/linkHandlers.tsx (1)
70-85: Normalize before handling — solid integrationUsing ensureStableSeizeLink consistently for validation, parsing, and previews tightens behavior and reduces edge cases. Good error-boundary fallbacks.
Also applies to: 91-99, 126-139, 149-149, 166-188
helpers/SeizeLinkParser.ts (3)
3-27: Cache BASE_ENDPOINT safely — LGTMCache refresh on env change is correct; null handling and parse guards are good.
51-71: Origin-validated query parsing — LGTMSwitching to URL with base-origin enforcement avoids false positives from relative/external links.
100-155: ensureStableSeizeLink is robustCorrectly rebases drop onto current same-origin path, preserving query and hash with safe fallbacks. Matches tests.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx (2)
218-231: Remove redundant cleanup call.Line 230 calls
setEnsureCurrentHref(undefined), but this is already handled in theafterEachhook at line 160.Apply this diff:
expect(screen.getByTestId("chat-buttons")).toHaveAttribute( "data-href", "https://6529.io/messages?wave=current-wave&drop=drop-123" ); - setEnsureCurrentHref(undefined); });
233-248: Remove redundant cleanup call.Line 247 calls
setEnsureCurrentHref(undefined), but this is already handled in theafterEachhook at line 160.Apply this diff:
expect(screen.getByTestId("chat-buttons")).toHaveAttribute( "data-href", "https://6529.io/messages?wave=current-wave&drop=drop-456" ); - setEnsureCurrentHref(undefined); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx(6 hunks)helpers/SeizeLinkParser.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before props
**/*.{ts,tsx}: Use TypeScript for source code and follow existing code style and naming conventions
Adhere to clean code standards as measured by SonarQube
Files:
helpers/SeizeLinkParser.ts__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingImplement React components as functional components using hooks (no class components)
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
__tests__/**
📄 CodeRabbit inference engine (tests/AGENTS.md)
Place Jest test suites under the
__tests__directory mirroring source folders (e.g., components, contexts, hooks, utils)
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
__tests__/components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use
@testing-library/reactand@testing-library/user-eventfor React component tests
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
{**/__tests__/**,**/*.test.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place tests in tests directories or alongside components as ComponentName.test.tsx
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
{**/__tests__/**,**/*.test.{ts,tsx}}
📄 CodeRabbit inference engine (AGENTS.md)
Mock external dependencies and APIs in tests
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
🧬 Code graph analysis (2)
helpers/SeizeLinkParser.ts (1)
next.config.mjs (2)
publicEnv(177-177)publicEnv(235-235)
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx (2)
helpers/SeizeLinkParser.ts (1)
ensureStableSeizeLink(100-160)components/drops/view/part/dropPartMarkdown/linkHandlers.tsx (1)
createLinkRenderer(56-195)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx (5)
6-6: LGTM! Import supports the new mock functionality.The import is correctly used in the SeizeLinkParser mock below to wrap the actual implementation with test-specific overrides.
18-20: LGTM! Mock correctly accepts and renders the src prop.
106-110: LGTM! Mock correctly reflects the updated DropItemChat signature.The mock now accepts both
dropIdandhrefparameters and renders the nested chat-buttons element with the href attribute, which aligns with the component changes in this PR.
113-128: LGTM! Well-designed test mock for URL normalization.The mock correctly wraps the actual implementation and provides a test-controlled override for
currentHref, enabling tests to simulate different page contexts without polluting global state.
134-139: LGTM! Helper function provides clean test control.helpers/SeizeLinkParser.ts (5)
3-4: LGTM! Cache variables support performance optimization.The cache is properly invalidated when
BASE_ENDPOINTchanges (line 15 checkscachedBaseEndpoint === current), so this should work correctly even in test environments.
6-27: LGTM! Robust caching implementation with proper error handling.The function correctly handles empty endpoints, invalid URLs, and cache invalidation when the endpoint changes.
29-32: LGTM! Clean helper for accessing the base origin.
58-70: LGTM! Improved origin validation using base URL.The function now correctly validates links against the configured base origin rather than a hardcoded list, and properly handles relative URLs with the fallback at line 67.
100-160: LGTM! Comprehensive URL normalization with robust error handling.The function correctly:
- Validates same-origin URLs
- Preserves the current page's path and existing query parameters
- Updates only the
dropparameter from the target URL- Maintains the current hash
- Handles all edge cases with appropriate fallbacks
The early return at lines 122-124 when there's no
dropparameter is appropriate, as this function specifically normalizes drop preview links.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx (3)
113-128: LGTM! Effective test infrastructure.The mock wrapper pattern correctly injects
currentHrefOverridefor deterministic testing of location-dependent behavior.
Optional: Consider type-safe alternative to
anyassertion.Line 120 uses
anyto attach the__setCurrentHrefsetter. While acceptable in tests, you could define a more specific type:type MockedEnsureStableSeizeLink = typeof ensureStableSeizeLink & { __setCurrentHref: (href?: string) => void; };
218-231: LGTM! Test correctly validates root drop link normalization.The test properly verifies that drop links from the root path are normalized to use the current location's path and parameters.
Optional: Redundant cleanup call.
Line 230 calls
setEnsureCurrentHref()butafterEachon Line 160 already performs this cleanup. The extra call is harmless but unnecessary.); - setEnsureCurrentHref(); });
233-248: LGTM! Test correctly validates cross-path drop link normalization.The test properly verifies that drop links from different paths are normalized to use the current location's context.
Optional: Redundant cleanup call.
Line 247 calls
setEnsureCurrentHref()butafterEachon Line 160 already performs this cleanup.); - setEnsureCurrentHref(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx(6 hunks)helpers/SeizeLinkParser.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before props
**/*.{ts,tsx}: Use TypeScript for source code and follow existing code style and naming conventions
Adhere to clean code standards as measured by SonarQube
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsxhelpers/SeizeLinkParser.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingImplement React components as functional components using hooks (no class components)
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
__tests__/**
📄 CodeRabbit inference engine (tests/AGENTS.md)
Place Jest test suites under the
__tests__directory mirroring source folders (e.g., components, contexts, hooks, utils)
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
__tests__/components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use
@testing-library/reactand@testing-library/user-eventfor React component tests
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
{**/__tests__/**,**/*.test.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place tests in tests directories or alongside components as ComponentName.test.tsx
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
{**/__tests__/**,**/*.test.{ts,tsx}}
📄 CodeRabbit inference engine (AGENTS.md)
Mock external dependencies and APIs in tests
Files:
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx
🧬 Code graph analysis (2)
__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx (2)
helpers/SeizeLinkParser.ts (1)
ensureStableSeizeLink(100-160)components/drops/view/part/dropPartMarkdown/linkHandlers.tsx (1)
createLinkRenderer(56-195)
helpers/SeizeLinkParser.ts (1)
next.config.mjs (2)
publicEnv(177-177)publicEnv(235-235)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
helpers/SeizeLinkParser.ts (4)
3-27: LGTM! Clean caching implementation.The memoization pattern correctly invalidates the cache when
BASE_ENDPOINTchanges and handles invalid URLs gracefully.
29-32: LGTM! Clean abstraction.The function provides a clear public API for retrieving the base origin.
51-98: LGTM! Solid refactor.Centralizing the base URL logic via
getBaseUrl()and simplifying the origin validation improves maintainability. The fallback handling for relative URLs is appropriate.
100-160: URL transformation behavior is intentional—confirmed by test suite.The function's URL restructuring is a design feature, not a bug. Tests explicitly validate this behavior ("rebases drop links from other paths onto current location"), confirming it extracts the
dropparameter and applies it to the current page location. This is appropriate for the drop preview use case.Optional: Simplify hash fallback.
Line 150: Remove unnecessary null-coalescing since
URL.hashalways returns a string:- const hash = currentUrl.hash ?? ""; + const hash = currentUrl.hash;__tests__/components/drops/view/part/dropPartMarkdown/linkHandlersRegistry.test.tsx (2)
104-111: LGTM! Mock properly updated.The
DropItemChatmock correctly accepts the newhrefparameter and exposes it viadata-hrefattributes for test assertions.
134-139: LGTM! Clean test helper.The helper provides a safer, more convenient API for controlling the mock's internal state.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests