fix(meetings): use occurrence data for join page timing and display #93
Conversation
- Add currentOccurrence computed signal to find nearest future occurrence - Update canJoinMeeting logic to use occurrence-based timing with 40-minute buffer - Modify template to display occurrence title, description, start time, and duration - Maintain fallback to meeting data when no occurrences exist - Preserve backward compatibility for non-recurring meetings Fixes LFXV2-525 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces occurrence-aware meeting join behavior. The template now prefers occurrence fields (title, start_time, duration, description) with meeting-level fallbacks. The component adds a currentOccurrence signal, selects a joinable or next future occurrence, computes per-occurrence join windows, updates canJoinMeeting to honor earliest/latest join times, and extracts links from occurrence descriptions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant C as MeetingJoinComponent
participant M as Meeting (base)
participant O as Occurrences[]
participant T as Clock/Now
rect rgba(233,246,255,0.6)
note over C: Initialization
C->>M: Read meeting data
C->>O: Scan occurrences
C->>T: Get now()
C->>C: initializeCurrentOccurrence()\n- pick joinable by window\n- else next future occurrence
C->>C: initializeImportantLinks()\n- extract from occurrence.description\n- fallback to meeting.description
end
rect rgba(240,255,233,0.6)
note over C: Rendering decisions
C->>C: Resolve title/start_time/duration/description\nprefer occurrence, fallback meeting
C->>T: now() for canJoinMeeting
C->>C: Compute earliestJoinTime = start_time - early_join_minutes\nCompute latestJoinTime = start_time + duration + 40m
C-->>User: Enable/disable Join, render details
end
alt Join window satisfied
User->>C: Click Join
C->>C: Proceed with join flow (unchanged)
else Outside window
C-->>User: Join disabled/not available
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the meeting join page to properly handle recurring meetings by using occurrence-specific data when available, ensuring accurate timing and display information.
- Added
currentOccurrencecomputed signal to find the nearest joinable occurrence for recurring meetings - Updated join timing logic to use occurrence-based timing with 40-minute buffer after meeting end
- Modified template to display occurrence-specific title, description, start time, and duration with fallback to meeting data
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| meeting-join.component.ts | Added occurrence detection logic and updated join timing calculations with 40-minute buffer |
| meeting-join.component.html | Updated template to display occurrence-specific data with fallback to meeting properties |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const joinableOccurrence = meeting.occurrences.find((occurrence) => { | ||
| const startTime = new Date(occurrence.start_time); | ||
| const earliestJoinTime = new Date(startTime.getTime() - earlyJoinMinutes * 60000); | ||
| const latestJoinTime = new Date(startTime.getTime() + occurrence.duration * 60000); // 40 minutes after end |
There was a problem hiding this comment.
The comment indicates '40 minutes after end' but the code calculates the time as start_time + duration, which is just the meeting end time, not 40 minutes after the end. This should be startTime.getTime() + (occurrence.duration + 40) * 60000 to match the intended behavior.
| const latestJoinTime = new Date(startTime.getTime() + occurrence.duration * 60000); // 40 minutes after end | |
| const latestJoinTime = new Date(startTime.getTime() + (occurrence.duration + 40) * 60000); // 40 minutes after end |
| const startTime = new Date(currentOccurrence.start_time); | ||
| const earlyJoinMinutes = meeting.early_join_time_minutes || 10; | ||
| const earliestJoinTime = new Date(startTime.getTime() - earlyJoinMinutes * 60000); | ||
| const latestJoinTime = new Date(startTime.getTime() + currentOccurrence.duration * 60000); // 40 minutes after end |
There was a problem hiding this comment.
Same issue as above - the comment indicates '40 minutes after end' but the code only calculates to the meeting end time. This should be startTime.getTime() + (currentOccurrence.duration + 40) * 60000 to add 40 minutes after the meeting ends.
| const startTime = new Date(meeting.start_time); | ||
| const earlyJoinMinutes = meeting.early_join_time_minutes || 10; // Default to 10 minutes | ||
| const earliestJoinTime = new Date(startTime.getTime() - earlyJoinMinutes * 60000); | ||
| const latestJoinTime = new Date(startTime.getTime() + meeting.duration * 60000); // 40 minutes after end |
There was a problem hiding this comment.
Inconsistent with the other two occurrences - this also calculates only to meeting end time, not 40 minutes after. Should be startTime.getTime() + (meeting.duration + 40) * 60000 to match the intended 40-minute buffer.
| const latestJoinTime = new Date(startTime.getTime() + meeting.duration * 60000); // 40 minutes after end | |
| const latestJoinTime = new Date(startTime.getTime() + (meeting.duration + 40) * 60000); // 40 minutes after end |
Signed-off-by: Asitha de Silva <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (2)
147-177: 40‑min buffer is correctly applied; consider deterministic selection and small tidy‑up.The join window now matches the requirement (end + 40m). Sort occurrences once to ensure the “first joinable” pick is deterministic, and keep the 40‑min math consistent.
Apply this diff within the function:
- const joinableOccurrence = meeting.occurrences.find((occurrence) => { + const occurrencesAsc = [...meeting.occurrences].sort( + (a, b) => new Date(a.start_time).getTime() - new Date(b.start_time).getTime() + ); + const joinableOccurrence = occurrencesAsc.find((occurrence) => { const startTime = new Date(occurrence.start_time); const earliestJoinTime = new Date(startTime.getTime() - earlyJoinMinutes * 60000); - const latestJoinTime = new Date(startTime.getTime() + occurrence.duration * 60000 + 40 * 60000); // 40 minutes after end + const latestJoinTime = new Date(startTime.getTime() + (occurrence.duration + 40) * 60000); // 40 minutes after end return now >= earliestJoinTime && now <= latestJoinTime; });Optional (outside this hunk): define a single source of truth for the buffer.
// place at class scope private static readonly LATE_JOIN_BUFFER_MINUTES = 40; private static readonly DEFAULT_EARLY_JOIN_MINUTES = 10;Then reference these constants where used.
254-279: Joinability logic is correct with the +40m window; dedupe the time‑window math.Logic now honors early join and late buffer for both occurrence and fallback paths. Extract the window check to a helper to avoid divergence across three sites.
Apply this diff to use a helper:
if (currentOccurrence) { - const now = new Date(); - const startTime = new Date(currentOccurrence.start_time); - const earlyJoinMinutes = meeting.early_join_time_minutes || 10; - const earliestJoinTime = new Date(startTime.getTime() - earlyJoinMinutes * 60000); - const latestJoinTime = new Date(startTime.getTime() + currentOccurrence.duration * 60000 + 40 * 60000); // 40 minutes after end - - return now >= earliestJoinTime && now <= latestJoinTime; + const startTime = new Date(currentOccurrence.start_time); + const earlyJoinMinutes = meeting.early_join_time_minutes || 10; + return this.isNowWithinJoinWindow(startTime, currentOccurrence.duration, earlyJoinMinutes); } @@ - const now = new Date(); const startTime = new Date(meeting.start_time); const earlyJoinMinutes = meeting.early_join_time_minutes || 10; // Default to 10 minutes - const earliestJoinTime = new Date(startTime.getTime() - earlyJoinMinutes * 60000); - const latestJoinTime = new Date(startTime.getTime() + meeting.duration * 60000 + 40 * 60000); // 40 minutes after end - - return now >= earliestJoinTime && now <= latestJoinTime; + return this.isNowWithinJoinWindow(startTime, meeting.duration, earlyJoinMinutes);Add this helper (outside the hunk):
private isNowWithinJoinWindow(startTime: Date, durationMinutes: number, earlyJoinMinutes: number): boolean { const now = new Date(); const earliest = new Date(startTime.getTime() - earlyJoinMinutes * 60000); const latest = new Date(startTime.getTime() + (durationMinutes + 40) * 60000); // 40m after end return now >= earliest && now <= latest; }
🧹 Nitpick comments (1)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (1)
60-60: Make the signal property readonly.Prevent accidental reassignment of the computed signal.
Apply this diff:
- public currentOccurrence: Signal<MeetingOccurrence | null>; + public readonly currentOccurrence: Signal<MeetingOccurrence | null>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html(2 hunks)apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/lfx-one/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.html: Always add data-testid attributes when creating new Angular components for reliable test targeting
Use data-testid naming convention [section]-[component]-[element]
Files:
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.htmlapps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (2)
packages/shared/src/interfaces/meeting.interface.ts (1)
MeetingOccurrence(150-161)packages/shared/src/utils/url.utils.ts (1)
extractUrlsWithDomains(95-117)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests / Playwright E2E Tests
🔇 Additional comments (7)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (3)
17-17: Import looks correct and scoped.Adding
MeetingOccurrenceandextractUrlsWithDomainsfrom the shared package is appropriate for the new occurrence-aware logic.
76-76: Good: initializecurrentOccurrenceearly.Keeps downstream computations reactive from the start.
234-242: Correct fallback for important links.Preferring
currentOccurrence.descriptionwith a clean fallback tomeeting.descriptionis spot on.apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html (4)
54-54: Title fallback looks good.Renders occurrence title when available, with a safe meeting-level fallback.
57-63: Datetime rendering correctly uses per‑occurrence duration.Alias usage keeps the template clean and avoids recomputations.
69-78: Description alias + linkification LGTM.Pulling from occurrence when present aligns with the PR objective.
137-137: Duration display follows occurrence‑first logic.Matches the behavior in TS and keeps UI consistent.
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
🧹 Deployment RemovedThe deployment for PR #93 has been removed. |
Summary
Updates the meeting join page to use occurrence-specific data when available, ensuring correct timing and display information for recurring meetings.
Test plan
Changes
currentOccurrencecomputed signal to find the nearest future occurrencecanJoinMeetinglogic to use occurrence-based timing with 40-minute buffer after meeting endimportantLinksto use occurrence description when availableImpact
Fixes LFXV2-525
🤖 Generated with Claude Code