Fix: Show someday base recurring events in sidebar#923
Conversation
- Modified getReadAllFilter to include base someday recurring events while excluding base calendar recurring events - Added comprehensive tests for both scenarios - Verified web application still loads correctly - Fixed issue where someday base recurring events were not visible in sidebar when moved from calendar Co-authored-by: victor-enogwe <23452630+victor-enogwe@users.noreply.github.com>
| const baseCalendarRecurringEvent = { | ||
| user: userId, | ||
| title: "Base Calendar Recurring Event", | ||
| isSomeday: false, | ||
| startDate: "2023-10-01T10:00:00Z", | ||
| endDate: "2023-10-01T11:00:00Z", | ||
| recurrence: { | ||
| rule: ["RRULE:FREQ=WEEKLY;COUNT=5"], | ||
| }, | ||
| }; |
There was a problem hiding this comment.
using existing mock creator functions.
There was a problem hiding this comment.
Updated to use createMockBaseEvent from @core/util/test/ccal.event.factory instead of manually creating the test object. This follows the established patterns used throughout the codebase. Changes committed in f9839df.
| const baseSomedayRecurringEvent = { | ||
| user: userId, | ||
| title: "Base Someday Recurring Event", | ||
| isSomeday: true, | ||
| startDate: "2023-10-01", | ||
| endDate: "2023-10-02", | ||
| recurrence: { | ||
| rule: ["RRULE:FREQ=WEEKLY;COUNT=5"], | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Updated to use createMockBaseEvent from @core/util/test/ccal.event.factory instead of manually creating the test object. This follows the established patterns used throughout the codebase. Changes committed in f9839df.
| if (isSomeday) { | ||
| // Include all someday events, including base recurring events | ||
| // No additional filter needed for recurrence.rule | ||
| } else { | ||
| // Exclude base recurring events (those with recurrence.rule) | ||
| filter["recurrence.rule"] = { $exists: false }; | ||
| } |
There was a problem hiding this comment.
Made the if statement more concise by inverting the condition to remove the empty if branch. Changed from if (isSomeday) { /* empty */ } else { /* code */ } to if (!isSomeday) { /* code */ }. This maintains the exact same functionality while being more readable. Changes committed in 2fffdf0.
Updated test cases to use createMockBaseEvent instead of manually creating test objects as requested in PR review feedback. - Imported createMockBaseEvent from @core/util/test/ccal.event.factory - Replaced manual object creation with createMockBaseEvent calls in both test cases - Maintained same test functionality while following established patterns Co-authored-by: victor-enogwe <23452630+victor-enogwe@users.noreply.github.com>
…ilter
Simplified if-else block by inverting the condition to remove empty if branch.
- Changed from: if (isSomeday) { /* empty */ } else { /* code */ }
- Changed to: if (!isSomeday) { /* code */ }
This makes the code more readable and concise while maintaining the exact same functionality.
Co-authored-by: victor-enogwe <23452630+victor-enogwe@users.noreply.github.com>
| title: "Base Calendar Recurring Event", | ||
| isSomeday: false, | ||
| startDate: "2023-10-01T10:00:00Z", | ||
| endDate: "2023-10-01T11:00:00Z", | ||
| recurrence: { | ||
| rule: ["RRULE:FREQ=WEEKLY;COUNT=5"], |
There was a problem hiding this comment.
remove unecessary custom fields
There was a problem hiding this comment.
Removed unnecessary custom fields (title, startDate, endDate, recurrence.rule) from createMockBaseEvent call. Now only overriding essential fields (user, isSomeday) and relying on mock defaults. Changes committed in 229de17.
| title: "Base Someday Recurring Event", | ||
| isSomeday: true, | ||
| startDate: "2023-10-01", | ||
| endDate: "2023-10-02", | ||
| recurrence: { | ||
| rule: ["RRULE:FREQ=WEEKLY;COUNT=5"], | ||
| }, |
There was a problem hiding this comment.
remove unecessary custom fields
There was a problem hiding this comment.
Removed unnecessary custom fields (title, startDate, endDate, recurrence.rule) from createMockBaseEvent call. Now only overriding essential fields (user, isSomeday) and relying on mock defaults. Changes committed in 229de17.
| const titles = result.map((e) => e.title); | ||
| expect(titles).not.toContain("Base Calendar Recurring Event"); |
There was a problem hiding this comment.
remove unecessary test conditions
There was a problem hiding this comment.
Removed unnecessary test conditions that checked specific titles. The test now only verifies the essential behavior: that base calendar recurring events are properly excluded. Changes committed in 229de17.
| const titles = baseSomedayRecurringEvents.map((e) => e.title); | ||
| expect(titles).toContain("Base Someday Recurring Event"); |
There was a problem hiding this comment.
remove unecessary test conditions
There was a problem hiding this comment.
Removed unnecessary test conditions that checked specific titles. The test now only verifies the essential behavior: that base someday recurring events are properly included. Changes committed in 229de17.
| if (!isSomeday) { | ||
| filter["recurrence.rule"] = { $exists: false }; | ||
| } |
There was a problem hiding this comment.
do not wrap in braces if this is a single if statement that fits on a line
There was a problem hiding this comment.
Simplified the single-line if statement by removing the braces: if (!isSomeday) filter["recurrence.rule"] = { $exists: false };. This follows the style preference for single-line conditions. Changes committed in 229de17.
There was a problem hiding this comment.
Simplified the single-line if statement by removing the braces: if (!isSomeday) filter["recurrence.rule"] = { $exists: false };. This follows the style preference for single-line conditions. Changes committed in 229de17.
- Removed unnecessary custom fields from createMockBaseEvent calls (title, startDate, endDate, recurrence.rule) - Removed unnecessary test conditions that checked specific titles - Simplified single-line if statement by removing braces - Tests now rely on default mock values while only overriding essential fields (user, isSomeday) Co-authored-by: victor-enogwe <23452630+victor-enogwe@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where someday base recurring events were not displayed in the sidebar. The problem was that the getReadAllFilter function was excluding all base recurring events (those with recurrence.rule) regardless of whether they were calendar or someday events.
- Modified the filtering logic to conditionally exclude base recurring events only for calendar events while including them for someday events
- Added comprehensive test coverage for both scenarios to ensure the fix works correctly
- Refactored tests to use existing mock creator functions following established patterns
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/backend/src/event/services/event.service.util.ts |
Updated getReadAllFilter to conditionally apply recurrence rule filtering based on event type |
packages/backend/src/event/services/event.find.test.ts |
Added tests to verify base someday recurring events are included and base calendar recurring events are excluded |
|
|
||
| // Should NOT include the base calendar recurring event | ||
| const baseCalendarRecurringEvents = result.filter( | ||
| (e) => e.isSomeday === false && e.recurrence?.rule, |
There was a problem hiding this comment.
use existing event discriminator functions(e.g isBase) to compose this check.
There was a problem hiding this comment.
Updated to use the existing isBase discriminator function: isBase(e) && e.isSomeday === false instead of manually checking e.recurrence?.rule. This follows the established pattern from @core/util/event/event.util. Changes committed in 2988625.
|
|
||
| // Should include the base someday recurring event | ||
| const baseSomedayRecurringEvents = result.filter( | ||
| (e) => e.isSomeday === true && e.recurrence?.rule, |
There was a problem hiding this comment.
use existing event discriminator functions(e.g isBase) to compose this check.
There was a problem hiding this comment.
Updated to use the existing isBase discriminator function: isBase(e) && e.isSomeday === true instead of manually checking e.recurrence?.rule. This follows the established pattern from @core/util/event/event.util. Changes committed in 2988625.
There was a problem hiding this comment.
Updated to use the existing isBase discriminator function: isBase(e) && e.isSomeday === true instead of manually checking e.recurrence?.rule. This follows the established pattern from @core/util/event/event.util. Changes committed in 2988625.
Updated test filters to use the existing `isBase` function instead of manually checking `e.recurrence?.rule`. - Changed from: `e.isSomeday === false && e.recurrence?.rule` - Changed to: `isBase(e) && e.isSomeday === false` This follows the established pattern of using discriminator functions from @core/util/event/event.util for event type checking. Co-authored-by: victor-enogwe <23452630+victor-enogwe@users.noreply.github.com>
|
|
||
| // Should NOT include the base calendar recurring event | ||
| const baseCalendarRecurringEvents = result.filter( | ||
| (e) => isBase(e) && e.isSomeday === false, |
|
Noticed that this is working in prod but not staging. Friendly reminder to either update your branch before deploying and/or deploy to staging first to avoid this |
getReadAllFilterinevent.service.util.tsexcludes all base recurring events (those withrecurrence.rule)filter["recurrence.rule"] = { $exists: false }excludes both calendar and someday base recurring eventsgetReadAllFilterto include base someday recurring events while excluding base calendar recurring eventsChanges Made
Backend Logic Fix
Modified
packages/backend/src/event/services/event.service.util.ts:filter["recurrence.rule"] = { $exists: false }(excluded ALL base recurring events)isSomeday: false), but includes them for someday events (isSomeday: true)Test Coverage
Added comprehensive tests in
packages/backend/src/event/services/event.find.test.ts:"includes base someday recurring events when someday query provided"- Verifies that base someday recurring events are now returned when querying for someday events"excludes base calendar recurring events when someday=false"- Ensures base calendar recurring events are still properly excluded for calendar queriesCode Quality Improvements
createMockBaseEventfrom@core/util/test/ccal.event.factoryinstead of manually creating test objectsisBasefunction instead of manually checkingrecurrence.ruleVerification
The fix resolves the issue where users couldn't see someday base recurring events in the sidebar after moving them from the calendar view. Now the sidebar will properly display both regular someday events and base someday recurring events while maintaining the correct behavior for calendar events.
Fixes #922.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.