-
Notifications
You must be signed in to change notification settings - Fork 1
feat(committees): add committee detail shell with overview tab and BFF endpoints #335
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
Changes from all commits
ada825d
f2ed216
da55ff5
8bf9c43
ab51db6
565ab1c
30fffb7
5450942
b52ed22
4ac0a8e
4c5f35c
05c7e45
0bee947
0a7344e
5c3f386
e54fa57
c4ef034
6993ec4
1a0c7c6
34c11a5
44c5a42
c5f5499
ebfeb18
602fb06
b1e3ada
a386498
e1f79de
1973bb1
fd66d0d
f3abd69
a8946b3
daccba6
8189248
85606e0
4a22ae4
b988e1e
649f316
9b2653f
3ceb7fd
bb322d8
d08453c
145e7e5
9a5b3cf
2721250
7574494
cb0640a
2d78552
da1d414
83a9d08
259bd24
696c5a0
2a66b38
0073545
212127a
46bdf90
3fe8e02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,13 +208,13 @@ export class CommitteeManageComponent { | |
| // Update existing committee | ||
| this.committeeService.updateCommittee(this.committeeId()!, committeeData).subscribe({ | ||
| next: () => this.handleCommitteeSuccess('updated'), | ||
| error: (error) => this.handleCommitteeError(error, 'update'), | ||
| error: (err: unknown) => this.handleCommitteeError('update', err), | ||
| }); | ||
| } else { | ||
| // Create new committee | ||
| this.committeeService.createCommittee(committeeData).subscribe({ | ||
| next: (committee) => this.handleCreateSuccess(committee), | ||
| error: (error) => this.handleCommitteeError(error, 'create'), | ||
| error: (err: unknown) => this.handleCommitteeError('create', err), | ||
| }); | ||
| } | ||
| } | ||
|
|
@@ -246,8 +246,7 @@ export class CommitteeManageComponent { | |
| this.showMemberOperationToast(totalSuccess, totalFailed, totalSuccess + totalFailed); | ||
| this.router.navigate(['/groups']); | ||
| }, | ||
| error: (error) => { | ||
| console.error('Error processing member changes:', error); | ||
| error: () => { | ||
| this.messageService.add({ | ||
| severity: 'error', | ||
| summary: 'Error', | ||
|
|
@@ -333,8 +332,7 @@ export class CommitteeManageComponent { | |
| // Navigate back to committees list | ||
| this.router.navigate(['/groups']); | ||
| }, | ||
| error: (error: unknown) => { | ||
| console.error('Error saving committee and members:', error); | ||
| error: () => { | ||
| this.messageService.add({ | ||
| severity: 'error', | ||
| summary: 'Error', | ||
|
|
@@ -455,10 +453,11 @@ export class CommitteeManageComponent { | |
| } | ||
| } | ||
|
|
||
| private handleCommitteeError(error: unknown, operation: 'create' | 'update'): void { | ||
| console.error(`Error ${operation} committee:`, error); | ||
| private handleCommitteeError(operation: 'create' | 'update', error?: unknown): void { | ||
| this.submitting.set(false); | ||
|
|
||
| console.error(`Failed to ${operation} committee:`, error); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leftover Problem: Why it's a problem: Every other error handler modified in this PR removed Fix: Remove the |
||
|
|
||
| this.messageService.add({ | ||
| severity: 'error', | ||
| summary: 'Error', | ||
|
|
@@ -570,10 +569,7 @@ export class CommitteeManageComponent { | |
| private createMemberOperation(type: string, operation: () => Observable<unknown>) { | ||
| return operation().pipe( | ||
| switchMap(() => of({ type, success: 1, failed: 0 })), | ||
| catchError((error) => { | ||
| console.error(`Error ${type} member:`, error); | ||
| return of({ type, success: 0, failed: 1 }); | ||
| }) | ||
| catchError(() => of({ type, success: 0, failed: 1 })) | ||
| ); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,9 +38,6 @@ <h2 class="text-lg font-semibold text-gray-900 mb-2">Something Went Wrong</h2> | |
| <lfx-breadcrumb [model]="breadcrumbItems()" data-testid="committee-view-breadcrumb"></lfx-breadcrumb> | ||
| <div class="flex items-center gap-3"> | ||
| <h1 data-testid="committee-view-name">{{ committee()?.name }}</h1> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Was removing the lock icon ( |
||
| @if (!committee()?.public) { | ||
| <i class="fa-light fa-lock w-4 h-4 text-gray-400"></i> | ||
| } | ||
| </div> | ||
| @if (committee()?.description) { | ||
| <p class="text-gray-500 max-w-2xl" data-testid="committee-view-description">{{ committee()?.description }}</p> | ||
|
|
@@ -54,7 +51,7 @@ <h1 data-testid="committee-view-name">{{ committee()?.name }}</h1> | |
| } | ||
| <span class="text-xs text-gray-400">Created {{ committee()?.created_at | date: 'MMM d, y' }}</span> | ||
| @if (committee()?.updated_at) { | ||
| <span class="text-xs text-gray-400">· Updated {{ committee()?.updated_at | date: 'MMM d, y' }}</span> | ||
| <span class="text-xs text-gray-400">· Updated {{ committee()?.updated_at | date: 'MMM d, y' }}</span> | ||
| } | ||
| </div> | ||
| </div> | ||
|
|
@@ -67,7 +64,7 @@ <h1 data-testid="committee-view-name">{{ committee()?.name }}</h1> | |
| } | ||
| </div> | ||
|
|
||
| <!-- Tabs --> | ||
| <!-- Tab Bar --> | ||
| <div class="flex gap-1 border-b" data-testid="committee-view-tabs"> | ||
| <button | ||
| (click)="activeTab.set('overview')" | ||
|
|
@@ -186,7 +183,4 @@ <h1 data-testid="committee-view-name">{{ committee()?.name }}</h1> | |
| } | ||
| </div> | ||
| } | ||
|
|
||
| <!-- Confirmation Dialog --> | ||
| <p-confirmDialog></p-confirmDialog> | ||
| </div> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,11 +8,10 @@ import { ActivatedRoute, Router, RouterLink } from '@angular/router'; | |
| import { BreadcrumbComponent } from '@components/breadcrumb/breadcrumb.component'; | ||
| import { ButtonComponent } from '@components/button/button.component'; | ||
| import { TagComponent } from '@components/tag/tag.component'; | ||
| import { RouteLoadingComponent } from '@components/loading/route-loading.component'; | ||
| import { Committee, CommitteeMemberVisibility, getCommitteeCategorySeverity, TagSeverity } from '@lfx-one/shared'; | ||
| import { CommitteeService } from '@services/committee.service'; | ||
| import { RouteLoadingComponent } from '@components/loading/route-loading.component'; | ||
| import { MenuItem, MessageService } from 'primeng/api'; | ||
| import { ConfirmDialogModule } from 'primeng/confirmdialog'; | ||
| import { catchError, combineLatest, finalize, of, switchMap } from 'rxjs'; | ||
|
|
||
| import { CommitteeOverviewComponent } from '../components/committee-overview/committee-overview.component'; | ||
|
|
@@ -25,7 +24,6 @@ type CommitteeTab = 'overview' | 'members' | 'votes' | 'meetings' | 'surveys' | | |
| BreadcrumbComponent, | ||
| ButtonComponent, | ||
| TagComponent, | ||
| ConfirmDialogModule, | ||
| RouterLink, | ||
| RouteLoadingComponent, | ||
| DatePipe, | ||
|
|
@@ -77,6 +75,17 @@ export class CommitteeViewComponent { | |
| this.refresh.update((v) => v + 1); | ||
| } | ||
|
|
||
| public createSurvey(): void { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code — Problem: This method is defined but never referenced from the template or any other component. Why it's a problem: Dead code adds confusion — future developers may assume it's wired up somewhere. Fix: Either remove the method entirely, or if it's intended for a "Create Survey" button in the surveys tab, wire it to a button in |
||
| const committee = this.committee(); | ||
| if (!committee) return; | ||
| this.router.navigate(['/surveys/create'], { | ||
| queryParams: { | ||
| committee_uid: committee.uid, | ||
| committee_name: committee.name, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| // -- Private initializer functions -- | ||
| private initializeCommittee(): Signal<Committee | null> { | ||
| return toSignal( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| <!-- Copyright The Linux Foundation and each contributor to LFX. --> | ||
| <!-- SPDX-License-Identifier: MIT --> | ||
|
|
||
| <form [formGroup]="form" (ngSubmit)="onSubmit()" class="flex flex-col gap-6"> | ||
| <!-- Member Selection --> | ||
| <div> | ||
| <label for="member-select" class="block text-sm font-medium text-gray-700 mb-1"> | ||
| Select Member <span class="text-red-500" aria-hidden="true">*</span> | ||
| <span class="sr-only">(required)</span> | ||
| </label> | ||
| <lfx-select | ||
| size="small" | ||
| [form]="form" | ||
| control="member_uid" | ||
| [options]="memberOptions" | ||
| [filter]="true" | ||
| filterBy="label" | ||
| filterPlaceholder="Search by name or organization..." | ||
| placeholder="Choose a member" | ||
| styleClass="w-full" | ||
| appendTo="body" | ||
| [showClear]="true" | ||
| id="member-select" | ||
| data-testid="assign-leadership-member-select"> | ||
| </lfx-select> | ||
| </div> | ||
|
|
||
| <!-- Effective Date --> | ||
| <div> | ||
| <label for="elected-date" class="block text-sm font-medium text-gray-700 mb-1">Effective Date</label> | ||
| <lfx-calendar | ||
| [form]="form" | ||
| control="elected_date" | ||
| placeholder="Select effective date" | ||
| [showButtonBar]="true" | ||
| appendTo="body" | ||
| id="elected-date" | ||
| data-testid="assign-leadership-elected-date"> | ||
| </lfx-calendar> | ||
| </div> | ||
|
|
||
| <!-- Form Actions --> | ||
| <div class="flex justify-between pt-6 border-t"> | ||
| <div> | ||
| @if (currentLeader) { | ||
| <lfx-button | ||
| [label]="'Remove ' + roleLabel" | ||
| severity="danger" | ||
| [outlined]="true" | ||
| [loading]="removing()" | ||
| [disabled]="submitting() || removing()" | ||
| (onClick)="onRemove()" | ||
| size="small" | ||
| type="button" | ||
| data-testid="assign-leadership-remove-btn"> | ||
| </lfx-button> | ||
| } | ||
| </div> | ||
| <div class="flex gap-3"> | ||
| <lfx-button | ||
| label="Cancel" | ||
| severity="secondary" | ||
| [outlined]="true" | ||
| (onClick)="onCancel()" | ||
| size="small" | ||
| type="button" | ||
| data-testid="assign-leadership-cancel-btn"> | ||
| </lfx-button> | ||
| <lfx-button | ||
| [label]="'Assign ' + roleLabel" | ||
| [loading]="submitting()" | ||
| [disabled]="!form.value.member_uid || submitting() || removing()" | ||
| type="submit" | ||
| size="small" | ||
| data-testid="assign-leadership-submit-btn"> | ||
| </lfx-button> | ||
| </div> | ||
| </div> | ||
| </form> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,175 @@ | ||
| // Copyright The Linux Foundation and each contributor to LFX. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| import { Component, inject, signal } from '@angular/core'; | ||
| import { FormControl, FormGroup, ReactiveFormsModule } from '@angular/forms'; | ||
| import { ButtonComponent } from '@components/button/button.component'; | ||
| import { CalendarComponent } from '@components/calendar/calendar.component'; | ||
| import { SelectComponent } from '@components/select/select.component'; | ||
| import { CommitteeMemberRole } from '@lfx-one/shared/enums'; | ||
| import { Committee, CommitteeLeadership, CommitteeMember, CreateCommitteeMemberRequest, LeadershipRole } from '@lfx-one/shared/interfaces'; | ||
| import { formatDateToISOString } from '@lfx-one/shared/utils'; | ||
| import { CommitteeService } from '@services/committee.service'; | ||
| import { MessageService } from 'primeng/api'; | ||
| import { DynamicDialogConfig, DynamicDialogRef } from 'primeng/dynamicdialog'; | ||
| import { catchError, of, switchMap } from 'rxjs'; | ||
|
|
||
| @Component({ | ||
| selector: 'lfx-assign-leadership-dialog', | ||
| imports: [ReactiveFormsModule, ButtonComponent, CalendarComponent, SelectComponent], | ||
| templateUrl: './assign-leadership-dialog.component.html', | ||
| }) | ||
| export class AssignLeadershipDialogComponent { | ||
| private readonly config = inject(DynamicDialogConfig); | ||
| private readonly dialogRef = inject(DynamicDialogRef); | ||
| private readonly committeeService = inject(CommitteeService); | ||
| private readonly messageService = inject(MessageService); | ||
|
|
||
| public readonly role: LeadershipRole; | ||
| public readonly committee: Committee; | ||
| public readonly members: CommitteeMember[]; | ||
| public readonly currentLeader: CommitteeLeadership | null; | ||
| public readonly roleLabel: string; | ||
|
|
||
| public form: FormGroup; | ||
|
|
||
| public submitting = signal(false); | ||
| public removing = signal(false); | ||
|
|
||
| public memberOptions: { label: string; value: string }[]; | ||
|
|
||
| public constructor() { | ||
| this.role = this.config.data?.role ?? 'chair'; | ||
| this.committee = this.config.data?.committee; | ||
| this.members = this.config.data?.members ?? []; | ||
| this.currentLeader = this.config.data?.currentLeader ?? null; | ||
|
|
||
| if (!this.committee) { | ||
| this.messageService.add({ | ||
| severity: 'error', | ||
| summary: 'Error', | ||
| detail: 'Committee data is missing. Please try again.', | ||
| }); | ||
| this.dialogRef.close(); | ||
| } | ||
|
|
||
| this.roleLabel = this.role === 'chair' ? 'Chair' : 'Co-Chair'; | ||
|
|
||
| this.form = new FormGroup({ | ||
| member_uid: new FormControl(this.currentLeader?.uid ?? null), | ||
| elected_date: new FormControl(this.currentLeader?.elected_date ? new Date(this.currentLeader.elected_date) : null), | ||
| }); | ||
|
|
||
| this.memberOptions = this.initializeMemberOptions(); | ||
| } | ||
|
|
||
| public onCancel(): void { | ||
| this.dialogRef.close(); | ||
| } | ||
|
|
||
| public onSubmit(): void { | ||
| const memberUid = this.form.value.member_uid; | ||
| if (!memberUid) return; | ||
|
|
||
| const selectedMember = this.members.find((m) => m.uid === memberUid); | ||
| if (!selectedMember) return; | ||
|
|
||
| this.submitting.set(true); | ||
|
|
||
| const leadership: CommitteeLeadership = { | ||
| uid: selectedMember.uid, | ||
| first_name: selectedMember.first_name, | ||
| last_name: selectedMember.last_name, | ||
| email: selectedMember.email, | ||
| elected_date: formatDateToISOString(this.form.value.elected_date) || undefined, | ||
| organization: selectedMember.organization?.name, | ||
| }; | ||
|
|
||
| const roleName = this.role === 'chair' ? CommitteeMemberRole.CHAIR : CommitteeMemberRole.VICE_CHAIR; | ||
| const roleUpdate: Partial<CreateCommitteeMemberRequest> = { | ||
| role: { | ||
| name: roleName, | ||
| start_date: formatDateToISOString(this.form.value.elected_date) || null, | ||
| }, | ||
| }; | ||
|
|
||
| this.committeeService | ||
| .updateCommitteeMember(this.committee.uid, memberUid, roleUpdate) | ||
| .pipe( | ||
| switchMap(() => { | ||
| if (this.currentLeader && this.currentLeader.uid !== memberUid) { | ||
| return this.committeeService | ||
| .updateCommitteeMember(this.committee.uid, this.currentLeader.uid, { | ||
| role: { name: CommitteeMemberRole.NONE }, | ||
| }) | ||
| .pipe( | ||
| catchError(() => { | ||
| // Assignment succeeded but clearing the previous leader's role failed. | ||
| // Warn the user so they can resolve the duplicate manually. | ||
| this.messageService.add({ | ||
| severity: 'warn', | ||
| summary: 'Partial Update', | ||
| detail: `New ${this.roleLabel.toLowerCase()} assigned, but the previous leader's role could not be cleared automatically.`, | ||
| }); | ||
| return of(null); | ||
| }) | ||
| ); | ||
| } | ||
| return of(null); | ||
| }) | ||
| ) | ||
| .subscribe({ | ||
| next: () => { | ||
| this.submitting.set(false); | ||
| this.messageService.add({ | ||
| severity: 'success', | ||
| summary: 'Success', | ||
| detail: `${this.roleLabel} assigned successfully`, | ||
| }); | ||
| this.dialogRef.close({ role: this.role, leadership }); | ||
| }, | ||
| error: () => { | ||
| this.submitting.set(false); | ||
| this.messageService.add({ | ||
| severity: 'error', | ||
| summary: 'Error', | ||
| detail: `Failed to assign ${this.roleLabel.toLowerCase()}`, | ||
| }); | ||
| this.dialogRef.close(); | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| public onRemove(): void { | ||
| if (!this.currentLeader) return; | ||
|
|
||
| this.removing.set(true); | ||
|
|
||
| this.committeeService.updateCommitteeMember(this.committee.uid, this.currentLeader.uid, { role: { name: CommitteeMemberRole.NONE } }).subscribe({ | ||
| next: () => { | ||
| this.removing.set(false); | ||
| this.messageService.add({ | ||
| severity: 'success', | ||
| summary: 'Success', | ||
| detail: `${this.roleLabel} removed`, | ||
| }); | ||
| this.dialogRef.close({ role: this.role, leadership: null }); | ||
| }, | ||
| error: () => { | ||
| this.removing.set(false); | ||
| this.messageService.add({ | ||
| severity: 'error', | ||
| summary: 'Error', | ||
| detail: `Failed to remove ${this.roleLabel.toLowerCase()}`, | ||
| }); | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| private initializeMemberOptions(): { label: string; value: string }[] { | ||
| return this.members.map((m) => ({ | ||
| label: `${m.first_name} ${m.last_name}${m.organization?.name ? ` — ${m.organization.name}` : ''}`, | ||
| value: m.uid, | ||
| })); | ||
| } | ||
| } |
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.
Silent error swallowing — All
console.errorcalls were removed from error handlers here (and in several other places in this file) but no replacement logging or telemetry was added. If there's a global HTTP error interceptor that handles this, this is fine — but if not, debugging production issues will be very difficult since errors are silently discarded. Please verify centralized error handling covers these cases, or consider keeping at minimum a structured log call.