Skip to content

Conversation

@corypride
Copy link
Contributor

@corypride corypride commented Oct 1, 2025

Description

Implemented automatic synchronization of class start_dt and end_dt fields when events are rescheduled or cancelled beyond the original class date boundaries. This fixes calendar navigation issues where rescheduled events (like moving Thanksgiving classes into December) were inaccessible because the parent class dates didn't reflect the actual schedule.

Additionally fixed critical bugs related to date discrepancies between dashboard and calendar views, and issues with restored events not properly reverting dates.

Key Changes Made

1. Dynamic RRULE UNTIL Calculation

File: backend/src/database/class_events.go
Function: calculateOriginalEndDate()

  • Before: Had hardcoded logic for specific date scenarios
  • After: Implemented dynamic algorithm that works for any RRULE pattern
  • Improvement: Now calculates the actual last occurrence instead of using hardcoded dates

2. Enhanced Date Boundary Synchronization

Functions Added/Updated:

  • syncClassDateBoundaries() - Recalculates actual class boundaries
  • calculateClassBoundariesWithContext() - Handles restore scenarios
  • updateClassDateBoundaries() - Updates only class date fields
  • updateRRuleUntilDates() - Updates RRULE UNTIL parameters

3. Multiple Event Series Support

  • Fixed: Classes with multiple event series now aggregate across ALL series (not just the first one)
  • Implementation: Changed from .First() to .Find() and loop through all events
  • Result: More accurate boundary calculations for complex class schedules

4. Improved Timezone Handling

  • Added: Proper time truncation to avoid timezone issues
  • Implementation: Truncate(24 * time.Hour) for date-only fields
  • Benefit: Consistent date handling across different timezones

5. Enhanced Transaction Integration

  • CreateOverrideEvents() - Syncs after creating/updating overrides
  • DeleteOverrideEvent() - Syncs after deleting overrides to revert boundaries
  • Result: Automatic boundary updates on all override operations

Bug Fixes Resolved

1. Date Discrepancy Issue

  • Problem: Dashboard and calendar views showed different end dates (Nov 27 vs Nov 28)
  • Root Cause: Different data sources and RRULE UNTIL calculation issues
  • Solution: Dynamic RRULE UNTIL calculation with proper occurrence handling

2. Restore Events Bug

  • Problem: Cancelling first event moved start_dt forward, but restoring didn't move it back
  • Root Cause: Circular dependency using already-modified dates
  • Solution: Use rRule.OrigOptions.Dtstart as source of truth

3. Multiple Event Series Bug

  • Problem: Classes with multiple event series only considered the first series
  • Root Cause: Using .First() instead of .Find()
  • Solution: Aggregate dates across all event series

Technical Implementation Details

Core Algorithm Flow

  1. Event Override Operation → Trigger boundary sync
  2. Boundary Calculation → Aggregate all occurrences across all series
  3. RRULE Update → Sync UNTIL parameter with actual last occurrence
  4. Class Update → Update start_dt/end_dt to match reality

Key Constants

  • MaxYearsForInfiniteRRule = 5 - Prevents infinite pattern issues
  • ClassEndDateBufferMonths = 3 - Allows reasonable override extensions

Error Handling

  • Graceful handling of malformed RRULEs with warning logs
  • Fallback to reasonable date ranges when calculations fail
  • Maintains backward compatibility

Testing Coverage

All integration tests pass, verifying:

  • ✅ Reschedule later extends end date correctly
  • ✅ Cancel/restore functionality works properly
  • ✅ Multiple event series aggregation
  • ✅ RRULE UNTIL synchronization
  • ✅ Date boundary calculations
  • ✅ All 20+ integration test suites (0.816s execution time)

Known Limitations

  1. Manual Date Edits: Class start_dt/end_dt are now computed fields - manual edits through Edit Class form are ignored. To change class dates, admins should reschedule actual events instead.

  2. Initial Creation: Sync only triggers on override operations. Initial event creation with mismatched dates won't auto-correct until first override is created (minor UX issue).

Future Enhancements Considered

  • Adding sync to CreateNewEvent for initial date correction (kept out of scope)

This implementation provides a robust, maintainable solution that resolves the core date synchronization issues while maintaining full backward compatibility and comprehensive test coverage.

Screenshot(s)

RRuleDateSync.mp4

@corypride corypride self-assigned this Oct 1, 2025
@corypride corypride requested a review from a team as a code owner October 1, 2025 16:13
@corypride corypride requested review from calisio and removed request for a team October 1, 2025 16:13
Copy link
Contributor

@jtucholski jtucholski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@corypride this works pretty well, I noticed one issue:

  1. I set up a recurring class 7 days per wk on 9/1 through 9/30 so the start_dt was 9/1/25
  2. I cancelled the first class. The start_dt was updated to 9/2/25.
  3. I restored the first class. The start_dt was still 9/2/25.

I would have thought the class start_dt goes back to 9/1/25 again.

Also, are you able to include tests that show that the class start_dt and end_dt update when overides are added/removed? We have some existing RRule tests in place so we can probably add it to those?

@calisio calisio removed their request for review October 3, 2025 19:34
Copy link
Member

@CK-7vn CK-7vn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just remove/think about those comments, and it works really well as long as we don't hit a class with multiple events, once that's all fixed it'll be great!

@corypride corypride requested review from CK-7vn and jtucholski October 8, 2025 17:16
Copy link
Contributor

