-
Notifications
You must be signed in to change notification settings - Fork 10
Add endpoint for fetching a post, Update docs #70
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new API endpoint was added to fetch a single post by ID, including both backend implementation and documentation. Next.js rewrite rules were updated to route requests to the new endpoint. Several API handlers had their error logging messages improved for clarity. No changes were made to existing public interfaces or exported entities except for the new endpoint. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Next.js Router
participant API Handler ([id].ts)
participant Supabase
participant Docs
Client->>Next.js Router: GET /changes.json/:id
Next.js Router->>API Handler ([id].ts): /api/post/:id
API Handler ([id].ts)->>Supabase: Query post by ID and page
Supabase-->>API Handler ([id].ts): Post data
API Handler ([id].ts)-->>Client: JSON response with post (or 404)
Docs-->>Client: Documents new endpoint usage
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/docs/content/docs/api/page.mdx (1)
74-74
: Fix grammar in headingThe singular article "a" doesn't match with the plural noun "posts".
-### Get a posts by id +### Get a post by id🧰 Tools
🪛 LanguageTool
[grammar] ~74-~74: It looks like ‘posts’ doesn’t match ‘a’. Did you mean “a post” or just “posts”?
Context: ...requested features!" } ] ``` ### Get a posts by id This endpoint will return a JSON...(A_NNS_IN)
apps/page/pages/api/post/[id].ts (4)
62-68
: Consider simplifying single post URL mappingSince you're only working with a single post (as indicated by the limit(1)), you could simplify this mapping operation.
- const postsWithUrl = posts.map((post) => { - return { - ...post, - url: getPostUrl(pageUrl, post), - }; - }); - - res.status(200).json(postsWithUrl[0] ?? null); + const post = posts[0]; + res.status(200).json({ + ...post, + url: getPostUrl(pageUrl, post), + });
70-70
: Fix error type annotation in catch clauseThe catch clause variable type annotation must be 'any' or 'unknown' if specified.
- } catch (e: Error | any) { + } catch (e: unknown) {🧰 Tools
🪛 Biome (1.9.4)
[error] 70-70: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
71-71
: Enhance error loggingConsider improving the error logging to include more context about the error.
- console.log("Failed to fetch post [Error]", e); + console.error("Failed to fetch post by ID:", id, e instanceof Error ? e.message : e);
20-24
: Validate ID format before processingConsider validating that the ID is in the expected UUID format before proceeding with the database query.
const { id } = req.query; - if (!id) { + // Validate ID is present and in the expected format + const postId = String(id); + const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + + if (!postId || !uuidRegex.test(postId)) { res.status(404).json(null); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (7)
apps/docs/content/docs/api/page.mdx
(2 hunks)apps/page/next.config.js
(1 hunks)apps/page/pages/api/json.ts
(2 hunks)apps/page/pages/api/latest.ts
(1 hunks)apps/page/pages/api/markdown.ts
(1 hunks)apps/page/pages/api/pinned.ts
(1 hunks)apps/page/pages/api/post/[id].ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/page/pages/api/post/[id].ts (3)
packages/supabase/types/page.ts (1)
IPost
(6-6)apps/page/lib/data.ts (2)
translateHostToPageIdentifier
(360-360)fetchRenderData
(357-357)packages/supabase/admin.ts (1)
supabaseAdmin
(4-7)
🪛 LanguageTool
apps/docs/content/docs/api/page.mdx
[grammar] ~74-~74: It looks like ‘posts’ doesn’t match ‘a’. Did you mean “a post” or just “posts”?
Context: ...requested features!" } ] ``` ### Get a posts by id This endpoint will return a JSON...
(A_NNS_IN)
🪛 Biome (1.9.4)
apps/page/pages/api/post/[id].ts
[error] 70-70: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
🔇 Additional comments (10)
apps/page/pages/api/latest.ts (1)
62-62
: Improved error logging for better debuggingThe error message is now more specific, which will improve debugging and error tracking capabilities.
apps/page/pages/api/pinned.ts (1)
63-63
: Enhanced error message for clarityThe error logging message is now more descriptive, clearly indicating that the failure occurred while fetching a pinned post.
apps/page/pages/api/markdown.ts (1)
43-43
: More detailed error loggingThe improved error message now provides context about both the operation that failed and the endpoint, which will help with debugging.
apps/page/pages/api/json.ts (2)
2-3
: Import reordering looks goodThe import statements have been reordered to place utility imports before framework imports, which improves organization.
46-46
: Enhanced error logging messageImproved error message clarity is consistent with the pattern applied to other API handlers in this PR.
apps/page/next.config.js (1)
90-93
: Well-implemented rewrite rule for new post endpointThe new rewrite rule correctly maps
/changes.json/:id
to the new/api/post/:id
endpoint, following the established pattern of other API routes in the application.This change properly exposes the new API functionality for fetching individual posts by ID, which aligns with the PR objective of adding a new endpoint for fetching a post.
apps/docs/content/docs/api/page.mdx (1)
75-93
: Documentation for new endpoint looks goodThe documentation follows the established pattern for API endpoints in this file, with clear request format and example response structure.
apps/page/pages/api/post/[id].ts (3)
1-10
: Imports look goodAll necessary imports are properly structured and organized.
11-15
: IPostWithUrl type definition is appropriateThe type definition properly extends the IPost type with a URL field.
41-49
: Query parameters and constraints look goodThe Supabase query properly filters for the specified post ID, page ID, and published status.
const posts = data as Array<IPost>; | ||
|
||
if (!posts?.length) { | ||
res.status(404).json(null); | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Add plain_text_content to response
Based on the example response in the documentation, the API should include a plain_text_content
field, but it's not selected in the query or added to the response.
🏁 Script executed:
#!/bin/bash
# Check if the IPost type includes plain_text_content field
rg -A 15 "export type IPost" packages/supabase/types/page.ts
# Check if other post endpoints include plain_text_content
rg -A 5 "plain_text_content" apps/page/pages/api
Length of output: 1050
Include plain_text_content
in the post detail response
The GET /api/post/[id]
handler currently returns the raw IPost
rows, but the example response in our docs and the /api/json
endpoint both include a computed plain_text_content
field. Since IPost
is just a direct mapping of your DB row, you need to append this field yourself.
• File: apps/page/pages/api/post/[id].ts
After you’ve fetched and validated posts
(around where you call res.status(200).json(...)
), map each post to include the plain‐text content:
import { convertMarkdownToPlainText } from 'path/to/markdown-utils';
const postsWithPlainText = posts.map(post => ({
...post,
plain_text_content: convertMarkdownToPlainText(post.content),
}));
res.status(200).json(postsWithPlainText);
This will keep your detail endpoint consistent with the docs and the JSON list endpoint.
🤖 Prompt for AI Agents
In apps/page/pages/api/post/[id].ts around lines 55 to 60, the API response
currently returns posts without the plain_text_content field required by the
documentation. To fix this, after validating the posts array, map each post to
add a plain_text_content property by converting the post's content from markdown
to plain text using a utility function like convertMarkdownToPlainText. Then
return this mapped array in the JSON response to ensure the API output matches
the documented format.
Summary by CodeRabbit
New Features
/changes.json/:id
.Documentation
Bug Fixes