feat: support users orgs#130
Conversation
929d2d3 to
debb502
Compare
|
@ilyaliao is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughA new API endpoint Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Server as "App Route\n(routes/users/[name]/orgs.ts)"
participant ghFetch as "ghFetch (HTTP helper)"
participant GitHub as "GitHub API"
Client->>Server: GET /users/{username}/orgs
Server->>ghFetch: request /users/{username}/orgs?per_page=100
ghFetch->>GitHub: HTTP GET
GitHub-->>ghFetch: orgs JSON[]
ghFetch-->>Server: raw orgs JSON[]
Server->>Server: map -> {id, name (login), description}
Server-->>Client: 200 { "orgs": [ ... ] }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 Tip You can disable the changed files summary in the walkthrough.Disable the |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
routes/users/[name]/orgs.ts (1)
4-6: Consider validating thenameparameter.
getRouterParamcan returnundefinedif the parameter is missing. While the route path should guarantee it exists, adding a guard improves robustness and provides a clearer error message.♻️ Suggested validation
const name = getRouterParam(event, "name"); +if (!name) { + throw new HTTPError({ message: "Username is required", statusCode: 400 }); +} // TODO: Do paginationNote:
HTTPErrorshould be imported fromnitro/h3per coding guidelines.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@routes/users/`[name]/orgs.ts around lines 4 - 6, Validate the router param returned by getRouterParam before using it: if name is undefined or empty, throw an HTTPError (import HTTPError from 'nitro/h3') with a clear 400 Bad Request message, and only call ghFetch(`users/${name}/orgs?per_page=100`) when name is present; update the top of users/[name]/orgs.ts to import HTTPError and add a short guard around the existing ghFetch usage to fail fast with a descriptive error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@routes/users/`[name]/orgs.ts:
- Around line 1-3: Replace the incorrect imports and handler: import the
GithubOrg type from "~/types", import defineHandler and getRouterParam from
"nitro" / "nitro/h3" respectively, and import ghFetch from "~/utils/github";
then change the exported default function to use defineHandler(...) instead of
eventHandler(...), and use getRouterParam(event, "name") inside that handler to
read the route param and call ghFetch to fetch orgs (referencing the GithubOrg
type for typing).
---
Nitpick comments:
In `@routes/users/`[name]/orgs.ts:
- Around line 4-6: Validate the router param returned by getRouterParam before
using it: if name is undefined or empty, throw an HTTPError (import HTTPError
from 'nitro/h3') with a clear 400 Bad Request message, and only call
ghFetch(`users/${name}/orgs?per_page=100`) when name is present; update the top
of users/[name]/orgs.ts to import HTTPError and add a short guard around the
existing ghFetch usage to fail fast with a descriptive error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 447e3e0f-4631-4515-9d16-1a3a634f24e0
📒 Files selected for processing (2)
README.mdroutes/users/[name]/orgs.ts
| import type { GithubOrg } from "~types"; | ||
|
|
||
| export default eventHandler(async (event) => { |
There was a problem hiding this comment.
Missing imports and incorrect handler function.
Several coding guideline violations:
- Use
defineHandlerfromnitroinstead ofeventHandler - Use
~/typespath alias format (not~types) - Import
getRouterParamfromnitro/h3 - Import
ghFetchfrom~/utils/github
🔧 Proposed fix
-import type { GithubOrg } from "~types";
+import { defineHandler, getRouterParam } from "nitro";
+import type { GithubOrg } from "~/types";
+import { ghFetch } from "~/utils/github";
-export default eventHandler(async (event) => {
+export default defineHandler(async (event) => {As per coding guidelines: "Route handlers must use defineHandler() from nitro" and "Use path alias ~/* for imports... with extensionless paths (e.g. ~/utils/github)". Based on learnings: "Use defineHandler() instead of eventHandler() or defineEventHandler() for route handlers".
📝 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.
| import type { GithubOrg } from "~types"; | |
| export default eventHandler(async (event) => { | |
| import { defineHandler, getRouterParam } from "nitro"; | |
| import type { GithubOrg } from "~/types"; | |
| import { ghFetch } from "~/utils/github"; | |
| export default defineHandler(async (event) => { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@routes/users/`[name]/orgs.ts around lines 1 - 3, Replace the incorrect
imports and handler: import the GithubOrg type from "~/types", import
defineHandler and getRouterParam from "nitro" / "nitro/h3" respectively, and
import ghFetch from "~/utils/github"; then change the exported default function
to use defineHandler(...) instead of eventHandler(...), and use
getRouterParam(event, "name") inside that handler to read the route param and
call ghFetch to fetch orgs (referencing the GithubOrg type for typing).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
routes/users/[name]/orgs.ts (1)
21-21:⚠️ Potential issue | 🟠 MajorUse
getRouterParaminstead ofevent.context.params!.name.Direct non-null assertion on
event.context.paramsis brittle and bypasses the H3 utility expected by this codebase.Suggested change
import { defineRouteMeta, defineHandler } from "nitro"; +import { getRouterParam, HTTPError } from "nitro/h3"; import { ghFetch } from "~/utils/github"; import type { GithubOrg } from "~/types"; @@ export default defineHandler(async (event) => { // TODO: Do pagination - const rawOrgs = await ghFetch(`users/${event.context.params!.name}/orgs?per_page=100`); + const name = getRouterParam(event, "name"); + if (!name) { + throw new HTTPError({ statusCode: 400, message: "Missing required path param: name" }); + } + const rawOrgs = await ghFetch(`users/${name}/orgs?per_page=100`);#!/bin/bash # Verify route-param extraction style in the sibling handlers. rg -n --type=ts -C2 'event\.context\.params!?\.name|getRouterParam\(event,\s*"name"\)' \ 'routes/users/[name]/orgs.ts' \ 'routes/users/[name]/repos.ts'As per coding guidelines: "Use H3 utilities from
nitro/h3:getRouterParam...".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@routes/users/`[name]/orgs.ts at line 21, Replace the brittle non-null assertion event.context.params!.name with the H3 utility by calling getRouterParam(event, "name") and use that value when building the ghFetch URL (the call to ghFetch(`users/${...}/orgs?per_page=100`)); update any surrounding logic that reads event.context.params to use getRouterParam, ensuring you import getRouterParam and handle the possibility of a missing param (e.g., throw or return a 400) before calling ghFetch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@routes/users/`[name]/orgs.ts:
- Around line 23-29: The mapping over rawOrgs currently uses rawOrg: any and a
force-cast <GithubOrg>, which allows GitHub's nullable description to slip into
the non-nullable GithubOrg.description; change the map to type the input (e.g.,
(rawOrg: { id: number; login: string; description: string | null }) => ...) or
use a typed helper, remove the <GithubOrg> assertion, and normalize description
with a safe fallback (e.g., rawOrg.description ?? '' or
String(rawOrg.description) as needed) so the produced object conforms to the
GithubOrg type without bypassing the compiler.
---
Duplicate comments:
In `@routes/users/`[name]/orgs.ts:
- Line 21: Replace the brittle non-null assertion event.context.params!.name
with the H3 utility by calling getRouterParam(event, "name") and use that value
when building the ghFetch URL (the call to
ghFetch(`users/${...}/orgs?per_page=100`)); update any surrounding logic that
reads event.context.params to use getRouterParam, ensuring you import
getRouterParam and handle the possibility of a missing param (e.g., throw or
return a 400) before calling ghFetch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d78edd7d-8d31-4342-abb6-7ceb2a30318b
📒 Files selected for processing (1)
routes/users/[name]/orgs.ts
| const orgs = rawOrgs.map( | ||
| (rawOrg: any) => | ||
| <GithubOrg>{ | ||
| id: rawOrg.id, | ||
| name: rawOrg.login, | ||
| description: rawOrg.description, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In GitHub REST API docs for "List organizations for a user", is the description field nullable (null) in the org payload?
💡 Result:
Yes, the description field in the organization payload for the "List organizations for a user" endpoint (GET /users/{username}/orgs) is nullable (can be null). The endpoint returns an array of Organization Simple objects. In the official GitHub REST API documentation, the schema for Organization Simple explicitly defines description as "required, string or null". This is confirmed across multiple pages, including the response schema for related endpoints like List organizations (GET /organizations), which uses the same object type. The documentation states for List organizations for a user: "Response schema (Status: 200): Same response schema as List organizations." And for List organizations: "Array of Organization Simple: - description: required, string or null".
Citations:
- 1: https://docs.github.com/rest/orgs/orgs
- 2: https://docs.github.com/en/rest/orgs/orgs?apiVersion=2022-11-28
- 3: https://docs.github.com/en/rest/orgs/orgs
- 4: https://github.apidog.io/api-3489096
- 5: http://github.apifox.cn/schema-1045422
🏁 Script executed:
#!/bin/bash
# Find and inspect the GithubOrg type definition
fd -t f '\.ts$' -x rg -l 'interface GithubOrg|type GithubOrg' {} \;Repository: unjs/ungh
Length of output: 69
🏁 Script executed:
#!/bin/bash
# Inspect the actual file mentioned in the review
cat -n routes/users/[name]/orgs.ts | head -40Repository: unjs/ungh
Length of output: 1078
🏁 Script executed:
#!/bin/bash
# Search for GithubOrg definition more broadly
rg 'interface GithubOrg|type GithubOrg' -A 5Repository: unjs/ungh
Length of output: 227
Avoid any + type assertion; handle nullable GitHub API description field.
The GitHub API returns description: string | null for organizations, but GithubOrg.description is typed as a required string. The current type assertion <GithubOrg> bypasses TypeScript's type checking and allows null values to be assigned to a non-nullable field, creating a type safety violation.
Suggested change
- const orgs = rawOrgs.map(
- (rawOrg: any) =>
- <GithubOrg>{
- id: rawOrg.id,
- name: rawOrg.login,
- description: rawOrg.description,
- },
- );
+ const orgs: GithubOrg[] = (rawOrgs as Array<{ id: number; login: string; description: string | null }>).map(
+ (rawOrg) => ({
+ id: rawOrg.id,
+ name: rawOrg.login,
+ description: rawOrg.description ?? "",
+ }),
+ );📝 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.
| const orgs = rawOrgs.map( | |
| (rawOrg: any) => | |
| <GithubOrg>{ | |
| id: rawOrg.id, | |
| name: rawOrg.login, | |
| description: rawOrg.description, | |
| }, | |
| const orgs: GithubOrg[] = (rawOrgs as Array<{ id: number; login: string; description: string | null }>).map( | |
| (rawOrg) => ({ | |
| id: rawOrg.id, | |
| name: rawOrg.login, | |
| description: rawOrg.description ?? "", | |
| }), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@routes/users/`[name]/orgs.ts around lines 23 - 29, The mapping over rawOrgs
currently uses rawOrg: any and a force-cast <GithubOrg>, which allows GitHub's
nullable description to slip into the non-nullable GithubOrg.description; change
the map to type the input (e.g., (rawOrg: { id: number; login: string;
description: string | null }) => ...) or use a typed helper, remove the
<GithubOrg> assertion, and normalize description with a safe fallback (e.g.,
rawOrg.description ?? '' or String(rawOrg.description) as needed) so the
produced object conforms to the GithubOrg type without bypassing the compiler.
resolves #129
This PR add a route to retrieve the user's orgs.
Summary by CodeRabbit
New Features
Documentation