:sparkles feat(sub-calendars): use event schema WIP#1163
:sparkles feat(sub-calendars): use event schema WIP#1163victor-enogwe wants to merge 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the codebase to use a new event schema that supports sub-calendars, moving away from user-based event organization to a calendar-based approach. The changes consolidate event-related types and update import paths to reflect the new schema structure.
Key changes:
- Migrated
Categories_Eventand related enums from@core/types/event.typesto@web/common/types/web.event.types - Updated event date handling to use native
Dateobjects instead of string-based date formats - Removed several deprecated types and utility functions related to the old event schema
- Updated import paths across the codebase to reflect the new type locations
Reviewed Changes
Copilot reviewed 168 out of 168 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/common/types/web.event.types.ts | Defined Categories_Event enum and updated schema definitions for web events |
| packages/core/src/types/event.types.ts | Removed Categories_Event enum and consolidated event schemas with calendar support |
| packages/web/src/views/Forms/EventForm/DateControlsSection/RecurrenceSection/useRecurrence/useRecurrence.ts | Updated to use Date objects instead of string-based dates |
| packages/core/src/util/event/event.util.ts | Removed deprecated date parsing functions and updated event categorization logic |
| packages/core/src/mappers/map.event.ts | Simplified mapper to remove provider data methods and consolidate metadata handling |
| packages/backend/src/user/services/user.service.ts | Updated user service methods to use ObjectId instead of strings |
| packages/scripts/src/migrations/*.ts | Updated migration scripts to use new event schema |
| Event_Core, | ||
| Schema_Event, | ||
| } from "@core/types/event.types"; | ||
| import { Event_Core, Schema_Event } from "@core/types/event.types"; |
There was a problem hiding this comment.
The function references EventSchema but it's not imported. This will cause a runtime error. Import EventSchema from @core/types/event.types.
| import { Event_Core, Schema_Event } from "@core/types/event.types"; | |
| import { Event_Core, Schema_Event, EventSchema } from "@core/types/event.types"; |
| const dbUser = await mongoService.user.findOne({ _id }); | ||
| const events = this.#generateEvents(userId); |
There was a problem hiding this comment.
Variable userId is used but never defined. It should be _id.toString() or the result should be assigned to a userId variable after line 138.
| ); | ||
|
|
||
| await userService.saveTimeFor("lastLoggedInAt", userId); | ||
| await userService.updateLastLoggedInTime("lastLoggedInAt", userId); |
There was a problem hiding this comment.
The method signature appears incorrect. Based on the changes in user.service.ts, updateLastLoggedInTime now takes only one parameter (userId as ObjectId), but here it's called with two string parameters.
| await userService.updateLastLoggedInTime("lastLoggedInAt", userId); | |
| await userService.updateLastLoggedInTime(mongoService.objectId(userId)); |
| for (const user of users) { | ||
| const userId = user?._id.toString(); | ||
| const summary = await userService.deleteCompassDataForUser(userId); | ||
| const summary = await userService.deleteCompassDataForUser(user._id); |
There was a problem hiding this comment.
The user._id field is already an ObjectId from the database query, but the iteration variable user shadows the outer scope user parameter (which is a string). This creates confusion. Consider renaming the loop variable to dbUser for clarity.
9bafc94 to
085f0ae
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 184 out of 184 changed files in this pull request and generated 29 comments.
Comments suppressed due to low confidence (1)
packages/backend/src/sync/services/sync/tests/compass.sync.processor.all-event.test.ts:1
- More references to removed properties:
gRecurringEventId,gEventId, anduserthat need to be updated to use the new metadata structure (metadata.recurringEventId,metadata.id) and calendar-based associations.
import { ObjectId } from "mongodb";
|
|
||
| const userId = (await this.#findUserOrThrow(user!))._id.toString(); | ||
| const dbUser = await mongoService.user.findOne({ _id }); | ||
| const events = this.#generateEvents(userId); |
There was a problem hiding this comment.
Variable userId is used on line 139 but is no longer defined after the changes. The code now uses _id and dbUser, but userId needs to be extracted from one of these. This will cause a ReferenceError.
| const events = this.#generateEvents(userId); | |
| const events = this.#generateEvents(dbUser._id.toString()); |
| summary: "New Standalone Event", | ||
| }); | ||
| const processor = new GcalSyncProcessor(user._id.toString()); | ||
| const processor = new GcalEventsSyncProcessor(user._id.toString()); |
There was a problem hiding this comment.
The constructor signature changed to accept a calendar object instead of a user ID string. This call should pass a calendar object but is still passing user._id.toString(). This will cause a type error.
|
|
||
| /* Act */ | ||
| const processor = new GcalSyncProcessor(user._id.toString()); | ||
| const processor = new GcalEventsSyncProcessor(user._id.toString()); |
There was a problem hiding this comment.
The constructor signature changed to accept a calendar object, but this is still passing a user ID string. This needs to be updated to pass a calendar object consistent with the new architecture.
| const processor = new GcalEventsSyncProcessor(user._id.toString()); | |
| const processor = new GcalEventsSyncProcessor(calendar); |
|
|
||
| /* Act */ | ||
| const processor = new GcalSyncProcessor(user._id.toString()); | ||
| const processor = new GcalEventsSyncProcessor(user._id.toString()); |
There was a problem hiding this comment.
Multiple instances where GcalEventsSyncProcessor is constructed with a user ID string instead of a calendar object. All these need to be updated to match the new constructor signature.
| }; | ||
|
|
||
| const processor = new GcalSyncProcessor(user._id.toString()); | ||
| const processor = new GcalEventsSyncProcessor(user._id.toString()); |
There was a problem hiding this comment.
Multiple instances where GcalEventsSyncProcessor is constructed with a user ID string instead of a calendar object. All these need to be updated to match the new constructor signature.
| const processor = new GcalEventsSyncProcessor(user._id.toString()); | |
| const calendar = await CalendarDriver.getByUserId(user._id); | |
| const processor = new GcalEventsSyncProcessor(calendar); |
| _getGcal(user, instanceUpdate!.gRecurringEventId!), | ||
| EventDriver.getGCalEvent( | ||
| user, | ||
| instanceUpdate!.gRecurringEventId!, |
There was a problem hiding this comment.
More references to removed properties: gRecurringEventId, gEventId, and user that need to be updated to use the new metadata structure (metadata.recurringEventId, metadata.id) and calendar-based associations.
| newInstances.map((instance) => | ||
| expect(_getGcal(user, instance.gEventId!)).rejects.toThrow( | ||
| expect( | ||
| EventDriver.getGCalEvent(user, instance.gEventId!), |
There was a problem hiding this comment.
More references to removed properties: gRecurringEventId, gEventId, and user that need to be updated to use the new metadata structure (metadata.recurringEventId, metadata.id) and calendar-based associations.
| gcalInstances.map(() => | ||
| expect.objectContaining({ | ||
| recurringEventId: event!.gEventId, | ||
| recurringEventId: event.metadata?.id, |
There was a problem hiding this comment.
More references to removed properties: gRecurringEventId, gEventId, and user that need to be updated to use the new metadata structure (metadata.recurringEventId, metadata.id) and calendar-based associations.
| recurringEventId: event.metadata?.id, | |
| recurringEventId: event.metadata?.recurringEventId, |
| _getGcal(updatedPayload.user!, updatedPayload.gEventId!), | ||
| EventDriver.getGCalEvent( | ||
| updatedPayload.user!, | ||
| updatedPayload.gEventId!, |
There was a problem hiding this comment.
More references to removed properties: gRecurringEventId, gEventId, and user that need to be updated to use the new metadata structure (metadata.recurringEventId, metadata.id) and calendar-based associations.
| instances.map((instance) => | ||
| expect( | ||
| _getGcal(instance.user!, instance.gEventId!), | ||
| EventDriver.getGCalEvent(instance.user!, instance.gEventId!), |
There was a problem hiding this comment.
More references to removed properties: gRecurringEventId, gEventId, and user that need to be updated to use the new metadata structure (metadata.recurringEventId, metadata.id) and calendar-based associations.
085f0ae to
d0b6831
Compare
d0b6831 to
36abb72
Compare
| const events = await getEventsInDb({ calendar: calendar._id, isSomeday }); | ||
| const typeFilter = type === "ALLDAY" ? isAllDay : isInstance; | ||
| const instanceEvents = events.filter(isInstance).filter(typeFilter); | ||
| console.log("Events:", events.filter(isInstance)); |
There was a problem hiding this comment.
Remove debug console.log statement from production code
| console.log("Events:", events.filter(isInstance)); |
| const updateChanges = await CompassSyncProcessor.processEvents([ | ||
| { | ||
| payload: updatedPayload as CompassThisEvent["payload"], | ||
| payload, |
There was a problem hiding this comment.
The payload should be updatedPayload instead of the original payload to match the test's intent of updating the priority field
| payload, | |
| payload: updatedPayload, |
| const updateChanges = await CompassSyncProcessor.processEvents([ | ||
| { | ||
| payload: updatedPayload as CompassThisEvent["payload"], | ||
| payload, |
There was a problem hiding this comment.
The payload should be updatedPayload instead of the original payload to match the test's intent of updating the endDate field
| payload, | |
| payload: updatedPayload, |
| const calendar = await CalendarDriver.getRandomUserCalendar(_user._id); | ||
| const isSomeday = false; | ||
| const recurrence = { rule: ["RRULE:FREQ=WEEKLY;COUNT=10"] }; | ||
| const payload = createMockBaseEvent({ isSomeday, user, recurrence }); |
There was a problem hiding this comment.
The parameter should be calendar: calendar._id instead of user to match the updated API signature
| const payload = createMockBaseEvent({ isSomeday, user, recurrence }); | |
| const payload = createMockBaseEvent({ isSomeday, calendar: calendar._id, recurrence }); |
| // check that event is deleted in db | ||
| await expect( | ||
| eventService.readById(user, deletedInstanceId), | ||
| eventService.readById(calendar, deletedInstanceId), |
There was a problem hiding this comment.
The first parameter should be calendar._id instead of calendar to match the service's expected signature
| eventService.readById(calendar, deletedInstanceId), | |
| eventService.readById(calendar._id, deletedInstanceId), |
| const updateChanges = await CompassSyncProcessor.processEvents([ | ||
| { | ||
| payload: updatedPayload as CompassAllEvents["payload"], | ||
| user: _user._id, |
There was a problem hiding this comment.
This field should not be present in the event payload as it's now handled through the calendar parameter
| user: _user._id, |
| const status = CompassEventStatus.CONFIRMED; | ||
| const _user = await AuthDriver.googleSignup(); | ||
| const calendar = await CalendarDriver.getRandomUserCalendar(user._id); | ||
| const status = EventStatus.CONFIRMED; |
There was a problem hiding this comment.
The status variable is declared but not used in the test. Either use it or remove the declaration
| const status = EventStatus.CONFIRMED; |
| const user = _user._id.toString(); | ||
| const status = CompassEventStatus.CONFIRMED; | ||
| const _user = await AuthDriver.googleSignup(); | ||
| const calendar = await CalendarDriver.getRandomUserCalendar(user._id); |
There was a problem hiding this comment.
The variable should be _user._id instead of user._id to match the declared variable name
| const calendar = await CalendarDriver.getRandomUserCalendar(user._id); | |
| const calendar = await CalendarDriver.getRandomUserCalendar(_user._id); |
| const calendar = await CalendarDriver.getRandomUserCalendar(user._id); | ||
| const status = EventStatus.CONFIRMED; | ||
| const recurrence = { rule: ["RRULE:FREQ=WEEKLY;COUNT=10"] }; | ||
| const payload = createMockBaseEvent({ recurrence, user }); |
There was a problem hiding this comment.
The parameter should be calendar: calendar._id instead of user to match the updated API signature
| const calendar = await CalendarDriver.getRandomUserCalendar(user._id); | |
| const status = EventStatus.CONFIRMED; | |
| const recurrence = { rule: ["RRULE:FREQ=WEEKLY;COUNT=10"] }; | |
| const payload = createMockBaseEvent({ recurrence, user }); | |
| const calendar = await CalendarDriver.getRandomUserCalendar(_user._id); | |
| const status = EventStatus.CONFIRMED; | |
| const recurrence = { rule: ["RRULE:FREQ=WEEKLY;COUNT=10"] }; | |
| const payload = createMockBaseEvent({ recurrence, calendar: calendar._id }); |
36abb72 to
d27d482
Compare
d27d482 to
20e53cb
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 103 out of 256 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/backend/src/sync/services/sync/tests/compass-sync-processor-this-event/instance.test.ts:1
- Debug console.log statement should be removed from production code. This appears to be leftover debug code that should be cleaned up.
import { faker } from "@faker-js/faker";
| ` | ||
| ImportAllEvents completed for calendar(${calendar._id.toString()}). |
There was a problem hiding this comment.
[nitpick] The template literal has an unnecessary leading newline character. The backtick should be placed directly after the opening parenthesis for better code formatting.
| ` | |
| ImportAllEvents completed for calendar(${calendar._id.toString()}). | |
| `ImportAllEvents completed for calendar(${calendar._id.toString()}). |
|
Closing PR until we pick it up again. |
What does this PR do?
This PR migrates the codebase to use the new event's schema with sub-calendar support.
Use Case
closes #1135