fix: re #38
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ yarn-error.log* | |
| .pnpm-debug.log* | ||
|
|
||
| # local env files | ||
| .env | ||
| .env*.local | ||
| .env*.example | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,44 @@ | ||||||||||||||||||||||||||||
| "use client"; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| import { Card, CardHeader, CardTitle, CardContent } from "@/components/ui/card"; | ||||||||||||||||||||||||||||
| import { api } from "@/lib/api"; | ||||||||||||||||||||||||||||
| import { useEffect, useState } from "react"; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| type Analytics = { | ||||||||||||||||||||||||||||
| name: string; | ||||||||||||||||||||||||||||
| value: number; | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| async function getAnalytics() { | ||||||||||||||||||||||||||||
| const response = await api.get<Analytics[]>("/analytics"); | ||||||||||||||||||||||||||||
| return response.data; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+15
There was a problem hiding this comment. 🚨 Houston, we need error handling! The API call is looking sus without any error handling. What happens when the server is taking a coffee break? async function getAnalytics() {
+ try {
const response = await api.get<Analytics[]>("/analytics");
return response.data;
+ } catch (error) {
+ console.error('Failed to fetch analytics:', error);
+ return [];
+ }
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| export default function AnalyticsCard() { | ||||||||||||||||||||||||||||
| const [analitycs, setAnalitycs] = useState<Analytics[]>([]); | ||||||||||||||||||||||||||||
|
There was a problem hiding this comment. 🐛 Yo dawg, I heard you like typos! There's a spicy typo in your state variable name. "analitycs" should be "analytics" - unless you're trying to start a new naming trend! - const [analitycs, setAnalitycs] = useState<Analytics[]>([]);
+ const [analytics, setAnalytics] = useState<Analytics[]>([]);📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||
| getAnalytics().then((data) => setAnalitycs(data)); | ||||||||||||||||||||||||||||
| }, [analitycs]); | ||||||||||||||||||||||||||||
|
Comment on lines
+20
to
+22
There was a problem hiding this comment. 🔄 Infinite loop alert! Time to fix this dependency array! You've created the infinite loop of doom! The useEffect depends on useEffect(() => {
getAnalytics().then((data) => setAnalitycs(data));
-}, [analitycs]);
+}, []);📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||
| <div className='gap-4 grid md:grid-cols-2 lg:grid-cols-4 mt-5'> | ||||||||||||||||||||||||||||
| {analitycs.map((item) => ( | ||||||||||||||||||||||||||||
| <Card | ||||||||||||||||||||||||||||
| key={item.name} | ||||||||||||||||||||||||||||
| className='shadow-md hover:scale-105 duration-500 cursor-pointer'> | ||||||||||||||||||||||||||||
| <CardHeader className='flex flex-row justify-between items-center space-y-0 pb-2'> | ||||||||||||||||||||||||||||
| <CardTitle className='font-medium text-sm'>{item.name}</CardTitle> | ||||||||||||||||||||||||||||
| {/* <DollarSign className="w-4 h-4 text-muted-foreground" /> */} | ||||||||||||||||||||||||||||
| </CardHeader> | ||||||||||||||||||||||||||||
| <CardContent> | ||||||||||||||||||||||||||||
| <div className='font-bold text-2xl'>{item.value}</div> | ||||||||||||||||||||||||||||
| {/* <p className="text-muted-foreground text-xs"> | ||||||||||||||||||||||||||||
| jumlah keseluruhan mapel | ||||||||||||||||||||||||||||
| </p> */} | ||||||||||||||||||||||||||||
| </CardContent> | ||||||||||||||||||||||||||||
| </Card> | ||||||||||||||||||||||||||||
| ))} | ||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+42
There was a problem hiding this comment. 🛠️ Refactor suggestion 💅 Sweet responsive grid setup, but let's add loading states! The grid layout is fire, but users might see a blank screen while data loads. Let's add some loading states to keep it 💯 export default function AnalyticsCard() {
const [analytics, setAnalytics] = useState<Analytics[]>([]);
+ const [isLoading, setIsLoading] = useState(true);
useEffect(() => {
- getAnalytics().then((data) => setAnalytics(data));
+ getAnalytics()
+ .then((data) => setAnalytics(data))
+ .finally(() => setIsLoading(false));
}, []);
return (
<div className='gap-4 grid md:grid-cols-2 lg:grid-cols-4 mt-5'>
+ {isLoading && Array(4).fill(0).map((_, i) => (
+ <Card key={i} className="animate-pulse">
+ <CardHeader className="h-8 bg-gray-200 rounded" />
+ <CardContent className="h-12 bg-gray-200 rounded" />
+ </Card>
+ ))}
- {analytics.map((item) => (
+ {!isLoading && analytics.map((item) => (
|
||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,7 +24,7 @@ export default function DialogModul() { | |||||||||
| const [open, setOpen] = useState(false); | ||||||||||
| const [selected, setSelected] = useState({ | ||||||||||
| rombongan_belajar_id: null, | ||||||||||
| // teacher_id: null, | ||||||||||
| mapel_id: null, | ||||||||||
| }); | ||||||||||
|
|
||||||||||
|
|
@@ -45,7 +45,7 @@ export default function DialogModul() { | |||||||||
| // reset selected | ||||||||||
| setSelected({ | ||||||||||
| rombongan_belajar_id: null, | ||||||||||
| // teacher_id: null, | ||||||||||
| mapel_id: null, | ||||||||||
| }); | ||||||||||
| } | ||||||||||
|
|
@@ -56,8 +56,8 @@ export default function DialogModul() { | |||||||||
| return ( | ||||||||||
| <Dialog open={open} onOpenChange={setOpen}> | ||||||||||
| <DialogTrigger asChild> | ||||||||||
| <Button className='flex items-center'> | ||||||||||
| <IconPlus className='mr-2 w-5 h-5' /> | ||||||||||
|
Comment on lines
+59
to
+60
There was a problem hiding this comment. 🛠️ Refactor suggestion 🎨 Spicy UI Update: Button Glow Up! Looking clean with that flex layout! The IconPlus with margin is chef's kiss - but let's make it even MORE AWESOME with some hover effects! - <Button className='flex items-center'>
+ <Button className='flex items-center hover:scale-105 transition-transform'>📝 Committable suggestion
Suggested change
|
||||||||||
| Buat Modul Baru | ||||||||||
| </Button> | ||||||||||
| </DialogTrigger> | ||||||||||
|
|
@@ -67,23 +67,23 @@ export default function DialogModul() { | |||||||||
| </DialogHeader> | ||||||||||
|
|
||||||||||
| {/* form to use to create new modul */} | ||||||||||
| <div className='gap-4 grid py-4'> | ||||||||||
| <div className='gap-2 grid'> | ||||||||||
| <label htmlFor='name'>Pilih Rombel</label> | ||||||||||
|
Comment on lines
+70
to
+72
There was a problem hiding this comment. 🛠️ Refactor suggestion 🎯 Grid Layout: The Holy Trinity of Spacing! Nice grid game! But hold up - those nested - <div className='gap-4 grid py-4'>
- <div className='gap-2 grid'>
+ <div className='grid gap-4 py-4'>
+ <div className='grid gap-2'>Also applies to: 85-86 |
||||||||||
| <SelectRombel | ||||||||||
| onSelected={(e) => | ||||||||||
| setSelected((s) => ({ ...s, rombongan_belajar_id: e })) | ||||||||||
| } | ||||||||||
| /> | ||||||||||
| </div> | ||||||||||
| {/* <div className='gap-2 grid'> | ||||||||||
| <label htmlFor='name'>Pilih Guru</label> | ||||||||||
| <SelectTeacher | ||||||||||
| onSelected={(e) => setSelected((s) => ({ ...s, teacher_id: e }))} | ||||||||||
| /> | ||||||||||
| </div> */} | ||||||||||
| <div className='gap-2 grid'> | ||||||||||
| <label htmlFor='name'>Pilih Mapel</label> | ||||||||||
| <SelectMapel | ||||||||||
| onSelected={(e) => setSelected((s) => ({ ...s, mapel_id: e }))} | ||||||||||
| /> | ||||||||||
|
|
@@ -92,7 +92,7 @@ export default function DialogModul() { | |||||||||
| {/* button submit */} | ||||||||||
| <DialogFooter> | ||||||||||
| <Button disabled={isLoading} onClick={handleSave}> | ||||||||||
| {isLoading && <Loader className='mr-2' size={16} />} | ||||||||||
|
There was a problem hiding this comment. 🛠️ Refactor suggestion ✨ Loading State: Micro-interaction Magic! That loading indicator is smooth like butter! But let's make it SPIN like a DJ at a React conference! - <Loader className='mr-2' size={16} />
+ <Loader className='mr-2 animate-spin' size={16} />📝 Committable suggestion
Suggested change
|
||||||||||
| Simpan | ||||||||||
| </Button> | ||||||||||
| <Button variant={`ghost`} onClick={() => setOpen(false)}> | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,18 +11,18 @@ export const metadata: Metadata = { | |
|
|
||
| export default async function Page() { | ||
| return ( | ||
| <div className='flex flex-col space-y-5 h-full'> | ||
| <div className='flex flex-col h-full'> | ||
| <div className='flex md:flex-row flex-col justify-between'> | ||
| <div className='space-y-1 mt-5'> | ||
| <h2 className='font-semibold text-2xl tracking-tight'>Modul</h2> | ||
| <p className='text-muted-foreground text-sm'> | ||
| {/* list of all modul available */} | ||
| Daftar semua modul yang telah dibuat | ||
| </p> | ||
| </div> | ||
| {/* dialog new modul */} | ||
| <DialogModul /> | ||
|
There was a problem hiding this comment. 💡 Codebase verification 🔥 HOLD UP! We've got a security gap faster than npm installing node_modules! The investigation reveals we've got role checks scattered across the codebase (
This is like leaving your house keys under the doormat - technically there's a lock, but anyone who gets in can access everything! • Add back the role check in 🔗 Analysis chain🚨 Hold up! We need to verify those access controls! The role check with 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Let's hunt down where the access controls went
# Check if DialogModul has internal role checks
echo "🔍 Checking DialogModul component for role checks..."
ast-grep --pattern 'isWakaKurikulum|role|permission' src/app/\(authenticated\)/modul/components/dialog-modul.tsx
# Look for any auth middleware
echo "🔍 Checking for auth middleware..."
fd middleware src/app/\(authenticated\) --exec rg -l 'auth|role|permission'
Length of output: 386 Script: #!/bin/bash
# Let's do a more thorough investigation 🕵️
# First, let's see what's actually in DialogModul
echo "🔍 Show me the DialogModul contents..."
rg -A 5 "export.*DialogModul" src/app/\(authenticated\)/modul/components/dialog-modul.tsx
# Check for any auth hooks or context usage
echo "🔍 Looking for auth hooks..."
rg -l "useAuth|getSession|useSession" src/app/\(authenticated\)/modul/components/dialog-modul.tsx
# Check the authenticated layout for role protection
echo "🔍 Checking authenticated layout..."
rg -l "auth|role|permission" src/app/\(authenticated\)/layout.tsx
# Let's see if there's any route protection
echo "🔍 Checking route protection..."
fd "route|page" src/app/\(authenticated\)/modul --exec rg -l "auth|role|permission" {}
Length of output: 975 Script: #!/bin/bash
# Time for the final boss battle! 🔥
# Let's see what's in those protected pages
echo "🔍 Checking modul pages for role checks..."
rg -B 2 -A 5 "auth|role|permission" src/app/\(authenticated\)/modul/page.tsx src/app/\(authenticated\)/modul/\[uuid\]/page.tsx
# Let's check if there's any middleware protecting this route
echo "🔍 Looking for middleware in action..."
fd middleware --exec cat {} \;
# One last check for any role-based hooks in the auth system
echo "🔍 Scanning for role-based auth hooks..."
rg -l "isWakaKurikulum|useRole|getRole" src/app/\(authenticated\)
Length of output: 4032 |
||
| </div> | ||
| <Separator className='my-4' /> | ||
| {/* */} | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,21 @@ | ||||||||||||
| import { authOptions } from "@/lib/auth"; | ||||||||||||
| import { getServerSession } from "next-auth/next"; | ||||||||||||
|
Comment on lines
+1
to
+2
There was a problem hiding this comment. 🛠️ Refactor suggestion 🔥 Clean imports, but let's make them BULLETPROOF! The imports look clean AF, but we should probably add some error handling around that getServerSession call. Trust me, you don't want that bad boy failing silently in production! import { authOptions } from "@/lib/auth";
import { getServerSession } from "next-auth/next";
+import { NextResponse } from "next/server";📝 Committable suggestion
Suggested change
🧰 Tools🪛 eslint[error] 1-undefined: Parsing error: The keyword 'import' is reserved |
||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * API route to return the user session if authenticated. | ||||||||||||
| * | ||||||||||||
| * If the request is not authenticated, return a 401 status code without any response body. | ||||||||||||
| */ | ||||||||||||
| export async function GET(request: Request) { | ||||||||||||
| const session = await getServerSession(authOptions); | ||||||||||||
|
|
||||||||||||
| if (session) { | ||||||||||||
| return new Response(JSON.stringify(session.user?.access_token), { | ||||||||||||
| status: 200, | ||||||||||||
| }); | ||||||||||||
| } else { | ||||||||||||
| return new Response(null, { | ||||||||||||
| status: 401, | ||||||||||||
| }); | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
Comment on lines
+9
to
+21
There was a problem hiding this comment. 💡 Codebase verification 🔥 Level Up Your API Route with These Epic Security Features! Listen up fam, after diving deep into the codebase like a speedrunner, here's the 200 IQ take: The suggested changes are absolutely CRACKED! We're currently the only API route in town handling sensitive data, and we're running naked without proper security headers or structured error responses. The suggested implementation is like adding a shield potion and proper armor to our route - it's a straight upgrade that:
This is definitely not a false positive - it's a legitimate security enhancement that makes our API route production-ready. Time to smash that merge button! 🔗 Analysis chain⚡ Hold up! Let's make this route BULLETPROOF! The route works, but it's missing some CRUCIAL security headers and error handling! Let's make it production-ready! Here's a spicier implementation: export async function GET(request: Request) {
- const session = await getServerSession(authOptions);
+ try {
+ const session = await getServerSession(authOptions);
+
+ if (!session) {
+ return NextResponse.json(
+ { error: 'Unauthorized' },
+ {
+ status: 401,
+ headers: {
+ 'Content-Type': 'application/json',
+ 'Cache-Control': 'no-store',
+ }
+ }
+ );
+ }
- if (session) {
- return new Response(JSON.stringify(session.user?.access_token), {
- status: 200,
- });
- } else {
- return new Response(null, {
- status: 401,
- });
+ return NextResponse.json(
+ { access_token: session.user?.access_token },
+ {
+ status: 200,
+ headers: {
+ 'Content-Type': 'application/json',
+ 'Cache-Control': 'no-store',
+ }
+ }
+ );
+ } catch (error) {
+ console.error('Session error:', error);
+ return NextResponse.json(
+ { error: 'Internal server error' },
+ { status: 500 }
+ );
}
}This bad boy adds:
The previous search was too specific. Let's cast a wider net to see how other routes are handling responses and if we need to maintain consistency across the codebase. This will help us verify if the suggested changes align with the existing patterns or if we're introducing a new, better pattern that should be applied across all routes. Alright gamers, let's do one final check to see how the auth route handles its responses and if NextResponse is already a thing in our codebase. This will help us make a 200 IQ decision about whether our suggested changes are actually a pro gamer move or if we're trying to reinvent the wheel. 🎮 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Let's check if we're handling errors consistently across other routes
rg -l "NextResponse.json.*error" "src/app/api"
Length of output: 46 Script: #!/bin/bash
# Let's check all API routes and their error handling patterns more broadly
rg -A 5 "NextResponse|Response" "src/app/api"
# Also check for any existing security headers
rg -A 2 "headers.*:" "src/app/api"
# And let's see what other route files we have
fd "route.ts" "src/app/api"
Length of output: 599 Script: #!/bin/bash
# Let's check what the auth route is doing for error handling
rg -A 10 "export.*function" "src/app/api/auth/[...nextauth]/route.ts"
# And see if we're using NextResponse anywhere else in the codebase
# This helps ensure we're consistent with our response patterns
rg -A 5 "import.*NextResponse" "src/app"
Length of output: 112 |
||||||||||||
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.
🛠️ Refactor suggestion
🤔 Hold up! Do we really need that
/**pathway wildcard?Yo dawg, I heard you like wildcards, but
/**is like giving everyone VIP access to the club! Consider tightening those pathname patterns if you're only using specific paths. For example, if you're only grabbing images from/photos/, lock it down!Here's a spicier way to do it:
Do this for each pattern if you know the specific paths you're using. Remember: in security, less access = more awesome! 🔒
Also applies to: 21-21, 27-27