feat(app): public witnessing - #5297
Open
ux-git wants to merge 28 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (24)
WalkthroughAdds a publisher-protected Public Witnessing feature with location management, recurring shift schedules, calendar views, publisher arrangements, IndexedDB persistence, responsive layouts, and reusable navigation components. ChangesPublic witnessing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Publisher
participant PublicWitnessing
participant ShiftsCard
participant ArrangementForm
participant IndexedDB
Publisher->>PublicWitnessing: open protected route
PublicWitnessing->>IndexedDB: load locations and arrangements
PublicWitnessing->>ShiftsCard: display selected location
Publisher->>ShiftsCard: select shift slot
ShiftsCard->>ArrangementForm: open arrangement form
ArrangementForm->>IndexedDB: save or delete arrangement
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ux-git
force-pushed
the
feat_public_witnessing
branch
from
July 16, 2026 16:37
db293df to
10a7ee3
Compare
Locations and arrangements tables follow the established vertical slice: definition types with the _deleted/updatedAt data-blob pattern, Dexie v13 schema, soft-delete services flagging metadata send_local, jotai db atoms wired through the IndexedDb wrapper, and a publisher route with a Ministry dashboard entry (reusing the existing tr_PW locale key). Arrangements are one record per booking action (author in created_by) so concurrent offline bookings merge without conflicts; day-view shifts are derived from the location's per-weekday schedule and never stored.
Two-column page: locations sidebar (settings_tab from the settings redesign, PR sws2apps#5224) and a details card with badges and description. Admin actions gated by usePermissions (app admin + service overseer): add/edit dialog with General information and Schedule tabs (approved days + per-day shift ranges), delete confirmation (soft delete), and drag reorder persisting sort_index. Shift time rows memoize their Date values in a dedicated ShiftRow — the shared TimePicker resets its in-progress edit whenever the value prop identity changes. Locale keys reuse the existing tr_PW*/general strings; only genuinely new labels were added.
Shifts card composes day navigation (arrows + jump-to-today, matching the field-service-meetings calendar pattern) with the segmented TabSwitcher from the clicker PR (week/month disabled for now). Day slots are derived at render time from the location's weekday schedule merged with booked arrangements — nothing is materialized per date. Cell states follow the design spec: available (accent border + chip), partner needed (orange, stays open until the requested partners join), fully occupied (dashed) and past. Cells are interactive only for bookable states, the arrangement author, or admins. The confirm-the-arrangement dialog covers all flows: partner needed with a wanted-count, I-have-a-partner with a freeSolo persons autocomplete (non-app partners stay name-only), admin for-myself / for-others tabs, joining a seeker (creates the joiner's own record — derived state keeps records conflict-free), edit and cancel, and an add-to-calendar ICS prompt after booking.
Schedule tab now matches the design measurements: fixed 240px approved days column, vertical divider with 32px gaps, 16px shifts-column spacing, 48px day rows, and a 16px dash between the start/end time fields. Tab content renders through the Tabs component's own panels, removing the double gap the empty tabpanels used to add (same fix in the arrangement dialog). Header actions reordered to Reorder / Edit / Add with Add as the main button, and the max publishers badge shortened to "Max. publishers".
Shift cells now behave like buttons: outlined states fill with accent-100 on hover instead of casting a shadow, and publisher names use the smaller text style. The day header drops the year from its label and paints the jump-to-today icon black; the no-shifts state uses the standard info-icon message row. IconReorder's center dot was a separate circle element that CSS recoloring (svg g path selectors) never reached — merged into the arrows path so the glyph is one shape. The cart badge icon gets an explicit accent-dark color for the same reason: its paths sit directly under the svg root with a hardcoded default fill. Location dialog: badges use the small size, the storage field label is now "Stored at", "Every day" shows an indeterminate state when only some days are approved, Add shift uses the small button variant, shift delete buttons sit in the 48px rounded container used by the circuit overseer week rows, and an unselected schedule shows a select-a-day hint with no divider.
The shift-row trash button cancels the IconButton edge-start negative margin that was eating its left spacing, and the day checkboxes drop the FormControlLabel offset so the row's 8px inner padding matches the design. The location delete action moves from a full-width button at the dialog bottom to a small red button beside the Edit location title. Max. publishers per shift is now optional: the field may stay empty, the badge hides when unset, and shift capacity falls back to the default of 3.
Mobile now mirrors the app's subpage pattern: the page shows only the locations list, and tapping a location opens its details as a subpage using the SubpageNavbar from the clicker PR (location name with the parent page underneath, animated back arrow returning to the list). The day navigation adopts the field service meetings calendar layout: single row on tablet+ with the toggle right-aligned, stacked on mobile with the arrows pushed to the card edges; the jump-to-today control always reserves its slot so the label never shifts, the label gets a 180px centered slot with ellipsis, and min-width guards keep the switcher and header clipped inside the card.
On mobile the schedule tab no longer stacks a detached shifts panel under the day list — the editor expands inline right under the tapped day, with expand/collapse chevrons on the far right of approved days and tap-to-toggle behavior. Desktop keeps the two-column layout. Small buttons (Add shift, Add person, the title Delete) now pass disableAutoStretch so they stay compact instead of stretching to the global full-width mobile button default.
The mobile detail view was rendering its own SubpageNavbar below the app navbar, so the screen showed two headers. The app navbar already renders a back arrow with a title and secondary title, so the detail is now a real route (/public-witnessing/:locationId) that drives it through PageTitle: one header, and the navbar back arrow returns to the list on its own. The cherry-picked SubpageNavbar is dropped — it exists for fullscreen overlays that cover the navbar, not for this. Also: the empty locations state uses the InfoNote card variant, the locations list keeps its chevron visible on touch devices (hover never fires there), the dashboard entry uses the cart icon, and a day's shifts only open once the day is approved — tapping an unapproved row approves it, unchecking closes its shifts again.
The shifts card now switches between the day, week and month periods instead of only showing a single day: - week: seven aligned columns of compact shift cells on wide screens, a stacked per-day list below it - month: a calendar grid with the available/occupied shift counts per day (dots when the card is narrow); tapping a day opens its day view - navigation, the period label and the jump-to-today control follow the selected period Slots now carry their own date, so booking from the week and month views stores the clicked day instead of the anchor date, and the arrangement dialog shows it. Arrangements are indexed once per render instead of being scanned for every shift, and the shift cells are reachable by keyboard. Also narrows the locations sidebar below 1400px and lets the card header wrap, which fixes the horizontal overflow of the whole page between 768px and 1200px.
Opening a location on mobile pushes its uid into the route, and the container re-applied that uid on every render — so a desktop window that landed on the detail route (a shared link, or a resized phone session) could no longer switch locations from the sidebar. Wide screens now fold the detail route back into the base one.
Review fixes across the day, week and month views: - the status pill is now the shared Badge component (accent / orange / grey) and every non-past state is labelled, including "Fully occupied"; the hand-rolled chip is gone - hover styles only apply to cells the user can actually open, so past and other-people's shifts no longer light up under the pointer, and partner-needed cells finally react - week cells carry a tooltip and an aria-label with their state, which the compact layout previously left to colour alone; month cells name their counts in the label - month days from the neighbouring months only mute their number instead of filling grey, matching the app's date picker, and the weekday row lines up with the date numbers - the feature uses the shared Divider component instead of the raw MUI one, like the rest of the app - getWeekStartDate moved to @utils/date (first-day-of-week aware and, unlike getWeekDate, non-mutating)
Month view now uses the same calendar as the field service meetings one (PR sws2apps#4369): its grid was extracted into @components/month_calendar — weekday header row, continuous cell borders, leading blanks filled and the outline stepping away after the last day, today's dot — with the day content left to the caller. Public witnessing fills it with the designed badges: a dashed "N occupied" over a filled "N available" that turns red when nothing is left. Days already over only report what was served instead of shouting "0 available". Week view follows its design too: each column is headed by the date and the weekday (muted once the day is over), and a booked cell keeps one line, revealing its publishers (and the partner-needed note) behind an expand chevron. Free cells keep the Available badge. Figma: section 5194:177629 — "Monthly view" 5265:217337, booking-flow "Publisher view" 5381:227347.
Schedule tab: the "every day" checkbox that sat above the seven weekday rows is replaced by an every-day / selected-days switcher — every day edits one set of shifts for the whole week, selected days keeps the per day editor. The checkbox alone approves a day now; the rest of the row opens it, with a chevron appearing under the pointer to say so, and the "select a day" hint moved to the top of the editor pane. Calendar: the weekday header uses the same border scheme as the cells, so its column lines meet theirs, and the rounded corners belong to the corner cells instead of a clipping wrapper. Occupied/available badges drop to the small label size; phone-sized cells show a single outlined count, coloured by what is still open, pinned to the bottom of the day. Week view: 16px column gaps with a divider down the middle, no Available badge (the tooltip covers it, after two seconds), smaller times and room between them and the expand chevron. Mobile: the navbar title centres between the back arrow and its balancing spacer (as in the clicker mode), the locations list drops the active indicator's slot down to an 8px inset, editing is offered inside a location and reordering on the list, and moving between the two slides in from the matching edge.
…holds The partner limit was derived from the location's maximum alone, with a floor of one, and it ignored the publishers already arranged for the shift — so a two-publisher location offered a name field per remaining step and could be saved with more people than the shift allows. The form now counts the seats other arrangements already hold, keeps one for the author when arranging for oneself, and caps the name fields, the "partner needed" count and what gets saved to what is actually left. A shift with a single seat left simply takes its author, without offering partners at all. Also shortens the location dialog's first tab to "Details" so it fits beside "Schedule" on a phone.
…eader Deleting an arrangement was a full-width button under Cancel, competing with the two actions that confirm or dismiss the dialog. It is now the same small red button in the top-right corner that the location dialog already uses.
Location badges follow the design's "badge-medium-light": 12px label, 6px radius, the lightest accent fill, 4/8 padding — and the publisher cap now carries the visitors icon it is drawn with instead of the groups one. Shift status badges drop to the same 12px label, and a taken shift simply reads "Occupied". The publisher headings in the arrangement dialog are grey like the other form subheadings.
Every size passed its font size through sx, where the default body-small-semibold class overruled it — small badges rendered at 14px instead of the 12px they are drawn at, and big ones at 14px instead of their own step. Each size now picks its class from the type scale, so the sizes mean what they say and an explicit className still wins. Adds a `light` badge for the lightest fill of its colour, and rounds the small badge to 4px like the rest. Public witnessing uses it for the location badges (with the visitors icon the design draws them with) instead of styling them by hand, its schedule setup switches "every day" with a switch rather than a second row of tabs, and the shift cells drop a step in the week columns.
ux-git
force-pushed
the
feat_public_witnessing
branch
from
July 30, 2026 22:01
10a7ee3 to
39ca7b1
Compare
Rows that come and go now carry their own id instead of leaning on the array index: partner names in the arrangement dialog, and shifts while a location is being edited (the id is dropped again on save, so nothing new reaches the database). Chained ternaries become named helpers — the slot status, the arrangement mode, the week header and month count colours, the days a shift edit touches — and the slot derivation lost two levels of nesting by moving the seeker rule and the status decision out of the loop.
ux-git
marked this pull request as ready for review
July 31, 2026 07:09
Badge resolved its text and background colours in two branching closures inside the component; they are now flat guard chains at module scope, taking the colour props they need. The shift cell splits its two layouts into their own components, and the arrangement dialog's publisher form comes out of a nested JSX ternary into a small function.
src/components/public_witnessing_card held place and time cards written before this feature; the shift cells replaced them and nothing imported them but the components barrel. They also carried fifteen of the repo's type errors and an untranslated string. Removing them takes the repo from 3216 to 3201 tsc errors. Also drops the arrangements bulk save, which nothing calls — unlike the locations one, which the reorder dialog uses. An expanded week cell no longer stretches the other cells in its row, and the "occupied" state loses its badge in the day and week views; in the week columns the tooltip still names it after two seconds.
The maximum publishers per shift was optional, with every consumer falling back to three. It is now part of a location like its name and schedule: the form will not save without it, and the shift capacity reads it directly. Week cells reserve the height the expand chevron needs, so a shift with publishers no longer sits taller than a free one beside it, and the schedule tab drops the "select a day" placeholder.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (3)
src/features/ministry/public_witnessing/shifts_card/useShiftsCard.tsx (1)
130-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe default shift capacity of 3 is hard-coded in two hooks. Slot status derivation and seat math read the same fallback independently, so a later change to one side produces inconsistent capacity behavior.
src/features/ministry/public_witnessing/shifts_card/useShiftsCard.tsx#L130-L130: read the fallback from a shared exported constant.src/features/ministry/public_witnessing/arrangement_form/useArrangementForm.tsx#L90-L90: read the same constant and keep the existing comment.🤖 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/features/ministry/public_witnessing/shifts_card/useShiftsCard.tsx` at line 130, The default shift capacity is duplicated across the two hooks and must come from one shared exported constant. In src/features/ministry/public_witnessing/shifts_card/useShiftsCard.tsx lines 130-130, replace the fallback in the capacity calculation with that constant; in src/features/ministry/public_witnessing/arrangement_form/useArrangementForm.tsx lines 90-90, use the same constant while preserving the existing comment.src/features/ministry/public_witnessing/arrangement_form/useArrangementForm.tsx (1)
199-223: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReport ICS generation failures to the user.
Handle the
createEventcallback error withdisplaySnackNotification, matchinghandleSave, so failed ICS generation does not close the dialog or silently log only.🤖 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/features/ministry/public_witnessing/arrangement_form/useArrangementForm.tsx` around lines 199 - 223, Update the createEvent error branch in handleDownloadCalendar to call displaySnackNotification with the same failure notification behavior used by handleSave, while preserving the early return so the dialog remains open and removing the console-only handling.src/features/ministry/public_witnessing/index.tsx (1)
55-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
selectedLocationderivation across two files. Both files independently computelocations.find(record => record.location_uid === selected)from the same two atoms, with slightly different null/undefined fallbacks.
src/features/ministry/public_witnessing/index.tsx#L55-L57: replace this inline.find()with a shared derived atom or hook that both files consume.src/pages/ministry/public_witnessing/usePublicWitnessing.tsx#L18-L22: replace this inline.find() ?? nullwith the same shared derived atom or hook, so both files stay in sync and share one fallback convention.🤖 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/features/ministry/public_witnessing/index.tsx` around lines 55 - 57, Replace the duplicated selectedLocation derivation with one shared derived atom or hook based on the locations and selected atoms. Update src/features/ministry/public_witnessing/index.tsx lines 55-57 and src/pages/ministry/public_witnessing/usePublicWitnessing.tsx lines 18-22 to consume it, using one consistent null/undefined fallback convention.
🤖 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/tab_switcher/index.tsx`:
- Around line 52-58: Update the tablist and its ButtonBase tabs to implement
roving keyboard focus: handle ArrowLeft/ArrowRight to move between tabs,
Home/End to focus the first/last tab, and maintain appropriate tabIndex so only
the active tab is in the Tab order. Preserve disabled-tab behavior and invoke
onChange when keyboard navigation selects a new tab.
In `@src/definition/public_witnessing.ts`:
- Around line 4-54: Update the schedule-edit flow that persists
PublicWitnessingShiftType entries to detect arrangements matching an affected
location, date, and original start_time/end_time before saving changed times.
Either warn and migrate those arrangements to the new shift time, or block the
save while existing arrangements remain; ensure bookings stay visible and avoid
relying on a shift ID, since PublicWitnessingShiftType has none.
In `@src/features/ministry/public_witnessing/arrangement_form/index.tsx`:
- Around line 66-74: Update handleConfirmClick to guard against concurrent
submissions by tracking an in-flight save and returning immediately when one is
already running; set the guard before awaiting handleConfirm and clear it after
completion, including failure paths. Ensure the Confirm button uses this state
to remain disabled while the save is running, preventing duplicate arrangement
records.
In `@src/features/ministry/public_witnessing/index.tsx`:
- Around line 38-53: Update the effect containing the locationId branch to
validate that locationId exists in locations before setting selected; when it is
missing, clear or reset the selection and navigate to the public-witnessing list
route, while preserving the existing valid-location behavior and laptop
navigation.
In `@src/features/ministry/public_witnessing/location_delete/index.tsx`:
- Around line 16-20: Update handleDelete to remove or reassign all future public
witnessing arrangements referencing the location’s location_uid before calling
dbPublicWitnessingLocationsSave. Reuse the existing arrangements database
operations and preserve the current soft-delete behavior after those bookings
are handled.
In `@src/features/ministry/public_witnessing/location_form/index.tsx`:
- Around line 258-299: Update the dayRow control to be keyboard-operable by
adding an appropriate interactive role, tabIndex, and keyboard handler that
invokes handleDayRowClick for Enter and Space while preserving the existing
click behavior. Ensure keyboard activation is limited to approved expandable
days and does not interfere with the nested dayCheckbox control.
In `@src/features/ministry/public_witnessing/location_form/useLocationForm.tsx`:
- Around line 27-75: The useLocationForm state is initialized only once, so
changing location leaves fields, shiftsByDay, approvedDays, selectedDay, and
scheduleMode from the previous location. Reset all location-derived state when
the location identity changes, or key the consuming LocationForm by a stable
location identifier so useLocationForm remounts; preserve the existing
empty/default initialization behavior.
- Around line 156-195: Update handleSave to validate every shift in shiftsByDay
before constructing and persisting schedule, requiring each end_time to be
strictly later than its start_time. Reject the save when any shift has an equal
or earlier end time, preserving the existing save flow for valid schedules and
using the form’s established validation or notification mechanism to report the
invalid range.
In
`@src/features/ministry/public_witnessing/locations_reorder/useLocationsReorder.tsx`:
- Around line 20-45: Update the locations_sorted useMemo to detect whether each
location’s findIndex differs from its existing location_data.sort_index before
changing metadata. Skip unchanged locations entirely, or at minimum preserve
their existing updatedAt and avoid saving them; only assign the new sort_index
and current timestamp for reordered locations before
dbPublicWitnessingLocationsBulkSave.
In `@src/features/ministry/public_witnessing/shifts_card/shift_cell.tsx`:
- Around line 123-133: Add the tr_shiftAvailable translation key to every
non-English ministry.json locale file that lacks it, using the locale’s existing
translation conventions and wording. Preserve the statusLabels mapping in
shift_cell.tsx and ensure all locales resolve the available shift label without
exposing the raw key.
- Around line 187-204: Update the expand IconButton and the surrounding outer
button semantics in the shift cell: use an existing key that names the expanded
publisher-list content for the IconButton aria-label, and prevent Enter/Space
events handled by the nested toggle from reaching the outer Box onKeyDown. Also
avoid appending an empty statusLabel to the outer aria-label for past slots,
preserving a valid label without a trailing comma.
In `@src/services/dexie/metadata.ts`:
- Around line 30-31: Update metadata initialization around the defaults
containing public_witnessing_locations and public_witnessing_arrangements so
existing id: 1 records receive these missing keys with the current defaults,
while preserving any existing values. Ensure this migration runs before metadata
consumers access the record, either by merging defaults during initialization or
through a version 13 Dexie upgrade.
In `@src/services/dexie/public_witnessing_arrangements.ts`:
- Line 1: Update dbPublicWitnessingArrangementsClear and
dbPublicWitnessingLocationsClear to call their respective metadata-update
helpers after completing the bulk soft-delete, ensuring both clear mutations are
flagged for synchronization like other writes.
- Line 1: Wrap the record writes and sync-flag metadata updates in both service
save flows with a single appDb.transaction using the corresponding table and
appDb.metadata. Update the put/bulkPut paths in public_witnessing_arrangements
and public_witnessing_locations so each transaction invokes its matching
metadata helper before completing.
- Around line 28-36: Update dbPublicWitnessingArrangementsClear to call
dbUpdatePublicWitnessingArrangementsMetadata() after bulkPut completes, ensuring
the mass soft-delete is marked with send_local: true for synchronization;
preserve the existing early return and soft-delete behavior.
- Around line 14-26: Wrap the arrangement write and metadata update in a single
Dexie read-write transaction in both dbPublicWitnessingArrangementsSave and
dbPublicWitnessingArrangementsBulkSave, ensuring the corresponding put or
bulkPut and dbUpdatePublicWitnessingArrangementsMetadata execute within the same
transaction and commit or fail together.
In `@src/services/dexie/public_witnessing_locations.ts`:
- Around line 28-36: Update dbPublicWitnessingLocationsClear to mark the mass
deletion for synchronization by calling
dbUpdatePublicWitnessingLocationsMetadata() with send_local enabled after
bulkPut completes, matching the metadata update behavior used by other mutations
in the file.
- Around line 14-26: Wrap the writes in both dbPublicWitnessingLocationsSave and
dbPublicWitnessingLocationsBulkSave in a single Dexie transaction that includes
the location put/bulkPut and dbUpdatePublicWitnessingLocationsMetadata call,
ensuring the location and metadata update commit or fail together.
---
Nitpick comments:
In
`@src/features/ministry/public_witnessing/arrangement_form/useArrangementForm.tsx`:
- Around line 199-223: Update the createEvent error branch in
handleDownloadCalendar to call displaySnackNotification with the same failure
notification behavior used by handleSave, while preserving the early return so
the dialog remains open and removing the console-only handling.
In `@src/features/ministry/public_witnessing/index.tsx`:
- Around line 55-57: Replace the duplicated selectedLocation derivation with one
shared derived atom or hook based on the locations and selected atoms. Update
src/features/ministry/public_witnessing/index.tsx lines 55-57 and
src/pages/ministry/public_witnessing/usePublicWitnessing.tsx lines 18-22 to
consume it, using one consistent null/undefined fallback convention.
In `@src/features/ministry/public_witnessing/shifts_card/useShiftsCard.tsx`:
- Line 130: The default shift capacity is duplicated across the two hooks and
must come from one shared exported constant. In
src/features/ministry/public_witnessing/shifts_card/useShiftsCard.tsx lines
130-130, replace the fallback in the capacity calculation with that constant; in
src/features/ministry/public_witnessing/arrangement_form/useArrangementForm.tsx
lines 90-90, use the same constant while preserving the existing comment.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 61fdea1c-6519-45a8-80d1-705c0b6ecc68
⛔ Files ignored due to path filters (2)
src/locales/en/general.jsonis excluded by!**/*.jsonsrc/locales/en/ministry.jsonis excluded by!**/*.json
📒 Files selected for processing (50)
src/App.tsxsrc/components/badge/index.tsxsrc/components/badge/index.types.tssrc/components/icons/IconReorder.tsxsrc/components/month_calendar/index.tsxsrc/components/month_calendar/index.types.tssrc/components/settings_tab/index.styles.tsxsrc/components/settings_tab/index.tsxsrc/components/tab_switcher/index.tsxsrc/components/tab_switcher/index.types.tssrc/definition/public_witnessing.tssrc/features/ministry/public_witnessing/arrangement_form/index.tsxsrc/features/ministry/public_witnessing/arrangement_form/index.types.tssrc/features/ministry/public_witnessing/arrangement_form/useArrangementForm.tsxsrc/features/ministry/public_witnessing/index.tsxsrc/features/ministry/public_witnessing/location_delete/index.tsxsrc/features/ministry/public_witnessing/location_delete/index.types.tssrc/features/ministry/public_witnessing/location_details/index.tsxsrc/features/ministry/public_witnessing/location_details/index.types.tssrc/features/ministry/public_witnessing/location_form/index.tsxsrc/features/ministry/public_witnessing/location_form/index.types.tssrc/features/ministry/public_witnessing/location_form/useLocationForm.tsxsrc/features/ministry/public_witnessing/locations_list/index.tsxsrc/features/ministry/public_witnessing/locations_list/index.types.tssrc/features/ministry/public_witnessing/locations_reorder/index.tsxsrc/features/ministry/public_witnessing/locations_reorder/index.types.tssrc/features/ministry/public_witnessing/locations_reorder/useLocationsReorder.tsxsrc/features/ministry/public_witnessing/shifts_card/day_view.tsxsrc/features/ministry/public_witnessing/shifts_card/index.tsxsrc/features/ministry/public_witnessing/shifts_card/index.types.tssrc/features/ministry/public_witnessing/shifts_card/month_view.tsxsrc/features/ministry/public_witnessing/shifts_card/shift_cell.tsxsrc/features/ministry/public_witnessing/shifts_card/shifts_empty.tsxsrc/features/ministry/public_witnessing/shifts_card/useShiftsCard.tsxsrc/features/ministry/public_witnessing/shifts_card/week_view.tsxsrc/features/ministry/public_witnessing/usePermissions.tsxsrc/indexedDb/appDb.tssrc/indexedDb/tables/public_witnessing_arrangements.tssrc/indexedDb/tables/public_witnessing_locations.tssrc/layouts/navbar/index.tsxsrc/pages/dashboard/ministry/index.tsxsrc/pages/ministry/public_witnessing/index.tsxsrc/pages/ministry/public_witnessing/usePublicWitnessing.tsxsrc/services/dexie/metadata.tssrc/services/dexie/public_witnessing_arrangements.tssrc/services/dexie/public_witnessing_locations.tssrc/states/public_witnessing.tssrc/utils/date.tssrc/wrapper/database_indexeddb/index.tsxsrc/wrapper/database_indexeddb/useIndexedDb.tsx
Comment on lines
+52
to
+58
| <ButtonBase | ||
| key={option.value} | ||
| role="tab" | ||
| aria-selected={isActive} | ||
| disabled={option.disabled} | ||
| disableRipple | ||
| onClick={() => onChange(option.value)} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find target file:"
fd -a 'index.tsx$' . | sed 's#^\./##' | rg 'src/components/tab_switcher/index.tsx|tab_switcher/index.tsx' || true
echo
echo "Target outline:"
ast-grep outline src/components/tab_switcher/index.tsx || true
echo
echo "Target relevant lines:"
cat -n src/components/tab_switcher/index.tsx | sed -n '1,140p'
echo
echo "Search for keyboard handling in related files:"
rg -n "ArrowRight|ArrowLeft|ArrowUp|ArrowDown|Home|End|key=|onKeyDown|onKeyUp|onKeyPress|tabIndex|role=\"tablist\"|role=\\\"tab\\\"" src/components/tab_switcher src || trueRepository: sws2apps/organized-app
Length of output: 50378
Implement keyboard navigation for the tablist.
role="tablist" with role="tab" items must expose roving Arrow/Home/End focus. The current ButtonBase tab only supports Tab navigation, so add tab-keyboard handling or use a control whose semantics match this interaction.
🤖 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/tab_switcher/index.tsx` around lines 52 - 58, Update the
tablist and its ButtonBase tabs to implement roving keyboard focus: handle
ArrowLeft/ArrowRight to move between tabs, Home/End to focus the first/last tab,
and maintain appropriate tabIndex so only the active tab is in the Tab order.
Preserve disabled-tab behavior and invoke onChange when keyboard navigation
selects a new tab.
Comment on lines
+4
to
+54
| export type PublicWitnessingShiftType = { | ||
| start_time: string; // "HH:mm" | ||
| end_time: string; // "HH:mm" | ||
| }; | ||
|
|
||
| export type PublicWitnessingDayScheduleType = { | ||
| weekday: number; // 1 (Monday) – 7 (Sunday) | ||
| shifts: PublicWitnessingShiftType[]; | ||
| }; | ||
|
|
||
| export type PublicWitnessingLocationType = { | ||
| location_uid: string; | ||
| location_data: { | ||
| _deleted: boolean; | ||
| updatedAt: string; | ||
| name: string; | ||
| address: string; | ||
| cart_stored_at: string; | ||
| // Optional — when unset, shifts fall back to the default capacity of 3. | ||
| max_publishers?: number; | ||
| description: string; | ||
| sort_index: number; | ||
| schedule: PublicWitnessingDayScheduleType[]; | ||
| }; | ||
| }; | ||
|
|
||
| export type PublicWitnessingPublisherType = { | ||
| name: string; | ||
| // Set when the publisher is a congregation person; free-text partners | ||
| // (non-app users) have a name only. | ||
| person_uid?: string; | ||
| }; | ||
|
|
||
| export type PublicWitnessingArrangementType = { | ||
| arrangement_uid: string; | ||
| arrangement_data: { | ||
| _deleted: boolean; | ||
| updatedAt: string; | ||
| location_uid: string; | ||
| date: string; // "yyyy/MM/dd" | ||
| start_time: string; // "HH:mm" | ||
| end_time: string; // "HH:mm" | ||
| // person_uid of the author — only the author (and admins) may edit. | ||
| created_by: string; | ||
| partner_needed: boolean; | ||
| // Publishers still wanted when partner_needed; the shift stays joinable | ||
| // until this many partners have been arranged. | ||
| partner_count?: number; | ||
| publishers: PublicWitnessingPublisherType[]; | ||
| }; | ||
| }; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect how shifts are derived from schedule + arrangements,
# and whether shift-time edits account for existing arrangement records.
fd useShiftsCard.tsx | xargs -I{} cat -n {}
rg -n 'start_time|end_time|schedule' src/features/ministry/public_witnessing/shifts_card/useShiftsCard.tsx 2>/dev/null
rg -n 'schedule' src/features/ministry/public_witnessing/location_form -A5 -B5Repository: sws2apps/organized-app
Length of output: 22238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## relevant location form sections\n'
sed -n '280,380p' src/features/ministry/public_witnessing/location_form/index.tsx | cat -n
printf '\n## shifts editor references\n'
rg -n 'selectedShifts|setSelectedShifts|shiftsEditor|Start|End|save|handleSave' src/features/ministry/public_witnessing/location_form -C 3
printf '\n## savedLocation usages and validation around public witnessing\n'
rg -n 'savePublicWitnessing|publicWitnessing|_deleted|location_data|save.*location|handleSave' src -g '!**/node_modules/**' | head -200Repository: sws2apps/organized-app
Length of output: 27686
Preserve existing bookings when schedule times change.
useShiftsCard builds arrangements from the date | start_time key and maps them into derived shifts. Locations save edited schedule shifts without comparing changed start_time/end_time values against arrangements, and PublicWitnessingShiftType has no stable id. A changed shift can leave bookings for the old time and omit that time from generated slots, so publishers lose visibility for the shifted arrangement. Add the change warning/migration step or prevent saving shifts that have existing arrangements for the affected date/time.
🤖 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/definition/public_witnessing.ts` around lines 4 - 54, Update the
schedule-edit flow that persists PublicWitnessingShiftType entries to detect
arrangements matching an affected location, date, and original
start_time/end_time before saving changed times. Either warn and migrate those
arrangements to the new shift time, or block the save while existing
arrangements remain; ensure bookings stay visible and avoid relying on a shift
ID, since PublicWitnessingShiftType has none.
Comment on lines
+66
to
+74
| const handleConfirmClick = async () => { | ||
| const saved = await handleConfirm(); | ||
| if (!saved) return; | ||
| if (mode === 'edit') { | ||
| props.onClose(); | ||
| return; | ||
| } | ||
| setStep('calendar'); | ||
| }; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Block repeated submits of the Confirm button.
handleConfirmClick is async and nothing disables the button while the save runs. In create mode, handleSave generates a new arrangement_uid on each call, so a double click writes two arrangement records for the same slot. Each duplicate consumes a seat and only its author can remove it.
🐛 Proposed fix
const [step, setStep] = useState<'form' | 'calendar'>('form');
+ const [isSaving, setIsSaving] = useState(false); const handleConfirmClick = async () => {
- const saved = await handleConfirm();
- if (!saved) return;
+ if (isSaving) return;
+ setIsSaving(true);
+ try {
+ const saved = await handleConfirm();
+ if (!saved) return;
+ } finally {
+ setIsSaving(false);
+ }
if (mode === 'edit') {
props.onClose();
return;
}
setStep('calendar');
};- <Button variant="main" onClick={handleConfirmClick}>
+ <Button
+ variant="main"
+ disabled={isSaving}
+ onClick={handleConfirmClick}
+ >
{t('tr_confirm')}
</Button>🤖 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/features/ministry/public_witnessing/arrangement_form/index.tsx` around
lines 66 - 74, Update handleConfirmClick to guard against concurrent submissions
by tracking an in-flight save and returning immediately when one is already
running; set the guard before awaiting handleConfirm and clear it after
completion, including failure paths. Ensure the Confirm button uses this state
to remain disabled while the save is running, preventing duplicate arrangement
records.
Comment on lines
+38
to
+53
| useEffect(() => { | ||
| if (locationId) { | ||
| setSelected(locationId); | ||
|
|
||
| if (laptopUp) navigate('/public-witnessing', { replace: true }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const isValid = locations.some( | ||
| (record) => record.location_uid === selected | ||
| ); | ||
| if (!isValid) { | ||
| setSelected(locations.at(0)?.location_uid ?? null); | ||
| } | ||
| }, [locationId, locations, selected, setSelected, laptopUp, navigate]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Deleting the currently-viewed subpage location leaves a blank screen.
The if (locationId) branch sets selected and returns early without checking that locationId still refers to an existing location. The else branch does this check (locations.some(...)), but the locationId branch does not.
If a user deletes the location shown on /public-witnessing/:locationId (through location_delete/index.tsx's handleDelete, which never navigates away), the app stays on that URL. On the next render, selected is re-set to the now-deleted locationId, selectedLocation becomes undefined once the location is filtered out of locations, and the mobile view renders neither the location details nor LocationsList, leaving a blank page with no recovery path other than manually changing the URL.
Validate locationId against locations and navigate back to the list when it no longer exists.
🐛 Proposed fix to validate `locationId` and fall back to the list
useEffect(() => {
if (locationId) {
- setSelected(locationId);
+ const isValidLocationId = locations.some(
+ (record) => record.location_uid === locationId
+ );
+
+ if (!isValidLocationId) {
+ navigate('/public-witnessing', { replace: true });
+ return;
+ }
+
+ setSelected(locationId);
if (laptopUp) navigate('/public-witnessing', { replace: true });
return;
}
const isValid = locations.some(
(record) => record.location_uid === selected
);
if (!isValid) {
setSelected(locations.at(0)?.location_uid ?? null);
}
}, [locationId, locations, selected, setSelected, laptopUp, navigate]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| if (locationId) { | |
| setSelected(locationId); | |
| if (laptopUp) navigate('/public-witnessing', { replace: true }); | |
| return; | |
| } | |
| const isValid = locations.some( | |
| (record) => record.location_uid === selected | |
| ); | |
| if (!isValid) { | |
| setSelected(locations.at(0)?.location_uid ?? null); | |
| } | |
| }, [locationId, locations, selected, setSelected, laptopUp, navigate]); | |
| useEffect(() => { | |
| if (locationId) { | |
| const isValidLocationId = locations.some( | |
| (record) => record.location_uid === locationId | |
| ); | |
| if (!isValidLocationId) { | |
| navigate('/public-witnessing', { replace: true }); | |
| return; | |
| } | |
| setSelected(locationId); | |
| if (laptopUp) navigate('/public-witnessing', { replace: true }); | |
| return; | |
| } | |
| const isValid = locations.some( | |
| (record) => record.location_uid === selected | |
| ); | |
| if (!isValid) { | |
| setSelected(locations.at(0)?.location_uid ?? null); | |
| } | |
| }, [locationId, locations, selected, setSelected, laptopUp, navigate]); |
🤖 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/features/ministry/public_witnessing/index.tsx` around lines 38 - 53,
Update the effect containing the locationId branch to validate that locationId
exists in locations before setting selected; when it is missing, clear or reset
the selection and navigate to the public-witnessing list route, while preserving
the existing valid-location behavior and laptop navigation.
Comment on lines
+16
to
+20
| const record = structuredClone(location); | ||
| record.location_data._deleted = true; | ||
| record.location_data.updatedAt = new Date().toISOString(); | ||
|
|
||
| await dbPublicWitnessingLocationsSave(record); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether location deletion cascades to or is checked against arrangements.
set -euo pipefail
rg -n -C 5 'location_uid' src/services/dexie/public_witnessing_arrangements.ts 2>/dev/null || true
rg -n -C 5 '_deleted' src/services/dexie/public_witnessing_locations.ts 2>/dev/null || true
rg -n -C 3 'location_uid' src/features/ministry/public_witnessing --glob '*.tsx' --glob '*.ts' | rg -v 'location_delete'Repository: sws2apps/organized-app
Length of output: 8674
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant file sizes =="
wc -l \
src/services/dexie/public_witnessing_arrangements.ts \
src/services/dexie/public_witnessing_locations.ts \
src/features/ministry/public_witnessing/location_delete/index.tsx \
src/features/ministry/public_witnessing/shifts_card/useShiftsCard.tsx \
src/features/ministry/public_witnessing/index.tsx \
src/features/ministry/public_witnessing/arrangement_form/useArrangementForm.tsx \
src/features/ministry/public_witnessing/locations_list/index.tsx \
2>/dev/null || true
echo
echo "== deletion service =="
sed -n '1,60p' src/services/dexie/public_witnessing_locations.ts
sed -n '1,180p' src/services/dexie/public_witnessing_arrangements.ts
echo
echo "== delete location component =="
sed -n '1,140p' src/features/ministry/public_witnessing/location_delete/index.tsx
echo
echo "== location selection/filter =="
sed -n '1,120p' src/features/ministry/public_witnessing/index.tsx
sed -n '90,130p' src/features/ministry/public_witnessing/shifts_card/useShiftsCard.tsx
echo
echo "== search deleted filters and cancellation =="
rg -n '_deleted|_deleted\.|deleted|cancel|cancelled|cancel_arrangement|arrangement_data\._deleted|_deleted\s*is|_deleted\s*==|_deleted\s*!' \
src/features/ministry/public_witnessing src/services/dexie \
--glob '*.ts' --glob '*.tsx' \
| head -200Repository: sws2apps/organized-app
Length of output: 19072
Cancel arrangements when deleting a location.
handleDelete only soft-deletes the location, while arrangements continue to store location_uid and remain in public_witnessing_arrangements. If a location with bookings is hidden from the lists, those bookings can become unreachable. Delete or reassign future arrangements before calling dbPublicWitnessingLocationsSave.
🤖 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/features/ministry/public_witnessing/location_delete/index.tsx` around
lines 16 - 20, Update handleDelete to remove or reassign all future public
witnessing arrangements referencing the location’s location_uid before calling
dbPublicWitnessingLocationsSave. Reuse the existing arrangements database
operations and preserve the current soft-delete behavior after those bookings
are handled.
| @@ -0,0 +1,36 @@ | |||
| import appDb from '@db/appDb'; | |||
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Clear soft-deletes records but never flags them for sync, in both dexie services. Every other mutation in these two files calls its metadata-update helper after writing so the local-sync layer knows to push the change. Clear skips this call in both files, so mass local deletions have no signal to be sent to the server until an unrelated write later sets the flag.
src/services/dexie/public_witnessing_arrangements.ts#L28-36: calldbUpdatePublicWitnessingArrangementsMetadata()after the bulk soft-delete indbPublicWitnessingArrangementsClear, or confirm this function is intentionally local-only.src/services/dexie/public_witnessing_locations.ts#L28-36: calldbUpdatePublicWitnessingLocationsMetadata()after the bulk soft-delete indbPublicWitnessingLocationsClear, or confirm this function is intentionally local-only.
🤖 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/services/dexie/public_witnessing_arrangements.ts` at line 1, Update
dbPublicWitnessingArrangementsClear and dbPublicWitnessingLocationsClear to call
their respective metadata-update helpers after completing the bulk soft-delete,
ensuring both clear mutations are flagged for synchronization like other writes.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Save/BulkSave writes aren't atomic with the sync-flag update, in both dexie services. In each file, the record write and the metadata write that sets send_local: true are two separate, independently-committed Dexie operations. A failure or app close between them leaves the record persisted without the sync flag set.
src/services/dexie/public_witnessing_arrangements.ts#L14-26: wrapappDb.public_witnessing_arrangements.put/bulkPutanddbUpdatePublicWitnessingArrangementsMetadata()in oneappDb.transaction('rw', [appDb.public_witnessing_arrangements, appDb.metadata], ...)call.src/services/dexie/public_witnessing_locations.ts#L14-26: wrapappDb.public_witnessing_locations.put/bulkPutanddbUpdatePublicWitnessingLocationsMetadata()in oneappDb.transaction('rw', [appDb.public_witnessing_locations, appDb.metadata], ...)call.
🤖 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/services/dexie/public_witnessing_arrangements.ts` at line 1, Wrap the
record writes and sync-flag metadata updates in both service save flows with a
single appDb.transaction using the corresponding table and appDb.metadata.
Update the put/bulkPut paths in public_witnessing_arrangements and
public_witnessing_locations so each transaction invokes its matching metadata
helper before completing.
Comment on lines
+28
to
+36
| export const dbPublicWitnessingArrangementsClear = async () => { | ||
| const records = await appDb.public_witnessing_arrangements.toArray(); | ||
| if (records.length === 0) return; | ||
| for (const record of records) { | ||
| record.arrangement_data._deleted = true; | ||
| record.arrangement_data.updatedAt = new Date().toISOString(); | ||
| } | ||
| await appDb.public_witnessing_arrangements.bulkPut(records); | ||
| }; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Clear never marks arrangements for sync.
dbPublicWitnessingArrangementsClear soft-deletes every arrangement (_deleted = true, refreshed updatedAt) but never calls dbUpdatePublicWitnessingArrangementsMetadata(). Every other mutation in this file sets send_local: true after writing. Without it here, a mass local deletion has no way to be picked up for sync until an unrelated write later flips the flag.
Call the metadata update after the bulk soft-delete, unless this function is intentionally local-only (e.g., a pre-resync cache wipe that must not be pushed back to the server).
🤖 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/services/dexie/public_witnessing_arrangements.ts` around lines 28 - 36,
Update dbPublicWitnessingArrangementsClear to call
dbUpdatePublicWitnessingArrangementsMetadata() after bulkPut completes, ensuring
the mass soft-delete is marked with send_local: true for synchronization;
preserve the existing early return and soft-delete behavior.
Comment on lines
+14
to
+26
| export const dbPublicWitnessingLocationsSave = async ( | ||
| location: PublicWitnessingLocationType | ||
| ) => { | ||
| await appDb.public_witnessing_locations.put(location); | ||
| await dbUpdatePublicWitnessingLocationsMetadata(); | ||
| }; | ||
|
|
||
| export const dbPublicWitnessingLocationsBulkSave = async ( | ||
| locations: PublicWitnessingLocationType[] | ||
| ) => { | ||
| await appDb.public_witnessing_locations.bulkPut(locations); | ||
| await dbUpdatePublicWitnessingLocationsMetadata(); | ||
| }; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Wrap the record write and metadata update in one transaction.
dbPublicWitnessingLocationsSave and dbPublicWitnessingLocationsBulkSave each perform two separate, independently-committed writes: the location write, then the metadata write that sets send_local: true. If the app closes or an error occurs between the two awaits, the location persists locally, but the sync flag never gets set.
Wrap both writes in a single Dexie transaction so they commit or fail together.
🤖 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/services/dexie/public_witnessing_locations.ts` around lines 14 - 26, Wrap
the writes in both dbPublicWitnessingLocationsSave and
dbPublicWitnessingLocationsBulkSave in a single Dexie transaction that includes
the location put/bulkPut and dbUpdatePublicWitnessingLocationsMetadata call,
ensuring the location and metadata update commit or fail together.
Comment on lines
+28
to
+36
| export const dbPublicWitnessingLocationsClear = async () => { | ||
| const records = await appDb.public_witnessing_locations.toArray(); | ||
| if (records.length === 0) return; | ||
| for (const record of records) { | ||
| record.location_data._deleted = true; | ||
| record.location_data.updatedAt = new Date().toISOString(); | ||
| } | ||
| await appDb.public_witnessing_locations.bulkPut(records); | ||
| }; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Clear never marks locations for sync.
dbPublicWitnessingLocationsClear soft-deletes every location (_deleted = true, refreshed updatedAt) but never calls dbUpdatePublicWitnessingLocationsMetadata(). Every other mutation in this file sets send_local: true after writing. Without it here, a mass local deletion has no way to be picked up for sync until an unrelated write later flips the flag.
🤖 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/services/dexie/public_witnessing_locations.ts` around lines 28 - 36,
Update dbPublicWitnessingLocationsClear to mark the mass deletion for
synchronization by calling dbUpdatePublicWitnessingLocationsMetadata() with
send_local enabled after bulkPut completes, matching the metadata update
behavior used by other mutations in the file.
The vertical divider only appeared once a day was open, so the empty editor lost its column. It stays now, and the prompt beside it is back as "Select a day to plan a shift".
…e form validity flag
|
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.
Description
Public witnessing: locations management plus day, week and month views where publishers book shifts. Frontend and local-first data layer — only the congregation sync endpoint is left for a later PR.
Follows the Figma sections "Public witnessing MVP", "Shift booking flow", and the Monthly/Daily view frames.
Data model
Two Dexie tables (
public_witnessing_locations,public_witnessing_arrangements) on schema v13, following the usual vertical slice:_deleted/updatedAtdata blobs, soft-delete services flaggingmetadata.send_local, jotai atoms through the IndexedDb wrapper.Two decisions worth a look:
created_by), so two publishers booking the same slot offline merge cleanly. Joining a partner-seeker writes the joiner's own record and never mutates the seeker's; the orange "partner needed" state is derived.Views
Day, week and month share one derivation (
useShiftsCard) and one cell component, switched by theTabSwitcherfrom #5291.@components/month_calendar(weekday header, continuous cell borders, outline stepping away after the last day, today dot), with the day content left to the caller: here a dashed "N occupied" over a filled "N available" that turns red when nothing is left, and a single outlined count on phones.Week and month respect the congregation's first day of the week. Selecting a month day opens it in the day view.
Locations and scheduling
Admin only (app admin + service overseer). Sidebar reuses
settings_tabfrom #5224; add/edit dialog has Details and Schedule tabs, soft delete, and drag reorder persistingsort_index.The schedule tab offers an Every day switch — on, one set of shifts repeats all week; off, each approved day gets its own. The checkbox approves a day, the rest of the row opens it for editing.
Booking
The "Confirm the arrangement" dialog covers the Figma flows: partner needed with a wanted count, "I have a partner" with a persons autocomplete that accepts free text, the admin For myself / For others tabs, joining a seeker, editing/cancelling, and the add-to-calendar ICS prompt.
Capacity is enforced against the shift, not just the location: seats left =
max_publishersminus what other arrangements already hold, minus one for the author when arranging for oneself. That caps the name fields, the wanted count, and what is saved. A shift with one seat left offers no partner options at all.Notes for reviewers
@components/badgechanged for the whole app. Every size passed its font size throughsx, where the defaultbody-small-semiboldclass overruled it — small badges rendered at 14px instead of the 12px they are drawn at, big ones at 14px instead of their own step. Each size now picks its class from the type scale (an explicitclassNamestill wins), plus a newlightfill and small radius 2px → 4px. Worth eyeballing badges elsewhere.@components/month_calendaris new and shared; feat(app): field service meetings #4369 can move onto it when it lands.IconReorderfix: its centre dot was a separate<circle>the icon recolouring never reached, so it kept a hardcoded fill. Merged into the arrows path — affects every existing usage.tr_PW*,tr_occupied/tr_available,tr_startTime/tr_endTime); only genuinely new strings were added.Type of change
Checklist: