Skip to content

Conversation

@asithade
Copy link
Contributor

Summary

Fixes LFXV2-757 - Recurring meetings with past occurrences were incorrectly showing as past meetings.

Changes

  • Refactored meetings-dashboard component to load upcoming and past meetings separately
  • 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 (now handled by API)

Impact

  • Proper categorization of recurring meetings based on occurrence times
  • Accurate counts for both upcoming and past meetings
  • Improved UX by hiding RSVP for past meetings

Generated with Claude Code

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>
@asithade asithade requested a review from jordane as a code owner November 18, 2025 17:44
Copilot AI review requested due to automatic review settings November 18, 2025 17:44
@coderabbitai
Copy link

coderabbitai bot commented Nov 18, 2025

Walkthrough

The 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

Cohort / File(s) Summary
Meetings Dashboard Refactoring
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts, apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html
Replaced single meetings signal with separate upcomingMeetings and pastMeetings streams; added pastMeetingsLoading signal; split initializeMeetings() into initializeUpcomingMeetings() and initializePastMeetings() with project-scoped API calls (getMeetingsByProject, getPastMeetingsByProject); updated template rendering conditions and loading state checks to account for timeFilter value; removed client-side time filtering in favor of API-driven separation.
Meeting Card RSVP Guard
apps/lfx-one/src/app/shared/components/meeting-card/meeting-card.component.html
Added conditional guard to render RSVP button group only for upcoming meetings (else-if (!pastMeeting())) instead of for all non-organizer states.

Sequence Diagram

sequenceDiagram
    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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

  • Moderate logic density with split initialization methods following similar patterns
  • Multiple file changes with interconnected data-flow updates (signals, loading states, API calls)
  • API contract changes (project-scoped fetch calls with new getPastMeetingsByProject)
  • Template changes require understanding of new signal structure and conditional rendering logic

Areas requiring extra attention:

  • Verify initializeUpcomingMeetings() and initializePastMeetings() properly handle reactivity and refresh triggers
  • Confirm API calls align with service definitions (getMeetingsByProject, getPastMeetingsByProject)
  • Validate that filteredMeetings correctly combines both streams and respects search filtering
  • Check that loading state transitions for both meetingsLoading and pastMeetingsLoading are correctly wired in templates

Possibly related PRs

Suggested labels

deploy-preview

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive While the PR addresses the core issue by separating upcoming and past meetings loading, it's unclear whether getCurrentOrNextOccurrence() is actually used or if the filtering is delegated entirely to the API without verification of the implementation. Verify that the API endpoints (getMeetingsByProject and getPastMeetingsByProject) properly implement occurrence-based filtering using getCurrentOrNextOccurrence() or equivalent logic to ensure recurring meetings are categorized correctly.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: refactoring to load past and upcoming meetings separately, which directly addresses the primary objective of the pull request.
Description check ✅ Passed The description is directly related to the changeset, detailing the refactoring of the meetings-dashboard component, the new API endpoints used, and the changes to RSVP button rendering.
Out of Scope Changes check ✅ Passed All changes are directly related to the issue: refactoring meetings loading separation, updating component initialization logic, and hiding RSVP for past meetings. No unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/LFXV2-757

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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 requests

Right now both initializeUpcomingMeetings and initializePastMeetings fetch 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 getPastMeetingsByProject until timeFilter is 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 initialize

Because both upcomingMeetings() and pastMeetings() are arrays (initialized via toSignal with initialValue: []), 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 @if entirely; 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 154cb09 and 2a77047.

📒 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.html
  • apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts
  • apps/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 meetings

The 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 consistent

The new upcomingMeetings, pastMeetings, pastMeetingsLoading, and filteredMeetings signals 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 solid

The initializeUpcomingMeetings pipeline correctly reacts to project and manual refresh, short-circuits when no projectId is available, and uses getCurrentOrNextOccurrence to 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

initializePastMeetings mirrors the upcoming flow with its own loading flag, project/reactive triggers, and safe error handling around getPastMeetingsByProject. This keeps the concerns separated cleanly while using the same reactive pattern.


158-176: Search-only filtering with backend-owned time bucketing is appropriate

The updated initializeFilteredMeetings uses timeFilter only to select between pastMeetings() and upcomingMeetings(), 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 correct

The conditional loading block keyed off timeFilter() with meetingsLoading() vs pastMeetingsLoading() correctly shows skeletons only for the active bucket, and the list rendering uses filteredMeetings() together with [pastMeeting]="timeFilter() === 'past'" to drive lfx-meeting-card behavior. This aligns well with the new upcoming/past split.

@asithade asithade merged commit fcef70a into main Nov 18, 2025
11 checks passed
@asithade asithade deleted the fix/LFXV2-757 branch November 18, 2025 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants