-
Notifications
You must be signed in to change notification settings - Fork 0
feat(meetings): add ability to cancel meeting occurrences #132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
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 support to cancel individual occurrences of recurring meetings: new UI dialogs for delete-scope selection and occurrence cancellation, frontend cancelOccurrence API call, backend DELETE endpoint/service, a MeetingOccurrence.is_cancelled field, and utilities to ignore cancelled occurrences in scheduling logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Card as Meeting Card UI
participant TypeModal as Delete Type Selection
participant CancelModal as Cancel Occurrence Confirmation
participant Client as Frontend Meeting Service
participant API as Backend API
User->>Card: Click "Delete" on recurring meeting
Card->>TypeModal: Open delete-type selection
User->>TypeModal: Select "Cancel This Occurrence"
TypeModal-->>Card: Return {deleteType:'occurrence'}
Card->>CancelModal: Open cancel-occurrence confirmation
User->>CancelModal: Confirm
CancelModal->>Client: cancelOccurrence(meetingId, occurrenceId)
Client->>API: DELETE /meetings/{uid}/occurrences/{occurrenceId}
alt success (204)
API-->>Client: 204 No Content
Client-->>CancelModal: success
CancelModal-->>User: close {confirmed:true}
Card->>Card: emit meetingDeleted -> refresh list
else error (404/403/500/0)
API-->>Client: error
Client-->>CancelModal: mapped error
CancelModal-->>User: close {confirmed:false,error}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Potential focus areas:
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements the ability to cancel individual occurrences of recurring meetings through a two-step modal flow. Users can now choose between canceling a single occurrence or deleting the entire meeting series, with cancelled occurrences automatically filtered from all displays.
Key Changes:
- Added backend endpoint and service methods for canceling meeting occurrences with ETag-based concurrency control
- Implemented frontend modal components for occurrence cancellation with comprehensive error handling
- Added filtering logic to hide cancelled occurrences from all meeting displays
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/meeting.interface.ts | Added is_cancelled field to MeetingOccurrence interface |
| packages/shared/src/utils/meeting.utils.ts | Added getActiveOccurrences() utility and updated getCurrentOrNextOccurrence() to filter cancelled occurrences |
| apps/lfx-one/src/server/routes/meetings.route.ts | Added DELETE endpoint for canceling meeting occurrences |
| apps/lfx-one/src/server/controllers/meeting.controller.ts | Added cancelOccurrence controller method with validation |
| apps/lfx-one/src/server/services/meeting.service.ts | Implemented cancelOccurrence service method with ETag concurrency control |
| apps/lfx-one/src/app/shared/services/meeting.service.ts | Added frontend cancelOccurrence method with error handling |
| apps/lfx-one/src/app/modules/project/meetings/components/meeting-delete-type-selection/* | New component for choosing between occurrence/series deletion |
| apps/lfx-one/src/app/modules/project/meetings/components/meeting-cancel-occurrence-confirmation/* | New component for confirming occurrence cancellation |
| apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts | Enhanced delete flow with modal chaining for recurring meetings |
| apps/lfx-one/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.html | Fixed button types to prevent page refresh |
| apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts | Filters cancelled occurrences from dashboard displays |
| apps/lfx-one/src/server/server.ts | Removed update:current_user_metadata scope from Auth0 configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts
Outdated
Show resolved
Hide resolved
Implement modal flow to cancel individual occurrences of recurring meetings: - Add delete type selection modal for recurring meetings - Add cancel occurrence confirmation modal with occurrence details - Implement ETag-based concurrency control for cancellation API - Filter cancelled occurrences from all meeting displays - Add centralized getActiveOccurrences utility function in shared package - Update frontend service with cancelOccurrence method - Add backend route, controller, and service for cancel occurrence API - Fix modal subscription logic to prevent unintended page refreshes Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
67b0433 to
46e6cd9
Compare
| import { MeetingTimePipe } from '@pipes/meeting-time.pipe'; | ||
| import { HttpErrorResponse } from '@angular/common/http'; | ||
|
|
||
| export interface MeetingCancelOccurrenceResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in the shared package under meeting interfaces
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. Done.
| <div class="space-y-3 mb-6"> | ||
| <div | ||
| class="border rounded-lg p-4 cursor-pointer transition-all" | ||
| [class.border-blue-500]="selectedType === 'occurrence'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use ngclass instead of individual [class.] values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Done.
| [class.fa-circle-dot]="selectedType === 'occurrence'" | ||
| [class.fa-circle]="selectedType !== 'occurrence'" | ||
| [class.text-blue-500]="selectedType === 'occurrence'" | ||
| [class.text-gray-400]="selectedType !== 'occurrence'"></i> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use ngClass instead
| [class.border-blue-500]="selectedType === 'series'" | ||
| [class.bg-blue-50]="selectedType === 'series'" | ||
| [class.border-gray-200]="selectedType !== 'series'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use ngclass
| [class.fa-circle-dot]="selectedType === 'series'" | ||
| [class.fa-circle]="selectedType !== 'series'" | ||
| [class.text-blue-500]="selectedType === 'series'" | ||
| [class.text-gray-400]="selectedType !== 'series'"></i> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use ngclass
| catchError((error) => { | ||
| console.error(`Failed to cancel occurrence ${occurrenceId} for meeting ${meetingId}:`, error); | ||
| return throwError(() => error); | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catch error should be handled by caller
Minor improvements to the cancel occurrence feature: - Move MeetingCancelOccurrenceResult interface to shared package - Refactor delete type selection to use ngClass instead of individual class bindings - Remove error handling from cancelOccurrence service method (handled in component) - Update imports to use shared interface These changes improve code organization and maintainability. Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
Signed-off-by: Andres Tobon <andrest2455@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/lfx-one/src/app/shared/services/meeting.service.ts (1)
202-204: API method looks good; keep errors bubbling to caller.Returning the raw DELETE observable with take(1) matches the prior guidance to let callers handle errors. Keep it consistent with the new confirmation dialog’s error mapping.
🧹 Nitpick comments (2)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-cancel-occurrence-confirmation/meeting-cancel-occurrence-confirmation.component.ts (2)
25-27: Harden dialog data handling (type + runtime guard).Accessing config.data.* at property init can throw if data is missing. Cast once and guard to avoid runtime errors.
Apply this diff locally and add a constructor guard:
- public readonly meeting: Meeting = this.config.data.meeting; - public readonly occurrence: MeetingOccurrence = this.config.data.occurrence; + private readonly data = this.config.data as Partial<{ meeting: Meeting; occurrence: MeetingOccurrence }>; + public readonly meeting: Meeting | undefined = this.data?.meeting; + public readonly occurrence: MeetingOccurrence | undefined = this.data?.occurrence; + + public constructor() { + if (!this.meeting || !this.occurrence) { + this.dialogRef.close({ confirmed: false, error: 'Invalid dialog data.' }); + } + }
33-58: Use finalize to avoid duplicate state toggles; map concurrency errors.Simplify spinner handling and cover 409/412 responses.
Apply this diff:
public onConfirm(): void { this.isCanceling.set(true); - this.meetingService.cancelOccurrence(this.meeting.uid, this.occurrence.occurrence_id).subscribe({ - next: () => { - this.isCanceling.set(false); - this.dialogRef.close({ confirmed: true }); - }, - error: (error: HttpErrorResponse) => { - this.isCanceling.set(false); + this.meetingService + .cancelOccurrence(this.meeting!.uid, this.occurrence!.occurrence_id) + .pipe(finalize(() => this.isCanceling.set(false))) + .subscribe({ + next: () => { + this.dialogRef.close({ confirmed: true }); + }, + error: (error: HttpErrorResponse) => { let errorMessage = 'Failed to cancel occurrence. Please try again.'; if (error.status === 404) { errorMessage = 'Meeting occurrence not found.'; } else if (error.status === 403) { errorMessage = 'You do not have permission to cancel this occurrence.'; + } else if (error.status === 409 || error.status === 412) { + errorMessage = 'Conflict detected. This occurrence may have changed or was already canceled. Please refresh and try again.'; } else if (error.status === 500) { errorMessage = 'Server error occurred while canceling occurrence.'; } else if (error.status === 0) { errorMessage = 'Network error. Please check your connection.'; } this.dialogRef.close({ confirmed: false, error: errorMessage }); - }, - }); + }, + }); }Additionally, import finalize:
- import { Component, inject, signal } from '@angular/core'; + import { Component, inject, signal } from '@angular/core'; + import { finalize } from 'rxjs';
📜 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 (6)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-cancel-occurrence-confirmation/meeting-cancel-occurrence-confirmation.component.ts(1 hunks)apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts(4 hunks)apps/lfx-one/src/app/modules/project/meetings/components/meeting-delete-type-selection/meeting-delete-type-selection.component.html(1 hunks)apps/lfx-one/src/app/shared/services/meeting.service.ts(1 hunks)apps/lfx-one/src/server/services/meeting.service.ts(1 hunks)packages/shared/src/interfaces/meeting.interface.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/lfx-one/src/app/modules/project/meetings/components/meeting-delete-type-selection/meeting-delete-type-selection.component.html
- packages/shared/src/interfaces/meeting.interface.ts
- apps/lfx-one/src/server/services/meeting.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/shared/services/meeting.service.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-cancel-occurrence-confirmation/meeting-cancel-occurrence-confirmation.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/app/shared/services/meeting.service.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-cancel-occurrence-confirmation/meeting-cancel-occurrence-confirmation.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/shared/services/meeting.service.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-cancel-occurrence-confirmation/meeting-cancel-occurrence-confirmation.component.ts
🧬 Code graph analysis (2)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (3)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-delete-type-selection/meeting-delete-type-selection.component.ts (1)
MeetingDeleteTypeResult(10-12)packages/shared/src/interfaces/meeting.interface.ts (2)
Meeting(75-150)MeetingCancelOccurrenceResult(556-559)packages/shared/src/utils/meeting.utils.ts (1)
getCurrentOrNextOccurrence(120-154)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-cancel-occurrence-confirmation/meeting-cancel-occurrence-confirmation.component.ts (2)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (1)
Component(52-854)packages/shared/src/interfaces/meeting.interface.ts (2)
Meeting(75-150)MeetingOccurrence(156-169)
🪛 Biome (2.1.2)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts
[error] 43-44: Expected a statement but instead found '<<<<<<< HEAD'.
Expected a statement here.
(parse)
[error] 45-46: Expected a statement but instead found '======='.
Expected a statement here.
(parse)
[error] 47-48: Expected a statement but instead found '>>>>>>> 42e9854'.
Expected a statement here.
(parse)
[error] 48-48: numbers cannot be followed by identifiers directly after
an identifier cannot appear here
(parse)
🔇 Additional comments (1)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (1)
565-567: LGTM: confirm delete result checked before emit.Guarding on result?.confirmed avoids false positives when dialog closes without confirmation.
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts
Show resolved
Hide resolved
…ogic - Remove merge conflict markers from meeting-card component imports - Fix cancel occurrence to use current/selected occurrence instead of always using next occurrence - Add fallback to next occurrence if no specific occurrence is selected Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
🚀 Deployment StatusYour branch has been deployed to: https://ui-pr-132.dev.v2.cluster.linuxfound.info Deployment Details:
The deployment will be automatically removed when this PR is closed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (2)
504-546: Good occurrence selection; harden destructive-confirm UX.Logic correctly prefers selected occurrence and falls back to next active. For a permanent action, consider disabling mask-click dismissal to avoid accidental closes:
- closable: true, - dismissableMask: true, + closable: true, + dismissableMask: false,
470-502: Event naming is misleading but functionally correct; refactoring optional.All consumers of
meetingDeletedhandle both occurrence cancellation and series deletion equally:
- Dashboard refreshes the meeting list in both cases
- Modal simply closes the dialog
The naming is semantically imprecise (emitting "Deleted" for occurrence cancellation), but no consumer assumes full deletion semantics. Renaming to
meetingChangedormeetingUpdatedwould improve clarity, though the current implementation causes no functional issues.
📜 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 (1)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/project/meetings/components/meeting-card/meeting-card.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/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (3)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-delete-type-selection/meeting-delete-type-selection.component.ts (1)
MeetingDeleteTypeResult(10-12)packages/shared/src/interfaces/meeting.interface.ts (2)
Meeting(75-150)MeetingCancelOccurrenceResult(556-559)packages/shared/src/utils/meeting.utils.ts (1)
getCurrentOrNextOccurrence(120-154)
🔇 Additional comments (3)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (3)
23-23: LGTM: typed dialog result import is correct.Importing MeetingCancelOccurrenceResult aligns the onClose result typing.
41-45: Imports resolved and correct.Both MeetingCancelOccurrenceConfirmationComponent and MeetingDeleteTypeSelectionComponent are properly imported; prior merge conflict is clean.
562-562: LGTM: explicitresult?.confirmedcheck.Prevents truthy misfires on dialog close without confirmation.
Summary
Implements the ability to cancel individual occurrences of recurring meetings through a two-step modal flow:
Changes
Frontend
New Components:
meeting-delete-type-selection: Modal for choosing between occurrence/series deletionmeeting-cancel-occurrence-confirmation: Modal showing occurrence details with cancellation confirmationUpdated Components:
meeting-card: Enhanced delete flow with modal chaining for recurring meetingsmy-meetings: Filters out cancelled occurrences from dashboard displaysmeeting-delete-confirmation: Fixed button types to prevent page refreshService Updates:
cancelOccurrence()method in meeting serviceBackend
DELETE /meetings/:uid/occurrences/:occurrenceIdcancelOccurrencewith validation and structured loggingShared Package
is_cancelledfield toMeetingOccurrencegetActiveOccurrences()function for centralized filtering of cancelled occurrencesgetCurrentOrNextOccurrence()to filter cancelled occurrencesTechnical Details
Test Plan
Ticket
https://linuxfoundation.atlassian.net/browse/LFXV2-650
Generated with Claude Code