Update home page buttons to be dynamic - #112
Conversation
📝 WalkthroughWalkthroughAdds a Next.js Edge route for validated home content retrieval and updates the home component to fetch and render visible action buttons dynamically instead of using a hard-coded registration button. ChangesHome content actions
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Home as Home component
participant Route as /api/home
participant API as Site content API
Home->>Route: Fetch home content
Route->>API: Request /api/v1/site-content
API-->>Route: Return site content JSON
Route-->>Home: Return validated home response
Home->>Home: Filter visible actions
Home-->>Home: Render external action buttons
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/api/home/route.tsOops! Something went wrong! :( ESLint: 9.39.2 TypeError: Converting circular structure to JSON components/Home.tsxOops! Something went wrong! :( ESLint: 9.39.2 TypeError: Converting circular structure to JSON 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 |
Deploying call-of-code with
|
| Latest commit: |
90bb83b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://51f740dc.call-of-code.pages.dev |
| Branch Preview URL: | https://dynamic-buttons.call-of-code.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
components/Home.tsx (2)
229-231: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueCompute filtered actions once instead of filtering twice.
actions.filter(action => action.isVisible)is called on line 229 for the length check and again on line 231 for the map. Extract the filtered list into a variable to avoid redundant computation and improve readability.♻️ Proposed fix: compute visible actions once
Add this before the
returnstatement (around line 118):+ const visibleActions = actions.filter(action => action.isVisible); + return (Then update the rendering block:
- {actions.filter(action => action.isVisible).length > 0 && ( + {visibleActions.length > 0 && ( <div className="flex flex-wrap justify-center items-center gap-6 mb-32 z-20"> - {actions.filter(action => action.isVisible).map((action) => ( + {visibleActions.map((action) => (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/Home.tsx` around lines 229 - 231, In the Home component, compute the visible actions once before the return using the existing actions collection and isVisible predicate. Update the conditional rendering and mapping block to reuse that filtered variable instead of calling actions.filter twice.
11-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport shared types instead of redefining them.
HomeAction,GalleryItem, andHomeResponseare defined identically in both this file andapp/api/home/route.ts(which exports them). Useimport typeto avoid duplication and prevent type drift.♻️ Proposed fix: import from route file or shared types module
-interface HomeAction { - key: string; - label: string; - url: string; - isVisible: boolean; -} - -interface GalleryItem { - imageUrl: string; - caption: string; - altText: string; -} - -interface HomeResponse { - success: boolean; - data: { - actions: HomeAction[]; - hero: { - imageUrl: string; - caption: string; - altText: string; - }; - gallery: GalleryItem[]; - }; -} +import type { HomeAction, HomeResponse } from "`@/app/api/home/route`";Alternatively, extract shared types to a dedicated
types/home.tsmodule and import from there in both files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/Home.tsx` around lines 11 - 35, Remove the local HomeAction, GalleryItem, and HomeResponse declarations in Home.tsx and import these types with import type from the existing app/api/home/route.ts exports, or from a shared home types module if one is established. Update the component references to use the imported types without changing their structure.app/api/home/route.ts (1)
62-65: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAvoid logging full upstream response payloads.
Lines 62 and 74 log the complete upstream response text and parsed data via
console.error. If the upstream response contains PII, internal paths, or secrets, these will be persisted to server logs. Log a redacted summary or response length instead.♻️ Proposed fix: redact logged payloads
- console.error("Invalid JSON from upstream API:", text, err.message); + console.error("Invalid JSON from upstream API (length=%d): %s", text.length, err.message);- console.error("Unexpected API response structure", data); + console.error("Unexpected API response structure (keys=%j)", Object.keys(data ?? {}));Also applies to: 74-74
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/home/route.ts` around lines 62 - 65, Update the JSON error logging near the invalid upstream response handling to stop emitting the full response text or parsed payload. In the relevant catch and fallback branches, log only a redacted summary such as response length and the parsing error message, while preserving the existing error context and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/home/route.ts`:
- Around line 73-79: Extend the response validation in the home route around the
existing data.data.actions check to validate every action object and its
required fields, including enforcing string types such as url and boolean types
such as isVisible. Reject the entire upstream response with the existing 500
Invalid response result when any action is malformed, while preserving valid
action arrays unchanged.
In `@components/Home.tsx`:
- Line 234: Validate action.url before assigning it to href in the
action-rendering flow of Home, allowing only safe protocols such as http: and
https:. Reject or omit actions with javascript:, data:, or other unsupported
protocols, while preserving valid URL rendering; apply the same validation at
the API filtering boundary if that is the established action-sanitization path.
---
Nitpick comments:
In `@app/api/home/route.ts`:
- Around line 62-65: Update the JSON error logging near the invalid upstream
response handling to stop emitting the full response text or parsed payload. In
the relevant catch and fallback branches, log only a redacted summary such as
response length and the parsing error message, while preserving the existing
error context and behavior.
In `@components/Home.tsx`:
- Around line 229-231: In the Home component, compute the visible actions once
before the return using the existing actions collection and isVisible predicate.
Update the conditional rendering and mapping block to reuse that filtered
variable instead of calling actions.filter twice.
- Around line 11-35: Remove the local HomeAction, GalleryItem, and HomeResponse
declarations in Home.tsx and import these types with import type from the
existing app/api/home/route.ts exports, or from a shared home types module if
one is established. Update the component references to use the imported types
without changing their structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f40d2d0-390d-4927-870a-3faf8cc5a7af
📒 Files selected for processing (2)
app/api/home/route.tscomponents/Home.tsx
| if (!data || !data.success || !data.data || !Array.isArray(data.data.actions)) { | ||
| console.error("Unexpected API response structure", data); | ||
| return NextResponse.json( | ||
| { success: false, message: "Invalid response from upstream API", data: { actions: [] } }, | ||
| { status: 500 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate individual action objects before passing through to the client.
The route checks that data.data.actions is an array but does not validate the shape of each element. If the upstream returns actions with missing or malformed fields (e.g., url as a non-string, isVisible as the string "false" instead of boolean false), the client will render broken UI or show buttons that should be hidden.
🛡️ Proposed fix: add per-action validation
if (!data || !data.success || !data.data || !Array.isArray(data.data.actions)) {
console.error("Unexpected API response structure", data);
return NextResponse.json(
{ success: false, message: "Invalid response from upstream API", data: { actions: [] } },
{ status: 500 }
);
}
+ const validActions = data.data.actions.filter(
+ (a): a is HomeAction =>
+ a != null &&
+ typeof a === "object" &&
+ typeof a.key === "string" &&
+ typeof a.label === "string" &&
+ typeof a.url === "string" &&
+ typeof a.isVisible === "boolean"
+ );
+
+ if (validActions.length !== data.data.actions.length) {
+ console.error("Some actions failed validation");
+ }
- return NextResponse.json({
+ return NextResponse.json({
success: true,
- data: data.data,
+ data: { ...data.data, actions: validActions },
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!data || !data.success || !data.data || !Array.isArray(data.data.actions)) { | |
| console.error("Unexpected API response structure", data); | |
| return NextResponse.json( | |
| { success: false, message: "Invalid response from upstream API", data: { actions: [] } }, | |
| { status: 500 } | |
| ); | |
| } | |
| if (!data || !data.success || !data.data || !Array.isArray(data.data.actions)) { | |
| console.error("Unexpected API response structure", data); | |
| return NextResponse.json( | |
| { success: false, message: "Invalid response from upstream API", data: { actions: [] } }, | |
| { status: 500 } | |
| ); | |
| } | |
| const validActions = data.data.actions.filter( | |
| (a): a is HomeAction => | |
| a != null && | |
| typeof a === "object" && | |
| typeof a.key === "string" && | |
| typeof a.label === "string" && | |
| typeof a.url === "string" && | |
| typeof a.isVisible === "boolean" | |
| ); | |
| if (validActions.length !== data.data.actions.length) { | |
| console.error("Some actions failed validation"); | |
| } | |
| return NextResponse.json({ | |
| success: true, | |
| data: { ...data.data, actions: validActions }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/home/route.ts` around lines 73 - 79, Extend the response validation
in the home route around the existing data.data.actions check to validate every
action object and its required fields, including enforcing string types such as
url and boolean types such as isVisible. Reject the entire upstream response
with the existing 500 Invalid response result when any action is malformed,
while preserving valid action arrays unchanged.
| {actions.filter(action => action.isVisible).map((action) => ( | ||
| <a | ||
| key={action.key} | ||
| href={action.url} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate action.url protocol before rendering as href.
The URL from the API response is used directly in href without protocol validation. A compromised or buggy upstream could return javascript: or data: URLs that execute arbitrary code. While target="_blank" blocks javascript: in modern browsers, data: URLs remain exploitable.
🛡️ Proposed fix: validate URL protocol before rendering
<a
key={action.key}
- href={action.url}
+ href={action.url.startsWith("http://") || action.url.startsWith("https://") ? action.url : "#"}
target="_blank"
rel="noopener noreferrer"
className="inline-block"
>Alternatively, perform this validation in the API route when filtering actions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| href={action.url} | |
| href={action.url.startsWith("http://") || action.url.startsWith("https://") ? action.url : "#"} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/Home.tsx` at line 234, Validate action.url before assigning it to
href in the action-rendering flow of Home, allowing only safe protocols such as
http: and https:. Reject or omit actions with javascript:, data:, or other
unsupported protocols, while preserving valid URL rendering; apply the same
validation at the API filtering boundary if that is the established
action-sanitization path.
Summary by CodeRabbit
New Features
Bug Fixes
Style