Repository navigation
Hours tab on the user detail with void - #47
Conversation
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| invalidateLedger(qc, id, eventId) | ||
| useUiStore.getState().showToast(voidedToast(userName, hours)) | ||
| if (res.affectedCertificates.length === 0) return | ||
| void confirmDialog({ |
There was a problem hiding this comment.
If a void finishes while a credit confirmation is open, this notice replaces the confirmation in the single-dialog store. The original confirmation never resolves; dismissing the notice leaves the credit form unable to submit or reopen its confirmation. Queue the notice or settle the open dialog first.
Knowledge Base Used: Application shell and navigation
Artifacts
Executed hours-panel UI harness
- This harness mounted the real hours panel and dialog host with controlled API responses, enabling the overlapping-dialog test.
Executed Chromium reproduction script
- This script drove both void-response conditions through the UI and asserted their credit API call counts, reproducing the failure.
▶ Credit confirmation remains available without affected certificates
- Chromium completed a pending void without affected certificates, then accepted the still-open credit confirmation and made one credit call.
Credit confirmation after a void with no affected certificates
- The captured frame shows the credit confirmation still available after the void response, so the operator can proceed.
▶ Certificate notice replaces credit confirmation
- Chromium completed the same pending void with an affected certificate, then dismissed the replacement notice and found that retrying made no credit call.
Certificate notice occupying the credit confirmation
- The captured frame shows the transcript notice where the credit confirmation had been, making the overwritten dialog visible.
- The captured command output records exit code 0, one credit call in the control run, and zero calls after the certificate notice.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/admin/src/features/users/use-user-hours.ts
Line: 102
Comment:
**Transcript notice strands confirmation**
If a void finishes while a credit confirmation is open, this notice replaces the confirmation in the single-dialog store. The original confirmation never resolves; dismissing the notice leaves the credit form unable to submit or reopen its confirmation. Queue the notice or settle the open dialog first.
**Knowledge Base Used:** [Application shell and navigation](https://app.greptile.com/civfix/-/custom-context/knowledge-base/civfix/civfix-admin/-/docs/application-shell-and-navigation.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| <button className="btn ghost" onClick={cancel}> | ||
| {current.kind === "confirm" ? (current.cancelLabel ?? "Cancel") : "Cancel"} | ||
| </button> | ||
| {!(current.kind === "confirm" && current.acknowledgeOnly) && ( |
There was a problem hiding this comment.
When a void affects issued transcripts, acknowledgeOnly removes Cancel but still lets Escape, an outside click, or Close dismiss the notice. The caller does not handle dismissal, so the operator can lose the only displayed codes needed to revoke transcripts that still verify with the old total. Keep the notice open until Done or preserve its information after dismissal.
Artifacts
Chromium transcript-notice validation script
- The authored script bundles the dialog and void-success callback, runs both comparison conditions, and asserts the observed dismissal behavior.
Chromium validation command and observed results
- The executed command exited 0 and recorded that all three dismissal routes stayed blocked in the control but dismissed the current-source notice.
▶ Guarded comparison keeps the transcript notice open
- Chromium exercised Escape, an outside click, and header Close against an in-memory guarded control; the notice remained until Done was selected.
Guarded comparison with affected transcript codes visible
- A frame captured during the guarded-control run shows the notice and its two affected codes.
▶ Current source dismisses the transcript notice without Done
- Chromium exercised the unchanged source and showed Escape, an outside click, and header Close each dismissing the notice without Done.
Current-source notice with affected transcript codes visible
- A frame captured during the current-source run shows the notice before its dismissal routes were exercised.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/admin/src/components/shared/dialog.tsx
Line: 152
Comment:
**Transcript codes can disappear**
When a void affects issued transcripts, `acknowledgeOnly` removes Cancel but still lets Escape, an outside click, or Close dismiss the notice. The caller does not handle dismissal, so the operator can lose the only displayed codes needed to revoke transcripts that still verify with the old total. Keep the notice open until Done or preserve its information after dismissal.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const list = useEventList( | ||
| { q: q === "" ? undefined : q, filter: "completed", limit: RESULT_LIMIT }, | ||
| { keepPreviousData: true }, | ||
| ) | ||
| const results = list.data?.items ?? [] |
There was a problem hiding this comment.
The picker displays only eight completed events and offers no next page. With nine events matching the same title and place, the ninth cannot be selected for a credit. The shared event response provides a cursor, and the backend list returns subsequent pages; expose those pages in the picker. This is a non-blocking limitation for operators crediting older events.
Knowledge Base Used: Event management
Artifacts
Executed Chromium event-picker repro script
- The authored command bundles the real picker, serves nine matching events, and runs baseline and pagination-overlay browser checks, making the test reproducible.
Rendered event-picker harness source
- The generated entry mounts EventPicker with a query client and displays the selected event ID, showing what the executed browser script rendered.
Successful picker validation run and observed API requests
- Running `node trex-artifacts/event-picker-pagination-repro.cjs` from `/home/user/repo` exited 0 and recorded no baseline cursor request versus a successful cursor request and selection in the comparison.
▶ Unchanged picker with the ninth matching event unreachable
- Chromium searched and scrolled the real picker with nine identical-search completed events; only the first eight rows were available.
Baseline picker after scrolling its eight results
- Playwright captured the unchanged picker after scrolling its result list, with no pagination control present.
▶ Pagination-overlay picker selects the ninth event
- Chromium used an in-memory pagination overlay, loaded the cursor page, and selected `event-9`, showing the comparison behavior without changing tracked code.
Comparison picker after selecting event nine
- Playwright captured the pagination-overlay picker after selecting the oldest event, confirming the selection state.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/admin/src/features/users/event-picker.tsx
Line: 39-43
Comment:
**Older matching events are unreachable**
The picker displays only eight completed events and offers no next page. With nine events matching the same title and place, the ninth cannot be selected for a credit. The [shared event response](https://github.com/civfix/civfix-app/blob/HEAD/packages/shared/src/schemas/admin/events.ts) provides a cursor, and the [backend list](https://github.com/civfix/civfix-backend/blob/HEAD/services/api/src/services/admin/admin-event-service.ts) returns subsequent pages; expose those pages in the picker. This is a non-blocking limitation for operators crediting older events.
**Knowledge Base Used:** [Event management](https://app.greptile.com/civfix/-/custom-context/knowledge-base/civfix/civfix-admin/-/docs/event-management.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| <Icons.X size={11} /> Void | ||
| </button> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
This PR adds roughly 1,500 lines, but its body has no Size: justification. That violates the repository directive requiring a Size: justification for changes above its 400-line cap. This repository requirement must be satisfied before merging.
Rule Used: # civfix review rules civfix is a live civic-tech platform that will hold government contracts. Review every PR for correctness, security and performance. Flag real defects with evidence; skip style nits that lint already covers. ## Repos - **... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/admin/src/features/users/user-hours-panel.tsx
Line: 121
Comment:
**Large PR needs justification**
This PR adds roughly 1,500 lines, but its body has no `Size:` justification. That violates the repository directive requiring a `Size:` justification for changes above its 400-line cap. This repository requirement must be satisfied before merging.
**Rule Used:** # civfix review rules civfix is a live civic-tech platform that will hold government contracts. Review every PR for **correctness, security and performance**. Flag real defects with evidence; skip style nits that lint already covers. ## Repos - **... ([source](https://app.greptile.com/civfix/-/custom-context?memory=39a53925-3d93-4c82-980e-27b67393717d))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comments Outside DiffThese findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.
|
430ef93 to
cf2c9a2
Compare
1 participant
What changed
An operator can open a person's volunteer hours in the admin panel and void an entry with a reason. Admin only.
Before you start
maininto this branch so it uses the published contract.Verify
[Admin]
[Web] (citizen)
[Admin] (account variants)
Regression
User detail tabs — [Admin]
Confirm and reason dialogs elsewhere — [Admin]
Not covered