fix(meetings): use direct endpoint calls and fix past meeting display #101
Conversation
- Replace query service calls with direct endpoint calls to /{meetingType}/{uid}
- Update meeting type parameters from 'meeting'/'past_meeting' to 'meetings'/'past_meetings'
- Simplify error handling by checking meeting.uid instead of resources array
- Remove redundant multiple meeting warnings since direct endpoint calls return single objects
- Maintain backward compatibility with access control and committee data fetching
Generated with [Claude Code](https://claude.ai/code)
Signed-off-by: Asitha de Silva <asithade@gmail.com>
|
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. WalkthroughServer meeting retrieval now fetches single meetings via /{meetingType}/{uid} with plural defaults; UI templates migrated to new @if syntax, member form added a cross-field organization validator, button tooltip input renamed, and several layout/message elements set to full-width/flex spacing. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Controller as Public/Past Controller
participant Service as MeetingService
participant API as Backend API
User->>Controller: GET /.../meetings/:uid
Controller->>Service: getMeetingById(req, uid, meetingType='meetings', access=true)
Service->>API: GET /{meetingType}/{uid}
API-->>Service: Meeting
alt Meeting has committees
Service->>API: GET /meetings/{uid}/committees
API-->>Service: Committees
Service->>Service: Merge committees into meeting
end
alt access augmentation enabled
Service->>Service: addAccessToResource(meeting, 'meeting', 'organizer')
end
Service-->>Controller: Meeting
Controller-->>User: 200 OK + Meeting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 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 refactors the getMeetingById service method to use direct endpoint calls instead of query service calls, improving simplicity and performance. The changes update meeting type parameters to use plural forms and simplify error handling.
Key changes:
- Replace query service calls with direct API endpoint calls (
/{meetingType}/{uid}) - Update meeting type parameters from singular to plural forms (
meeting→meetings,past_meeting→past_meetings) - Simplify error handling and remove redundant multiple meeting warnings
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| meeting.service.ts | Core refactoring to use direct endpoint calls and simplified error handling |
| public-meeting.controller.ts | Updated meeting type parameter from 'meeting' to 'meetings' |
| past-meeting.controller.ts | Updated meeting type parameter from 'past_meeting' to 'past_meetings' |
| message.component.html | Added w-full class for better layout |
| button.component.ts/html | Renamed tooltip property from pTooltip to tooltip |
| meeting-card.component.html | Updated tooltip property usage and conditional rendering |
| member-form.component.ts/html | Added cross-field validation and modernized template syntax |
| committee-form.component.html | Modernized template syntax from *ngIf to @if |
| meeting-join.component.html | Updated CSS classes from space-y-* to flex flex-col gap-* |
| app.component.scss | Added CSS rules for message component width |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
apps/lfx-one/src/app/modules/project/committees/components/member-form/member-form.component.ts
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html (2)
41-51: Gate copy action to non‑past meetings: confirm product intent and add accessible label
- Confirm UX: hiding “Copy Meeting” for past meetings is intentional.
- Icon‑only button lacks an accessible name. Add aria label.
Apply:
<lfx-button icon="fa-light fa-copy" [text]="true" [rounded]="true" size="small" severity="secondary" data-testid="copy-meeting-button" (click)="copyMeetingLink()" - tooltip="Copy Meeting"></lfx-button> + tooltip="Copy Meeting" + [ariaLabel]="'Copy Meeting'"></lfx-button>If
lfx-buttondoesn’t exposeariaLabel, please add it in the component and forward it to the underlying button’saria-label.
70-71: Add aria label to icon‑only Edit buttonEnsure screen readers announce the action meaningfully.
- tooltip="Edit Meeting" + tooltip="Edit Meeting" + [ariaLabel]="'Edit Meeting'" [routerLink]="['/project', project()!.slug, 'meetings', meeting().uid, 'edit']"></lfx-button>apps/lfx-one/src/app/shared/components/button/button.component.ts (1)
64-64: Narrow tooltipPosition type to prevent typosConstrain to allowed values for better type safety.
Apply this diff:
- public readonly tooltipPosition = input<string>('top'); + public readonly tooltipPosition = input<'top' | 'bottom' | 'left' | 'right'>('top');apps/lfx-one/src/app/modules/project/committees/components/member-form/member-form.component.ts (1)
180-206: Trim values in cross-field validator to avoid whitespace false-positivesWhitespace-only inputs currently pass the “provided” checks.
Apply this diff:
- const organization = control.get('organization')?.value; - const organizationUrl = control.get('organization_url')?.value; + const organization = (control.get('organization')?.value ?? '').toString().trim(); + const organizationUrl = (control.get('organization_url')?.value ?? '').toString().trim();apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html (1)
203-204: Use (onClick) output from lfx-button, not (click)Binding to
(click)onlfx-buttoncan bypass ButtonComponent’s click gating and naming consistency.Apply this diff at both sites:
- (click)="onJoinMeeting()"></lfx-button> + (onClick)="onJoinMeeting()"></lfx-button>Also applies to: 312-313
apps/lfx-one/src/server/services/meeting.service.ts (1)
69-71: Defensively normalize/validate meetingType (support singular aliases)Prevent accidental calls to non-existent endpoints and ease migration by mapping singular → plural.
public async getMeetingById(req: Request, meetingUid: string, meetingType: string = 'meetings', access: boolean = true): Promise<Meeting> { + // Normalize legacy types + if (meetingType === 'meeting') meetingType = 'meetings'; + if (meetingType === 'past_meeting') meetingType = 'past_meetings';Optionally, introduce an enum (MeetingEntityPath) for allowed values to avoid stringly-typed calls.
apps/lfx-one/src/server/controllers/public-meeting.controller.ts (1)
206-206: Drop redundant await on returnMinor cleanup: returning the Promise directly is fine here.
- return await this.meetingService.getMeetingById(req, id, 'meetings', false); + return this.meetingService.getMeetingById(req, id, 'meetings', false);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/lfx-one/src/app/app.component.scss(1 hunks)apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html(3 hunks)apps/lfx-one/src/app/modules/project/committees/components/committee-form/committee-form.component.html(4 hunks)apps/lfx-one/src/app/modules/project/committees/components/member-form/member-form.component.html(2 hunks)apps/lfx-one/src/app/modules/project/committees/components/member-form/member-form.component.ts(3 hunks)apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html(3 hunks)apps/lfx-one/src/app/shared/components/button/button.component.html(2 hunks)apps/lfx-one/src/app/shared/components/button/button.component.ts(1 hunks)apps/lfx-one/src/app/shared/components/message/message.component.html(1 hunks)apps/lfx-one/src/server/controllers/past-meeting.controller.ts(1 hunks)apps/lfx-one/src/server/controllers/public-meeting.controller.ts(1 hunks)apps/lfx-one/src/server/services/meeting.service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/server/controllers/public-meeting.controller.tsapps/lfx-one/src/app/shared/components/button/button.component.tsapps/lfx-one/src/server/controllers/past-meeting.controller.tsapps/lfx-one/src/server/services/meeting.service.tsapps/lfx-one/src/app/modules/project/committees/components/member-form/member-form.component.ts
**/*.{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/server/controllers/public-meeting.controller.tsapps/lfx-one/src/app/modules/project/committees/components/committee-form/committee-form.component.htmlapps/lfx-one/src/app/shared/components/message/message.component.htmlapps/lfx-one/src/app/shared/components/button/button.component.tsapps/lfx-one/src/server/controllers/past-meeting.controller.tsapps/lfx-one/src/server/services/meeting.service.tsapps/lfx-one/src/app/modules/project/committees/components/member-form/member-form.component.tsapps/lfx-one/src/app/modules/project/committees/components/member-form/member-form.component.htmlapps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.htmlapps/lfx-one/src/app/shared/components/button/button.component.htmlapps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-one/src/app/app.component.scss
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/server/controllers/public-meeting.controller.tsapps/lfx-one/src/app/shared/components/button/button.component.tsapps/lfx-one/src/server/controllers/past-meeting.controller.tsapps/lfx-one/src/server/services/meeting.service.tsapps/lfx-one/src/app/modules/project/committees/components/member-form/member-form.component.ts
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/project/committees/components/committee-form/committee-form.component.htmlapps/lfx-one/src/app/shared/components/message/message.component.htmlapps/lfx-one/src/app/modules/project/committees/components/member-form/member-form.component.htmlapps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.htmlapps/lfx-one/src/app/shared/components/button/button.component.htmlapps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html
🧬 Code graph analysis (1)
apps/lfx-one/src/server/services/meeting.service.ts (1)
packages/shared/src/interfaces/meeting.interface.ts (1)
Meeting(75-148)
⏰ 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: Code Quality Checks
🔇 Additional comments (12)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html (1)
60-60: Add accessible name to icon-only Share button & verify tooltip API migration
- apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html (≈ line 60): add an accessible name and confirm tooltip API rename.
- tooltip="Share Meeting"></lfx-button> + tooltip="Share Meeting" + [ariaLabel]="'Share Meeting'"></lfx-button>
- Verify no remaining pTooltip usages (run locally):
rg -n -P '<lfx-button\b[^>]*\bpTooltip=' --hidden --glob '!**/node_modules/**'apps/lfx-one/src/app/shared/components/button/button.component.html (1)
21-21: Tooltip binding alignment LGTMBinding
[pTooltip]="tooltip()"correctly maps the renamed input to PrimeNG’s directive in both branches.Also applies to: 52-52
apps/lfx-one/src/app/shared/components/message/message.component.html (1)
22-22: Full-width message containers LGTMAdding
w-fullto the inner containers is consistent with the layout updates.Also applies to: 29-29
apps/lfx-one/src/app/shared/components/button/button.component.ts (1)
63-63: Input rename LGTMRenaming
pTooltip→tooltipkeeps the public API clear and avoids confusion with PrimeNG’s directive input.apps/lfx-one/src/app/app.component.scss (1)
114-120: Enable full-width message content LGTMThe applied Tailwind utilities match the intent and are scoped via ::ng-deep appropriately.
apps/lfx-one/src/app/modules/project/committees/components/committee-form/committee-form.component.html (1)
24-26: Control-flow syntax migration and validations LGTM@if blocks/readability improve without changing behavior. Validation conditions and messages look correct.
Also applies to: 40-42, 122-134, 172-174
apps/lfx-one/src/app/modules/project/committees/components/member-form/member-form.component.ts (1)
156-175: Cross-field validator wiring LGTMValidator is attached at the FormGroup level with clear control defs and patterns.
apps/lfx-one/src/app/modules/project/committees/components/member-form/member-form.component.html (1)
12-14: Error rendering and dynamic required indicators LGTMMessages are correctly scoped to touched state, and cross-field errors align with the new validator.
Also applies to: 21-23, 31-36, 50-55, 63-65, 70-75, 83-88
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html (1)
150-150: Layout updates LGTMSwitching to
flex flex-col gap-*and widening messages (w-full) improves consistency.Also applies to: 173-173, 250-250, 253-253
apps/lfx-one/src/server/services/meeting.service.ts (2)
85-91: LGTM: access augmentation matches the new single-object flowUsing addAccessToResource for the single Meeting object aligns with the refactor.
69-71: Quick audit: find getMeetingById calls still passing singular 'meeting'/'past_meeting'Sandbox search couldn't verify (ripgrep skipped files). Run locally to locate and fix call sites:
rg -n -C2 -uu --pcre2 "getMeetingById\s*(\s*[^,]+,\s*[^,]+,\s*(?:'meeting'|'past_meeting'|"meeting"|"past_meeting")" --glob '!node_modules/**'
Replace matches with the plural resource types ('meetings' / 'past_meetings') where appropriate.
apps/lfx-one/src/server/controllers/public-meeting.controller.ts (1)
206-206: LGTM: correct use of pluralized endpoint typePassing 'meetings' aligns with the new service contract.
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
- Create MeetingSession and PastMeeting interfaces for proper typing - Fix past meeting time display by handling scheduled_start_time field - Add shared getCurrentOrNextOccurrence utility to follow DRY principle - Update services and components to use PastMeeting type instead of Meeting - Remove 'as any' casts for better type safety - Implement proper occurrence priority logic in meeting cards Related JIRA: - LFXV2-564: Add PastMeeting interface and improve type safety - LFXV2-565: Fix past meeting time display in meeting cards - LFXV2-566: Create shared utility for meeting occurrence logic 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Asitha de Silva <asithade@gmail.com>
asithade
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (1)
149-153: Good: delegate occurrence selection to shared helperSwitching initializeCurrentOccurrence to use getCurrentOrNextOccurrence is correct. Consider also simplifying initializeCanJoinMeeting to rely solely on currentOccurrence to avoid duplicating the join-window calculation.
apps/lfx-one/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts (1)
64-65: Avoid union array type for filteredMeetingsPastMeeting extends Meeting, so you can type this as Signal<Meeting[]> to simplify.
Apply this diff:
- public filteredMeetings: Signal<Meeting[] | PastMeeting[]>; + public filteredMeetings: Signal<Meeting[]>;apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (2)
73-76: Consider a type guard instead of repeating Meeting | PastMeeting unionsTo align with maintainability guidance, keep meeting typed as Meeting and narrow via a type guard when needed for PastMeeting-only fields.
Example:
function isPastMeeting(m: Meeting | PastMeeting): m is PastMeeting { return 'scheduled_start_time' in m; }Use isPastMeeting(this.meeting()) where accessing scheduled_start_time.
81-81: Union on meeting signal mirrors input unionAcceptable; can be simplified with the type guard approach above if desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts(2 hunks)apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html(4 hunks)apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts(5 hunks)apps/lfx-one/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts(3 hunks)apps/lfx-one/src/app/shared/services/meeting.service.ts(4 hunks)apps/lfx-one/src/server/controllers/past-meeting.controller.ts(3 hunks)packages/shared/src/interfaces/meeting.interface.ts(1 hunks)packages/shared/src/utils/meeting.utils.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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.tsapps/lfx-one/src/app/shared/services/meeting.service.tsapps/lfx-one/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/server/controllers/past-meeting.controller.tspackages/shared/src/interfaces/meeting.interface.tspackages/shared/src/utils/meeting.utils.ts
**/*.{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.tsapps/lfx-one/src/app/shared/services/meeting.service.tsapps/lfx-one/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/server/controllers/past-meeting.controller.tspackages/shared/src/interfaces/meeting.interface.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.htmlpackages/shared/src/utils/meeting.utils.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.tsapps/lfx-one/src/app/shared/services/meeting.service.tsapps/lfx-one/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/server/controllers/past-meeting.controller.tspackages/shared/src/interfaces/meeting.interface.tspackages/shared/src/utils/meeting.utils.ts
packages/shared/src/interfaces/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all TypeScript interfaces in the shared package at packages/shared/src/interfaces
Files:
packages/shared/src/interfaces/meeting.interface.ts
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/project/meetings/components/meeting-card/meeting-card.component.html
🧠 Learnings (1)
📚 Learning: 2025-09-16T03:32:46.518Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T03:32:46.518Z
Learning: All PrimeNG components are wrapped in LFX components to keep UI library independence
Applied to files:
apps/lfx-one/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts
🧬 Code graph analysis (6)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (1)
packages/shared/src/utils/meeting.utils.ts (1)
getCurrentOrNextOccurrence(111-138)
apps/lfx-one/src/app/shared/services/meeting.service.ts (1)
packages/shared/src/interfaces/meeting.interface.ts (1)
PastMeeting(500-513)
apps/lfx-one/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts (1)
packages/shared/src/interfaces/meeting.interface.ts (2)
PastMeeting(500-513)Meeting(75-148)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (2)
packages/shared/src/interfaces/meeting.interface.ts (3)
Meeting(75-148)PastMeeting(500-513)MeetingOccurrence(154-165)packages/shared/src/utils/meeting.utils.ts (1)
getCurrentOrNextOccurrence(111-138)
apps/lfx-one/src/server/controllers/past-meeting.controller.ts (1)
packages/shared/src/interfaces/meeting.interface.ts (1)
PastMeeting(500-513)
packages/shared/src/utils/meeting.utils.ts (1)
packages/shared/src/interfaces/meeting.interface.ts (2)
Meeting(75-148)MeetingOccurrence(154-165)
⏰ 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 (21)
apps/lfx-one/src/server/controllers/past-meeting.controller.ts (3)
6-6: Strong typing to PastMeeting is appropriateImporting PastMeeting here aligns controller responses with the new shared interface.
25-28: Cast to PastMeeting[] is fine (list path stays on query service)The list endpoint still uses meetingType 'past_meeting'. The cast ensures downstream code sees PastMeeting fields.
76-78: Comment drift: update to 'past_meetings'The comment says 'past_meeting' while the code calls getMeetingById with 'past_meetings'.
Apply this diff:
- // Get the past meeting by ID using meetingType 'past_meeting' + // Get the past meeting by ID using meetingType 'past_meetings'packages/shared/src/interfaces/meeting.interface.ts (1)
483-514: PastMeeting and MeetingSession additions look goodInterfaces are clear, documented, and placed in shared as per guidelines.
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (1)
18-18: Imports updated to use shared helperImporting getCurrentOrNextOccurrence centralizes occurrence logic.
packages/shared/src/utils/meeting.utils.ts (2)
106-138: Helper is correct and side‑effect freeLogic for joinable vs next future occurrence matches existing usage and defaults early_join_time_minutes sensibly.
5-6: No action needed — helper already re-exported from shared barrelgetCurrentOrNextOccurrence is exported in packages/shared/src/utils/meeting.utils.ts and re‑exported via packages/shared/src/utils/index.ts (and packages/shared/src/index.ts), so imports from @lfx-one/shared resolve.
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html (2)
41-51: Hide copy button for past meetings + use tooltip input: LGTMConditional render and use of tooltip on lfx-button align with wrapper component usage.
Confirm lfx-button supports a tooltip input consistently across the app.
96-100: Switch to meetingStartTime(): LGTMUsing computed meetingStartTime in the header centralizes date/time source selection.
apps/lfx-one/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts (3)
15-16: PastMeeting import: LGTMType import aligns with new past-meetings data shape.
60-61: Type pastMeetings as PastMeeting[]: LGTMKeeps past meetings strongly typed.
190-201: Initialize past meetings with PastMeeting[]: LGTMReturn type and service call match the new interface.
apps/lfx-one/src/app/shared/services/meeting.service.ts (4)
18-20: Import PastMeeting: LGTMPreps service methods to return PastMeeting responses.
44-51: getPastMeetings returns PastMeeting[]: LGTMEndpoint and error handling unchanged; typed result adds clarity.
82-90: getPastMeetingsByProject: LGTMTyped to PastMeeting[] and delegates to getPastMeetings.
102-109: getPastMeeting returns PastMeeting: LGTMAlso updates shared meeting signal via tap; compatible since PastMeeting extends Meeting.
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (5)
18-27: Shared imports addition: LGTMConsolidates helpers and types from @lfx-one/shared.
103-105: Add currentOccurrence and meetingStartTime signals: LGTMGood separation of concerns for rendering logic.
118-127: Occurrence selection precedence is soundExplicit occurrenceInput > computed for upcoming > null for past keeps behavior predictable.
627-632: Use shared helper for current occurrence: LGTMKeeps logic consistent with other components.
634-664: Past meeting start-time precedence: prefer scheduled_start_time firstFor PastMeeting display, scheduled_start_time is typically the canonical scheduled value. Recommend checking it before start_time to avoid inconsistencies.
Apply this diff:
- } else { - // For past meetings, use occurrence input or fallback to scheduled_start_time/start_time + } else { + // For past meetings, use occurrence input or fallback to scheduled_start_time/start_time const occurrence = this.occurrence(); if (occurrence?.start_time) { return occurrence.start_time; } - if (meeting?.start_time) { - return meeting.start_time; - } - // Handle past meetings that use scheduled_start_time (type-safe check) - if ('scheduled_start_time' in meeting && meeting.scheduled_start_time) { + // Prefer the scheduled start time if present (PastMeeting) + if ('scheduled_start_time' in meeting && meeting.scheduled_start_time) { return meeting.scheduled_start_time; } + if (meeting?.start_time) { + return meeting.start_time; + } }
Summary
getMeetingByIdto use direct endpoint calls instead of query servicemeetings/past_meetings)Related JIRA
Changes Made
Original Changes
/{meetingType}/{uid}meeting/past_meetingtomeetings/past_meetingsmeeting.uidinstead of resources arrayNew Changes
scheduled_start_timefieldgetCurrentOrNextOccurrenceutility to shared package following DRY principleFiles Modified
Original Files
apps/lfx-one/src/server/services/meeting.service.tsapps/lfx-one/src/server/routes/past-meetings.route.tsNew Files
packages/shared/src/interfaces/meeting.interface.ts- Added MeetingSession and PastMeeting interfacespackages/shared/src/utils/meeting.utils.ts- New shared utility for occurrence logicapps/lfx-one/src/app/shared/services/meeting.service.ts- Updated to use PastMeeting typeapps/lfx-one/src/server/controllers/past-meeting.controller.ts- Added proper typingapps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts- Fixed time displayapps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html- Updated templateapps/lfx-one/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts- Updated typesapps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts- Uses shared utilityTest Plan
🤖 Generated with Claude Code