feat(meetings): implement custom recurrence and refactor utilities #85
Conversation
LFXV2-484: Implement comprehensive custom recurrence pattern functionality - Add new meeting-recurrence-pattern component with full UI support - Support daily, weekly, and monthly recurrence with advanced options - Add end conditions: never, specific date, or number of occurrences - Fix end date population bug (ISO string to Date object conversion) - Enhance meeting-manage component with custom recurrence integration LFXV2-483: Refactor components to use shared utilities and remove stale code - Replace local getWeekOfMonth with shared utility import - Remove duplicate methods and unused event handlers - Consolidate setAiEstimatedDuration and setTemplateDuration methods - Reduce bundle size by ~1KB and eliminate ~48 lines of duplicate code 🤖 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. WalkthroughAdds a custom recurrence system: new MeetingRecurrencePatternComponent, RecurrenceSummaryPipe, utilities/constants/interfaces for recurrence, switches forms to a recurrenceType+recurrence FormGroup, updates UI bindings/tooltips/summaries, adjusts payload shape, removes disabled inputs from some shared controls, and tweaks an E2E workflow env var. Changes
Sequence Diagram(s)sequenceDiagram
actor U as User
participant MD as MeetingDetailsComponent
participant RP as MeetingRecurrencePatternComponent
participant MM as MeetingManageComponent
participant API as Server
U->>MD: Open create/edit meeting
MD->>MD: Init form (recurrenceType + recurrence FormGroup)
MD-->>RP: Render custom UI when recurrenceType == "custom"
Note over RP,MD: User configures pattern in RP
U->>RP: Set pattern / end conditions
RP->>MD: Patch recurrence FormGroup fields
MD->>MD: handleStartDateChange -> update pattern/options
U->>MM: Save meeting
MM->>MM: Build payload (recurrenceType, recurrence object if custom)
MM->>API: POST/PUT meeting
API-->>MM: Response
MM-->>U: Confirmation
sequenceDiagram
participant MC as MeetingCard
participant Pipe as RecurrenceSummaryPipe
participant Util as buildRecurrenceSummary
MC->>Pipe: transform(meeting.recurrence, "full")
Pipe->>Util: buildRecurrenceSummary(pattern)
Util-->>Pipe: RecurrenceSummary
Pipe-->>MC: Full summary (tooltip)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
Comment |
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive custom recurrence pattern feature for meetings and refactors shared utilities to reduce code duplication. The implementation includes a new recurrence pattern component with advanced configuration options, enhanced end condition support, and consolidated utility functions for improved maintainability.
- Custom recurrence pattern component with daily, weekly, and monthly configuration options
- Enhanced recurrence summary generation with shared utilities
- Refactored code to use shared
getWeekOfMonthutility instead of duplicate implementations
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/utils/meeting.utils.ts | New shared utility for building recurrence summaries from custom patterns |
| packages/shared/src/utils/index.ts | Export new meeting utilities |
| packages/shared/src/interfaces/meeting.interface.ts | Extended interfaces for custom recurrence patterns and summaries |
| packages/shared/src/constants/meeting.constants.ts | Added recurrence pattern configuration constants |
| apps/lfx-pcc/src/server/errors/microservice.error.ts | Reordered imports for consistency |
| apps/lfx-pcc/src/app/shared/pipes/recurrence-summary.pipe.ts | New pipe for formatting recurrence summaries |
| apps/lfx-pcc/src/app/shared/components/select/select.component.ts | Removed unused disabled input property |
| apps/lfx-pcc/src/app/shared/components/radio-button/radio-button.component.ts | Removed unused disabled input property and cleaned up template |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.ts | Enhanced with custom recurrence summary support |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts | New comprehensive custom recurrence pattern component |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts | Integrated custom recurrence support and fixed end date population |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts | Refactored to use shared utilities and support custom recurrence |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts | Added recurrence summary pipe import |
| .github/workflows/e2e-tests.yml | Updated M2M auth audience URL |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...x-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts
Show resolved
Hide resolved
...ect/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.html
Outdated
Show resolved
Hide resolved
...oject/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts
Outdated
Show resolved
Hide resolved
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (27)
.github/workflows/e2e-tests.yml (1)
288-291: Skip message mentions an AWS AI secret that isn’t fetched.The “E2E tests skipped” message references /cloudops/managed-secrets/ai/ai_config, but the workflow doesn’t read that secret; AI creds are taken from GitHub secrets. Update the message (or fetch that secret) to avoid confusion.
Suggested tweak:
- echo " - /cloudops/managed-secrets/ai/ai_config (AI configuration)" + echo "GitHub Secrets (AI configuration):" + echo " - AI_API_KEY" + echo " - AI_PROXY_URL"apps/lfx-pcc/src/server/errors/microservice.error.ts (5)
12-26: Type safety: replace any with a typed error body interface.Avoid
anyon public API surfaces; model the backend error shape explicitly to improve safety and IntelliSense.Apply this diff:
export class MicroserviceError extends BaseApiError { - public readonly errorBody?: any; + public readonly errorBody?: MicroserviceErrorBody; public readonly originalMessage?: string; public constructor( message: string, statusCode: number, code: string, - options: { + options: { operation?: string; service?: string; path?: string; - errorBody?: any; + errorBody?: MicroserviceErrorBody; originalMessage?: string; originalError?: Error; } = {} ) {Add this interface near the top of the file (after imports):
+interface MicroserviceErrorBody { + message?: string; + error?: string; + details?: unknown; + errors?: unknown; +}
39-58: Narrow return type; avoid leaking arbitrary shapes.Return
Record<string, unknown>and keep the response surface minimal. This is non‑breaking but improves type hygiene.- public override toResponse(): Record<string, any> { + public override toResponse(): Record<string, unknown> { const response = super.toResponse();
42-55: Consider sanitizing passthrough fields to prevent PII/oversharing.Passing through
details/errorsfrom upstream may leak sensitive info or large payloads. Gate or scrub these for 5xx responses and/or production, and cap payload size.Example approach (conceptual; adapt to your BaseApiError API and env handling):
- Only include
details/errorswhenstatusCode < 500.- In production, include only whitelisted keys or an error trace ID.
- Truncate oversized arrays/strings to a safe maximum.
60-66: Log context: watch for sensitive data; consider redaction.
error_bodyin logs can contain PII. Recommend redacting known sensitive fields and/or logging a hash/trace ID instead in production.
71-91: Useunknownfor errorBody and normalize before property access.rg shows the factory is only called from apps/lfx-pcc/src/server/services/microservice-proxy.service.ts (≈lines 45 & 82). Changing the signature to
errorBody: unknownand guarding withtypeof errorBody === 'object' && errorBody !== null(then treating it as a narrow shape orRecord<string, unknown>and extracting.message/.erroronly if they are strings) is safe — callers need no changes. Leave directnew MicroserviceError(...)usages untouched.packages/shared/src/constants/meeting.constants.ts (1)
271-275: Tighten types for option arrays.Mark these arrays readonly with literal types to catch regressions at compile time.
-export const RECURRENCE_PATTERN_TYPE_OPTIONS = [ +export const RECURRENCE_PATTERN_TYPE_OPTIONS = [ { label: 'Days', value: 'daily' }, { label: 'Weeks', value: 'weekly' }, { label: 'Months', value: 'monthly' }, -]; +] as const; -export const RECURRENCE_END_TYPE_OPTIONS = [ +export const RECURRENCE_END_TYPE_OPTIONS = [ { label: 'Never', value: 'never' }, { label: 'On date', value: 'date' }, { label: 'After number of occurrences', value: 'occurrences' }, -]; +] as const; -export const RECURRENCE_MONTHLY_TYPE_OPTIONS = [ +export const RECURRENCE_MONTHLY_TYPE_OPTIONS = [ { label: 'Day of month', value: 'dayOfMonth' }, { label: 'Day of week', value: 'dayOfWeek' }, -]; +] as const;Also applies to: 281-285, 291-295
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.html (1)
205-208: Add data-testid for the new Recurrence row (testability guideline).- <div class="col-span-2"> + <div class="col-span-2" data-testid="resources-summary-recurrence-row"> <span class="text-slate-500">Recurrence:</span> <span class="ml-1 text-slate-900">{{ recurrenceLabel() }}</span> </div>apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.html (1)
50-57: Don’t rely on index for weekly selection; use day.value.Index-coupling is brittle if the order changes. Prefer passing/using the actual day value.
- [class.bg-blue-50]="isWeeklyDaySelected(i)" - [class.border-blue-300]="isWeeklyDaySelected(i)" + [class.bg-blue-50]="isWeeklyDaySelected(day.value)" + [class.border-blue-300]="isWeeklyDaySelected(day.value)" [attr.data-testid]="'recurrence-day-' + day.label.toLowerCase()"> - <input type="checkbox" [checked]="isWeeklyDaySelected(i)" (change)="onWeeklyDayChange(i, $any($event.target).checked)" class="sr-only" /> + <input type="checkbox" [checked]="isWeeklyDaySelected(day.value)" (change)="onWeeklyDayChange(day.value, $any($event.target).checked)" class="sr-only" />packages/shared/src/interfaces/meeting.interface.ts (1)
455-464: Export shared UI union types; day-mapping verified
- Add exported aliases in packages/shared/src/interfaces/meeting.interface.ts (e.g. RecurrencePatternType = 'daily'|'weekly'|'monthly'; RecurrenceMonthlyType = 'dayOfMonth'|'dayOfWeek'; RecurrenceEndType = 'never'|'date'|'occurrences') and replace inline unions in meeting-recurrence-pattern.component.ts, meeting-resources-summary.component.ts, recurrence-summary.pipe.ts, meeting-manage.component.ts, etc.
- Verified UI 0–6 ↔ API 1–7 conversions are present and consistent (uses parseInt(...) - 1 and getDay() + 1) in meeting.utils.ts, meeting-recurrence-pattern.component.ts, meeting-resources-summary.component.ts, meeting-details.component.ts, date-time.utils.ts — no change required to mapping.
apps/lfx-pcc/src/app/shared/pipes/recurrence-summary.pipe.ts (3)
5-6: Replace magic numbers with RecurrenceType enum.Avoid 1/2/3 literals for
recurrence.type. UseRecurrenceTypefor clarity and type‑safety.import { Pipe, PipeTransform } from '@angular/core'; -import { buildRecurrenceSummary, CustomRecurrencePattern, MeetingRecurrence } from '@lfx-pcc/shared'; +import { buildRecurrenceSummary, CustomRecurrencePattern, MeetingRecurrence } from '@lfx-pcc/shared'; +import { RecurrenceType } from '@lfx-pcc/shared/enums'; @@ - if (recurrence.type === 1) patternType = 'daily'; - else if (recurrence.type === 2) patternType = 'weekly'; - else if (recurrence.type === 3) patternType = 'monthly'; + if (recurrence.type === RecurrenceType.DAILY) patternType = 'daily'; + else if (recurrence.type === RecurrenceType.WEEKLY) patternType = 'weekly'; + else if (recurrence.type === RecurrenceType.MONTHLY) patternType = 'monthly';Also applies to: 34-36
47-50: End-type detection should allow zero and be explicit.
else if (recurrence.end_times)treats 0 as falsy. Prefer explicit numeric check.- else if (recurrence.end_times) endType = 'occurrences'; + else if (typeof recurrence.end_times === 'number' && recurrence.end_times >= 0) endType = 'occurrences';
52-55: Harden weekly_days parsing (radix, sanitize).Add radix 10 and guard invalid/oo‑b values.
- if (recurrence.weekly_days) { - weeklyDaysArray = recurrence.weekly_days.split(',').map((d) => parseInt(d.trim()) - 1); - } + if (recurrence.weekly_days) { + weeklyDaysArray = recurrence.weekly_days + .split(',') + .map((d) => parseInt(d.trim(), 10)) + .filter((n) => Number.isInteger(n)) + .map((n) => n - 1) + .filter((n) => n >= 0 && n <= 6); + }apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.ts (3)
241-251: Avoid duplicating recurrence conversion logic; centralize it.
convertToCustomPatternduplicates the pipe’s conversion. Extract a sharedtoCustomRecurrencePattern(recurrence: MeetingRecurrence): CustomRecurrencePatternin@lfx-pcc/shared/utilsand reuse here and in the pipe to keep behavior consistent.I can draft the shared util and wire both call sites.
281-283: Harden weekly_days parsing (radix, sanitize).- if (recurrenceObject.weekly_days) { - weeklyDaysArray = recurrenceObject.weekly_days.split(',').map((d: string) => parseInt(d.trim()) - 1); - } + if (recurrenceObject.weekly_days) { + weeklyDaysArray = recurrenceObject.weekly_days + .split(',') + .map((d: string) => parseInt(d.trim(), 10)) + .filter((n) => Number.isInteger(n)) + .map((n) => n - 1) + .filter((n) => n >= 0 && n <= 6); + }
10-13: Type the converter input; avoidany.-import { CustomRecurrencePattern, MeetingAttachment, PendingAttachment } from '@lfx-pcc/shared/interfaces'; +import { CustomRecurrencePattern, MeetingAttachment, PendingAttachment, MeetingRecurrence } from '@lfx-pcc/shared/interfaces'; @@ - private convertToCustomPattern(recurrenceObject: any): CustomRecurrencePattern { + private convertToCustomPattern(recurrenceObject: MeetingRecurrence): CustomRecurrencePattern {Also applies to: 264-292
packages/shared/src/utils/meeting.utils.ts (1)
36-45: Stable ordering and de-duplication of weekly days.Ensure consistent summaries regardless of selection order, and remove duplicates.
- if (pattern.weeklyDaysArray) { - selectedDays = pattern.weeklyDaysArray - .map((dayIndex: number) => RECURRENCE_DAYS_OF_WEEK[dayIndex]?.fullLabel) - .filter((day: string | undefined) => day !== undefined); + if (pattern.weeklyDaysArray) { + const uniqueSorted = Array.from(new Set(pattern.weeklyDaysArray)).sort((a, b) => a - b); + selectedDays = uniqueSorted + .map((dayIndex: number) => RECURRENCE_DAYS_OF_WEEK[dayIndex]?.fullLabel) + .filter((day: string | undefined) => day !== undefined); } else if (pattern.weekly_days) { // Parse from comma-separated string and convert from 1-based to 0-based - const days = pattern.weekly_days.split(',').map((d) => parseInt(d.trim()) - 1); - selectedDays = days.map((dayIndex: number) => RECURRENCE_DAYS_OF_WEEK[dayIndex]?.fullLabel).filter((day: string | undefined) => day !== undefined); + const days = pattern.weekly_days + .split(',') + .map((d) => parseInt(d.trim(), 10)) + .filter((n) => Number.isInteger(n)) + .map((n) => n - 1) + .filter((n) => n >= 0 && n <= 6) + .sort((a, b) => a - b); + selectedDays = days + .map((dayIndex: number) => RECURRENCE_DAYS_OF_WEEK[dayIndex]?.fullLabel) + .filter((day: string | undefined) => day !== undefined); }apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts (1)
858-874: Custom recurrence detection is minimal (OK), but consider monthly “dayOfMonth/dayOfWeek” toggles.If monthly patterns with non-default intervals (e.g., every 2 months) should be classified as custom, add a monthly check.
- // Custom interval (not 1) + // Custom interval (not 1) for any type if (recurrence.repeat_interval && recurrence.repeat_interval !== 1) return true;apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts (1)
17-18: Use RecurrenceType enum instead of numeric literals.-import { getWeekOfMonth } from '@lfx-pcc/shared/utils'; +import { RecurrenceType } from '@lfx-pcc/shared/enums'; +import { getWeekOfMonth } from '@lfx-pcc/shared/utils'; @@ case 'daily': recurrenceFormGroup.patchValue({ - type: 1, // Daily + type: RecurrenceType.DAILY, repeat_interval: 1, }); break; @@ case 'weekly': recurrenceFormGroup.patchValue({ - type: 2, // Weekly + type: RecurrenceType.WEEKLY, repeat_interval: 1, weekly_days: String(startDate.getDay() + 1), // Convert 0-6 to 1-7 }); break; @@ case 'weekdays': recurrenceFormGroup.patchValue({ - type: 2, // Weekly + type: RecurrenceType.WEEKLY, repeat_interval: 1, weekly_days: '2,3,4,5,6', // Monday to Friday }); break; @@ case 'monthly_nth': case 'monthly_last': { const { weekOfMonth, isLastWeek } = getWeekOfMonth(startDate); recurrenceFormGroup.patchValue({ - type: 3, // Monthly + type: RecurrenceType.MONTHLY, repeat_interval: 1, monthly_week: isLastWeek ? -1 : weekOfMonth, monthly_week_day: startDate.getDay() + 1, // Convert 0-6 to 1-7 }); break; }Also applies to: 375-407
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts (8)
65-80: Numeric sort comparator (minor).Use numeric comparator to avoid lexicographic sort surprises.
- if (checked) { - newDays = [...currentDays, dayIndex].sort(); + if (checked) { + newDays = [...currentDays, dayIndex].sort((a, b) => a - b);
110-136: End type default is hardcoded; prefer a constant and minimal patching.Avoid magic number 10; define a file‑level constant. Also only patch when a value actually changes to reduce churn on valueChanges.
+// default number of occurrences when switching to 'occurrences' +const DEFAULT_END_TIMES = 10; @@ - if (endType === 'occurrences') { - // If there's no existing end_times value, set default of 10 - if (!currentValue.end_times) { - updateData.end_times = 10; - } + if (endType === 'occurrences') { + if (!currentValue.end_times) { + updateData.end_times = DEFAULT_END_TIMES; + }
142-160: Drive start‑date updates from the canonical recurrence.type, not UI signal.patternType() can drift from data; branching on recurrence.value.type is more robust.
- const patternType = this.patternType(); - const currentValue = recurrenceForm.value; - if (!currentValue.type) return; // No recurrence pattern set - - switch (patternType) { - case 'daily': + const currentValue = recurrenceForm.value; + switch (currentValue.type) { + case 1: // daily // Daily patterns don't need updates for date changes break; - - case 'weekly': + case 2: // weekly this.updateWeeklyPatternForNewDate(newDate, currentValue); break; - - case 'monthly': + case 3: // monthly this.updateMonthlyPatternForNewDate(newDate); break; }
166-180: Respect monthlyTypeUI when switching pattern type and guard start date.Switching to monthly currently defaults to dayOfMonth; align with UI selection and avoid NPE when startDate is null.
- recurrenceForm.patchValue({ - type: type, - // Clear pattern-specific fields when changing pattern type - weekly_days: patternType === 'weekly' ? this.getDefaultWeeklyDays() : null, - monthly_day: patternType === 'monthly' ? this.startDate().getDate() : null, - monthly_week: null, - monthly_week_day: null, - }); + const start = this.startDate(); + const monthlyTypeUI = recurrenceForm.get('monthlyTypeUI')?.value || 'dayOfMonth'; + const basePatch: any = { type }; + basePatch.weekly_days = patternType === 'weekly' ? this.getDefaultWeeklyDays() : null; + if (patternType === 'monthly' && start) { + if (monthlyTypeUI === 'dayOfMonth') { + basePatch.monthly_day = start.getDate(); + basePatch.monthly_week = null; + basePatch.monthly_week_day = null; + } else { + const { weekOfMonth, isLastWeek } = getWeekOfMonth(start); + basePatch.monthly_day = null; + basePatch.monthly_week = isLastWeek ? -1 : weekOfMonth; + basePatch.monthly_week_day = start.getDay() + 1; + } + } else { + basePatch.monthly_day = null; + basePatch.monthly_week = null; + basePatch.monthly_week_day = null; + } + recurrenceForm.patchValue(basePatch);
234-270: Type recurrence shape and harden weekly_days parsing.Use shared interface to avoid any, and filter/uniq parsed days.
- private updateUISignals(recurrenceValue: any): void { + private updateUISignals(recurrenceValue: Partial<MeetingRecurrence>): void { @@ - if (recurrenceValue.weekly_days) { - const daysArray = recurrenceValue.weekly_days.split(',').map((d: string) => parseInt(d.trim()) - 1); - this.weeklyDaysArray.set(daysArray); + if (recurrenceValue.weekly_days) { + const daysArray = Array.from( + new Set( + recurrenceValue.weekly_days + .split(',') + .map((d: string) => Number.parseInt(d.trim(), 10)) + .filter((n) => Number.isFinite(n)) + .map((n) => n - 1) + ) + ).sort((a, b) => a - b); + this.weeklyDaysArray.set(daysArray); } else { this.weeklyDaysArray.set([]); }Add import:
import { RECURRENCE_DAYS_OF_WEEK, @@ } from '@lfx-pcc/shared/constants'; import { getWeekOfMonth } from '@lfx-pcc/shared/utils'; +import type { MeetingRecurrence } from '@lfx-pcc/shared/interfaces';
288-293: Default weekly day should not assume startDate exists.Fallback to today when startDate is not yet set.
- const startDate = this.startDate(); - return String(startDate.getDay() + 1); // Convert 0-6 to 1-7 + const start = this.startDate() ?? new Date(); + return String(start.getDay() + 1); // Convert 0-6 to 1-7
294-309: updateWeeklyPatternForNewDate: minor typing and safety.Type currentValue for clarity and early‑return only when weekly_days is a non‑empty string.
- private updateWeeklyPatternForNewDate(newDate: Date, currentValue: any): void { + private updateWeeklyPatternForNewDate(newDate: Date, currentValue: Partial<MeetingRecurrence>): void { const recurrenceForm = this.recurrenceForm(); - if (!recurrenceForm || !currentValue.weekly_days) return; + if (!recurrenceForm || !currentValue.weekly_days?.trim()) return;
21-26: Nit: consider adding changeDetection: OnPush.Standalone, signal‑driven component benefits from OnPush to minimize checks.
@Component({ selector: 'lfx-meeting-recurrence-pattern', standalone: true, imports: [CommonModule, ReactiveFormsModule, CalendarComponent, InputTextComponent, RadioButtonComponent, SelectComponent], templateUrl: './meeting-recurrence-pattern.component.html', + changeDetection: ChangeDetectionStrategy.OnPush, })Add import:
-import { Component, computed, DestroyRef, inject, input, OnInit, signal, Signal } from '@angular/core'; +import { Component, computed, DestroyRef, inject, input, OnInit, signal, Signal, ChangeDetectionStrategy } from '@angular/core';
📜 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 (19)
.github/workflows/e2e-tests.yml(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts(2 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.html(2 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts(8 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts(6 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.ts(2 hunks)apps/lfx-pcc/src/app/shared/components/radio-button/radio-button.component.html(2 hunks)apps/lfx-pcc/src/app/shared/components/radio-button/radio-button.component.ts(0 hunks)apps/lfx-pcc/src/app/shared/components/select/select.component.ts(0 hunks)apps/lfx-pcc/src/app/shared/pipes/recurrence-summary.pipe.ts(1 hunks)apps/lfx-pcc/src/server/errors/microservice.error.ts(1 hunks)packages/shared/src/constants/meeting.constants.ts(1 hunks)packages/shared/src/interfaces/meeting.interface.ts(1 hunks)packages/shared/src/utils/index.ts(1 hunks)packages/shared/src/utils/meeting.utils.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/lfx-pcc/src/app/shared/components/select/select.component.ts
- apps/lfx-pcc/src/app/shared/components/radio-button/radio-button.component.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer TypeScript interfaces over union types when modeling shapes and contracts
Files:
packages/shared/src/utils/index.tsapps/lfx-pcc/src/server/errors/microservice.error.tspackages/shared/src/interfaces/meeting.interface.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tspackages/shared/src/utils/meeting.utils.tsapps/lfx-pcc/src/app/shared/pipes/recurrence-summary.pipe.tspackages/shared/src/constants/meeting.constants.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
packages/shared/src/utils/index.tsapps/lfx-pcc/src/server/errors/microservice.error.tspackages/shared/src/interfaces/meeting.interface.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tspackages/shared/src/utils/meeting.utils.tsapps/lfx-pcc/src/app/shared/pipes/recurrence-summary.pipe.tspackages/shared/src/constants/meeting.constants.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts
{apps/lfx-pcc/src,packages/shared/src}/**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid barrel exports; use direct imports instead of index.ts barrels
Files:
packages/shared/src/utils/index.ts
**/*.{ts,tsx,js,jsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
packages/shared/src/utils/index.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.htmlapps/lfx-pcc/src/app/shared/components/radio-button/radio-button.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-pcc/src/server/errors/microservice.error.tspackages/shared/src/interfaces/meeting.interface.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tspackages/shared/src/utils/meeting.utils.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.htmlapps/lfx-pcc/src/app/shared/pipes/recurrence-summary.pipe.tspackages/shared/src/constants/meeting.constants.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts
apps/lfx-pcc/src/**/*.{ts,html}
📄 CodeRabbit inference engine (CLAUDE.md)
Use LFX wrapper components instead of importing PrimeNG components directly
Files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.htmlapps/lfx-pcc/src/app/shared/components/radio-button/radio-button.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-pcc/src/server/errors/microservice.error.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.htmlapps/lfx-pcc/src/app/shared/pipes/recurrence-summary.pipe.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts
apps/lfx-pcc/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-pcc/src/**/*.html: Add data-testid attributes to new components for reliable test targeting
Follow data-testid naming convention: [section]-[component]-[element]
Files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.htmlapps/lfx-pcc/src/app/shared/components/radio-button/radio-button.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.html
apps/lfx-pcc/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-pcc/src/**/*.ts: In the Angular app, use direct imports for standalone components (no barrel imports)
When defining types for PrimeNG usage, reference PrimeNG’s component interfaces
Files:
apps/lfx-pcc/src/server/errors/microservice.error.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-pcc/src/app/shared/pipes/recurrence-summary.pipe.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts
packages/shared/src/interfaces/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all shared interfaces in the shared package
Files:
packages/shared/src/interfaces/meeting.interface.ts
packages/shared/src/constants/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all reusable constants (design tokens, etc.) in the shared package
Files:
packages/shared/src/constants/meeting.constants.ts
🧠 Learnings (3)
📚 Learning: 2025-09-05T18:09:48.792Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.792Z
Learning: Applies to {apps/lfx-pcc/src,packages/shared/src}/**/index.ts : Avoid barrel exports; use direct imports instead of index.ts barrels
Applied to files:
packages/shared/src/utils/index.ts
📚 Learning: 2025-09-05T18:09:48.792Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.792Z
Learning: Applies to apps/lfx-pcc/src/**/*.{ts,html} : Use LFX wrapper components instead of importing PrimeNG components directly
Applied to files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts
📚 Learning: 2025-09-05T18:09:48.792Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.792Z
Learning: Applies to apps/lfx-pcc/src/**/*.ts : When defining types for PrimeNG usage, reference PrimeNG’s component interfaces
Applied to files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.ts
🧬 Code graph analysis (6)
packages/shared/src/utils/meeting.utils.ts (2)
packages/shared/src/interfaces/meeting.interface.ts (2)
CustomRecurrencePattern(455-464)RecurrenceSummary(470-477)packages/shared/src/constants/meeting.constants.ts (2)
RECURRENCE_DAYS_OF_WEEK(300-308)RECURRENCE_WEEKLY_ORDINALS(314-320)
apps/lfx-pcc/src/app/shared/pipes/recurrence-summary.pipe.ts (2)
packages/shared/src/interfaces/meeting.interface.ts (2)
MeetingRecurrence(25-42)CustomRecurrencePattern(455-464)packages/shared/src/utils/meeting.utils.ts (1)
buildRecurrenceSummary(12-104)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts (1)
packages/shared/src/utils/date-time.utils.ts (1)
generateRecurrenceObject(306-354)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.ts (2)
packages/shared/src/utils/meeting.utils.ts (1)
buildRecurrenceSummary(12-104)packages/shared/src/interfaces/meeting.interface.ts (1)
CustomRecurrencePattern(455-464)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts (2)
packages/shared/src/constants/meeting.constants.ts (5)
RECURRENCE_PATTERN_TYPE_OPTIONS(271-275)RECURRENCE_END_TYPE_OPTIONS(281-285)RECURRENCE_MONTHLY_TYPE_OPTIONS(291-294)RECURRENCE_DAYS_OF_WEEK(300-308)RECURRENCE_WEEKLY_ORDINALS(314-320)packages/shared/src/utils/date-time.utils.ts (1)
getWeekOfMonth(281-297)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts (1)
packages/shared/src/utils/date-time.utils.ts (1)
getWeekOfMonth(281-297)
⏰ 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 (17)
.github/workflows/e2e-tests.yml (1)
162-168: Ensure M2M audience matches Auth0 API Identifier exactly (trailing slash matters)M2M_AUTH_AUDIENCE in .github/workflows/e2e-tests.yml (lines ~162-168) includes a trailing slash; Auth0 treats the audience literally, so a mismatch (presence/absence of trailing slash) can cause invalid_audience/401. Also confirm that PCC_AUTH0_AUDIENCE and M2M_AUTH_AUDIENCE intentionally differ.
Suggested fix if the Auth0 API Identifier has no trailing slash:
- echo "M2M_AUTH_AUDIENCE=https://lfx-api.dev.v2.cluster.linuxfound.info/" >> $GITHUB_ENV + echo "M2M_AUTH_AUDIENCE=https://lfx-api.dev.v2.cluster.linuxfound.info" >> $GITHUB_ENVAttempted repo search returned "No files were searched." Verify occurrences of M2M_AUTH_AUDIENCE and the Auth0 API Identifier (or run this search):
rg -n -C2 --hidden --no-ignore 'M2M_AUTH_AUDIENCE|lfx-api\.dev\.v2\.cluster\.linuxfound\.info' || trueapps/lfx-pcc/src/app/shared/components/radio-button/radio-button.component.html (1)
7-8: Removal of [disabled] is fine for reactive forms — confirm CVA flow works end-to-end.File: apps/lfx-pcc/src/app/shared/components/radio-button/radio-button.component.html (lines 7–8)
Reactive Forms propagate disabled via ControlValueAccessor; dropping [disabled] is OK if PrimeNG RadioButton’s setDisabledState is honored — verify that FormControl.disable() updates the rendered radio.
The ripgrep run returned no matches, so I could not confirm there are no callers still passing [disabled]. Verify usages manually or run a broader search (example): rg -nP "<[^>](radio-button|app-radio-button)[^>]\s+[?disabled\b" -S
apps/lfx-pcc/src/server/errors/microservice.error.ts (1)
4-4: Import reorder is harmless; keep if it satisfies lint rules.No functional change; named import order is irrelevant. Proceed if this matches your import‑ordering conventions.
If your linter enforces sorted imports, run the formatter to avoid churn in future diffs.
packages/shared/src/constants/meeting.constants.ts (2)
314-320: LGTM: ordinals align with monthly_week semantics (1–4, -1 last).
300-308: Resolved — UI 0..6 ↔ API 1..7 conversions are implemented.
- packages/shared/src/utils/meeting.utils.ts — parses API weekly_days using .split(...).map(d => parseInt(d.trim()) - 1) and uses pattern.monthly_week_day - 1 for monthly day names.
- apps/lfx-pcc/.../meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts — UI writes weekly_days with newDays.map(d => d + 1).join(','); updateUISignals reads weekly_days and sets weeklyDaysArray via parseInt(...)-1; monthly fields set with getDay() + 1.
- apps/lfx-pcc/.../meeting-details/meeting-details.component.ts — patches weekly_days and monthly_week_day using getDay() + 1.
- packages/shared/src/utils/date-time.utils.ts — recurrence payload includes weekly_days (dayOfWeek.toString()) when building API payloads.
No code changes required; conversions are handled consistently across components.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (1)
17-17: LGTM: recurrence summary pipe integration is correct.Import and standalone registration look good.
Also applies to: 40-41
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html (1)
31-34: LGTM: tooltip shows human-readable recurrence summary with test id.packages/shared/src/interfaces/meeting.interface.ts (1)
470-477: LGTM: RecurrenceSummary shape is clear and useful for display.apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.html (2)
278-283: Approve — custom recurrence component gated and imported directly. The template uses showCustomRecurrence() and meeting-details.component.ts imports MeetingRecurrencePatternComponent from ../meeting-recurrence-pattern/meeting-recurrence-pattern.component; no changes required.
269-269: Confirm parent FormGroup provides 'recurrenceType' (or add a local initialization/alias).
This component reads/sets this.form().get('recurrenceType') (meeting-details.component.ts) but does not create that control — the FormGroup is supplied via input. Confirm the parent form was updated to add/rename the control to 'recurrenceType' (and preserve/alias the existing 'recurrence' FormGroup used for recurrence details) to avoid template/runtime errors.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts/.html — control="recurrenceType".apps/lfx-pcc/src/app/shared/pipes/recurrence-summary.pipe.ts (1)
11-29: LGTM overall.Nice standalone pipe; summary routing by format is clear.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.ts (1)
235-262: LGTM: concise summary generation path.Custom vs. simple handling and labels mapping look good.
packages/shared/src/utils/meeting.utils.ts (1)
1-12: LGTM overall.Utility is cohesive and well-scoped.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts (1)
333-352: Monthly toggle on start-date change: verify no oscillation.Changing
recurrenceTypeinside the startDate subscription triggersupdateRecurrenceFormGroup. Ensure it doesn’t cause flicker or unintended resets when moving between last/nth weeks.Would you like a small harness to simulate end-of-month date changes and log the form diffs?
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts (3)
1-3: License header present and correct.MIT/SPDX header included as required.
8-12: Good: using LFX wrapper components and direct imports.No direct PrimeNG imports; paths are specific (no barrel for standalones).
54-61: Overall: solid feature encapsulation.Good use of signals, DestroyRef, and Reactive Forms; aligns with PR goals.
Please confirm validators ensure at least one weekly day is selected when type=weekly and that monthly fields are mutually exclusive in the form group.
...x-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts
Show resolved
Hide resolved
...ect/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.html
Show resolved
Hide resolved
...ect/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.html
Show resolved
Hide resolved
...oject/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts
Show resolved
Hide resolved
...oject/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts
Show resolved
Hide resolved
...oject/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts
Show resolved
Hide resolved
apps/lfx-pcc/src/app/shared/components/radio-button/radio-button.component.html
Show resolved
Hide resolved
- Update onWeeklyDayChange method to accept Event instead of boolean - Remove need for $any() type casting in template - Add validation for parseInt conversion to prevent NaN values - Use Number() with isNaN checks to filter invalid weekly_days strings - Prevent accessing undefined array indices from malformed data LFXV2-483 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
🧹 Deployment RemovedThe deployment for PR #85 has been removed. |
Summary
This PR implements two distinct pieces of work:
🆕 Custom Recurrence Pattern Feature (LFXV2-484)
meeting-recurrence-patternwith comprehensive UI supportmeeting-managecomponent with custom recurrence support🔧 Code Refactoring & Cleanup (LFXV2-483)
getWeekOfMonthimplementations with shared utility importssetAiEstimatedDurationandsetTemplateDurationinto single methodFiles Changed
New Files
meeting-recurrence-pattern.component.ts- Complete custom recurrence componentmeeting-recurrence-pattern.component.html- Component templaterecurrence-summary.pipe.ts- Display formatting pipemeeting.utils.ts- Shared meeting utilitiesModified Files
meeting-manage.component.ts- Custom recurrence integration + bug fixmeeting-details.component.ts- Refactored to use shared utilitiesmeeting.constants.ts- Extended recurrence configurationTest plan
Related JIRA Tickets
🤖 Generated with Claude Code