ciphera-net/pulse
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 15 Wiki Activity

[PULSE-13] Org Audit Log UI with advanced filtering #10

Merged
uz1mani merged 9 commits from staging into main 2026-02-05 13:53:56 +00:00
Conversation 13 Commits 9 Files Changed 3 +248 -4
2 changed files with 248 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit bcb221eb41 - Show all commits

8
components/settings/OrganizationSettings.tsx
View File

@@ -177,8 +177,8 @@ export default function OrganizationSettings() {
if (auditStartDate) params.start_date = auditStartDate
if (auditEndDate) params.end_date = auditEndDate
const { entries, total } = await getAuditLog(params)
setAuditEntries(entries)
setAuditTotal(total)
setAuditEntries(Array.isArray(entries) ? entries : [])
setAuditTotal(typeof total === 'number' ? total : 0)
} catch (error) {
console.error('Failed to load audit log', error)
toast.error(getAuthErrorMessage(error as Error) || 'Failed to load audit log')
greptile-apps[bot] commented 2026-02-05 13:28:16 +00:00 (Migrated from github.com)
Review
Copy Link

infinite loop risk: loadAudit in dependency array

since loadAudit depends on auditPage (line 208), changing filters triggers the debounced effect, which calls loadAudit() and resets auditPage to 0 (line 215). this causes loadAudit to be recreated, potentially re-triggering the effect

remove loadAudit from dependencies since you're already using filtersRef to access latest values:

  useEffect(() => {
    if (activeTab !== 'audit') return
    
    const timer = setTimeout(() => {
        setAuditPage(0)
        loadAudit()
    }, 500)
    return () => clearTimeout(timer)
  }, [auditActionFilter, auditLogIdFilter, auditStartDate, auditEndDate, activeTab])
Prompt To Fix With AI 
This is a comment left during a code review.
Path: components/settings/OrganizationSettings.tsx
Line: 211:219

Comment:
infinite loop risk: `loadAudit` in dependency array

since `loadAudit` depends on `auditPage` (line 208), changing filters triggers the debounced effect, which calls `loadAudit()` and resets `auditPage` to 0 (line 215). this causes `loadAudit` to be recreated, potentially re-triggering the effect

remove `loadAudit` from dependencies since you're already using `filtersRef` to access latest values:

```suggestion
  useEffect(() => {
    if (activeTab !== 'audit') return
    
    const timer = setTimeout(() => {
        setAuditPage(0)
        loadAudit()
    }, 500)
    return () => clearTimeout(timer)
  }, [auditActionFilter, auditLogIdFilter, auditStartDate, auditEndDate, activeTab])
```

How can I resolve this? If you propose a fix, please make it concise.
infinite loop risk: `loadAudit` in dependency array since `loadAudit` depends on `auditPage` (line 208), changing filters triggers the debounced effect, which calls `loadAudit()` and resets `auditPage` to 0 (line 215). this causes `loadAudit` to be recreated, potentially re-triggering the effect remove `loadAudit` from dependencies since you're already using `filtersRef` to access latest values: ```suggestion useEffect(() => { if (activeTab !== 'audit') return const timer = setTimeout(() => { setAuditPage(0) loadAudit() }, 500) return () => clearTimeout(timer) }, [auditActionFilter, auditLogIdFilter, auditStartDate, auditEndDate, activeTab]) ``` <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: components/settings/OrganizationSettings.tsx Line: 211:219 Comment: infinite loop risk: `loadAudit` in dependency array since `loadAudit` depends on `auditPage` (line 208), changing filters triggers the debounced effect, which calls `loadAudit()` and resets `auditPage` to 0 (line 215). this causes `loadAudit` to be recreated, potentially re-triggering the effect remove `loadAudit` from dependencies since you're already using `filtersRef` to access latest values: ```suggestion useEffect(() => { if (activeTab !== 'audit') return const timer = setTimeout(() => { setAuditPage(0) loadAudit() }, 500) return () => clearTimeout(timer) }, [auditActionFilter, auditLogIdFilter, auditStartDate, auditEndDate, activeTab]) ``` How can I resolve this? If you propose a fix, please make it concise. ````` </details>
greptile-apps[bot] commented 2026-02-05 13:28:17 +00:00 (Migrated from github.com)
Review
Copy Link

double API call when page changes

when user clicks pagination, setAuditPage triggers this effect AND the debounced filter effect (211-219) since loadAudit is in both dependency arrays. causes 2 API calls

add auditPage to the debounced effect's dependencies and remove this separate effect

Prompt To Fix With AI 
This is a comment left during a code review.
Path: components/settings/OrganizationSettings.tsx
Line: 221:225

Comment:
double API call when page changes

when user clicks pagination, `setAuditPage` triggers this effect AND the debounced filter effect (211-219) since `loadAudit` is in both dependency arrays. causes 2 API calls

add `auditPage` to the debounced effect's dependencies and remove this separate effect

How can I resolve this? If you propose a fix, please make it concise.
double API call when page changes when user clicks pagination, `setAuditPage` triggers this effect AND the debounced filter effect (211-219) since `loadAudit` is in both dependency arrays. causes 2 API calls add `auditPage` to the debounced effect's dependencies and remove this separate effect <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: components/settings/OrganizationSettings.tsx Line: 221:225 Comment: double API call when page changes when user clicks pagination, `setAuditPage` triggers this effect AND the debounced filter effect (211-219) since `loadAudit` is in both dependency arrays. causes 2 API calls add `auditPage` to the debounced effect's dependencies and remove this separate effect How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-05 13:32:49 +00:00 (Migrated from github.com)
Review
Copy Link

Fix:
Introduced a auditFetchTrigger state to reliably trigger fetches when filters change, even if the page number remains 0.
Debounced Effect: Removed loadAudit from dependencies (fixing the loop). Now it only resets the page and increments the trigger.
Fetch Effect: Merged the pagination logic. It now listens to loadAudit (which changes on page change) AND auditFetchTrigger (which changes on filter change).

Fix: Introduced a auditFetchTrigger state to reliably trigger fetches when filters change, even if the page number remains 0. Debounced Effect: Removed loadAudit from dependencies (fixing the loop). Now it only resets the page and increments the trigger. Fetch Effect: Merged the pagination logic. It now listens to loadAudit (which changes on page change) AND auditFetchTrigger (which changes on filter change).
uz1mani commented 2026-02-05 13:32:58 +00:00 (Migrated from github.com)
Review
Copy Link

Fix:
Introduced a auditFetchTrigger state to reliably trigger fetches when filters change, even if the page number remains 0.
Debounced Effect: Removed loadAudit from dependencies (fixing the loop). Now it only resets the page and increments the trigger.
Fetch Effect: Merged the pagination logic. It now listens to loadAudit (which changes on page change) AND auditFetchTrigger (which changes on filter change).

Fix: Introduced a auditFetchTrigger state to reliably trigger fetches when filters change, even if the page number remains 0. Debounced Effect: Removed loadAudit from dependencies (fixing the loop). Now it only resets the page and increments the trigger. Fetch Effect: Merged the pagination logic. It now listens to loadAudit (which changes on page change) AND auditFetchTrigger (which changes on filter change).
@@ -842,7 +842,7 @@ export default function OrganizationSettings() {
<div className="animate-spin w-6 h-6 border-2 border-neutral-400 border-t-transparent rounded-full mx-auto mb-3"></div>
Loading audit log...
</div>
) : auditEntries.length === 0 ? (
) : (auditEntries ?? []).length === 0 ? (
<div className="p-8 text-center text-neutral-500">No audit events found.</div>
) : (
<div className="overflow-x-auto">
@@ -857,7 +857,7 @@ export default function OrganizationSettings() {
</tr>
</thead>
<tbody>
{auditEntries.map((entry) => (
{(auditEntries ?? []).map((entry) => (
<tr key={entry.id} className="border-b border-neutral-100 dark:border-neutral-800 hover:bg-neutral-50 dark:hover:bg-neutral-800/30">
<td className="px-4 py-3 text-neutral-600 dark:text-neutral-400 whitespace-nowrap">
{new Date(entry.occurred_at).toLocaleString()}

7
lib/api/audit.ts
View File

@@ -55,6 +55,7 @@ async function auditFetch<T>(endpoint: string, options: RequestInit = {}): Promi
/**
* Fetches paginated audit log entries for the current org (org from JWT; admin-only on backend).
* Normalizes response so entries is always an array (backend may return null when empty).
*/
export async function getAuditLog(params: GetAuditLogParams = {}): Promise<GetAuditLogResponse> {
const search = new URLSearchParams()
@@ -65,5 +66,9 @@ export async function getAuditLog(params: GetAuditLogParams = {}): Promise<GetAu
if (params.end_date) search.set('end_date', params.end_date)
const qs = search.toString()
const url = qs ? `/api/audit?${qs}` : '/api/audit'
return await auditFetch<GetAuditLogResponse>(url, { method: 'GET' })
const data = await auditFetch<GetAuditLogResponse>(url, { method: 'GET' })
return {
entries: Array.isArray(data?.entries) ? data.entries : [],
total: typeof data?.total === 'number' ? data.total : 0,
}
}