adhit-r / adhit-r/fairmind

Fix ApiResponse<T> handling + wire up broken compliance-automation feature (~19 TS errors)

未關閉
#162 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
bug frontend help wanted typescript
主要語言
Python
星號
9
分支
12
平均合併
2 小時 23 分鐘
30 天內合併 PR
2

描述

## Summary

The frontend has **~19 TypeScript errors** caused by inconsistent handling of the `ApiResponse` wrapper type. The errors cluster into two categories:

1. **Half-built `compliance-automation` feature** — the service file (`compliance-automation-service.ts`) imports a non-existent `api` export and exports `scheduleAPI`/`violationAPI`/`reportAPI` while consumers (`ScheduleManager.tsx`, `ViolationAlerts.tsx`, `compliance-automation/page.tsx`) call methods on a `complianceAutomationService` object that doesn't exist. The whole feature needs to be wired up correctly.
2. **Admin pages and auth hook** — call sites that forget to unwrap `ApiResponse` via `.data` (or that wrongly unwrap raw data as if it were an `ApiResponse`).

This is a great intermediate-level contribution: real bugs, well-scoped, no infra needed, and the fix teaches the codebase's API client conventions.

## Background

Our `apiClient` returns a wrapped response shape:

```typescript
// apps/frontend/src/lib/api/api-client.ts
export interface ApiResponse {
success: boolean
data?: T
message?: string
error?: string
}
```

Every call site is supposed to check `response.success` first, then read `response.data`. About 19 sites get this wrong in two opposite ways.

## The Two Patterns

### Pattern A — Forgot to unwrap (admin pages, useAuthentik, compliance-automation/page.tsx)

```typescript
// BEFORE - wrong: response is ApiResponse<{users: User[]}>, not {users: User[]}
const response = await apiClient.get<{ users: User[] }>(API_ENDPOINTS.admin.users)
setUsers(response.users) // TS2339: Property 'users' does not exist on type 'ApiResponse<...>'

// AFTER
const response = await apiClient.get<{ users: User[] }>(API_ENDPOINTS.admin.users)
if (response.success && response.data) {
setUsers(response.data.users)
} else {
setError(response.error ?? 'Failed to load users')
}
```

### Pattern B — Treats raw data as if it were a wrapper (ScheduleManager, ViolationAlerts)

```typescript
// BEFORE - wrong: complianceAutomationService.listSchedules() returns raw data, not ApiResponse
const res = await complianceAutomationService.listSchedules()
if (res.success && res.data) { // TS2339: Property 'success' does not exist
setSchedules(res.data)
}

// AFTER (option 1: change consumer to match service contract)
const res = await complianceAutomationService.listSchedules()
setSchedules(res.schedules) // res is { schedules: ComplianceSchedule[]; total; offset; limit }

// AFTER (option 2: change service to use apiClient's ApiResponse wrapper for consistency)
// - see "Recommended approach" below
```

## Recommended Approach

**Unify on `apiClient` + `ApiResponse`.** The codebase's other services use `apiClient` directly and expose typed methods that return `ApiResponse`. The compliance-automation service is the odd one out:

1. Rewrite `apps/frontend/src/lib/api/compliance-automation-service.ts`:
- Replace `import { api } from '@/lib/api/api-client'` with `import { apiClient } from '@/lib/api/api-client'`.
- Export a single `complianceAutomationService` object with methods (`listSchedules`, `createSchedule`, `listViolations`, `acknowledgeViolation`, `listReports`, etc.) that all return `ApiResponse`.
- The current `scheduleAPI`/`violationAPI`/`reportAPI`/`remediationAPI` namespacing can be kept as a thin wrapper if you prefer, but consumers need a single import.
2. Update consumers (`ScheduleManager.tsx`, `ViolationAlerts.tsx`, `compliance-automation/page.tsx`) to use the new `ApiResponse`-shaped return - `if (res.success && res.data) { ... }`.
3. Fix the admin pages and `useAuthentik.ts` to properly unwrap `response.data` before reading inner properties.

## Files to Fix

