-
Notifications
You must be signed in to change notification settings - Fork 0
fix(meetings): load past and upcoming meetings separately #160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Fixed recurring meetings incorrectly showing as past by refactoring to use separate API endpoints for upcoming and past meetings: - Load upcoming meetings via getMeetingsByProject() - Load past meetings via getPastMeetingsByProject() - React to timeFilter changes to call appropriate endpoint - Hide RSVP button for non-organizers on past meetings - Removed client-side time filtering (handled by API) This ensures proper meeting categorization and enables accurate counts for both upcoming and past meetings. Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
WalkthroughThe PR refactors the meetings dashboard to separately manage upcoming and past meetings through distinct data streams instead of a single combined stream. It introduces separate loading signals, splits initialization logic, and updates API calls to fetch data per category while guarding RSVP UI to upcoming meetings only. Changes
Sequence DiagramsequenceDiagram
participant UI as Dashboard UI
participant Comp as MeetingsDashboardComponent
participant API as Meeting Service
UI->>Comp: Load dashboard (timeFilter selected)
alt timeFilter === 'upcoming'
Comp->>Comp: initializeUpcomingMeetings()
Comp->>API: getMeetingsByProject(projectId)
API-->>Comp: upcomingMeetings[]
Comp->>Comp: Set meetingsLoading = false
else timeFilter === 'past'
Comp->>Comp: initializePastMeetings()
Comp->>API: getPastMeetingsByProject(projectId)
API-->>Comp: pastMeetings[]
Comp->>Comp: Set pastMeetingsLoading = false
end
Comp->>Comp: Combine into filteredMeetings
Comp->>UI: Render top-bar + meetings list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Areas requiring extra attention:
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts (1)
127-156: Consider lazy-loading past meetings to avoid unnecessary requestsRight now both
initializeUpcomingMeetingsandinitializePastMeetingsfetch their respective lists on every project change and refresh, even if the user never switches to the “Past” view. If API load or latency become an issue, you might consider:
- Loading upcoming meetings eagerly as now.
- Deferring
getPastMeetingsByProjectuntiltimeFilteris first set to'past', then caching results for subsequent toggles.This is purely an optimization and not required for correctness.
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html (1)
48-55: Top-bar guard condition is effectively always truthy once signals initializeBecause both
upcomingMeetings()andpastMeetings()are arrays (initialized viatoSignalwithinitialValue: []), this@if (upcomingMeetings() || pastMeetings())will evaluate to a truthy array as soon as the component is constructed, even when both lists are empty. If you intend the top bar to be always visible, you can drop the@ifentirely; if you want it to appear only when there is at least one meeting, consider checking.length:- @if (upcomingMeetings() || pastMeetings()) { + @if (upcomingMeetings().length || pastMeetings().length) {Either way, clarifying this will make the intent more obvious to future readers.
📜 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 (3)
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html(1 hunks)apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts(7 hunks)apps/lfx-one/src/app/shared/components/meeting-card/meeting-card.component.html(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-21T21:19:13.599Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-ui PR: 125
File: apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts:345-350
Timestamp: 2025-10-21T21:19:13.599Z
Learning: In the Angular meeting card component (apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts), when selecting between `summary.summary_data.edited_content` and `summary.summary_data.content`, the logical OR operator (`||`) is intentionally used instead of nullish coalescing (`??`) because empty string edited_content should fall back to the original content rather than being displayed as empty.
Applied to files:
apps/lfx-one/src/app/shared/components/meeting-card/meeting-card.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html
🧬 Code graph analysis (1)
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts (1)
packages/shared/src/interfaces/meeting.interface.ts (2)
Meeting(90-167)PastMeeting(538-551)
🔇 Additional comments (6)
apps/lfx-one/src/app/shared/components/meeting-card/meeting-card.component.html (1)
270-283: RSVP controls correctly restricted to upcoming meetingsThe organizer vs non-organizer split plus the
@else if (!pastMeeting())guard cleanly ensures RSVP selection is hidden for non-organizers on past meetings while keeping organizer details available. This aligns with the stated UX requirements.apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts (4)
30-65: State and signal setup for upcoming/past streams looks consistentThe new
upcomingMeetings,pastMeetings,pastMeetingsLoading, andfilteredMeetingssignals are wired cleanly in the constructor and align with the intended separation of upcoming vs past meetings. Public API surface is coherent and backward-compatible with the template usage.
77-125: Upcoming meetings loading and sorting by next occurrence are solidThe
initializeUpcomingMeetingspipeline correctly reacts to project and manual refresh, short-circuits when noprojectIdis available, and usesgetCurrentOrNextOccurrenceto sort by the next relevant occurrence, which addresses the recurring-meeting classification concerns. Error handling falls back to an empty list and clears the loading flag.
127-156: Past meetings loading flow is symmetric and robust
initializePastMeetingsmirrors the upcoming flow with its own loading flag, project/reactive triggers, and safe error handling aroundgetPastMeetingsByProject. This keeps the concerns separated cleanly while using the same reactive pattern.
158-176: Search-only filtering with backend-owned time bucketing is appropriateThe updated
initializeFilteredMeetingsusestimeFilteronly to select betweenpastMeetings()andupcomingMeetings(), then applies a search filter over shared fields, leaving time-based inclusion to the APIs. This is a good simplification and keeps the frontend out of recurrence/occurrence edge cases.apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html (1)
61-99: Per-bucket loading skeleton and past-meeting flag wiring look correctThe conditional loading block keyed off
timeFilter()withmeetingsLoading()vspastMeetingsLoading()correctly shows skeletons only for the active bucket, and the list rendering usesfilteredMeetings()together with[pastMeeting]="timeFilter() === 'past'"to drivelfx-meeting-cardbehavior. This aligns well with the new upcoming/past split.
Summary
Fixes LFXV2-757 - Recurring meetings with past occurrences were incorrectly showing as past meetings.
Changes
getMeetingsByProject()getPastMeetingsByProject()timeFilterchanges to call appropriate endpointImpact
Generated with Claude Code