Membuat api - #5
Closed
migthyhbb wants to merge 5 commits into
Closed
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdded server-side Supabase authentication, role-based middleware, partner registration, profile access, login, verification, and E-Contract API routes. Removed the legacy Supabase client and connection-test route. ChangesSupabase authentication and partner workflows
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RegistrationRoute
participant SupabaseAuth
participant AdminClient
Client->>RegistrationRoute: submit registration data
RegistrationRoute->>SupabaseAuth: create Auth user
SupabaseAuth-->>RegistrationRoute: return user id
RegistrationRoute->>AdminClient: assign role and insert profile
AdminClient-->>RegistrationRoute: return operation result
RegistrationRoute-->>Client: return success or error response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
lib/supabase/server.ts (2)
5-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRead the admin credentials inside
createAdminClientinstead of accepting them as parameters.
createAdminClientrequires callers to pass the service-role key. Both registration routes bypass this helper and callcreateClientfrom@supabase/supabase-jsdirectly withprocess.env.SUPABASE_SERVICE_ROLE_KEY!. The non-null assertion hides a missing key until requests fail at runtime. Move the env read and validation into this factory, then use it in the registration routes.♻️ Proposed refactor
-export function createAdminClient(url: string, key: string) { - return createSupabaseAdmin(url, key, { +export function createAdminClient() { + const url = process.env.NEXT_PUBLIC_SUPABASE_URL; + const key = process.env.SUPABASE_SERVICE_ROLE_KEY; + if (!url || !key) { + throw new Error('Supabase admin environment variables are missing!'); + } + return createSupabaseAdmin(url, key, { auth: { autoRefreshToken: false, persistSession: false, }, }); }🤖 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 `@lib/supabase/server.ts` around lines 5 - 12, Update createAdminClient to read the Supabase URL and service-role key from environment configuration, validate that the required key exists, and stop accepting credentials as parameters. Replace the direct createClient calls in both registration routes with createAdminClient so all admin clients use the centralized validated configuration.
25-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMigrate the cookie adapter to
getAll/setAll.
@supabase/ssr0.12.4 deprecatesget,set, andremove. UsegetAllandsetAllto support chunked session cookies and matchmiddleware.ts. Keep one documented catch for the Server Component cookie-write restriction. Remove the unusedCookieOptionsimport.🤖 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 `@lib/supabase/server.ts` around lines 25 - 44, The cookie adapter around the cookies object must migrate from get/set/remove to getAll/setAll, matching the middleware adapter and supporting chunked session cookies. Replace the individual cookie operations with the corresponding bulk methods, retain one documented catch for the Server Component cookie-write restriction, and remove the now-unused CookieOptions import.app/api/registrasi_agen/route.ts (1)
5-24: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winValidate the field formats, not only their presence.
The check rejects empty values only.
nikNib, andnoTeleponreach Supabase and the database without format checks, andpasswordhas no length rule. Non-string values also pass, because only falsiness is tested. Consider a schema validator such aszod, which the two registration routes can share.🤖 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/registrasi_agen/route.ts` around lines 5 - 24, Enhance validation in the POST handler using a shared schema for both registration routes, such as Zod. Validate that all fields are strings, enforce proper email formatting, apply the expected NIK/NIB and phone-number formats, and require a minimum password length before contacting Supabase; return the existing 400-style validation response for schema failures.
🤖 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/admin/verifikasi/route.ts`:
- Around line 44-52: Validate that the parsed body in the request handler is a
non-null object before destructuring id_target, tipe, and status_baru. Return
the existing 400 invalid-body response for null or other non-object JSON values,
while preserving the current handling for valid object bodies.
In `@app/api/auth/login/route.ts`:
- Around line 55-66: Update the role lookup in the login route to read
data.user?.app_metadata?.role instead of user_metadata, while preserving the
existing response shape and redirect behavior.
In `@app/api/legal/e-contract/route.ts`:
- Around line 23-37: Update the Supabase update flow in the e-contract route to
request the modified agen row and verify that exactly one row was returned for
user.id. Treat zero or multiple matches as an error instead of returning the
success response, while preserving the existing update fields and success
payload for exactly one updated row.
In `@app/api/profil/me/route.ts`:
- Around line 20-39: Update both profile lookups in the role branches of the GET
handler to map Supabase’s PGRST116 “no rows” error to HTTP 404, while preserving
other errors for server-side handling. In the catch block, replace the explicit
any type with a safe error type and return a generic client-facing message
instead of err.message, avoiding exposure of PostgREST or schema details.
In `@app/api/registrasi_agen/route.ts`:
- Around line 29-37: Update the registration flow around the signUp call to stop
placing the role in options.data/user_metadata. Reuse the shared
createAdminClient from `@/lib/supabase/server`, create it before the profile
insert, and use it after signUp to set the new user’s app_metadata.role to agen
so middleware authorization reads the trusted value.
- Around line 69-77: Update the rollback block after the profile creation
failure to capture and inspect the result of
supabaseAdmin.auth.admin.deleteUser(userId), log any deletion failure, and
preserve the rollback failure handling. Replace the client-facing
profileError.message with a generic error response while retaining detailed
error information only in server-side logs.
In `@app/api/registrasi_perusahaan/route.ts`:
- Around line 18-23: Update the validation error returned by the required-field
check to name all six validated fields, including alamatKantor and noTelepon,
while preserving the existing 400 response and condition.
In `@middleware.ts`:
- Around line 32-33: Ensure registration populates app_metadata.role before
authorization by assigning the submitted role through
supabaseAdmin.auth.admin.updateUserById or an equivalent server-side hook.
Update the registration flow and the role checks in login, profil/me, and
e-contract handlers to consistently read app_metadata.role, including replacing
the user-editable metadata access in the e-contract authorization check.
---
Nitpick comments:
In `@app/api/registrasi_agen/route.ts`:
- Around line 5-24: Enhance validation in the POST handler using a shared schema
for both registration routes, such as Zod. Validate that all fields are strings,
enforce proper email formatting, apply the expected NIK/NIB and phone-number
formats, and require a minimum password length before contacting Supabase;
return the existing 400-style validation response for schema failures.
In `@lib/supabase/server.ts`:
- Around line 5-12: Update createAdminClient to read the Supabase URL and
service-role key from environment configuration, validate that the required key
exists, and stop accepting credentials as parameters. Replace the direct
createClient calls in both registration routes with createAdminClient so all
admin clients use the centralized validated configuration.
- Around line 25-44: The cookie adapter around the cookies object must migrate
from get/set/remove to getAll/setAll, matching the middleware adapter and
supporting chunked session cookies. Replace the individual cookie operations
with the corresponding bulk methods, retain one documented catch for the Server
Component cookie-write restriction, and remove the now-unused CookieOptions
import.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c874d3e-856d-482c-97d6-e1398ba3a020
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
app/api/admin/calon-mitra/route.tsapp/api/admin/verifikasi/route.tsapp/api/auth/login/route.tsapp/api/legal/e-contract/route.tsapp/api/profil/me/route.tsapp/api/registrasi_agen/route.tsapp/api/registrasi_perusahaan/route.tsapp/api/test-koneksi/route.tslib/supabase.tslib/supabase/server.tsmiddleware.tspackage.json
💤 Files with no reviewable changes (2)
- lib/supabase.ts
- app/api/test-koneksi/route.ts
Comment on lines
+23
to
+37
| // 4. Update tabel 'agen', ubah syarat_disetujui menjadi true | ||
| const { error: updateError } = await supabase | ||
| .from('agen') | ||
| .update({ | ||
| syarat_disetujui: true, | ||
| waktu_persetujuan: waktuSekarang | ||
| }) | ||
| .eq('auth_id', user.id); | ||
|
|
||
| if (updateError) throw updateError; | ||
|
|
||
| return NextResponse.json({ | ||
| message: 'E-Contract berhasil disetujui secara digital!', | ||
| waktu_persetujuan: waktuSekarang | ||
| }, { status: 200 }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Critical: the route reports success when no agent row matches.
A Supabase update that matches zero rows returns no error. If the agen row is missing, or if RLS filters it out, updateError stays null and the response states that the E-Contract is approved. No consent is recorded. This endpoint records a legal agreement, so the false confirmation is severe. Request the updated rows and verify that exactly one row changed.
🐛 Proposed fix
- const { error: updateError } = await supabase
+ const { data: updated, error: updateError } = await supabase
.from('agen')
.update({
syarat_disetujui: true,
waktu_persetujuan: waktuSekarang
})
- .eq('auth_id', user.id);
+ .eq('auth_id', user.id)
+ .select('auth_id');
if (updateError) throw updateError;
+
+ if (!updated || updated.length === 0) {
+ return NextResponse.json(
+ { error: 'Data agen tidak ditemukan' },
+ { status: 404 }
+ );
+ }🤖 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/legal/e-contract/route.ts` around lines 23 - 37, Update the Supabase
update flow in the e-contract route to request the modified agen row and verify
that exactly one row was returned for user.id. Treat zero or multiple matches as
an error instead of returning the success response, while preserving the
existing update fields and success payload for exactly one updated row.
Comment on lines
+20
to
+39
| // 3. Tarik data dari tabel yang sesuai menggunakan .single() karena data pasti unik | ||
| if (role === 'agen') { | ||
| const { data, error } = await supabase | ||
| .from('agen') | ||
| .select('*') | ||
| .eq('auth_id', user.id) | ||
| .single(); | ||
|
|
||
| if (error) throw error; | ||
| profileData = data; | ||
|
|
||
| } else if (role === 'perusahaan') { | ||
| const { data, error } = await supabase | ||
| .from('perusahaan_industri') | ||
| .select('*') | ||
| .eq('auth_id', user.id) | ||
| .single(); | ||
|
|
||
| if (error) throw error; | ||
| profileData = data; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Return 404 for a missing profile, and stop returning the raw error message.
.single() returns a PGRST116 error when no row matches. A registered user without a profile row therefore receives HTTP 500. Map that case to 404. The catch block also sends err.message to the client, which exposes Postgrest and schema details. The any type on line 52 fails ESLint @typescript-eslint/no-explicit-any.
🛡️ Proposed fix
if (role === 'agen') {
const { data, error } = await supabase
.from('agen')
.select('*')
.eq('auth_id', user.id)
- .single();
-
- if (error) throw error;
+ .maybeSingle();
+
+ if (error) throw error;
+ if (!data) {
+ return NextResponse.json({ error: 'Profil belum tersedia' }, { status: 404 });
+ }
profileData = data;Apply the same change to the perusahaan_industri branch, then:
- } catch (err: any) {
- return NextResponse.json(
- { error: 'Terjadi kesalahan server: ' + err.message },
- { status: 500 }
- );
+ } catch (err: unknown) {
+ const message = err instanceof Error ? err.message : 'Unknown server error';
+ console.error('Profil error:', message);
+ return NextResponse.json(
+ { error: 'Terjadi kesalahan server' },
+ { status: 500 }
+ );
}Also applies to: 52-57
🤖 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/profil/me/route.ts` around lines 20 - 39, Update both profile lookups
in the role branches of the GET handler to map Supabase’s PGRST116 “no rows”
error to HTTP 404, while preserving other errors for server-side handling. In
the catch block, replace the explicit any type with a safe error type and return
a generic client-facing message instead of err.message, avoiding exposure of
PostgREST or schema details.
Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/supabase/server.ts (1)
9-10: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd explicit Supabase environment checks.
@supabase/ssralready rejects missing values, but explicit checks provide a clearer configuration error. Apply the same check inlib/supabase/server.tsandmiddleware.ts, consistent withcreateAdminClient().🤖 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 `@lib/supabase/server.ts` around lines 9 - 10, Add explicit presence checks for NEXT_PUBLIC_SUPABASE_URL and NEXT_PUBLIC_SUPABASE_ANON_KEY before constructing the Supabase client in the server setup, matching the validation used by createAdminClient(). Apply the same validation in middleware.ts and raise the established clear configuration error when either variable is missing.
🤖 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/profil/me/route.ts`:
- Around line 21-40: Update the role lookup in the profile route to read the
trusted user.app_metadata?.role value instead of user_metadata.role. Preserve
the existing agen and perusahaan_industri profile branches and their response
behavior.
In `@app/api/registrasi_agen/route.ts`:
- Around line 53-57: Update the agen insertion in the registration handler
before the HTTP 201 response: replace the empty insert payload with one object
keyed by the actual table columns, including auth_id: userId, mappings for
namaAgen, nikNib, alamatLengkap, and noTelepon, plus required defaults such as
status_verifikasi: 'pending'. Preserve the existing profileError handling.
---
Nitpick comments:
In `@lib/supabase/server.ts`:
- Around line 9-10: Add explicit presence checks for NEXT_PUBLIC_SUPABASE_URL
and NEXT_PUBLIC_SUPABASE_ANON_KEY before constructing the Supabase client in the
server setup, matching the validation used by createAdminClient(). Apply the
same validation in middleware.ts and raise the established clear configuration
error when either variable is missing.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8eac49dc-008d-4934-a9e2-2ac6d65a803e
📒 Files selected for processing (6)
app/api/admin/verifikasi/route.tsapp/api/auth/login/route.tsapp/api/profil/me/route.tsapp/api/registrasi_agen/route.tsapp/api/registrasi_perusahaan/route.tslib/supabase/server.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- app/api/registrasi_perusahaan/route.ts
- app/api/admin/verifikasi/route.ts
- app/api/auth/login/route.ts
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cekk coderabbit
Summary by CodeRabbit
New Features
Bug Fixes
Chores