| # | File | Line | Error |
|---|------|------|-------|
| 1 | `apps/frontend/src/lib/api/compliance-automation-service.ts` | 12 | TS2305: `'api'` not exported from api-client |
| 2 | `apps/frontend/src/components/compliance-automation/ScheduleManager.tsx` | 30 | TS2339: `success` does not exist on `{data: ComplianceSchedule[]}` |
| 3 | `apps/frontend/src/components/compliance-automation/ScheduleManager.tsx` | 45 | TS2322: `string` not assignable to `'daily' \| 'weekly' \| 'monthly'` (state needs literal type) |
| 4 | `apps/frontend/src/components/compliance-automation/ScheduleManager.tsx` | 49 | TS2339: `success` does not exist on `ComplianceSchedule` |
| 5 | `apps/frontend/src/components/compliance-automation/ViolationAlerts.tsx` | 22 | TS2339: `success`/`data` do not exist on raw violations payload |
| 6 | `apps/frontend/src/components/compliance-automation/ViolationAlerts.tsx` | 23 | TS2339: same |
| 7 | `apps/frontend/src/app/(dashboard)/compliance-automation/page.tsx` | 41 | TS2345: `ApiResponse` not assignable to `SystemStatus \| null` |
| 8 | `apps/frontend/src/app/(dashboard)/compliance-automation/page.tsx` | 223 | TS2339: `schedules` does not exist on `ApiResponse` |
| 9 | `apps/frontend/src/app/(dashboard)/compliance-automation/page.tsx` | 299 | TS2339: `violations` does not exist on `ApiResponse` |
| 10 | `apps/frontend/src/app/(dashboard)/compliance-automation/page.tsx` | 384 | TS2339: `reports` does not exist on `ApiResponse` |
| 11 | `apps/frontend/src/app/(dashboard)/admin/audit-log/page.tsx` | 56 | TS2339: `logs` does not exist on `ApiResponse<{logs: AuditLog[]}>` |
| 12 | `apps/frontend/src/app/(dashboard)/admin/registrations/page.tsx` | 46 | TS2339: `requests` does not exist on `ApiResponse<{requests: ...}>` |
| 13 | `apps/frontend/src/app/(dashboard)/admin/users/page.tsx` | 40 | TS2339: `users` does not exist on `ApiResponse<{users: User[]}>` |
| 14 | `apps/frontend/src/lib/api/hooks/useAuthentik.ts` | 181 | TS2339: `access_token` does not exist on `ApiResponse` |
| 15 | `apps/frontend/src/lib/api/hooks/useAuthentik.ts` | 181 | TS2339: `refresh_token` does not exist on `ApiResponse` |
| 16 | `apps/frontend/src/lib/api/hooks/useAuthentik.ts` | 181 | TS2339: `user` does not exist on `ApiResponse` |
| 17 | `apps/frontend/src/lib/api/hooks/useAuthentik.ts` | 194 | TS2345: `ApiResponse` not assignable to `AuthTokenResponse` |

## Acceptance Criteria

- [ ] `cd apps/frontend && npx tsc --noEmit` reports **zero** errors in any file listed above
- [ ] `compliance-automation-service.ts` uses `apiClient` (not the non-existent `api`)
- [ ] All admin pages handle `response.success === false` (show an error state, not crash)
- [ ] No `as any` / `as ApiResponse` casts used to silence errors
- [ ] No runtime behavior change in successful-path responses - only error handling and types
- [ ] If you change the compliance-automation service shape, update *both* `ScheduleManager.tsx` and `ViolationAlerts.tsx` to match

## How to Reproduce

```bash
git clone https://github.com/adhit-r/fairmind.git
cd fairmind/apps/frontend
npm install
npx tsc --noEmit
```

You should see 19 errors in the files listed above.

## How to Test

1. **Type check (the gate):** `npx tsc --noEmit` - must pass.
2. **Manual smoke (optional, needs a running backend):** Log in, navigate to `/dashboard/admin/users`, `/dashboard/admin/audit-log`, `/dashboard/admin/registrations`, and `/dashboard/compliance-automation`. Pages should render data when the backend responds, and show an error state (not a blank screen) when it doesn't.
3. If you can't run the backend, just the type check is acceptable for review.

## Estimated Effort

2-4 hours for someone comfortable with TypeScript generics and React state management. The compliance-automation rewrite is the largest piece (~1-2h); admin pages and useAuthentik are smaller mechanical fixes (~1h).

## Why This Is a Good Contribution

- **Real bugs, not busywork.** Several of these would cause runtime crashes today if anyone tried to use the affected pages.
- **Self-contained.** Frontend only, no backend changes required.
- **Teaches the codebase's conventions.** You'll understand our API client pattern after fixing this.
- **Maintainer support.** Comment on this issue with questions and a maintainer will respond within 48 hours.

## Help / Questions

Comment on this issue and a maintainer will respond. If you're new to the repo, check `CONTRIBUTING.md` first. Please claim the issue (comment "I'd like to take this") before starting so we don't have duplicate work.

貢獻指南

開啟貢獻指南

研究方向

Run `cd apps/frontend && npx tsc --noEmit` first, then inspect `compliance-automation-service.ts` and the listed consumers, admin pages, and `useAuthentik.ts`. Follow the existing `apiClient` and `ApiResponse` conventions without casts. Done means the type check reports zero errors in the listed files and failed responses show error states.

由索引模型根據 Issue 內容生成。

評估

技術堆疊
nextjs, react, typescript
領域
api, frontend, testing
Issue 類型
缺陷
難度
4/5
預估耗時
3-5 天
活躍度
冷清
描述清晰度
描述清楚
新手友好度
65/100

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。