@jtucholski jtucholski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments but looks and works great!

Copy link
Member

@CK-7vn CK-7vn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite literally just the same stuff Josh said. Id add that comment so we don't forget to refactor it, add the UTC and make that constant, works great man!

@carddev81 carddev81 requested review from carddev81 and removed request for jtucholski October 15, 2025 13:44
Copy link
Contributor

@carddev81 carddev81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@corypride code looks good, but there are a couple functionality issues when rescheduling the event and restoring that event:

When creating an override event as described in the asana ticket (Nov 1 - 30; override last scheduled event to be December 2). Duplicate events are created. Also when restoring that event, the series is still ending on December 2. See video:

2025-10-15.09-22-57.mp4

@corypride corypride force-pushed the cpride/ticket_id478_sync_class_start_dt_and_end_dt branch from a4ea872 to 60bc28a Compare October 20, 2025 03:21
@corypride corypride requested a review from carddev81 October 20, 2025 04:34
Copy link
Member

@CK-7vn CK-7vn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple easy things, it works and tests great though!


originalEndRange := class.StartDt.AddDate(0, 3, 0)
if class.EndDt != nil {
monthsDiff := int(class.EndDt.Sub(class.StartDt).Hours() / (24 * 30))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are assuming that all months have 30 days here, this will cause some issues. Example fix:

startYear, startMonth, _ := class.StartDt.Date()
endYear, endMonth, _ := class.EndDt.Date()
monthsDiff := (endYear-startYear)*12 + int(endMonth-startMonth)


func (db *DB) calculateOriginalEndDate(trans *gorm.DB, classID uint) (time.Time, error) {
var events []models.ProgramClassEvent
if err := trans.Where("class_id = ?", classID).Find(&events).Error; err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should preload Overrides here since we are calling calculateClassBoundariesWithContext(), since both of these methods are called in the same transaction it could cause stale data.

newOptions.Until = newUntil

newRule, err := rrule.NewRRule(newOptions)
if err == nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should bubble up the error from this, because if we ignore it some events might not get their UNTIL updated, and we won't know why. I don't mind the if err == nil logic, but we should probably and an else just in case.

@CK-7vn CK-7vn self-requested a review October 20, 2025 16:09
@corypride corypride force-pushed the cpride/ticket_id478_sync_class_start_dt_and_end_dt branch from ff3c352 to de6fc5c Compare October 20, 2025 19:00
@carddev81
Copy link
Contributor

@corypride I'm still experiencing the problem from before of duplicating the data. Also I noticed that the days leading up to the changed end date is also being scheduled rather than just the one event. We can discuss the issue and work on it together, this seems like a weird issue that we may need to work out together. Let me know

image

@calisio calisio requested review from calisio and removed request for CK-7vn October 22, 2025 17:49
@calisio
Copy link
Contributor

calisio commented Oct 22, 2025

Please let me know when this PR is ready for review, and I will look over it. (waiting because I know you both had a work session today about this).

@corypride corypride force-pushed the cpride/ticket_id478_sync_class_start_dt_and_end_dt branch from de6fc5c to 2af8772 Compare October 23, 2025 04:24
@corypride corypride force-pushed the cpride/ticket_id478_sync_class_start_dt_and_end_dt branch from 2af8772 to 8eed054 Compare October 23, 2025 04:58
Copy link
Contributor

@carddev81 carddev81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just have to fix the javascript logic for determining the next scheduled date

}

func (db *DB) syncClassDateBoundaries(trans *gorm.DB, classID uint) error {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a nit: remove blank line

updates["end_dt"] = computedEnd
}
} else {
if (!hasOverrides && !ruleUntil.IsZero() && !ruleUntil.Equal(*class.EndDt)) || (!ruleUntil.IsZero() && ruleUntil.After(*class.EndDt)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this up into the else changing it to else if

type ProgramClassEvent struct {
DatabaseFields
ClassID uint `json:"class_id" gorm:"not null" validate:"required"`
ClassID uint `json:"class_id" gorm:"not null"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious why validate:"required" was removed? Can this be added back?

);

function getNextOccurrenceDateAsStr(): string {
const now = new Date();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rework to calculate the next scheduled date. You will have to check overrides.

@corypride corypride requested a review from carddev81 October 24, 2025 17:13
Copy link
Contributor

@carddev81 carddev81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@corypride Looks great!! Tests PASSED!!

Copy link
Contributor

@calisio calisio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will approve this PR for now, pending that one comment about a large chunk of tests being commented out but still in the code base. I would be curious to hear from the field if canceling a class would move back or forward a start date. I would assume that even if the first class is cancelled, then the start date would remain on the day of the cancelled first class. I think that the only time the start and end dates should change are if there are events that are NOT CANCELLED taking place before or after the start and end date. I will post in slack about this concern.

require.True(t, updatedClass.EndDt.After(*createdClass.EndDt), "End date should be later than original November 30")
}

// // Test that deleting event overrides properly restores class boundaries
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are all of these lines commented out? should we delete them?

@corypride corypride force-pushed the cpride/ticket_id478_sync_class_start_dt_and_end_dt branch from 73bb5d0 to 43a44b1 Compare November 3, 2025 17:24
@carddev81 carddev81 merged commit d2ad1d9 into main Nov 7, 2025
9 checks passed
@carddev81 carddev81 deleted the cpride/ticket_id478_sync_class_start_dt_and_end_dt branch November 7, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants