feat: improve attendee notes access and display - #1031
Conversation
📝 WalkthroughWalkthroughAttendeeForm now uses hooks for state, validation, submission, and section visibility. NotesPanel provides controlled admin-notes rendering with updated translations. The attendee form and edit-ticket page now use the renamed panel component. ChangesAttendee and notes panel refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 3
🤖 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 `@src/components/notes/notes-panel.js`:
- Around line 58-61: Update the confirmation message construction in the delete
flow around the msg object to remove the extra literal separator before noteId,
relying on the trailing space provided by
T.translate("notes_panel.remove_warning"). Preserve the existing translated
warning text and note ID in the rendered dialog.
- Around line 123-125: Update the showColumns calculation to call the standard
columns.includes method when filtering tableColumns, replacing the invalid
columns.include call while preserving the existing fallback to all tableColumns.
- Around line 41-51: Update the notes-loading useEffect to depend on attendeeId,
ticketId, open, and onToggle, while preventing completions from previous
requests from toggling current accordion state or updating stale data; retain
cleanup via clearNotesParams. Correct the columns method to includes and remove
the extra space in the delete warning text.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 473fef30-6a90-4c97-ad66-01cedbc8d9c0
📒 Files selected for processing (4)
src/components/forms/attendee-form/attendee-form.jssrc/components/notes/notes-panel.jssrc/i18n/en.jsonsrc/pages/orders/edit-ticket-page.js
| useEffect(() => { | ||
| getNotes(attendeeId, ticketId, term, 1, perPage, order, orderDir).then( | ||
| () => { | ||
| if (!open) onToggle(); | ||
| } | ||
| ); | ||
|
|
||
| return () => { | ||
| clearNotesParams(); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for key-based remounting of AttendeeForm/EditTicketPage on id changes.
rg -n '<AttendeeForm' -C5
rg -n '<EditTicketPage' -C5 --type=js
rg -n 'ticket_id' -C3 src/pages/orders/edit-ticket-page.jsRepository: fntechgit/summit-admin
Length of output: 160
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- NotesPanel implementation ---'
sed -n '1,220p' src/components/notes/notes-panel.js
printf '%s\n' '--- AttendeeForm call site ---'
sed -n '390,455p' src/components/forms/attendee-form/attendee-form.js
printf '%s\n' '--- EditTicketPage call site and effects ---'
sed -n '1,100p' src/pages/orders/edit-ticket-page.js
sed -n '425,475p' src/pages/orders/edit-ticket-page.js
printf '%s\n' '--- NotesPanel references and key props ---'
rg -n -C5 'NotesPanel|<AttendeeForm|<EditTicketPage|key=' src --glob '*.js'
printf '%s\n' '--- columns prop usage ---'
rg -n -C3 'columns|include\\(' src/components/notes src --glob '*.js'
printf '%s\n' '--- remove_warning translations ---'
rg -n -C2 'remove_warning' .Repository: fntechgit/summit-admin
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- AttendeeForm state and toggleSection ---'
rg -n -C8 'toggleSection|openSections|useState' src/components/forms/attendee-form/attendee-form.js | head -160
printf '%s\n' '--- AttendeeForm and EditTicketPage route declarations ---'
rg -n -C6 'attendee-form|AttendeeForm|edit-ticket-page|EditTicketPage|ticket/:|ticket_id|attendee_id' src --glob '*.js' --glob '*.jsx' | head -240
printf '%s\n' '--- All NotesPanel call sites only ---'
rg -n -C4 'NotesPanel' src --glob '*.js' --glob '*.jsx'
printf '%s\n' '--- Translation files containing remove_warning ---'
rg -l '"remove_warning"' . | while IFS= read -r file; do
printf '%s\n' "--- $file"
rg -n -C2 '"remove_warning"' "$file"
done
printf '%s\n' '--- Notes state shape and actions ---'
rg -n -C6 'notesState|columns:|getNotes|clearNotesParams' src/actions src/reducers src --glob '*.js' | head -260Repository: fntechgit/summit-admin
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- AttendeeForm consumers and route paths ---'
rg -n -C5 'AttendeeForm|EditTicketPage|edit-ticket|edit_attendee|attendees?/|tickets?/' src/pages src/routes src --glob '*.js' --glob '*.jsx' 2>/dev/null | head -260
printf '%s\n' '--- Notes reducer and action request completion ---'
sed -n '1,230p' src/reducers/notes-reducer.js 2>/dev/null || true
sed -n '1,195p' src/actions/notes-actions.js
printf '%s\n' '--- Targeted static verifier ---'
python3 - <<'PY'
import json
from pathlib import Path
notes = Path("src/components/notes/notes-panel.js").read_text()
translation = json.loads(Path("src/i18n/en.json").read_text())
warning = translation["notes_panel"]["remove_warning"]
print("effect_has_empty_deps:", " }, []);" in notes)
print("uses_columns_include:", "columns.include(c.columnKey)" in notes)
print("remove_warning_repr:", repr(warning))
print("rendered_delete_text_repr:", repr(f"{warning} 123"))
print("double_space_in_rendered_delete_text:", " " in f"{warning} 123")
PYRepository: fntechgit/summit-admin
Length of output: 23264
Refetch notes when attendeeId or ticketId changes. The empty dependency array captures stale identifiers and accordion state. Use current open and onToggle, and ignore completions from previous requests. Also replace columns.include(...) with columns.includes(...) and remove the extra space in the delete warning.
🤖 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 `@src/components/notes/notes-panel.js` around lines 41 - 51, Update the
notes-loading useEffect to depend on attendeeId, ticketId, open, and onToggle,
while preventing completions from previous requests from toggling current
accordion state or updating stale data; retain cleanup via clearNotesParams.
Correct the columns method to includes and remove the extra space in the delete
warning text.
There was a problem hiding this comment.
@santipalenque Confirmed this part applies. Verified that EditSummitAttendeePage.componentDidUpdate and EditTicketPage's useEffect(..., [match.params.ticket_id]) both re-fetch the entity without unmounting the tree when the route id changes — so NotesPanel can stay mounted while showing a different attendee's/ticket's notes.
That said, it's not a regression introduced by this PR: the previous Notes component had the exact same useEffect(() => { getNotes(...); ... }, []) with no attendeeId/ticketId in its deps, so this predates this change. Worth fixing while the file is already being touched, but flagging it as inherited rather than newly introduced.
| const msg = { | ||
| title: T.translate("general.are_you_sure"), | ||
| text: `${T.translate("notes.remove_warning")} ${noteId}`, | ||
| text: `${T.translate("notes_panel.remove_warning")} ${noteId}`, | ||
| type: "warning" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Delete-confirmation text will render a double space.
text: \${T.translate("notes_panel.remove_warning")} ${noteId}`adds a literal space before${noteId}. The translation "notes_panel.remove_warning"insrc/i18n/en.json` already ends with a trailing space, so the rendered dialog shows two spaces between "note" and the ID.
✏️ Proposed fix
- "remove_warning": "Are you sure you want to delete note ",
+ "remove_warning": "Are you sure you want to delete note",🤖 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 `@src/components/notes/notes-panel.js` around lines 58 - 61, Update the
confirmation message construction in the delete flow around the msg object to
remove the extra literal separator before noteId, relying on the trailing space
provided by T.translate("notes_panel.remove_warning"). Preserve the existing
translated warning text and note ID in the rendered dialog.
| const showColumns = columns | ||
| ? tableColumns.filter((c) => columns.include(c.columnKey)) | ||
| : tableColumns; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file="$(fd -t f '^notes-panel\.js$' . | head -n 1)"
printf '%s\n' "FILE=$file"
ast-grep outline "$file" --match 'showColumns' --view expanded || true
sed -n '100,140p' "$file"
printf '%s\n' '--- columns references ---'
rg -n -C 3 '\bcolumns\b|notes-panel|NotesPanel' . -g '*.js' -g '*.jsx' -g '*.ts' -g '*.tsx'Repository: fntechgit/summit-admin
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- NotesPanel definition and prop types ---'
sed -n '1,55p' src/components/notes/notes-panel.js
sed -n '195,225p' src/components/notes/notes-panel.js
printf '%s\n' '--- NotesPanel call sites ---'
rg -n -B 8 -A 16 '<NotesPanel|<NotesPanel\b' src
printf '%s\n' '--- Runtime method check ---'
node - <<'JS'
const columns = ["id"];
console.log({
includesType: typeof columns.includes,
includeType: typeof columns.include
});
try {
columns.include("id");
} catch (error) {
console.log(`${error.name}: ${error.message}`);
}
JSRepository: fntechgit/summit-admin
Length of output: 5951
Replace columns.include(...) with columns.includes(...). When columns is provided, the current call throws TypeError: columns.include is not a function.
🤖 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 `@src/components/notes/notes-panel.js` around lines 123 - 125, Update the
showColumns calculation to call the standard columns.includes method when
filtering tableColumns, replacing the invalid columns.include call while
preserving the existing fallback to all tableColumns.
There was a problem hiding this comment.
Pull request overview
This PR improves how admin/attendee notes are accessed and displayed by introducing a dedicated NotesPanel component and wiring it into both ticket editing and attendee editing flows, alongside related i18n key updates.
Changes:
- Replaced the previous notes section usage with a reusable
NotesPanelin ticket and attendee editing screens. - Refactored
AttendeeFormfrom a class component to a function component with hooks and independent section open/close state. - Updated English translations to support the new notes panel labels/messages and moved the “note created” success message key.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/pages/orders/edit-ticket-page.js | Switches ticket admin notes UI to the new NotesPanel and makes section toggle event handling null-safe. |
| src/i18n/en.json | Adjusts translation keys for notes vs. notes panel and removes unused admin notes labels. |
| src/components/notes/notes-panel.js | Introduces/renames the notes UI into a dedicated panel component with table/search/export behaviors. |
| src/components/forms/attendee-form/attendee-form.js | Converts attendee form to hooks, updates section toggling, and embeds NotesPanel for attendee notes. |
Suppressed comments (2)
src/components/notes/notes-panel.js:46
- The initial load effect toggles the panel after the getNotes promise resolves using the initial
openvalue from the mount render. If the user opens/closes the panel while the request is in flight, this callback can flip the panel back to the opposite state unexpectedly.
src/components/notes/notes-panel.js:125 columns.include(...)is not a standard Array API (the standard method isincludes). Ifcolumnsis ever provided as an array (the likely intent), this will throw at runtime.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // field dint change , so remove it from submit | ||
| delete copyOfEntity[f]; |
| }) => { | ||
| const [newNote, setNewNote] = useState(""); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
@santipalenque This useEffect closes over open and onToggle at mount time (empty dep array), then calls onToggle() once getNotes resolves — and that stale call can silently undo a user's own action.
In edit-ticket-page.js, showSection is a single shared string across refund_requests/admin_notes/audit_log (mutually exclusive), initialized to "main". Since mount always happens with showSection === "main", the closure's toggleSection deterministically resolves to setShowSection("admin_notes") — a literal (non-functional) set. If an admin opens "Refund Requests" or "Audit Log" while the notes request is still in flight (any nonzero latency), that panel gets forcibly closed and replaced with "Admin Notes" the moment the fetch resolves, with no visual warning.
In attendee-form.js, toggleSection uses a functional update (setOpenSections(prev => ...)), so the flip itself is safe, but the gate if (!open) is still the stale mount-time false — meaning the deferred call always fires a toggle once. If the admin manually opens "Admin Notes" before the fetch resolves, this closes it right back.
Suggested fix: don't gate on the closed-over open/onToggle. Track "already auto-opened" and the current open state via a useRef synced every render, and expose a dedicated idempotent onOpen rather than reusing the generic toggle. Note: simply adding attendeeId, ticketId, open, onToggle to the effect's deps (as another bot comment on this PR suggests) would introduce a different bug — it would re-run getNotes/clearNotesParams on every panel toggle.
| return copyOfEntity; | ||
| }; | ||
|
|
||
| const AttendeeForm = ({ |
There was a problem hiding this comment.
@santipalenque This PR's commit message describes moving the notes panel and adding auto-open, but the diff also rewrites the entire AttendeeForm from a class component to hooks — validation (validate), submission (triggerFormSubmit, handleSubmit), tag creation (handleNewTag), and unchanged-field stripping (removeUnchangedFields) all moved to a new state/effect model. That's the attendee save/validate pipeline, not just the notes panel.
yarn jest src/components/forms/attendee-form src/components/notes src/pages/orders currently matches zero test files, and a repo-wide search finds no test anywhere referencing AttendeeForm, NotesPanel, or notes-panel — so this rewrite ships with no regression coverage at all.
Suggested fix: add a behavioral test class for AttendeeForm covering required-field validation, that onSubmit still receives a removeUnchangedFields-filtered payload after submit, and that the form resyncs when the entity prop changes — per summit-admin-testing-patterns.md's component/page test guidance.
https://app.clickup.com/t/9014802374/86b72t1x0
Summary by CodeRabbit