Fix row doc cell updates#340
Merged
Merged
Conversation
Reviewer's GuideRefactors cell update logic to be row-doc aware and resilient to missing row data, centralizing it in a dedicated dispatch module, adding async handling for calendar interactions, and introducing tests to ensure rows are ensured/loaded before cell updates are applied. Sequence diagram for async row-aware cell update flowsequenceDiagram
actor User
participant FullCalendar
participant useCalendarEvents
participant useUpdateStartEndTimeCell as updateCell
participant DatabaseContext as useDatabaseContext
participant waitForWritableRowTarget
participant writeCellToRow
User->>FullCalendar: drag/resize event
FullCalendar->>useCalendarEvents: handleEventDrop/handleEventResize
useCalendarEvents->>useCalendarEvents: updateEventTime(rowId, startTimestamp, endTimestamp, isAllDay)
useCalendarEvents->>updateCell: updateCell(rowId, fieldId, startTimestamp, endTimestamp, isAllDay)
activate updateCell
updateCell->>DatabaseContext: rowMap[rowId]
alt rowDoc and writable target exist
updateCell->>writeCellToRow: writeCellToRow(rowDoc, row, cells, fieldId, FieldType.DateTime, data, dateOpts)
else rowDoc or target missing
updateCell->>DatabaseContext: ensureRow(rowId)
DatabaseContext-->>updateCell: rowDoc
updateCell->>waitForWritableRowTarget: waitForWritableRowTarget(rowDoc)
waitForWritableRowTarget-->>updateCell: { row, cells } | null
alt target ready
updateCell->>writeCellToRow: writeCellToRow(rowDoc, row, cells, fieldId, FieldType.DateTime, data, dateOpts)
else timed out / null
updateCell->>updateCell: Log.warn("Row doc not ready for cell update")
end
end
deactivate updateCell
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
writeCellToRow, the guardtypeof data === 'string' || typeof data === 'number'is inconsistent withCellUpdateData(which excludesnumber) andupdateDateCell(which expects a string), so you can tighten the type and drop thenumberbranch to avoid confusion and potential misuse. - The fixed
ROW_DATA_WAIT_MS = 3000timeout inwaitForWritableRowTargetis a hidden magic value; consider either documenting why 3s is appropriate or making it configurable so callers can tune behavior for different environments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `writeCellToRow`, the guard `typeof data === 'string' || typeof data === 'number'` is inconsistent with `CellUpdateData` (which excludes `number`) and `updateDateCell` (which expects a string), so you can tighten the type and drop the `number` branch to avoid confusion and potential misuse.
- The fixed `ROW_DATA_WAIT_MS = 3000` timeout in `waitForWritableRowTarget` is a hidden magic value; consider either documenting why 3s is appropriate or making it configurable so callers can tune behavior for different environments.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
# Conflicts: # src/application/database-yjs/__tests__/useUpdateCellDispatch.test.tsx # src/application/database-yjs/dispatch/cell.ts
Row dispatch hooks no longer return a Promise after the merge from main, so `await` on the result is a lint error and a no-op. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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
Checklist
General
Testing
Feature-Specific
Summary by Sourcery
Ensure database row documents and cells are created and ready before applying cell and calendar time updates, and adapt calendar event handlers to the new async update flow.
Bug Fixes:
Enhancements:
Tests: