Skip to content

Commit 4bea0ca

Browse files
andrest50andrest50
committed
refactor(meetings): address PR review comments for recording and summary features
- Use toSignal instead of effect for recording/summary data loading - Move recording/summary initialization to ngOnInit lifecycle hook - Replace click handler with native href for recording links - Convert summary modal to use reactive forms with lfx-textarea - Move error handling from service to component callers Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
1 parent e3be355 commit 4bea0ca

File tree

6 files changed

+93
-117
lines changed

6 files changed

+93
-117
lines changed

‎apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts‎

Lines changed: 51 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,24 @@ export class MeetingCardComponent implements OnInit {
8585
public meeting: WritableSignal<Meeting | PastMeeting> = signal({} as Meeting | PastMeeting);
8686
public occurrence: WritableSignal<MeetingOccurrence | null> = signal(null);
8787
public registrantsLoading: WritableSignal<boolean> = signal(true);
88-
public recordingShareUrl: WritableSignal<string | null> = signal(null);
89-
public hasRecording: Signal<boolean> = computed(() => this.recordingShareUrl() !== null);
90-
public summaryContent: WritableSignal<string | null> = signal(null);
91-
public summaryUid: WritableSignal<string | null> = signal(null);
92-
public summaryApproved: WritableSignal<boolean> = signal(false);
93-
public hasSummary: Signal<boolean> = computed(() => this.summaryContent() !== null);
9488
private refresh$: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(false);
9589
public registrants = this.initRegistrantsList();
9690
public pastMeetingParticipants = this.initPastMeetingParticipantsList();
9791
public registrantsLabel: Signal<string> = this.initRegistrantsLabel();
92+
public recording: WritableSignal<PastMeetingRecording | null> = signal(null);
93+
public recordingShareUrl: Signal<string | null> = computed(() => {
94+
const recording = this.recording();
95+
return recording ? this.getLargestSessionShareUrl(recording) : null;
96+
});
97+
public hasRecording: Signal<boolean> = computed(() => this.recordingShareUrl() !== null);
98+
public summary: WritableSignal<PastMeetingSummary | null> = signal(null);
99+
public summaryContent: Signal<string | null> = computed(() => {
100+
const summary = this.summary();
101+
return summary?.summary_data ? summary.summary_data.edited_content || summary.summary_data.content : null;
102+
});
103+
public summaryUid: Signal<string | null> = computed(() => this.summary()?.uid || null);
104+
public summaryApproved: Signal<boolean> = computed(() => this.summary()?.approved || false);
105+
public hasSummary: Signal<boolean> = computed(() => this.summaryContent() !== null);
98106
public additionalRegistrantsCount: WritableSignal<number> = signal(0);
99107
public additionalParticipantsCount: WritableSignal<number> = signal(0);
100108
public actionMenuItems: Signal<MenuItem[]> = this.initializeActionMenuItems();
@@ -136,24 +144,12 @@ export class MeetingCardComponent implements OnInit {
136144
this.occurrence.set(null);
137145
}
138146
});
139-
140-
// Fetch recording for past meetings
141-
effect(() => {
142-
if (this.pastMeeting() && this.meeting().uid) {
143-
this.fetchRecording();
144-
}
145-
});
146-
147-
// Fetch summary for past meetings
148-
effect(() => {
149-
if (this.pastMeeting() && this.meeting().uid) {
150-
this.fetchSummary();
151-
}
152-
});
153147
}
154148

155149
public ngOnInit(): void {
156150
this.attachments = this.initAttachments();
151+
this.initRecording();
152+
this.initSummary();
157153
}
158154

159155
public onRegistrantsToggle(event: Event): void {
@@ -305,26 +301,21 @@ export class MeetingCardComponent implements OnInit {
305301
// Update local content and approval status when changes are made
306302
ref.onClose.pipe(take(1)).subscribe((result?: { updated: boolean; content: string; approved: boolean }) => {
307303
if (result && result.updated) {
308-
this.summaryContent.set(result.content);
309-
this.summaryApproved.set(result.approved);
304+
const currentSummary = this.summary();
305+
if (currentSummary) {
306+
this.summary.set({
307+
...currentSummary,
308+
approved: result.approved,
309+
summary_data: {
310+
...currentSummary.summary_data,
311+
edited_content: result.content,
312+
},
313+
});
314+
}
310315
}
311316
});
312317
}
313318

314-
private fetchRecording(): void {
315-
this.meetingService
316-
.getPastMeetingRecording(this.meeting().uid)
317-
.pipe(take(1))
318-
.subscribe((recording) => {
319-
if (recording) {
320-
const shareUrl = this.getLargestSessionShareUrl(recording);
321-
this.recordingShareUrl.set(shareUrl);
322-
} else {
323-
this.recordingShareUrl.set(null);
324-
}
325-
});
326-
}
327-
328319
private getLargestSessionShareUrl(recording: PastMeetingRecording): string | null {
329320
if (!recording.sessions || recording.sessions.length === 0) {
330321
return null;
@@ -337,24 +328,6 @@ export class MeetingCardComponent implements OnInit {
337328
return largestSession.share_url || null;
338329
}
339330

340-
private fetchSummary(): void {
341-
this.meetingService
342-
.getPastMeetingSummary(this.meeting().uid)
343-
.pipe(take(1))
344-
.subscribe((summary: PastMeetingSummary | null) => {
345-
if (summary && summary.summary_data) {
346-
const content = summary.summary_data.edited_content || summary.summary_data.content;
347-
this.summaryContent.set(content);
348-
this.summaryUid.set(summary.uid);
349-
this.summaryApproved.set(summary.approved);
350-
} else {
351-
this.summaryContent.set(null);
352-
this.summaryUid.set(null);
353-
this.summaryApproved.set(false);
354-
}
355-
});
356-
}
357-
358331
private initMeetingRegistrantCount(): Signal<number> {
359332
return computed(() => {
360333
if (this.pastMeeting()) {
@@ -597,6 +570,30 @@ export class MeetingCardComponent implements OnInit {
597570
});
598571
}
599572

573+
private initRecording(): void {
574+
runInInjectionContext(this.injector, () => {
575+
toSignal(
576+
this.meetingService.getPastMeetingRecording(this.meetingInput().uid).pipe(
577+
catchError(() => of(null)),
578+
tap((recording) => this.recording.set(recording))
579+
),
580+
{ initialValue: null }
581+
);
582+
});
583+
}
584+
585+
private initSummary(): void {
586+
runInInjectionContext(this.injector, () => {
587+
toSignal(
588+
this.meetingService.getPastMeetingSummary(this.meetingInput().uid).pipe(
589+
catchError(() => of(null)),
590+
tap((summary) => this.summary.set(summary))
591+
),
592+
{ initialValue: null }
593+
);
594+
});
595+
}
596+
600597
private initAttendancePercentage(): Signal<number> {
601598
return computed(() => {
602599
if (this.pastMeeting()) {

‎apps/lfx-one/src/app/modules/project/meetings/components/recording-modal/recording-modal.component.html‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ <h4 class="text-lg font-semibold text-gray-900 mb-1">{{ meetingTitle }}</h4>
1414
<div class="flex items-center gap-2">
1515
<div
1616
class="flex-1 px-3 py-2 text-sm border border-gray-300 rounded-md bg-white text-gray-900 focus:outline-none focus:ring-2 focus:ring-blue-500 overflow-x-auto whitespace-nowrap cursor-text select-all"
17-
data-testid="share-url-input"
18-
(click)="selectUrl($event)">
17+
data-testid="share-url-input">
1918
{{ shareUrl }}
2019
</div>
2120
<lfx-button label="Copy" icon="fa-light fa-copy" size="small" severity="secondary" data-testid="copy-url-button" (click)="copyShareUrl()"></lfx-button>
@@ -30,7 +29,8 @@ <h4 class="text-lg font-semibold text-gray-900 mb-1">{{ meetingTitle }}</h4>
3029
size="small"
3130
severity="primary"
3231
data-testid="open-recording-button"
33-
(click)="openRecording()"></lfx-button>
32+
[href]="shareUrl"
33+
target="_blank"></lfx-button>
3434
<lfx-button label="Close" size="small" severity="secondary" data-testid="close-button" (click)="onClose()"></lfx-button>
3535
</div>
3636
</div>

‎apps/lfx-one/src/app/modules/project/meetings/components/recording-modal/recording-modal.component.ts‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,12 @@ export class RecordingModalComponent {
1818
// Injected services
1919
private readonly clipboard = inject(Clipboard);
2020
private readonly messageService = inject(MessageService);
21-
private readonly ref = inject(DynamicDialogRef);
22-
private readonly config = inject(DynamicDialogConfig);
21+
private readonly dialogRef = inject(DynamicDialogRef);
22+
private readonly dialogConfig = inject(DynamicDialogConfig);
2323

2424
// Inputs from dialog config
25-
public readonly shareUrl = this.config.data.shareUrl as string;
26-
public readonly meetingTitle = this.config.data.meetingTitle as string;
27-
28-
public selectUrl(event: MouseEvent): void {
29-
const target = event.target as HTMLElement;
30-
const selection = window.getSelection();
31-
const range = document.createRange();
32-
range.selectNodeContents(target);
33-
selection?.removeAllRanges();
34-
selection?.addRange(range);
35-
}
25+
public readonly shareUrl = this.dialogConfig.data.shareUrl as string;
26+
public readonly meetingTitle = this.dialogConfig.data.meetingTitle as string;
3627

3728
// Public methods
3829
public copyShareUrl(): void {
@@ -53,11 +44,7 @@ export class RecordingModalComponent {
5344
}
5445
}
5546

56-
public openRecording(): void {
57-
window.open(this.shareUrl, '_blank', 'noopener,noreferrer');
58-
}
59-
6047
public onClose(): void {
61-
this.ref.close();
48+
this.dialogRef.close();
6249
}
6350
}

‎apps/lfx-one/src/app/modules/project/meetings/components/summary-modal/summary-modal.component.html‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@ <h4 class="text-lg font-semibold text-gray-900 mb-1">{{ meetingTitle }}</h4>
1212
@if (isEditMode()) {
1313
<div class="bg-gray-50 rounded-lg p-4 border border-gray-200" data-testid="summary-edit-container">
1414
<label class="block text-sm font-medium text-gray-700 mb-2">Edit Summary</label>
15-
<textarea
16-
[ngModel]="editedContent()"
17-
(ngModelChange)="editedContent.set($event)"
18-
class="w-full min-h-[500px] px-3 py-2 text-sm border border-gray-300 rounded-md bg-white text-gray-900 focus:outline-none focus:ring-2 focus:ring-blue-500 font-mono"
15+
<lfx-textarea
16+
[form]="editForm"
17+
control="content"
18+
[rows]="20"
19+
[autoResize]="false"
1920
placeholder="Edit the meeting summary..."
20-
data-testid="summary-edit-textarea"></textarea>
21+
styleClass="w-full font-mono"
22+
dataTest="summary-edit-textarea"></lfx-textarea>
2123
</div>
2224
} @else {
2325
<div class="bg-gray-50 rounded-lg p-6 border border-gray-200 max-h-[500px] overflow-y-auto" data-testid="summary-content-container">

‎apps/lfx-one/src/app/modules/project/meetings/components/summary-modal/summary-modal.component.ts‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33

44
import { CommonModule } from '@angular/common';
55
import { Component, computed, inject, Signal, signal, WritableSignal } from '@angular/core';
6-
import { FormsModule } from '@angular/forms';
6+
import { FormControl, FormGroup, ReactiveFormsModule } from '@angular/forms';
77
import { DomSanitizer, SafeHtml } from '@angular/platform-browser';
88
import { ButtonComponent } from '@components/button/button.component';
9+
import { TextareaComponent } from '@components/textarea/textarea.component';
910
import { MeetingService } from '@services/meeting.service';
1011
import { MessageService } from 'primeng/api';
1112
import { DynamicDialogConfig, DynamicDialogRef } from 'primeng/dynamicdialog';
@@ -14,53 +15,58 @@ import { take } from 'rxjs';
1415
@Component({
1516
selector: 'lfx-summary-modal',
1617
standalone: true,
17-
imports: [CommonModule, ButtonComponent, FormsModule],
18+
imports: [CommonModule, ButtonComponent, ReactiveFormsModule, TextareaComponent],
1819
templateUrl: './summary-modal.component.html',
1920
})
2021
export class SummaryModalComponent {
2122
// Injected services
22-
private readonly ref = inject(DynamicDialogRef);
23-
private readonly config = inject(DynamicDialogConfig);
23+
private readonly dialogRef = inject(DynamicDialogRef);
24+
private readonly dialogConfig = inject(DynamicDialogConfig);
2425
private readonly sanitizer = inject(DomSanitizer);
2526
private readonly meetingService = inject(MeetingService);
2627
private readonly messageService = inject(MessageService);
2728

2829
// Inputs from dialog config
29-
private readonly summaryUid = this.config.data.summaryUid as string;
30-
private readonly pastMeetingUid = this.config.data.pastMeetingUid as string;
31-
public readonly meetingTitle = this.config.data.meetingTitle as string;
30+
private readonly summaryUid = this.dialogConfig.data.summaryUid as string;
31+
private readonly pastMeetingUid = this.dialogConfig.data.pastMeetingUid as string;
32+
public readonly meetingTitle = this.dialogConfig.data.meetingTitle as string;
3233

3334
// Edit mode state
3435
public readonly isEditMode: WritableSignal<boolean> = signal(false);
35-
public readonly originalContent: WritableSignal<string> = signal(this.config.data.summaryContent as string);
36-
public readonly editedContent: WritableSignal<string> = signal(this.config.data.summaryContent as string);
36+
public readonly originalContent: WritableSignal<string> = signal(this.dialogConfig.data.summaryContent as string);
3737
public readonly isSaving: WritableSignal<boolean> = signal(false);
3838
public readonly isApproving: WritableSignal<boolean> = signal(false);
39-
public readonly isApproved: WritableSignal<boolean> = signal(this.config.data.approved as boolean);
39+
public readonly isApproved: WritableSignal<boolean> = signal(this.dialogConfig.data.approved as boolean);
4040
private readonly wasUpdated: WritableSignal<boolean> = signal(false);
4141

42+
// Reactive form for editing
43+
public readonly editForm: FormGroup = new FormGroup({
44+
content: new FormControl(this.dialogConfig.data.summaryContent as string),
45+
});
46+
4247
// Sanitized HTML content for display
4348
public readonly summaryContent: Signal<SafeHtml> = computed(() => {
44-
const content = this.isEditMode() ? this.editedContent() : this.originalContent();
49+
const content = this.isEditMode() ? this.editForm.get('content')?.value : this.originalContent();
4550
return this.sanitizer.bypassSecurityTrustHtml(content || '');
4651
});
4752

4853
// Public methods
4954
public enterEditMode(): void {
50-
this.editedContent.set(this.originalContent() || '');
55+
this.editForm.get('content')?.setValue(this.originalContent() || '');
5156
this.isEditMode.set(true);
5257
}
5358

5459
public cancelEdit(): void {
55-
this.editedContent.set(this.originalContent() || '');
60+
this.editForm.get('content')?.setValue(this.originalContent() || '');
5661
this.isEditMode.set(false);
5762
}
5863

5964
public saveEdit(): void {
6065
this.isSaving.set(true);
66+
const editedContent = this.editForm.get('content')?.value || '';
6167

6268
this.meetingService
63-
.updatePastMeetingSummary(this.pastMeetingUid, this.summaryUid, { edited_content: this.editedContent() })
69+
.updatePastMeetingSummary(this.pastMeetingUid, this.summaryUid, { edited_content: editedContent })
6470
.pipe(take(1))
6571
.subscribe({
6672
next: () => {
@@ -70,7 +76,7 @@ export class SummaryModalComponent {
7076
detail: 'Summary updated successfully',
7177
});
7278
// Update the original content with the saved changes
73-
this.originalContent.set(this.editedContent());
79+
this.originalContent.set(editedContent);
7480
this.wasUpdated.set(true);
7581
this.isSaving.set(false);
7682
this.isEditMode.set(false);
@@ -120,9 +126,9 @@ export class SummaryModalComponent {
120126
public onClose(): void {
121127
// Return updated content if changes were saved
122128
if (this.wasUpdated()) {
123-
this.ref.close({ updated: true, content: this.originalContent(), approved: this.isApproved() });
129+
this.dialogRef.close({ updated: true, content: this.originalContent(), approved: this.isApproved() });
124130
} else {
125-
this.ref.close();
131+
this.dialogRef.close();
126132
}
127133
}
128134
}

‎apps/lfx-one/src/app/shared/services/meeting.service.ts‎

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -322,28 +322,12 @@ export class MeetingService {
322322
);
323323
}
324324

325-
public getPastMeetingRecording(pastMeetingUid: string): Observable<PastMeetingRecording | null> {
326-
return this.http.get<PastMeetingRecording>(`/api/past-meetings/${pastMeetingUid}/recording`).pipe(
327-
catchError((error) => {
328-
if (error.status === 404) {
329-
return of(null);
330-
}
331-
console.error(`Failed to load recording for past meeting ${pastMeetingUid}:`, error);
332-
return of(null);
333-
})
334-
);
325+
public getPastMeetingRecording(pastMeetingUid: string): Observable<PastMeetingRecording> {
326+
return this.http.get<PastMeetingRecording>(`/api/past-meetings/${pastMeetingUid}/recording`);
335327
}
336328

337-
public getPastMeetingSummary(pastMeetingUid: string): Observable<PastMeetingSummary | null> {
338-
return this.http.get<PastMeetingSummary>(`/api/past-meetings/${pastMeetingUid}/summary`).pipe(
339-
catchError((error) => {
340-
if (error.status === 404) {
341-
return of(null);
342-
}
343-
console.error(`Failed to load summary for past meeting ${pastMeetingUid}:`, error);
344-
return of(null);
345-
})
346-
);
329+
public getPastMeetingSummary(pastMeetingUid: string): Observable<PastMeetingSummary> {
330+
return this.http.get<PastMeetingSummary>(`/api/past-meetings/${pastMeetingUid}/summary`);
347331
}
348332

349333
public updatePastMeetingSummary(pastMeetingUid: string, summaryUid: string, updateData: UpdatePastMeetingSummaryRequest): Observable<PastMeetingSummary> {

0 commit comments

Comments
 (0)