Admin UI changes #94
Admin UI changes #94
Conversation
…t NUll constraint with default value USER
…ngo.ai into admin-ui-changes
|
Added @poojalandejosh |
|
Added All Required Admin APIS and Its postman collection. with reqest Body and responses. |
|
…sers and subscription
|
Let’s address the current build errors first. Also, please take a look at the following warnings related to missing dependencies in useEffect hooks. It would be good to refactor these as well: |
|
/review |
Changelist by BitoThis pull request implements the following key changes.
|
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #d83474
Actionable Suggestions - 4
-
app/src/app/api/admin/subscriptions/[id]/route.ts - 1
- Unvalidated input allows unauthorized field modification · Line 35-49
-
app/src/app/admin/users/page.tsx - 1
- Redundant API calls in useEffect · Line 37-42
-
app/src/app/admin/recordings/[id]/page.tsx - 1
- Non-functional delete button implementation · Line 82-82
-
app/src/app/admin/subscriptions/page.tsx - 1
- Infinite loop in useEffect dependency · Line 47-49
Additional Suggestions - 4
-
app/src/app/api/admin/users/route.ts - 1
-
Redundant authorization validation logic · Line 9-9Redundant authorization check detected. The `withAdmin` wrapper already validates admin access by calling `validateRequest()` and checking user role. Remove the duplicate validation (lines 9-13) to eliminate unnecessary database calls and improve performance.
Code suggestion
@@ -9,6 +9,1 @@ - const { user } = await validateRequest(); - - if (!user || user.role !== "ADMIN") { - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); - } - const { searchParams } = new URL(req.url); + const { searchParams } = new URL(req.url);
-
-
app/src/app/admin/components/SubscriptionEdit.tsx - 1
-
Unused state variable and setter · Line 15-15The `editingField` state variable and its setter `setEditingField` are declared but never used in the component. Consider removing them to improve code cleanliness.
Code suggestion
@@ -14,6 +14,5 @@ const SubscriptionEdit = ({ recording, onClose }: Props) => { const [editData, setEditData] = useState<SubscriptionData>(recording); - const [editingField, setEditingField] = useState<string | null>(null); const [updating, setUpdating] = useState(false); console.log("recording", recording); useEffect(() => {
-
-
app/src/app/api/admin/users/transcriptions/[userId]/route.ts - 1
-
Unused import 'and' should be removed · Line 4-4The `and` import from drizzle-orm is defined but never used in this file. Please remove it to keep the code clean and avoid potential confusion.
Code suggestion
@@ -4,1 +4,1 @@ -import { and, eq, sql } from "drizzle-orm"; +import { eq, sql } from "drizzle-orm";
-
-
app/src/app/api/admin/users/[id]/route.ts - 1
-
Unused destructured variable in object pattern · Line 79-79The `password_hash` variable is destructured but never used. Consider using the rest operator directly without explicitly naming the unused variable.
Code suggestion
@@ -79,3 +79,3 @@ - const { password_hash, ...user } = updated; // removed password hash from response + const { password_hash: _, ...user } = updated; // removed password hash from response
-
Review Details
-
Files reviewed - 27 · Commit Range:
d75249a..d199bbc- app/migrations/0011_sour_maria_hill.sql
- app/migrations/meta/0011_snapshot.json
- app/migrations/meta/_journal.json
- app/src/app/admin/components/EditSubscription.tsx
- app/src/app/admin/components/SubscriptionEdit.tsx
- app/src/app/admin/layout.tsx
- app/src/app/admin/recordings/[id]/page.tsx
- app/src/app/admin/subscriptions/page.tsx
- app/src/app/admin/users/[id]/recordings/RecordingsPage.tsx
- app/src/app/admin/users/[id]/recordings/page.tsx
- app/src/app/admin/users/page.tsx
- app/src/app/api/admin/subscriptions/[id]/route.ts
- app/src/app/api/admin/subscriptions/route.ts
- app/src/app/api/admin/transcriptions/[id]/route.ts
- app/src/app/api/admin/transcriptions/sample/route.ts
- app/src/app/api/admin/users/[id]/route.ts
- app/src/app/api/admin/users/route.ts
- app/src/app/api/admin/users/transcriptions/[userId]/route.ts
- app/src/app/api/admin/users/update-role/route.ts
- app/src/app/api/admin/users/update-subscription/route.ts
- app/src/app/api/signin/route.ts
- app/src/auth.ts
- app/src/constants/roles.ts
- app/src/db/schema.ts
- app/src/hooks/useUser.ts
- app/src/lib/axios.ts
- app/src/lib/withAdmin.ts
-
Files skipped - 0
-
Tools
- Eslint (Linter) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at [email protected].
Documentation & Help
AI Code Review powered by
| const data = await req.json(); | ||
| const { id } = params; | ||
|
|
||
| if (!id) { | ||
| return NextResponse.json( | ||
| { error: "Subscription ID is required" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| const [updated] = await db | ||
| .update(subscriptionTable) | ||
| .set(data) | ||
| .where(eq(subscriptionTable.id, id)) |
There was a problem hiding this comment.
The PATCH endpoint accepts arbitrary data without validation and directly passes it to the database update. This creates a security vulnerability where malicious users could modify system fields like id or createdAt. Add input validation with a whitelist of allowed fields: ['name', 'recordingCount', 'fileSizeLimitMB', 'durationDays', 'price'].
Code suggestion
Check the AI-generated fix before applying
| const data = await req.json(); | |
| const { id } = params; | |
| if (!id) { | |
| return NextResponse.json( | |
| { error: "Subscription ID is required" }, | |
| { status: 400 } | |
| ); | |
| } | |
| try { | |
| const [updated] = await db | |
| .update(subscriptionTable) | |
| .set(data) | |
| .where(eq(subscriptionTable.id, id)) | |
| const requestData = await req.json(); | |
| const { id } = params; | |
| if (!id) { | |
| return NextResponse.json( | |
| { error: "Subscription ID is required" }, | |
| { status: 400 } | |
| ); | |
| } | |
| // Whitelist allowed fields for update | |
| const allowedFields = ['name', 'recordingCount', 'fileSizeLimitMB', 'durationDays', 'price']; | |
| const sanitizedData = Object.fromEntries( | |
| Object.entries(requestData).filter(([key]) => allowedFields.includes(key)) | |
| ); | |
| if (Object.keys(sanitizedData).length === 0) { | |
| return NextResponse.json( | |
| { error: "No valid fields to update" }, | |
| { status: 400 } | |
| ); | |
| } | |
| try { | |
| const [updated] = await db | |
| .update(subscriptionTable) | |
| .set(sanitizedData) | |
| .where(eq(subscriptionTable.id, id)) |
Code Review Run #d83474
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
| useEffect(() => { | ||
| if (!isModalOpen) { | ||
| fetchUsers(); | ||
| } | ||
| fetchUsers(); | ||
| }, [isModalOpen]); |
There was a problem hiding this comment.
Remove the redundant fetchUsers() call that causes duplicate API requests. The function is already called conditionally on line 39, making line 41 unnecessary and wasteful.
Code suggestion
Check the AI-generated fix before applying
| useEffect(() => { | |
| if (!isModalOpen) { | |
| fetchUsers(); | |
| } | |
| fetchUsers(); | |
| }, [isModalOpen]); | |
| useEffect(() => { | |
| if (!isModalOpen) { | |
| fetchUsers(); | |
| } | |
| }, [isModalOpen]); |
Code Review Run #d83474
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
| </TableCell> | ||
| <TableCell className="flex gap-2"> | ||
| <div className="w-8 h-8 p-2 rounded-full flex items-center justify-center shadow-xl hover:bg-red-500 cursor-pointer hover:text-white"> | ||
| <Trash2 onClick={() => {}} size={20} /> |
There was a problem hiding this comment.
The Trash2 delete button has an empty onClick={() => {}} handler that does nothing when clicked. This creates a non-functional delete button that appears interactive but doesn't perform any action. Implement a proper delete function that calls the API to delete the recording and updates the local state.
Code suggestion
Check the AI-generated fix before applying
-export default function RecordingsPage({ params }: PageProps) {
- const [recordings, setRecordings] = useState<Recording[]>([]);
- const searchParams = useSearchParams();
- const page = searchParams.get("page") || "1";
- const limit = searchParams.get("limit") || "20";
+export default function RecordingsPage({ params }: PageProps) {
+ const [recordings, setRecordings] = useState<Recording[]>([]);
+ const searchParams = useSearchParams();
+ const page = searchParams.get("page") || "1";
+ const limit = searchParams.get("limit") || "20";
+
+ const handleDelete = async (recordingId: string) => {
+ try {
+ await API.delete(`/admin/users/transcriptions/${params.id}/${recordingId}`);
+ setRecordings(recordings.filter((recording) => recording.id !== recordingId));
+ } catch (error) {
+ console.error("Failed to delete recording:", error);
+ }
+ };
@@ -81,7 +81,7 @@
- <Trash2 onClick={() => {}} size={20} />
+ <Trash2 onClick={() => handleDelete(audioFile.id)} size={20} />
Code Review Run #d83474
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
| useEffect(() => { | ||
| fetchSubscriptions(); | ||
| }, [isModalOpen]); |
There was a problem hiding this comment.
The useEffect with isModalOpen dependency creates an infinite loop. When modal opens/closes → triggers fetchSubscriptions() → updates subs → triggers first useEffect → updates isModalOpen → repeats endlessly. Fix: Remove isModalOpen from dependency array and use empty array for initial fetch.
Code suggestion
Check the AI-generated fix before applying
| useEffect(() => { | |
| fetchSubscriptions(); | |
| }, [isModalOpen]); | |
| useEffect(() => { | |
| fetchSubscriptions(); | |
| }, []); // Run only on mount | |
| useEffect(() => { | |
| if (isModalOpen === null) fetchSubscriptions(); | |
| }, [isModalOpen]); |
Code Review Run #d83474
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
✅ Completed : Admin Flow changes:
Summary by Bito
This pull request enhances the admin interface with new pages for managing users, recordings, and subscriptions, while adding corresponding API endpoints with improved validation. The changes include database schema updates to support pricing and role modifications, along with a standardized navigation system for admin features.