fix(meetings): use consistent meeting identifier for v1/v2 compatibility #203

asithade · 2025-12-08T20:41:44Z

Summary

Fix meeting identifier handling to use meetingIdentifier() consistently for v1 (id) and v2 (uid) meetings
Add parseToInt utility for v1 meetings that return numeric fields as strings
Fix duration parsing in user.service for v1 meeting compatibility
Refactor dashboard-meeting-card to use init function pattern

🤖 Generated with Claude Code

LFXV2-891 - Update dashboard-meeting-card to use meetingIdentifier() for attachments - Update meeting-card to use meetingIdentifier() for attachments and join URL - Add joinQueryParams computed signal for proper v1 query param handling - Add parseToInt utility for v1 meetings that return numeric fields as strings - Fix duration parsing in user.service for v1 meeting compatibility - Refactor recurrence-summary.pipe to use shared parseToInt utility - Refactor dashboard-meeting-card to use init function pattern Signed-off-by: Asitha De Silva <[email protected]> Signed-off-by: Asitha de Silva <[email protected]>

coderabbitai · 2025-12-08T20:42:01Z

Walkthrough

This pull request refactors component property declarations from inline computed signals to private initializer methods, introduces shared meetingIdentifier and meetingDetailUrl signals to standardize meeting reference handling, adds a parseToInt utility for robust number parsing, and updates templates and services to use these new abstractions consistently.

Changes

Cohort / File(s)	Summary
Template Updates `apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.html`, `apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.html`	Replaced `meeting().uid` references with `meetingIdentifier()` in attachment URL construction and join button routing; replaced router-based navigation for meeting details with direct `href` using `meetingDetailUrl()`.
Dashboard Card Component Refactor `apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts`	Migrated all computed properties (`meetingTypeInfo`, `meetingStartTime`, `formattedTime`, `isTodayMeeting`, `isPrivate`, `hasYoutubeUploads`, `hasRecording`, `hasTranscripts`, `canJoinMeeting`, `meetingTitle`, `hasAiSummary`, `isLegacyMeeting`) to private initializer methods; added public `meetingIdentifier` and `meetingDetailUrl` signals; replaced `meetingDetailRouterLink` and `meetingDetailQueryParams` signals with URL-based approach.
Meeting Card Component Enhancement `apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.ts`	Added `joinQueryParams` computed signal and private `initJoinQueryParams()` method to encapsulate query parameter construction for the join button.
Shared Utility & Service Updates `packages/shared/src/utils/string.utils.ts`, `apps/lfx-one/src/app/shared/pipes/recurrence-summary.pipe.ts`, `apps/lfx-one/src/server/services/user.service.ts`	Introduced new exported `parseToInt()` utility function for consistent string/number-to-integer conversion with null/undefined handling; updated `recurrence-summary.pipe.ts` to use shared `parseToInt` instead of local implementation; updated `user.service.ts` duration calculations to use `parseToInt()` for robust numeric handling in v1 and v2 meeting logic.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Areas requiring attention:

dashboard-meeting-card.component.ts: Verify that all 12+ initializer methods correctly replicate the logic of their original inline computed properties, particularly initCanJoinMeeting(), initMeetingIdentifier(), and initMeetingDetailUrl() which involve conditional logic and state dependencies.
Attachment and join URL reworking: Confirm that the shift from meeting().uid to meetingIdentifier() across templates and services maintains URL correctness and doesn't break API endpoints.
parseToInt utility consistency: Ensure the new utility's null/undefined/NaN handling aligns with expected behavior in user.service.ts duration calculations, especially for mixed v1/v2 meeting scenarios.
Signal wiring: Validate that the new meetingDetailUrl signal replacement for router-based navigation (meetingDetailRouterLink + queryParams) produces equivalent URLs and preserves all query parameters.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: ensuring consistent meeting identifier handling for v1/v2 compatibility, which is the core objective of the PR.
Description check	✅ Passed	The description is related to the changeset and summarizes the key fixes and refactoring, though it could provide more context about why these changes matter.
Linked Issues check	✅ Passed	All coded requirements from LFXV2-891 are met: meetingIdentifier() is used consistently [HTML files], parseToInt() utility is added [string.utils.ts], duration parsing is fixed [user.service.ts], recurrence pipe refactored [recurrence-summary.pipe.ts], and dashboard-meeting-card uses init pattern [component.ts].
Out of Scope Changes check	✅ Passed	All changes are within scope: five files modified directly address the objectives, and the refactoring of dashboard-meeting-card to the init pattern aligns with code quality improvements mentioned in LFXV2-891.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ 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-891

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

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts (1)
253-259: Verify signal dependency resolution.

The initMeetingIdentifier() method calls this.isLegacyMeeting() internally, but isLegacyMeeting is declared on Line 59 after meetingIdentifier on Line 60. Since Angular's computed() uses lazy evaluation, this should work correctly at runtime - the dependency will be resolved when meetingIdentifier is first accessed. However, for clarity and to avoid potential confusion, consider reordering the declarations so isLegacyMeeting appears before meetingIdentifier.
   // TODO(v1-migration): Remove once all meetings are migrated to V2
   public readonly isLegacyMeeting: Signal<boolean> = this.initIsLegacyMeeting();
+  // TODO(v1-migration): Simplify to use V2 uid only once all meetings are migrated to V2
   public readonly meetingIdentifier: Signal<string> = this.initMeetingIdentifier();
+  // TODO(v1-migration): Remove V1 parameter handling once all meetings are migrated to V2
   public readonly meetingDetailUrl: Signal<string> = this.initMeetingDetailUrl();
-  // TODO(v1-migration): Simplify to use V2 uid only once all meetings are migrated to V2
-  public readonly meetingIdentifier: Signal<string> = this.initMeetingIdentifier();

📜 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 5f980ca and 29436fb.

📒 Files selected for processing (7)

apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.html (2 hunks)
apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts (2 hunks)
apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.html (2 hunks)
apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.ts (2 hunks)
apps/lfx-one/src/app/shared/pipes/recurrence-summary.pipe.ts (2 hunks)
apps/lfx-one/src/server/services/user.service.ts (3 hunks)
packages/shared/src/utils/string.utils.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (7)

**/*.{ts,tsx}