refactor(web): simplify optimistic rendering

[tyler-dane]

Follow-up to #1394

[Copilot]

Pull request overview

This PR refactors optimistic rendering by replacing the optimistic ID prefix approach with a pending events tracking system. Events now receive their final MongoDB IDs immediately upon creation instead of temporary prefixed IDs that are later replaced. The key architectural change is the introduction of a pendingEventsSlice that tracks event IDs during creation/conversion, allowing the UI to disable interactions with events still being saved to the database.

• Replaces optimistic ID prefix pattern (optimistic-{id}) with pending event tracking via Redux state
• Events now maintain the same ID throughout their lifecycle (no more ID replacement)
• UI components check pending state instead of ID patterns to prevent premature interactions

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
packages/web/src/ducks/events/slices/pending.slice.ts	New Redux slice to track pending event IDs during creation/conversion
packages/web/src/ducks/events/selectors/pending.selectors.ts	Selectors for accessing pending event state
packages/web/src/ducks/events/sagas/event.sagas.ts	Updated create/delete sagas to track pending events; removed ID replacement logic
packages/web/src/ducks/events/sagas/someday.sagas.ts	Removed ID replacement in convert saga
packages/web/src/ducks/events/sagas/saga.util.ts	Removed replaceOptimisticId saga; updated _createOptimisticGridEvent to use new addId function
packages/web/src/common/utils/event/event.util.ts	Replaced replaceIdWithOptimisticId and isOptimisticEvent with addId; updated cursor style functions to use isPending
packages/web/src/common/types/web.event.types.ts	Removed optimisticIdSchema; added WithId utility type
packages/web/src/store/reducers.ts	Integrated pendingEventsSlice into store
packages/web/src/views/Forms/hooks/useOpenEventForm.ts	Checks pending state instead of ID pattern to prevent opening form for pending events
packages/web/src/views/Day/hooks/events/useOpenAgendaEventPreview.ts	Checks pending state to prevent opening preview for pending events
packages/web/src/views/Day/components/Agenda/Events/*/Draggable*.tsx	Components use pending state selector instead of isOptimisticEvent
packages/web/src/views/Calendar/components/Grid/MainGrid/MainGridEvents.tsx	Prevents click handler from opening form for pending events
packages/web/src/views/Calendar/components/Grid/AllDayRow/AllDayEvent.tsx	Uses pending state for cursor styling and resize prevention
packages/web/src/views/Calendar/components/Event/Grid/GridEvent/GridEvent.tsx	Uses pending state for cursor styling and resize prevention
packages/web/src/views/Calendar/components/Event/styled.ts	Renamed isOptimistic prop to isPending
packages/web/src/views/Calendar/components/Draft/hooks/actions/useDraftActions.ts	Uses pending state to determine if event is existing vs new
packages/web/src/components/ContextMenu/GridContextMenuWrapper.tsx	Prevents context menu from opening for pending events
packages/web/src/components/ContextMenu/ContextMenuItems.tsx	Disables edit/duplicate actions for pending events with visual feedback
Test files	Updated mocks and assertions to reflect new pending state approach and same-ID lifecycle

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

packages/web/src/ducks/events/sagas/event.sagas.ts:143

• The pending event ID should be removed from the pending state when an event is deleted. Without this cleanup, the pending event IDs array will grow indefinitely as events are deleted, causing a memory leak.

After line 127 (after deleting from entities), add a cleanup action to remove the event from pending state regardless of whether it was pending or not (the remove action is idempotent).

export function* deleteEvent({ payload }: Action_DeleteEvent) {
  try {
    yield put(getWeekEventsSlice.actions.delete(payload));
    yield put(getDayEventsSlice.actions.delete(payload));
    yield put(eventsEntitiesSlice.actions.delete(payload));

    const pendingEventIds = (yield select(
      (state: RootState) => state.events.pendingEvents.eventIds,
    )) as string[];
    const isPending = pendingEventIds.includes(payload._id);
    // Only call delete API if event is not pending (i.e., exists in DB)
    if (!isPending) {
      yield call(EventApi.delete, payload._id, payload.applyTo);
    }

    yield put(deleteEventSlice.actions.success());
  } catch (error) {
    yield put(deleteEventSlice.actions.error());
    handleError(error as Error);
  }
}

[Copilot]

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.

[Copilot]

Critical bug: The optimistic event is created with a new ID on line 39, but then _editEvent is called with the original gridEvent which still has the old ID from the existing someday event. This means the backend will try to edit an event with the old ID, while the frontend has already inserted an optimistic event with a different new ID into state.

The fix should be to call _editEvent with optimisticEvent instead of gridEvent, after ensuring optimisticEvent has the correct data from gridEvent (minus the _id change).

Suggested change

	// Mark event as pending when edit starts
	yield put(pendingEventsSlice.actions.add(optimisticEvent._id));
	yield* _editEvent(gridEvent);
	// Ensure optimisticEvent has the latest gridEvent data, while preserving
	// the optimistic-specific fields (like the new _id).
	optimisticEvent = {
	...gridEvent,
	...optimisticEvent,
	_id: optimisticEvent._id,
	} as Schema_OptimisticEvent;
	// Mark event as pending when edit starts
	yield put(pendingEventsSlice.actions.add(optimisticEvent._id));
	yield* _editEvent(optimisticEvent);

[Copilot]

Critical bug: The optimistic event is created with a new ID on line 62, but then _editEvent is called on line 67 with the original gridEvent which still has the old ID. This mismatch will cause the backend to attempt editing the event with the old ID while the frontend has already inserted an optimistic event with a different new ID into state.

The fix should be to call _editEvent with optimisticEvent instead of gridEvent.

Suggested change

	yield* _editEvent(gridEvent, { applyTo });
	yield* _editEvent(optimisticEvent, { applyTo });

[Copilot]

The comment on line 61 is now misleading. It says "optimistic event will have the same ID as that eventually saved", but this is only true for newly created events (createEvent saga). For conversion operations like this one, the optimistic event gets a new ID while the original event that's being converted has its own ID. The backend edit call should use the new optimistic event's ID, not the old event's ID.

Suggested change

	// optimistic event will have the same ID as that eventually saved
	optimisticEvent = yield* _createOptimisticGridEvent(gridEvent, true);
	// Mark event as pending when edit starts
	yield put(pendingEventsSlice.actions.add(optimisticEvent._id));
	yield* _editEvent(gridEvent, { applyTo });
	// For conversions, the optimistic event represents the new event and has its own ID
	optimisticEvent = yield* _createOptimisticGridEvent(gridEvent, true);
	// Mark event as pending when edit starts
	yield put(pendingEventsSlice.actions.add(optimisticEvent._id));
	yield* _editEvent(optimisticEvent, { applyTo });

[Copilot]

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

packages/web/src/ducks/events/sagas/event.sagas.ts:143

• The deleteEvent saga does not remove the event ID from the pendingEvents state when deleting a pending event. This can lead to memory leaks where pending event IDs remain in state even after the event has been deleted. The saga should call pendingEventsSlice.actions.remove(payload._id) before or after deleting the event from other slices.

export function* deleteEvent({ payload }: Action_DeleteEvent) {
  try {
    yield put(getWeekEventsSlice.actions.delete(payload));
    yield put(getDayEventsSlice.actions.delete(payload));
    yield put(eventsEntitiesSlice.actions.delete(payload));

    const pendingEventIds = (yield select(
      (state: RootState) => state.events.pendingEvents.eventIds,
    )) as string[];
    const isPending = pendingEventIds.includes(payload._id);
    // Only call delete API if event is not pending (i.e., exists in DB)
    if (!isPending) {
      yield call(EventApi.delete, payload._id, payload.applyTo);
    }

    yield put(deleteEventSlice.actions.success());
  } catch (error) {
    yield put(deleteEventSlice.actions.error());
    handleError(error as Error);
  }
}

[Copilot]

The code uses non-null assertion (event._id!) when checking if an event is pending, but Schema_GridEvent can have an optional _id field. If _id is undefined, this will cause undefined to be passed to includes(), which will always return false but is semantically incorrect. Add a null check before the includes call: event._id && pendingEventIds.includes(event._id) or use optional chaining.

[Copilot]

The code uses non-null assertion (event._id!) when checking if an event is pending, but Schema_GridEvent can have an optional _id field. If _id is undefined, this will cause undefined to be passed to includes(), which will always return false but is semantically incorrect. Add a null check before the includes call: event._id && pendingEventIds.includes(event._id) or use optional chaining.

[Copilot]

The code uses non-null assertion (event._id!) when checking if an event is pending, but event._id can be undefined in the context menu. If _id is undefined, this will pass undefined to the selector which may cause issues. Add a null check: event._id && selectIsEventPending(state, event._id) or handle the undefined case in the selector.

Suggested change

	selectIsEventPending(state, event._id!),
	event._id ? selectIsEventPending(state, event._id) : false,