Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an article import feature: server POST /api/import for Hive and external URLs with strict DNS/redirect/security checks and Readability extraction; client import UI (dialog + page), PublishActionBar integration to apply imports, publish-state setters, i18n keys, dependency updates, tests, and simplified list-item image fallbacks. Changes
Sequence DiagramsequenceDiagram
actor User
participant Browser as Browser / UI
participant ImportAPI as Import API (/api/import)
participant HiveBridge as Hive Bridge API
participant FetchSite as External Site
participant Readability as Mozilla Readability
User->>Browser: Enter URL & trigger Import
Browser->>ImportAPI: POST /api/import { url }
alt Hive URL
ImportAPI->>ImportAPI: parse author/permlink
ImportAPI->>HiveBridge: fetch post data
HiveBridge->>ImportAPI: return post data
else External URL
ImportAPI->>FetchSite: fetch HTML (DNS/redirect checks)
FetchSite->>ImportAPI: return HTML stream
ImportAPI->>ImportAPI: preprocess HTML (lazy images, size limits)
ImportAPI->>Readability: extract article content
Readability->>ImportAPI: return article
end
ImportAPI->>ImportAPI: convert to Markdown, extract thumbnail/date/tags
ImportAPI->>Browser: return ImportResult
Browser->>Browser: apply result to editor state / navigate
Browser->>User: show imported content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/src/app/api/import/route.ts (2)
390-391: Avoid duplicate URL validation on the external path.Line 391 validates once, then Line 198 validates again in
fetchPage. This duplicates DNS work for non-Hive imports. Consider validating only once for external URLs (while keeping per-redirect validation infetchPage).Also applies to: 198-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/import/route.ts` around lines 390 - 391, The code currently calls validateUrl(url) before calling fetchPage and fetchPage also re-validates per-redirect, causing duplicate DNS/validation work for external (non-Hive) imports; remove the pre-fetch duplicate call to validateUrl(url) in the external import path and rely on fetchPage's internal per-redirect validation instead, but retain the initial validateUrl call for the Hive-specific import path if that behavior is needed; update the route handler in route.ts accordingly by removing or gating the validateUrl(url) invocation around the external-fetch branch while keeping fetchPage's validation intact.
144-144: Extract inline object types to interfaces for consistency with coding guidelines.Line 144's return type (
{ author: string; permlink: string } | null) and the identical result shapes returned byfetchHivePost(lines 178–184) andfetchExternalArticle(lines 363–369) should be defined as interfaces. Both functions return:{ title: string; content: string; thumbnail: string; tags: string[]; source: "hive" | "external" as const }Defining explicit interfaces will improve type strictness, reduce duplication, and align with the coding guideline: "Prefer
interfacefor defining object shapes in TypeScript."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/import/route.ts` at line 144, Replace the inline object type used as the return for parseHiveUrl and the duplicated anonymous result shapes returned by fetchHivePost and fetchExternalArticle with named interfaces: define an interface (e.g., HiveUrl { author: string; permlink: string } | null for parseHiveUrl) and a shared interface (e.g., ArticleData { title: string; content: string; thumbnail: string; tags: string[]; source: "hive" | "external" }) for the post payload, then update parseHiveUrl, fetchHivePost, and fetchExternalArticle signatures and return annotations to use these interfaces (and use the literal union "hive" | "external" on the source field) to remove inline object types and eliminate duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/api/import/route.ts`:
- Around line 381-415: The POST handler conflates client errors (bad JSON,
invalid URL, not-found imports) with server errors and uses any for the outer
catch; wrap request.json() in its own try/catch to return 400 on parse failures,
change inner catch for validateUrl to narrow the error type (use unknown and
type guards) instead of swallowing, and update the outer catch to accept unknown
(e: unknown) and map known error messages via ERROR_CODES to non-500 statuses
(e.g., 400 for invalid input, 404 for not-found) before defaulting to 500;
reference POST, request.json, validateUrl, parseHiveUrl, fetchHivePost,
fetchExternalArticle, and ERROR_CODES when making these changes.
- Around line 381-416: Add unit/integration tests for the POST handler exported
from route.ts to cover validation, Hive post handling, external-article
handling, and error mapping: write tests that send a request JSON with an
invalid url to assert a 400 and error "import-error-invalid-url" (exercise
validateUrl failure), a Hive post URL that mocks parseHiveUrl to return
author/permlink and mocks fetchHivePost to return null to assert 404 and
"import-error-not-found", a successful Hive post path that mocks fetchHivePost
to return result and asserts JSON response, a successful external article path
that mocks parseHiveUrl to return null and fetchExternalArticle to return result
and asserts JSON response, and a thrown error from
fetchExternalArticle/fetchHivePost to assert 500 and mapped ERROR_CODES value
("import-failed" when unmapped). Use the POST function export, and stub/mock
validateUrl, parseHiveUrl, fetchHivePost, fetchExternalArticle and the
ERROR_CODES lookup to isolate behavior.
---
Nitpick comments:
In `@apps/web/src/app/api/import/route.ts`:
- Around line 390-391: The code currently calls validateUrl(url) before calling
fetchPage and fetchPage also re-validates per-redirect, causing duplicate
DNS/validation work for external (non-Hive) imports; remove the pre-fetch
duplicate call to validateUrl(url) in the external import path and rely on
fetchPage's internal per-redirect validation instead, but retain the initial
validateUrl call for the Hive-specific import path if that behavior is needed;
update the route handler in route.ts accordingly by removing or gating the
validateUrl(url) invocation around the external-fetch branch while keeping
fetchPage's validation intact.
- Line 144: Replace the inline object type used as the return for parseHiveUrl
and the duplicated anonymous result shapes returned by fetchHivePost and
fetchExternalArticle with named interfaces: define an interface (e.g., HiveUrl {
author: string; permlink: string } | null for parseHiveUrl) and a shared
interface (e.g., ArticleData { title: string; content: string; thumbnail:
string; tags: string[]; source: "hive" | "external" }) for the post payload,
then update parseHiveUrl, fetchHivePost, and fetchExternalArticle signatures and
return annotations to use these interfaces (and use the literal union "hive" |
"external" on the source field) to remove inline object types and eliminate
duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5aa3d6f9-f20d-4b97-b1fa-107ebcff6b06
📒 Files selected for processing (3)
apps/web/src/app/api/import/route.tsapps/web/src/app/publish/_components/publish-import-dialog.tsxapps/web/src/app/publish/import/page.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/app/publish/_components/publish-import-dialog.tsx
- apps/web/src/app/publish/import/page.tsx
Comment on lines
+381
to
+416
| export async function POST(request: NextRequest) { | ||
| try { | ||
| const body = await request.json(); | ||
| const { url } = body; | ||
|
|
||
| if (!url || typeof url !== "string") { | ||
| return Response.json({ error: "import-error-invalid-url" }, { status: 400 }); | ||
| } | ||
|
|
||
| try { | ||
| await validateUrl(url); | ||
| } catch { | ||
| return Response.json({ error: "import-error-invalid-url" }, { status: 400 }); | ||
| } | ||
|
|
||
| // Check if it's a Hive post URL | ||
| const hivePost = parseHiveUrl(url); | ||
| if (hivePost) { | ||
| const result = await fetchHivePost(hivePost.author, hivePost.permlink); | ||
| if (!result) { | ||
| return Response.json( | ||
| { error: "import-error-not-found" }, | ||
| { status: 404 } | ||
| ); | ||
| } | ||
| return Response.json(result); | ||
| } | ||
|
|
||
| // External article | ||
| const result = await fetchExternalArticle(url); | ||
| return Response.json(result); | ||
| } catch (e: any) { | ||
| const code = ERROR_CODES[e.message] || "import-failed"; | ||
| return Response.json({ error: code }, { status: 500 }); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether tests for the import API route exist in the repository
fd -H -I '(*import*test*|*import*spec*|*route*test*|*route*spec*).(ts|tsx|js|jsx)' apps/web
rg -n --glob 'apps/web/**/*.{test,spec}.{ts,tsx,js,jsx}' '/api/import|import-error-|fetchExternalArticle|parseHiveUrl|POST\(request'Repository: ecency/vision-next
Length of output: 413
🏁 Script executed:
#!/bin/bash
# First, let's find test files related to import or api routes
find apps/web -type f \( -name "*import*.test.*" -o -name "*import*.spec.*" \) 2>/dev/null
echo "---"
# Check for test files in the api directory
find apps/web/src/app/api -type f -name "*.test.*" -o -name "*.spec.*" 2>/dev/null
echo "---"
# Look for __tests__ directories
find apps/web -type d -name "__tests__" -o -type d -name "tests" 2>/dev/nullRepository: ecency/vision-next
Length of output: 245
🏁 Script executed:
#!/bin/bash
# Check if the import route file exists and see when it was added
ls -la apps/web/src/app/api/import/route.ts 2>/dev/null && echo "File exists"
# List the structure of the api directory
ls -la apps/web/src/app/api/ 2>/dev/null | head -20Repository: ecency/vision-next
Length of output: 561
🏁 Script executed:
#!/bin/bash
# Search for test files more broadly
find apps/web -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "*.test.tsx" -o -name "*.spec.tsx" \) | head -20Repository: ecency/vision-next
Length of output: 1897
🏁 Script executed:
#!/bin/bash
# Check for all test files in apps/web/src/app/api directory tree
find apps/web/src/app/api -type f \( -name "*.test.*" -o -name "*.spec.*" \) 2>/dev/null
echo "---"
# List all files in the import api directory
ls -laR apps/web/src/app/api/import/Repository: ecency/vision-next
Length of output: 268
🏁 Script executed:
#!/bin/bash
# Check if there's a broader test pattern used for API routes
find apps/web -type f -name "*.test.ts" -o -name "*.test.js" 2>/dev/null | head -10
echo "---"
# Look for jest or test configuration that might include API routes
find apps/web -name "jest.config.*" -o -name "vitest.config.*" -o -name "tsconfig.test.json" 2>/dev/nullRepository: ecency/vision-next
Length of output: 119
🏁 Script executed:
#!/bin/bash
# Check other API route files to see if they have tests
for route in auth-api healthcheck mattermost stats; do
echo "=== Checking $route ==="
ls -la apps/web/src/app/api/$route/
doneRepository: ecency/vision-next
Length of output: 1237
🏁 Script executed:
#!/bin/bash
# Search for any references to the import route/functionality in test files
rg -l 'import.*route|fetchExternalArticle|parseHiveUrl|/api/import' apps/web --type ts --type tsxRepository: ecency/vision-next
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Check git status and history of the import route file to confirm it's new
cd apps/web && git log --oneline -20 src/app/api/import/route.ts 2>/dev/null || echo "Git log not available or file is new"Repository: ecency/vision-next
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Verify if there are any integration or e2e tests for the import feature
find apps/web -type f \( -name "*.e2e.*" -o -name "*.integration.*" \) 2>/dev/null
echo "---"
# Check if any test files reference the import functionality
rg -i 'import.*api|import.*feature' apps/web --glob '*.{test,spec}.{ts,tsx}' 2>/dev/null | head -5Repository: ecency/vision-next
Length of output: 683
🏁 Script executed:
#!/bin/bash
# Search for any test files in specs directory related to import
find apps/web/src/specs -type f -name "*import*" 2>/dev/null
echo "---"
# Check if there's a pattern for API route tests in specs
ls -la apps/web/src/specs/ 2>/dev/nullRepository: ecency/vision-next
Length of output: 881
🏁 Script executed:
#!/bin/bash
# Look for test files that might test the import API indirectly
rg -l 'import.*api|/api/import|fetchExternalArticle|parseHiveUrl' apps/web/src/specs --type ts 2>/dev/null
echo "---"
# Check the full structure of specs directory
find apps/web/src/specs -type d | head -20Repository: ecency/vision-next
Length of output: 809
🏁 Script executed:
#!/by/bash
# Check what files are in the import directory besides route.ts
git ls-tree -r --name-only HEAD apps/web/src/app/api/import/ 2>/dev/null || echo "Git info not available"
echo "---"
# Check git diff for the import route to see what changed
git show --stat e3e39fad2 2>/dev/null | head -30 || echo "Commit info not available"Repository: ecency/vision-next
Length of output: 1728
🏁 Script executed:
#!/bin/bash
# Final verification: check if import route or its functions are tested elsewhere
rg -r 'import.*route|POST.*import|fetchExternalArticle|parseHiveUrl|validateUrl' apps/web/src/specs --type ts 2>/dev/null | head -20
echo "---"
# Check if the import route functions are exported and tested as utilities
grep -n "export.*function\|export const" apps/web/src/app/api/import/route.ts | head -10Repository: ecency/vision-next
Length of output: 120
🏁 Script executed:
#!/bin/bash
# Verify the scope: check if API routes are supposed to have tests by looking at other projects
find apps -type d -name "api" | xargs -I {} find {} -name "*.spec.*" -o -name "*.test.*" 2>/dev/null | head -20
echo "---"
# Check the vitest config to see if it covers API routes
cat apps/web/vitest.config.ts | head -50Repository: ecency/vision-next
Length of output: 1455
Add tests for the new /api/import route handler.
This route introduces substantial new behavior (URL validation, Hive post parsing, external article fetching, error mapping), but no tests are included. Similar API utility functions elsewhere in the codebase (e.g., apps/web/src/features/market/market-swap-form/api/) have test coverage. Please add tests covering success paths and key error codes (import-error-invalid-url, import-error-not-found, import-failed).
Per coding guidelines: "All new features in @ecency/web require tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/api/import/route.ts` around lines 381 - 416, Add
unit/integration tests for the POST handler exported from route.ts to cover
validation, Hive post handling, external-article handling, and error mapping:
write tests that send a request JSON with an invalid url to assert a 400 and
error "import-error-invalid-url" (exercise validateUrl failure), a Hive post URL
that mocks parseHiveUrl to return author/permlink and mocks fetchHivePost to
return null to assert 404 and "import-error-not-found", a successful Hive post
path that mocks fetchHivePost to return result and asserts JSON response, a
successful external article path that mocks parseHiveUrl to return null and
fetchExternalArticle to return result and asserts JSON response, and a thrown
error from fetchExternalArticle/fetchHivePost to assert 500 and mapped
ERROR_CODES value ("import-failed" when unmapped). Use the POST function export,
and stub/mock validateUrl, parseHiveUrl, fetchHivePost, fetchExternalArticle and
the ERROR_CODES lookup to isolate behavior.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/web/src/app/api/import/route.ts (1)
389-402:⚠️ Potential issue | 🟡 MinorReplace
anywithunknownfor TypeScript strict mode compliance.Line 391 uses
anyfor the parsed body, which violates TypeScript strict mode requirements from the coding guidelines. Useunknownand narrow the type explicitly.🔧 Proposed fix
export async function POST(request: NextRequest) { try { - let body: any; + let body: unknown; try { body = await request.json(); } catch { return Response.json({ error: "import-error-invalid-url" }, { status: 400 }); } - const { url } = body; + const { url } = body as { url?: unknown }; if (!url || typeof url !== "string") { return Response.json({ error: "import-error-invalid-url" }, { status: 400 }); }As per coding guidelines: "TypeScript strict mode is enabled; all new code should include proper types."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/import/route.ts` around lines 389 - 402, In the POST handler replace the loose any with unknown for the parsed body (let body: unknown = await request.json()), then explicitly narrow/guard before destructuring: check body is a non-null object and that it has a 'url' property of type string (e.g., typeof body === 'object' && body !== null && 'url' in body && typeof (body as Record<string, unknown>).url === 'string') and only then extract url; update subsequent validation to use the narrowed type so TypeScript strict mode is satisfied; keep the try/catch around request.json() and return the same 400 error paths on failure.
🧹 Nitpick comments (3)
apps/web/src/app/api/import/route.ts (2)
270-279: Server-side innerHTML assignment is acceptable but document the context.Static analysis flagged this as XSS risk, but since this runs in server-side JSDOM (not a browser), the content is processed through Readability/Turndown before being served, and no scripts are executed in the virtual DOM, the risk is mitigated. Consider adding a brief comment to clarify this is intentional server-side DOM manipulation.
📝 Optional: Add clarifying comment
// 1. Unwrap <noscript> images — many sites hide the real <img> inside <noscript> for (const noscript of Array.from(document.querySelectorAll("noscript"))) { const content = noscript.textContent || ""; if (/<img\s/i.test(content)) { const wrapper = document.createElement("div"); + // Safe: server-side JSDOM manipulation; content is sanitized through Readability/Turndown wrapper.innerHTML = content; noscript.parentNode?.replaceChild(wrapper, noscript); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/import/route.ts` around lines 270 - 279, The innerHTML assignment in fixLazyImages (wrapper.innerHTML = content) is intentionally done in a server-side JSDOM context to unwrap <noscript> images; add a brief comment above that line explaining this is safe because it runs on the server with no script execution, content has been run through Readability/Turndown, and this is deliberate to recover real <img> elements from noscript blocks so static-analysis XSS warnings can be ignored; reference the function name fixLazyImages and the wrapper.innerHTML usage in the comment so future reviewers understand the rationale.
426-430: Consider returning appropriate HTTP status codes for client errors.Currently all caught errors return 500, but some error codes represent client issues (e.g.,
RESPONSE_TOO_LARGE→ 413,NOT_HTML→ 415) rather than server failures. This makes error categorization clearer for clients.♻️ Proposed fix to map errors to appropriate status codes
} catch (e: unknown) { const message = e instanceof Error ? e.message : ""; const code = ERROR_CODES[message] || "import-failed"; - return Response.json({ error: code }, { status: 500 }); + const status = + message === "RESPONSE_TOO_LARGE" ? 413 : + message === "NOT_HTML" ? 415 : + message === "EXTRACT_FAILED" ? 422 : + message === "FETCH_FAILED" ? 502 : + 500; + return Response.json({ error: code }, { status }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/import/route.ts` around lines 426 - 430, The catch block currently always returns status 500; update it to map specific error codes from ERROR_CODES (e.g., RESPONSE_TOO_LARGE, NOT_HTML) to appropriate HTTP statuses (413, 415, etc.) before returning the Response. Create a small status mapping (e.g., STATUS_FOR_ERROR or a getStatusForError function) and use it to look up the status for the computed code, defaulting to 500 for unknown codes; then return Response.json({ error: code }, { status }). Update the catch logic that computes message and code to use this new mapping so client errors are returned with correct HTTP status codes.apps/web/src/specs/api/import-route.spec.ts (1)
172-189: Consider adding tests for additional error scenarios.The error mapping tests cover
FETCH_FAILEDand unknown errors, but the route also handlesRESPONSE_TOO_LARGE,NOT_HTML, andEXTRACT_FAILED. Adding tests for these would improve coverage.📝 Optional: Add tests for other error codes
it("returns 500 with error code for non-HTML content", async () => { const mockResponse = { ok: true, status: 200, headers: new Headers({ "content-type": "application/json" }), body: null }; vi.spyOn(globalThis, "fetch").mockResolvedValue(mockResponse as any); const res = await POST(makeRequest({ url: "https://example.com/api" })); expect(res.status).toBe(500); const data = await res.json(); expect(data.error).toBe("import-error-not-html"); }); it("returns 500 with error code when extraction fails", async () => { vi.mocked(JSDOM).mockImplementation(() => ({ window: { document: { querySelector: vi.fn().mockReturnValue(null), querySelectorAll: vi.fn().mockReturnValue([]) } } }) as any); vi.mocked(Readability).mockImplementation(() => ({ parse: () => null }) as any); const mockResponse = { ok: true, status: 200, headers: new Headers({ "content-type": "text/html" }), body: { getReader: () => ({ read: vi.fn() .mockResolvedValueOnce({ done: false, value: new TextEncoder().encode("<html></html>") }) .mockResolvedValueOnce({ done: true, value: undefined }) }) } }; vi.spyOn(globalThis, "fetch").mockResolvedValue(mockResponse as any); const res = await POST(makeRequest({ url: "https://example.com/article" })); expect(res.status).toBe(500); const data = await res.json(); expect(data.error).toBe("import-error-extract-failed"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/specs/api/import-route.spec.ts` around lines 172 - 189, Add unit tests covering the other mapped error scenarios: simulate RESPONSE_TOO_LARGE by mocking globalThis.fetch to return a response with large content/stream behavior that triggers the route's size check and assert POST(makeRequest(...)) returns 500 with error "import-error-response-too-large"; simulate NOT_HTML by returning a response with content-type "application/json" and assert error "import-error-not-html"; simulate EXTRACT_FAILED by mocking JSDOM and Readability (as shown in the suggestion) so parsing returns null and assert error "import-error-extract-failed". Use the same test helpers (POST, makeRequest) and mocking patterns in the existing tests to keep style consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/web/src/app/api/import/route.ts`:
- Around line 389-402: In the POST handler replace the loose any with unknown
for the parsed body (let body: unknown = await request.json()), then explicitly
narrow/guard before destructuring: check body is a non-null object and that it
has a 'url' property of type string (e.g., typeof body === 'object' && body !==
null && 'url' in body && typeof (body as Record<string, unknown>).url ===
'string') and only then extract url; update subsequent validation to use the
narrowed type so TypeScript strict mode is satisfied; keep the try/catch around
request.json() and return the same 400 error paths on failure.
---
Nitpick comments:
In `@apps/web/src/app/api/import/route.ts`:
- Around line 270-279: The innerHTML assignment in fixLazyImages
(wrapper.innerHTML = content) is intentionally done in a server-side JSDOM
context to unwrap <noscript> images; add a brief comment above that line
explaining this is safe because it runs on the server with no script execution,
content has been run through Readability/Turndown, and this is deliberate to
recover real <img> elements from noscript blocks so static-analysis XSS warnings
can be ignored; reference the function name fixLazyImages and the
wrapper.innerHTML usage in the comment so future reviewers understand the
rationale.
- Around line 426-430: The catch block currently always returns status 500;
update it to map specific error codes from ERROR_CODES (e.g.,
RESPONSE_TOO_LARGE, NOT_HTML) to appropriate HTTP statuses (413, 415, etc.)
before returning the Response. Create a small status mapping (e.g.,
STATUS_FOR_ERROR or a getStatusForError function) and use it to look up the
status for the computed code, defaulting to 500 for unknown codes; then return
Response.json({ error: code }, { status }). Update the catch logic that computes
message and code to use this new mapping so client errors are returned with
correct HTTP status codes.
In `@apps/web/src/specs/api/import-route.spec.ts`:
- Around line 172-189: Add unit tests covering the other mapped error scenarios:
simulate RESPONSE_TOO_LARGE by mocking globalThis.fetch to return a response
with large content/stream behavior that triggers the route's size check and
assert POST(makeRequest(...)) returns 500 with error
"import-error-response-too-large"; simulate NOT_HTML by returning a response
with content-type "application/json" and assert error "import-error-not-html";
simulate EXTRACT_FAILED by mocking JSDOM and Readability (as shown in the
suggestion) so parsing returns null and assert error
"import-error-extract-failed". Use the same test helpers (POST, makeRequest) and
mocking patterns in the existing tests to keep style consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb550320-c5b8-4a4c-880b-0f01ec587b7e
📒 Files selected for processing (2)
apps/web/src/app/api/import/route.tsapps/web/src/specs/api/import-route.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes / UI
Tests