feat(calendar): NCAL-4 — team-event grid, event modals, link-to-lead - #284
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds a team-event calendar entry flow end-to-end: shared types and schemas, CalDAV patch and merge logic, calendar UI modals and grid rendering, page wiring, a link-to-lead API route, unit/e2e tests, and process docs. ChangesNCAL-4 Calendar Event UI
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CalendarPage
participant EventDetailModal
participant LinkAPI
participant directPatchEvent
participant Nextcloud
User->>CalendarPage: click team-event chip
CalendarPage->>EventDetailModal: open with entry
User->>EventDetailModal: select lead, click Convert
EventDetailModal->>CalendarPage: onlink(uid, leadId, startAt)
CalendarPage->>LinkAPI: POST /api/calendar/events/[uid]/link
LinkAPI->>LinkAPI: create crm_meetings row
LinkAPI->>directPatchEvent: patch(uid, categories, leadHref)
directPatchEvent->>Nextcloud: GET then PUT updated .ics
Nextcloud-->>directPatchEvent: success or error
directPatchEvent-->>LinkAPI: resolve or CalDavWebhookError
LinkAPI-->>CalendarPage: success or rollback + error
CalendarPage->>CalendarPage: invalidateAll()
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: one or more packages not found in the registry. Comment |
…Modal, link-to-lead - Show all Nextcloud events as purple chips in calendar grid (server-side CalDAV REPORT merge with graceful degradation; excludes CRM-synced categories) - Legend toggle checkbox to show/hide team events client-side - EventFormModal: create/edit team events (title, date/time, all-day, location, description, color, status) via POST/PUT /api/calendar/events - EventDetailModal: view/edit/delete team events + Convert to CRM Meeting flow (LeadCombobox + POST /api/calendar/events/[uid]/link creates crm_meetings row and patches Nextcloud via direct CalDAV PUT for CATEGORIES + CRM-HREF) - directPatchEvent() in writer.ts: GET ICS + ical.js patch + PUT back - CalendarEntry type union extended with team-event variant and optional fields - Zod schemas extended with attendees, color, status, rrule optional fields - 45 new Fully-Automated Vitest tests; 627 total green Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
hdmGOAT
force-pushed
the
feat/ncal-4-event-ui
branch
from
July 9, 2026 05:11
08d1042 to
58c1131
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (12)
src/lib/zod/schemas.ts (2)
221-241: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate schema body between create/update — extract shared fields.
createCalendarEventSchemaandupdateCalendarEventSchemashare an almost identical object shape (this diff adds the same 4 fields to both). Extracting a shared base (e.g..extend()off a common object) would prevent the two schemas drifting apart as fields are added.♻️ Suggested consolidation
+const calendarEventBaseFields = { + title: z.string().trim().min(1), + start: z.iso.datetime(), + end: z.iso.datetime(), + location: z.string().optional(), + description: z.string().optional(), + categories: z.string().optional(), + leadHref: z.string().optional(), + attendees: z.array(z.string().email()).optional(), + color: z.string().regex(/^#[0-9a-fA-F]{6}$/).optional(), + status: z.enum(['confirmed', 'tentative', 'cancelled']).optional(), + rrule: z.string().optional() +}; + export const createCalendarEventSchema = z - .object({ - title: z.string().trim().min(1), - ... - }) + .object(calendarEventBaseFields) .refine((v) => new Date(v.end) > new Date(v.start), { message: 'end must be after start', path: ['end'] }); export const updateCalendarEventSchema = z - .object({ - title: z.string().trim().min(1), - ... - }) + .object(calendarEventBaseFields) .refine((v) => new Date(v.end) > new Date(v.start), { message: 'end must be after start', path: ['end'] });Also applies to: 245-265
🤖 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/lib/zod/schemas.ts` around lines 221 - 241, The `createCalendarEventSchema` and `updateCalendarEventSchema` bodies are duplicated and should be consolidated into a shared base schema. Extract the common fields into a reusable object in `src/lib/zod/schemas.ts`, then build both schemas from it using the existing `createCalendarEventSchema` and `updateCalendarEventSchema` symbols so future field changes stay in sync.
229-236: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
.email()chained method is deprecated in Zod v4.Confirmed via Zod v4 migration docs: The method forms (z.string().email()) still exist and work as before, but are now deprecated. Consider
z.email()for new code going forward; not blocking since the chained form still validates correctly.Also applies to: 253-260
🤖 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/lib/zod/schemas.ts` around lines 229 - 236, The email validation uses the deprecated chained form in Zod v4, so update the affected schema fields to use the new standalone email validator instead of z.string().email(). Look for the email-related entries in schemas.ts, including the attendees array and the other schema block noted in the comment, and switch them to the current z.email() pattern while keeping the same optional/array structure.src/routes/calendar/+page.server.ts (1)
92-100: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSilent catch loses diagnostic signal on Nextcloud fetch failures.
The graceful degradation (empty
teamEventEntrieson error) is reasonable UX, but swallowing the error entirely with no log means an auth misconfiguration, network outage, or ICS parsing regression all look identical to "no Nextcloud events today." Consider logging the error server-side.📋 Suggested logging
} catch { - // Degrade gracefully — calendar still loads without Nextcloud events + } catch (err) { + // Degrade gracefully — calendar still loads without Nextcloud events + console.error('Failed to load Nextcloud team events:', err); }🤖 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/routes/calendar/`+page.server.ts around lines 92 - 100, The team events fallback in +page.server.ts is fine, but the catch around fetchCalendarReport/parseIcsToEvents/mapTeamEvents is swallowing failures entirely and hiding useful diagnostics. Update that try/catch to log the caught error server-side with enough context before leaving teamEventEntries empty, so failures in the Nextcloud fetch/parsing path can be distinguished from a legitimate “no events” result.src/lib/caldav/writer.ts (1)
211-211: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueCategories update fully replaces rather than merges the property.
updatePropertyWithValue('categories', options.categories)discards any pre-existing CATEGORIES values (e.g. user-set labels unrelated to CRM sync). Given current call sites always pass a single fixed category, impact is limited today, but worth confirming this is intentional before more category values are introduced.🤖 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/lib/caldav/writer.ts` at line 211, The CATEGORIES update in the CalDAV writer currently replaces the entire property, which can discard existing labels. Review the update logic in the write path around vevent.updatePropertyWithValue and decide whether categories should be merged instead of overwritten; if merge is intended, preserve any existing CATEGORIES values and append the sync category only when missing. Keep the change localized to the writer flow that sets options.categories so future call sites with multiple categories remain safe.src/lib/types/index.ts (1)
209-227: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a discriminated union instead of shared optional fields.
typeplus six type-specific optional fields on one flat interface allows invalid combinations to type-check (e.g. a'meeting'entry withcategoriesset, or a'team-event'entry missinguid). A discriminated union ({ type: 'team-event'; uid: string; ... } | { type: 'meeting'; ... } | ...) would let TypeScript enforce the field/type pairing that the comments currently only document.🤖 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/lib/types/index.ts` around lines 209 - 227, The current event type definition uses one flat shape with shared optional fields, which allows invalid field/type combinations to compile. Refactor the type around a discriminated union keyed by the existing type field so each variant in src/lib/types/index.ts has only the fields it supports, with required properties like uid for team-event enforced by TypeScript. Update the event type aliases/interfaces near the current type declaration to preserve the documented pairing between type and fields while removing the unnecessary optional cross-compatibility.src/routes/api/calendar/events/[uid]/link/+server.ts (2)
25-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
z.string().uuid()is deprecated in Zod v4; prefer top-levelz.uuid().
startAtalready uses the new top-levelz.iso.datetime()form, butleadIdstill uses the deprecated chained.string().uuid(). Zod 4 deprecates method-chained string formats in favor of top-level functions, and notez.uuid()is now stricter (validates RFC 9562/4122 variant bits) — verify this doesn't reject any currently-accepted UUIDs before switching.const linkBodySchema = z.object({ - leadId: z.string().uuid(), + leadId: z.uuid(), startAt: z.iso.datetime() });🤖 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/routes/api/calendar/events/`[uid]/link/+server.ts around lines 25 - 28, The `linkBodySchema` still uses the deprecated chained UUID validator on `leadId`; update it to the top-level Zod v4 UUID form to match the existing `startAt` pattern. Make the change in the schema definition itself, and verify the stricter `z.uuid()` validation still accepts the UUID values this endpoint currently receives.
49-59: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winNo idempotency guard against duplicate link submissions/retries.
A client retry (e.g., network timeout after a successful response, or a double-click before the button disables) hits this route again and unconditionally inserts a second
crm_meetingsrow for the sameuid+leadId, then re-patches the same Nextcloud event. There's no check for an existing meeting already linked to thisuidbefore inserting.Consider checking for an existing non-deleted
crm_meetingsrow withnextcloudUid = uidbefore inserting, and short-circuiting with the existing meeting id if found.🤖 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/routes/api/calendar/events/`[uid]/link/+server.ts around lines 49 - 59, The link handler in the route for calendar events currently inserts a new meeting every time, so retries or double submits can create duplicate crm_meetings rows for the same uid and leadId. Update the link flow around createMeeting to first look up an existing non-deleted meeting by nextcloudUid = uid, and if one exists, reuse that meeting id and skip inserting again before proceeding with the Nextcloud patch.process/features/calendar/active/ncal-4-event-ui_09-07-26/ncal4-event-ui_REPORT_09-07-26.md (1)
46-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReported test counts don't match the actual test files, and one row is self-contradictory.
- Line 46 claims "AC12 schemas ... 20/20", but
calendar-schemas.spec.tscontains 16it()blocks (14 in the create-schema describe + 2 in the update-schema describe).- Line 47 claims "AC3 merge ... 16/16", but
calendar-merge.spec.tscontains 17it()blocks.- Line 48 claims "AC9+AC10 link ... 12/12" then adds a contradictory parenthetical "(wait: 9 tests total per run — some batched)" —
calendar-link-to-lead.spec.tsactually contains 13it()blocks (10 + 3).These look like leftover/uncorrected placeholder numbers from report generation. Worth fixing so the gate table is trustworthy for future audits.
🤖 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 `@process/features/calendar/active/ncal-4-event-ui_09-07-26/ncal4-event-ui_REPORT_09-07-26.md` around lines 46 - 48, Update the test summary entries in the report so the PASS counts match the actual `it()` totals in the referenced specs: adjust the AC12 schemas, AC3 merge, and AC9+AC10 link rows in the report section. Remove the self-contradictory parenthetical on the `calendar-link-to-lead.spec.ts` row and replace all placeholder totals with the correct numbers derived from `calendar-schemas.spec.ts`, `calendar-merge.spec.ts`, and `calendar-link-to-lead.spec.ts`, keeping the table consistent and trustworthy.process/features/calendar/active/ncal-4-event-ui_09-07-26/ncal-4-event-ui_SPEC_09-07-26.md (1)
139-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSPEC references a
titlefield oncrm_meetingsthat doesn't exist oncreateMeeting().Line 139 and AC8 (lines 189-192) describe the link-to-lead insert as
crm_meetings (leadId, title, startAt from event, ...), but the actualcreateMeeting()contract (per themeetings.tssnippet used elsewhere in this cohort) acceptsleadId, startAt, organizerId, leadOrganizerId, meetingUrl, venue, notes, outcome, attendeeIds— notitle. The implemented route correctly omitstitle, but the SPEC/AC text wasn't updated to reflect this, which could mislead future readers auditing AC8 against the shipped code.Also applies to: 189-192
🤖 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 `@process/features/calendar/active/ncal-4-event-ui_09-07-26/ncal-4-event-ui_SPEC_09-07-26.md` at line 139, Update the SPEC/AC text for the crm_meetings insert to match the actual createMeeting() contract: remove the nonexistent title field from the documented payload and align the link-to-lead insert description with the real parameters used by createMeeting() (leadId, startAt, organizerId, leadOrganizerId, meetingUrl, venue, notes, outcome, attendeeIds). Keep the guidance around updateMeetingNextcloudUid and the lead association, but make sure AC8 and the related insert step no longer imply a title column exists.src/tests/calendar-link-to-lead.spec.ts (1)
195-257: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRollback tests simulate the pattern inline rather than exercising the real route handler.
These tests re-implement the try/catch rollback logic locally instead of importing and invoking the actual
POSThandler from+server.ts. As explicitly noted in the test comments (lines 197-201) and the PLAN's "Test Infra Improvement Notes," this is a known, accepted limitation. However, sincePOSTis a plain async function taking{ locals, request, params }, it could be invoked directly with mockedcreateMeeting/softDeleteMeeting/directPatchEvent(viavi.mock('$lib/server/db/meetings', ...)andvi.mock('$lib/caldav/writer', ...)), similar to how$env/dynamic/privateandfetchare already mocked above. This would catch real divergence between the route implementation and the rollback contract these tests assert.🤖 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/tests/calendar-link-to-lead.spec.ts` around lines 195 - 257, The rollback specs in calendar-link-to-lead.spec.ts are re-implementing the catch/cleanup flow instead of validating the real POST handler, so they can drift from the route logic. Update these tests to invoke the actual POST function from +server.ts with mocked dependencies for createMeeting, softDeleteMeeting, and directPatchEvent (using the existing vi.mock patterns already used for $env/dynamic/private and fetch). Keep the current assertions, but drive them through the real route handler so the rollback contract is exercised end-to-end.src/lib/components/calendar/CalendarGrid.svelte (1)
109-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate team-event chip markup.
The purple team-event
<button>block is duplicated verbatim between the inline and overflow renderers. Extract into a shared{#snippet}to avoid the two copies drifting apart as styling/behavior evolves.♻️ Suggested extraction
{`#snippet` teamEventChip(entry: CalendarEntry)} <button data-entry-type="team-event" data-testid="calendar-entry" class="w-full rounded-[5px] px-1.5 py-0.5 text-left text-[11px] font-medium text-white truncate" style="background-color: `#7c3aed`;" onclick={() => onteameventclick?.(entry)} title={entry.title} > {entry.title} </button> {/snippet}Then replace both
{#ifentry.type === 'team-event'} ... {:else}blocks with{@renderteamEventChip(entry)}.Also applies to: 138-151
🤖 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/lib/components/calendar/CalendarGrid.svelte` around lines 109 - 122, The team-event button markup in CalendarGrid.svelte is duplicated between the inline and overflow renderers, so extract the purple team-event button into a shared snippet (for example, a teamEventChip snippet near the CalendarGrid rendering logic) and render it from both places. Keep the existing data attributes, title, onclick handler, and styling in that single snippet so changes to team-event presentation only need to be made once.src/lib/components/calendar/EventDetailModal.svelte (1)
76-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
$derived.by()instead of$derived(() => {...}).
$derived(expression)evaluatesexpressiononce to produce the derived value; wrapping an arrow function inside$derived(...)makeslinkedLeadIditself a plain function (not the computed id), which is why callsites have to invoke it aslinkedLeadId(). This works today, but it forgoes$derived's dependency tracking/memoization and is a common source of confusion (a future edit that readslinkedLeadIdwithout calling it will silently break).♻️ Suggested fix
- const linkedLeadId = $derived(() => { - if (!event?.url) return null; - const match = event.url.match(/\/leads\/([^/]+)$/); - return match ? match[1] : null; - }); + const linkedLeadId = $derived.by(() => { + if (!event?.url) return null; + const match = event.url.match(/\/leads\/([^/]+)$/); + return match ? match[1] : null; + });And update the two template usages from
linkedLeadId()tolinkedLeadId.🤖 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/lib/components/calendar/EventDetailModal.svelte` around lines 76 - 80, `linkedLeadId` in EventDetailModal.svelte is being created as a plain function because `$derived(() => {...})` is used instead of `$derived.by(...)`; switch the derived declaration for `linkedLeadId` to `$derived.by` so it computes the actual id with dependency tracking, then update the template callsites in the same component to use `linkedLeadId` directly rather than invoking it.
🤖 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/lib/caldav/team-events.ts`:
- Around line 4-6: The CRM filter in team-events is missing the category written
by directPatchEvent(), so converted CRM items can still be treated as team
events and appear twice. Update CRM_CATEGORIES in team-events.ts to include the
crm-meeting value, or make directPatchEvent() write one of the already-filtered
canonical categories if that is the intended source of truth. Keep the filter
and writer aligned so the team event chip logic and CRM-synced event handling
use the same category names.
In `@src/lib/caldav/writer.ts`:
- Around line 186-201: The directPatchEvent flow performs a GET, mutates the ICS
locally, then PUTs it back without any concurrency check, so add conditional
update handling around the existing fetch/PUT path in directPatchEvent. Capture
the resource’s ETag from the GET response and include it in the subsequent PUT
as an If-Match header, or otherwise fail if the tag is missing so stale writes
are rejected. Apply the same conditional update logic to the matching PUT flow
mentioned in the comment so both paths protect against overwriting concurrent
edits.
- Around line 182-187: The CalDAV URL in directPatchEvent is built from a
decoded uid value, so raw string concatenation can alter the requested Nextcloud
resource when path-significant characters are present. Update the icsUrl निर्माण
to pass uid through encodeURIComponent before appending it to
calendarCollectionUrl(), keeping the change localized to directPatchEvent and
preserving the existing authHeader flow.
In `@src/lib/components/calendar/EventFormModal.svelte`:
- Around line 124-132: The all-day reset logic in EventFormModal.svelte is
re-running whenever `start` changes, which overwrites a user-set multi-day `end`
value. Update the `$effect` tied to `allDay`/`start` so it only initializes
`start` and `end` when `allDay` first becomes true, instead of resetting on
every later `start` edit. Use the existing `allDay` state and the date-handling
block in `EventFormModal` to gate the assignment and preserve any custom `end`
date once the user has extended it.
- Around line 49-79: `EventFormModal` edit-mode seeding currently rebuilds `end`
from `event.startAt` and always sets `allDay = false`, which flattens existing
event duration/state. Update the `$effect` in `EventFormModal.svelte` to prefill
from the event’s stored end and all-day values instead of defaulting to start +
1h, and thread `endAt`/`allDay` (or the equivalent fields) through
`CalendarEntry` so those values are available when editing.
In `@src/routes/api/calendar/events/`[uid]/link/+server.ts:
- Around line 67-81: Both empty catch blocks in the calendar link handler are
swallowing failures without any observability, so add real server-side logging
in the `catch` for `softDeleteMeeting` and the `catch` for
`updateMeetingNextcloudUid`. In
`src/routes/api/calendar/events/[uid]/link/+server.ts`, use the existing handler
context to log the rollback failure and the non-fatal UID wiring failure with
enough detail to identify the meeting and uid, while still preserving the
current client-facing behavior. Keep the 404/502 error handling in the main
`try/catch` unchanged, and only augment the two silent failure paths with logs.
- Around line 25-28: The route in the link handler should validate params.uid
before building the ICS URL, since calendarCollectionUrl() is only a base URL
and direct concatenation in the directPatchEvent() call can normalize
dot-segments. Add a strict UID allowlist check at the route boundary in
+server.ts (around linkBodySchema/use of uid) so only expected calendar UID
characters are accepted, and reject any value that could escape the calendar
collection.
In `@src/routes/calendar/`+page.svelte:
- Around line 117-172: The four event handlers in calendar/+page.svelte silently
ignore non-OK fetch responses, so add failure handling in handleCreateEvent,
handleEditEvent, handleDeleteEvent, and handleLinkToLead. After each fetch, keep
the existing saving flag reset but add an else branch that surfaces an error to
the user, reusing the existing linkError-style pattern from EventDetailModal or
a toast, so users get feedback when the API returns validation/auth/service
errors.
- Around line 135-148: The edit/link flow in +page.svelte leaves the detail
modal open and keeps rendering stale selectedEvent data after success. Update
the Edit action handler that sets editOpen to also close detailOpen before
opening the form, and in handleEditEvent and handleLinkToLead refresh or clear
selectedEvent on successful mutation so the detail view does not keep showing
outdated state. Make sure the EventDetailModal/EventFormModal open flags stay
mutually exclusive and that success handlers reconcile the local state after
invalidateAll.
---
Nitpick comments:
In
`@process/features/calendar/active/ncal-4-event-ui_09-07-26/ncal-4-event-ui_SPEC_09-07-26.md`:
- Line 139: Update the SPEC/AC text for the crm_meetings insert to match the
actual createMeeting() contract: remove the nonexistent title field from the
documented payload and align the link-to-lead insert description with the real
parameters used by createMeeting() (leadId, startAt, organizerId,
leadOrganizerId, meetingUrl, venue, notes, outcome, attendeeIds). Keep the
guidance around updateMeetingNextcloudUid and the lead association, but make
sure AC8 and the related insert step no longer imply a title column exists.
In
`@process/features/calendar/active/ncal-4-event-ui_09-07-26/ncal4-event-ui_REPORT_09-07-26.md`:
- Around line 46-48: Update the test summary entries in the report so the PASS
counts match the actual `it()` totals in the referenced specs: adjust the AC12
schemas, AC3 merge, and AC9+AC10 link rows in the report section. Remove the
self-contradictory parenthetical on the `calendar-link-to-lead.spec.ts` row and
replace all placeholder totals with the correct numbers derived from
`calendar-schemas.spec.ts`, `calendar-merge.spec.ts`, and
`calendar-link-to-lead.spec.ts`, keeping the table consistent and trustworthy.
In `@src/lib/caldav/writer.ts`:
- Line 211: The CATEGORIES update in the CalDAV writer currently replaces the
entire property, which can discard existing labels. Review the update logic in
the write path around vevent.updatePropertyWithValue and decide whether
categories should be merged instead of overwritten; if merge is intended,
preserve any existing CATEGORIES values and append the sync category only when
missing. Keep the change localized to the writer flow that sets
options.categories so future call sites with multiple categories remain safe.
In `@src/lib/components/calendar/CalendarGrid.svelte`:
- Around line 109-122: The team-event button markup in CalendarGrid.svelte is
duplicated between the inline and overflow renderers, so extract the purple
team-event button into a shared snippet (for example, a teamEventChip snippet
near the CalendarGrid rendering logic) and render it from both places. Keep the
existing data attributes, title, onclick handler, and styling in that single
snippet so changes to team-event presentation only need to be made once.
In `@src/lib/components/calendar/EventDetailModal.svelte`:
- Around line 76-80: `linkedLeadId` in EventDetailModal.svelte is being created
as a plain function because `$derived(() => {...})` is used instead of
`$derived.by(...)`; switch the derived declaration for `linkedLeadId` to
`$derived.by` so it computes the actual id with dependency tracking, then update
the template callsites in the same component to use `linkedLeadId` directly
rather than invoking it.
In `@src/lib/types/index.ts`:
- Around line 209-227: The current event type definition uses one flat shape
with shared optional fields, which allows invalid field/type combinations to
compile. Refactor the type around a discriminated union keyed by the existing
type field so each variant in src/lib/types/index.ts has only the fields it
supports, with required properties like uid for team-event enforced by
TypeScript. Update the event type aliases/interfaces near the current type
declaration to preserve the documented pairing between type and fields while
removing the unnecessary optional cross-compatibility.
In `@src/lib/zod/schemas.ts`:
- Around line 221-241: The `createCalendarEventSchema` and
`updateCalendarEventSchema` bodies are duplicated and should be consolidated
into a shared base schema. Extract the common fields into a reusable object in
`src/lib/zod/schemas.ts`, then build both schemas from it using the existing
`createCalendarEventSchema` and `updateCalendarEventSchema` symbols so future
field changes stay in sync.
- Around line 229-236: The email validation uses the deprecated chained form in
Zod v4, so update the affected schema fields to use the new standalone email
validator instead of z.string().email(). Look for the email-related entries in
schemas.ts, including the attendees array and the other schema block noted in
the comment, and switch them to the current z.email() pattern while keeping the
same optional/array structure.
In `@src/routes/api/calendar/events/`[uid]/link/+server.ts:
- Around line 25-28: The `linkBodySchema` still uses the deprecated chained UUID
validator on `leadId`; update it to the top-level Zod v4 UUID form to match the
existing `startAt` pattern. Make the change in the schema definition itself, and
verify the stricter `z.uuid()` validation still accepts the UUID values this
endpoint currently receives.
- Around line 49-59: The link handler in the route for calendar events currently
inserts a new meeting every time, so retries or double submits can create
duplicate crm_meetings rows for the same uid and leadId. Update the link flow
around createMeeting to first look up an existing non-deleted meeting by
nextcloudUid = uid, and if one exists, reuse that meeting id and skip inserting
again before proceeding with the Nextcloud patch.
In `@src/routes/calendar/`+page.server.ts:
- Around line 92-100: The team events fallback in +page.server.ts is fine, but
the catch around fetchCalendarReport/parseIcsToEvents/mapTeamEvents is
swallowing failures entirely and hiding useful diagnostics. Update that
try/catch to log the caught error server-side with enough context before leaving
teamEventEntries empty, so failures in the Nextcloud fetch/parsing path can be
distinguished from a legitimate “no events” result.
In `@src/tests/calendar-link-to-lead.spec.ts`:
- Around line 195-257: The rollback specs in calendar-link-to-lead.spec.ts are
re-implementing the catch/cleanup flow instead of validating the real POST
handler, so they can drift from the route logic. Update these tests to invoke
the actual POST function from +server.ts with mocked dependencies for
createMeeting, softDeleteMeeting, and directPatchEvent (using the existing
vi.mock patterns already used for $env/dynamic/private and fetch). Keep the
current assertions, but drive them through the real route handler so the
rollback contract is exercised end-to-end.
🪄 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: 4a16383d-1ef8-454d-898b-2c5eaaa60f9d
📒 Files selected for processing (17)
e2e/ncal4-event-ui.e2e.tsprocess/features/calendar/active/ncal-4-event-ui_09-07-26/ncal-4-event-ui_PLAN_09-07-26.mdprocess/features/calendar/active/ncal-4-event-ui_09-07-26/ncal-4-event-ui_SPEC_09-07-26.mdprocess/features/calendar/active/ncal-4-event-ui_09-07-26/ncal4-event-ui_REPORT_09-07-26.mdsrc/lib/caldav/team-events.tssrc/lib/caldav/writer.tssrc/lib/components/calendar/CalendarGrid.sveltesrc/lib/components/calendar/EventDetailModal.sveltesrc/lib/components/calendar/EventFormModal.sveltesrc/lib/types/index.tssrc/lib/zod/schemas.tssrc/routes/api/calendar/events/[uid]/link/+server.tssrc/routes/calendar/+page.server.tssrc/routes/calendar/+page.sveltesrc/tests/calendar-link-to-lead.spec.tssrc/tests/calendar-merge.spec.tssrc/tests/calendar-schemas.spec.ts
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
hdmGOAT
added a commit
that referenced
this pull request
Jul 9, 2026
feat(calendar): NCAL-4 — team-event grid, event modals, link-to-lead Co-Authored-By: Claude Sonnet 4.6 <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
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.
Summary
#7c3aed) on the calendar grid alongside CRM meetings, follow-ups, and milestones.mapTeamEventsfilters out CRM categories (meeting,golive,go-live,eventstart,event-start) and passes everything else through (including events with no CATEGORIES set).POST /api/calendar/events/[uid]/linkcreates a CRM meeting tied to a lead, patchesCATEGORIES:meeting+CRM-HREF:/leads/<id>onto the Nextcloud ICS via direct CalDAV PUT (bypassing n8n which silently drops CATEGORIES), and links the meeting to the UID. On CalDAV patch failure the meeting is rolled back viasoftDeleteMeeting.AbortSignal.timeout(5000)and gracefully degrades to[]on error, merging team events into the entries array.Test plan
bun run test:unit:ci— 583 pass (16 calendar-schemas, 17 calendar-merge, 13 calendar-link-to-lead)bun run check— 0 errorsvite build— clean/calendar; legend checkbox hides/shows theme2e/ncal4-event-ui.e2e.tsself-skip pending shared Playwright auth fixture (pre-accepted known-gap — same root cause as NCAL-1/2/3)Known gaps (pre-accepted)
directPatchEventlive round-trip test requires a live Nextcloud env (same class as NCAL-1/2 known-gaps)🤖 Generated with Claude Code
Summary by CodeRabbit