fix(committees): committee view polish and settings cleanup#344
Merged
fix(committees): committee view polish and settings cleanup#344
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughCommittee management components are refactored to remove error logging, eliminate unused service dependencies, simplify authorization logic to use only writer permissions, remove confirmation dialog UI, and add a new Join Mode settings section with simplified member list empty states. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~18 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
This was referenced Mar 19, 2026
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-leadership-polish
branch
from
dabcd3c to
8e54719
Compare
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-leadership-polish
branch
from
8e54719 to
962e6f7
Compare
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-leadership-polish
branch
from
962e6f7 to
f8fcdec
Compare
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-leadership-polish
branch
from
f8fcdec to
4bf5202
Compare
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-leadership-polish
branch
from
4bf5202 to
a3ec82f
Compare
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-leadership-polish
branch
from
a3ec82f to
e472e1c
Compare
asithade
requested changes
Mar 19, 2026
| this.refresh.update((v) => v + 1); | ||
| } | ||
|
|
||
| public createSurvey(): void { |
Contributor
There was a problem hiding this comment.
Dead code — createSurvey() is never called.
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 committee-view.component.html with a click handler.
| 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.
Leftover console.error.
Problem: handleCommitteeError still calls console.error here.
Why it's a problem: Every other error handler modified in this PR removed console.error in favor of toast-only feedback (e.g., createMemberOperation, the two error: () handlers above). Leaving this one creates an inconsistency and leaks error details to the browser console in production.
Fix: Remove the console.error line. The messageService.add() toast below already handles user-facing feedback. If server-side visibility is needed, the backend controller already logs the error.
| <div class="flex flex-col gap-3" data-testid="committee-view-header"> | ||
| <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.
Question: Was removing the lock icon (fa-light fa-lock) for private/non-public committees intentional? Previously there was an @if (!committee()?.public) block showing a lock icon next to the committee name here. It's been removed with no replacement or mention in the PR description.
| // Server-side enrichment will be added in a follow-up PR. | ||
| // ── Leadership ── | ||
| /** Committee chair (derived from member with role "Chair") */ | ||
| chair?: CommitteeLeadership | null; |
Contributor
There was a problem hiding this comment.
CommitteeLeadership is redundant, chair/co_chair don't exist upstream, and the dialog is dead code.
Problem (4 issues):
(a) The CommitteeLeadership interface (line 109) is a redundant subset of CommitteeMember — it has uid, first_name, last_name, email, organization, and an elected_date field that is just CommitteeMember.role.start_date renamed.
(b) Committee.chair and Committee.co_chair fields added here do not exist on the upstream committee service response. No server-side enrichment populates them, so they will always be undefined. Adding phantom fields to a shared interface that represents an API contract is misleading.
(c) The AssignLeadershipDialogComponent is never opened — no component calls dialogService.open(AssignLeadershipDialogComponent, ...).
(d) CommitteeUpdateData (line 277) also adds chair/co_chair, but the upstream PUT/PATCH endpoint doesn't accept them.
Why it's a problem: The shared Committee interface should match the upstream API contract. Adding fields the API never returns creates a false contract. The redundant CommitteeLeadership type and unused dialog add dead code.
Fix: Remove chair/co_chair from Committee and CommitteeUpdateData interfaces. Remove CommitteeLeadership interface and LeadershipRole type. Use CommitteeMember | null if a typed reference to the leader is needed — derive leadership at the component level by filtering the members list by role.name === 'Chair' / 'Vice Chair'. If the dialog is deferred to a follow-up PR, note that explicitly in the PR description and don't add the interface fields until the enrichment exists.
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-leadership-polish
branch
from
e472e1c to
f6f8466
Compare
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-leadership-polish
branch
from
f6f8466 to
18324f1
Compare
github-actions
Bot
requested review from
andrest50,
asithade,
dealako,
jordane and
mauriciozanettisalomao
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the committees feature set by introducing typed leadership data in @lfx-one/shared, adding a (new) leadership assignment dialog component, and polishing committee settings/view UI.
Changes:
- Add leadership-related types (
LeadershipRole,CommitteeLeadership) and extend committee/vote interfaces with new fields. - Add a join mode select to committee settings and minor committee view polish (remove confirm dialog + UI tweaks).
- Introduce a standalone
assign-leadership-dialogcomponent and add acreateSurvey()navigation helper in committee view.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/committee.interface.ts | Adds leadership types/fields plus key_topics and total_responses on vote interface. |
| apps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.ts | Exposes join mode options to the settings template. |
| apps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.html | Adds Join Mode select UI bound to join_mode. |
| apps/lfx-one/src/app/modules/committees/components/assign-leadership-dialog/assign-leadership-dialog.component.ts | New dialog logic for assigning/removing chair/co-chair via member role updates. |
| apps/lfx-one/src/app/modules/committees/components/assign-leadership-dialog/assign-leadership-dialog.component.html | New dialog template: member select, effective date, assign/remove actions. |
| apps/lfx-one/src/app/modules/committees/committee-view/committee-view.component.ts | Removes ConfirmDialogModule import and adds createSurvey() navigation helper. |
| apps/lfx-one/src/app/modules/committees/committee-view/committee-view.component.html | Removes confirm dialog + lock icon, tweaks header separator text, renames “Tabs” comment. |
| apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts | Fixes error handler signature usage and reduces direct console.error usage in some paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+47
to
+64
| 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(); | ||
| } |
Comment on lines
+17
to
+21
| @Component({ | ||
| selector: 'lfx-assign-leadership-dialog', | ||
| imports: [ReactiveFormsModule, ButtonComponent, CalendarComponent, SelectComponent], | ||
| templateUrl: './assign-leadership-dialog.component.html', | ||
| }) |
Comment on lines
+69
to
+78
| public createSurvey(): void { | ||
| const committee = this.committee(); | ||
| if (!committee) return; | ||
| this.router.navigate(['/surveys/create'], { | ||
| queryParams: { | ||
| committee_uid: committee.uid, | ||
| committee_name: committee.name, | ||
| }, | ||
| }); | ||
| } |
Comment on lines
+72
to
+77
| this.router.navigate(['/surveys/create'], { | ||
| queryParams: { | ||
| committee_uid: committee.uid, | ||
| committee_name: committee.name, | ||
| }, | ||
| }); |
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-leadership-polish
branch
from
369e769 to
aa2cd97
Compare
manishdixitlfx
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.ts (1)
209-218:⚠️ Potential issue | 🟡 MinorInconsistent:
console.errorretained here but removed elsewhere.The PR objective states console.error usages were removed, and
committee-manage.component.tsremoved all console.error calls. This delete error handler still logs to console, which is inconsistent with the rest of the PR.🔧 Suggested fix
error: (err: HttpErrorResponse) => { this.isDeleting.set(false); - console.error('Failed to delete member:', err); this.messageService.add({ severity: 'error', summary: 'Error', detail: 'Failed to remove member. Please try again.', }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.ts` around lines 209 - 218, The delete-member error handler in committee-members.component.ts is still calling console.error (see the block using this.isDeleting.set(false) and this.messageService.add); remove the console.error('Failed to delete member:', err) call to match the PR convention and, if you need to preserve error visibility, replace it with a structured logging call or include the error in the messageService.add detail or a central logger method instead of using console.error.
🧹 Nitpick comments (4)
apps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.html (1)
152-162: Minor:target="_blank"is unnecessary for mailto links.The
target="_blank"attribute on the mailto button has no effect sincemailto:links open the user's email client rather than a browser window.🔧 Suggested fix
<lfx-button icon="fa-light fa-envelope" [text]="true" [rounded]="true" size="small" severity="secondary" [href]="'mailto:' + member.email" - target="_blank" pTooltip="Send Message"> </lfx-button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.html` around lines 152 - 162, Remove the unnecessary target attribute from the mailto button in the CommitteeMembers component template: locate the <lfx-button> used for sending email (the branch that sets [href]="'mailto:' + member.email") and delete the target="_blank" attribute so the mailto link behaves normally; ensure only target is removed and other attributes (icon, [text], [rounded], size, severity, pTooltip) remain unchanged.apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.html (2)
138-154: Minor: Extraneous blank lines within element attributes.Lines 144-145 and 152-153 contain blank lines within the
<lfx-calendar>element attributes. While this doesn't affect functionality, it appears to be a formatting artifact.🔧 Suggested cleanup
<lfx-calendar [form]="form()" control="role_start" placeholder="Start date" [showButtonBar]="true" appendTo="body" - data-testid="member-form-role-start"></lfx-calendar> <lfx-calendar [form]="form()" control="role_end" placeholder="End date" [showButtonBar]="true" appendTo="body" - data-testid="member-form-role-end"></lfx-calendar>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.html` around lines 138 - 154, Remove the extraneous blank lines within the two <lfx-calendar> component attribute blocks to tidy formatting: collapse the attribute list for both instances (the one with control="role_start" and the one with control="role_end") so attributes like [form]="form()", control, placeholder, [showButtonBar], appendTo="body", and data-testid are contiguous without blank lines; update the member-form.component.html's <lfx-calendar> elements accordingly to remove the empty lines between attributes.
189-205: Minor: Same blank line artifacts in voting date calendars.Same formatting issue as above with blank lines within the
<lfx-calendar>attributes.🔧 Suggested cleanup
<lfx-calendar [form]="form()" control="voting_status_start" placeholder="Start date" [showButtonBar]="true" appendTo="body" - data-testid="member-form-voting-start"></lfx-calendar> <lfx-calendar [form]="form()" control="voting_status_end" placeholder="End date" [showButtonBar]="true" appendTo="body" - data-testid="member-form-voting-end"></lfx-calendar>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.html` around lines 189 - 205, Remove the extraneous blank-line artifacts inside the two <lfx-calendar> elements for controls voting_status_start and voting_status_end in member-form.component.html: collapse the attribute blocks so there are no empty lines between attributes (e.g., between appendTo="body" and data-testid="...") to match the rest of the template's formatting.apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.ts (1)
227-236: Consider clarifying the error key naming.The error key
${startKey}_after_${endKey}producesrole_start_after_role_end, which reads as "start is after end" (the error condition). This is technically correct but slightly confusing since the template message says "end date must be after start date."This is a minor readability concern and the current implementation works correctly with the template.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.ts` around lines 227 - 236, The validator dateRangeValidator currently returns an error key `${startKey}_after_${endKey}`, which reads counterintuitively; change the returned key to reflect the template wording (e.g. `${endKey}_before_${startKey}`) so the error key clearly indicates "end is before start" when the condition new Date(start) > new Date(end) is met; update the return inside dateRangeValidator to use the new key name and adjust any template/consumers that reference the old key.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@apps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.ts`:
- Around line 209-218: The delete-member error handler in
committee-members.component.ts is still calling console.error (see the block
using this.isDeleting.set(false) and this.messageService.add); remove the
console.error('Failed to delete member:', err) call to match the PR convention
and, if you need to preserve error visibility, replace it with a structured
logging call or include the error in the messageService.add detail or a central
logger method instead of using console.error.
---
Nitpick comments:
In
`@apps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.html`:
- Around line 152-162: Remove the unnecessary target attribute from the mailto
button in the CommitteeMembers component template: locate the <lfx-button> used
for sending email (the branch that sets [href]="'mailto:' + member.email") and
delete the target="_blank" attribute so the mailto link behaves normally; ensure
only target is removed and other attributes (icon, [text], [rounded], size,
severity, pTooltip) remain unchanged.
In
`@apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.html`:
- Around line 138-154: Remove the extraneous blank lines within the two
<lfx-calendar> component attribute blocks to tidy formatting: collapse the
attribute list for both instances (the one with control="role_start" and the one
with control="role_end") so attributes like [form]="form()", control,
placeholder, [showButtonBar], appendTo="body", and data-testid are contiguous
without blank lines; update the member-form.component.html's <lfx-calendar>
elements accordingly to remove the empty lines between attributes.
- Around line 189-205: Remove the extraneous blank-line artifacts inside the two
<lfx-calendar> elements for controls voting_status_start and voting_status_end
in member-form.component.html: collapse the attribute blocks so there are no
empty lines between attributes (e.g., between appendTo="body" and
data-testid="...") to match the rest of the template's formatting.
In
`@apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.ts`:
- Around line 227-236: The validator dateRangeValidator currently returns an
error key `${startKey}_after_${endKey}`, which reads counterintuitively; change
the returned key to reflect the template wording (e.g.
`${endKey}_before_${startKey}`) so the error key clearly indicates "end is
before start" when the condition new Date(start) > new Date(end) is met; update
the return inside dateRangeValidator to use the new key name and adjust any
template/consumers that reference the old key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9a339ff-6264-4ff1-af76-c1b4fc72db3f
📒 Files selected for processing (10)
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.tsapps/lfx-one/src/app/modules/committees/committee-view/committee-view.component.htmlapps/lfx-one/src/app/modules/committees/committee-view/committee-view.component.tsapps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.tsapps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.tsapps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.htmlapps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.tspackages/shared/src/interfaces/member.interface.ts
Contributor
Author
All review findings addressed — PR stripped down@asithade @MRashad26 — this PR has been significantly reworked based on Asitha's review. Here's what changed: Removed from scope (all 4 findings addressed)
What remains (~68 lines, 5 files)
Ready for re-review when you get a chance. |
MRashad26
requested changes
Mar 19, 2026
Contributor
MRashad26
left a comment
MRashad26
left a comment
There was a problem hiding this comment.
Review — 3 blocking issues
All three must be addressed before merge. See inline comments.
| public committee = input.required<Committee | null>(); | ||
| public members = input.required<CommitteeMember[]>(); | ||
| public membersLoading = input<boolean>(true); | ||
| public groupBehavioralClass = input<GroupBehavioralClass>('other'); |
Contributor
There was a problem hiding this comment.
Blocker: groupBehavioralClass input is never passed from the parent.
committee-view.component.html renders <lfx-committee-members> without binding [groupBehavioralClass]:
<lfx-committee-members
[committee]="committee()"
[members]="members()"
[membersLoading]="membersLoading()"
(refresh)="refreshMembers()"></lfx-committee-members>This means it always defaults to "other", making the governing-board / oversight-committee contextual empty state messages ("No Board Members Found") unreachable dead code.
Fix: Either pass [groupBehavioralClass] from the parent (which means committee-view needs to know the behavioral class), or derive it from the committee() input inside committee-members itself.
| error: (err: HttpErrorResponse) => { | ||
| this.isDeleting.set(false); | ||
| console.error('Failed to delete member:', error); | ||
| console.error('Failed to delete member:', err); |
Contributor
There was a problem hiding this comment.
Blocker: console.error retained here but removed everywhere else in this PR.
This PR removes console.error from 5 places in committee-manage.component.ts and member-form.component.ts, but this delete-member handler keeps it. This creates an inconsistency.
Fix: Remove this console.error line. The messageService.add() toast below already handles user-facing feedback, and the backend logs the error server-side.
| // Computed signals — inline per component-organization.md | ||
| public readonly isBoardMember = computed(() => this.personaService.currentPersona() === 'board-member'); | ||
| public readonly isMaintainer = computed(() => this.personaService.currentPersona() === 'maintainer'); | ||
| public readonly canManageMembers = computed(() => !this.isBoardMember() && (!!this.committee()?.writer || this.isMaintainer())); |
Contributor
There was a problem hiding this comment.
Blocker: Significant permission change not documented in PR description.
Old logic: !!this.committee()?.writer
New logic: !this.isBoardMember() && (!!this.committee()?.writer || this.isMaintainer())
This introduces two behavioral changes:
- Board members lose manage capability — even if
committee.writeristrue, board members can no longer add/edit/delete members. - Maintainers gain manage capability — maintainers can now manage members without the
writerflag from the API.
This is a permission boundary change that affects who can modify committee membership. It needs to be:
- Explicitly documented in the PR description with justification
- Validated that the upstream service enforces these same constraints (otherwise the UI restriction is cosmetic and inconsistent with the API)
- Tested for both personas
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-leadership-polish
branch
from
aa2cd97 to
02f7a54
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.html (1)
153-155: Cross-field validation messages appear withouttouchedcheck—intentional?Unlike individual field validations that check
&& form().get('field')?.touched, these cross-field errors display immediately when the form-level validator fails. This provides instant feedback when dates conflict, which is reasonable UX. Just confirming this is the intended behavior.Also applies to: 202-204
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.html` around lines 153 - 155, The cross-field error for 'role_start_after_role_end' is shown immediately; update the template checks (where form().errors?.['role_start_after_role_end'] is used around lines with member-form.component.html) to only display when the form or relevant controls are touched—e.g., add "&& (form().touched || form().get('role_start')?.touched || form().get('role_end')?.touched)" so the error appears only after user interaction; apply the same change to the other occurrence at the later block (the one referenced as also applies to 202-204).apps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.html (1)
152-162: Consider guarding against empty email.If
member.emailis empty or undefined, the button will havehref="mailto:"orhref="mailto:undefined". Consider conditionally rendering this button only when the email is valid.♻️ Suggested fix
} `@else` { + `@if` (member.email) { <lfx-button icon="fa-light fa-envelope" [text]="true" [rounded]="true" size="small" severity="secondary" [href]="'mailto:' + member.email" pTooltip="Send Message"> </lfx-button> + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.html` around lines 152 - 162, The template renders an email button even when member.email can be empty; update the committee-members.component.html to conditionally render the <lfx-button> only when the email is present/valid (e.g. add *ngIf="member.email" or *ngIf="isValidEmail(member.email)") and implement an isValidEmail(email: string): boolean helper on CommitteeMembersComponent to centrally validate emails (simple regex or truthy check) so the mailto href is never generated with empty/undefined values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.html`:
- Around line 152-162: The template renders an email button even when
member.email can be empty; update the committee-members.component.html to
conditionally render the <lfx-button> only when the email is present/valid (e.g.
add *ngIf="member.email" or *ngIf="isValidEmail(member.email)") and implement an
isValidEmail(email: string): boolean helper on CommitteeMembersComponent to
centrally validate emails (simple regex or truthy check) so the mailto href is
never generated with empty/undefined values.
In
`@apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.html`:
- Around line 153-155: The cross-field error for 'role_start_after_role_end' is
shown immediately; update the template checks (where
form().errors?.['role_start_after_role_end'] is used around lines with
member-form.component.html) to only display when the form or relevant controls
are touched—e.g., add "&& (form().touched || form().get('role_start')?.touched
|| form().get('role_end')?.touched)" so the error appears only after user
interaction; apply the same change to the other occurrence at the later block
(the one referenced as also applies to 202-204).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c879cfe-f33b-4b51-9926-6d4fb1a5b9bb
📒 Files selected for processing (8)
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.tsapps/lfx-one/src/app/modules/committees/committee-view/committee-view.component.htmlapps/lfx-one/src/app/modules/committees/committee-view/committee-view.component.tsapps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.tsapps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.tsapps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.html
✅ Files skipped from review due to trivial changes (1)
- apps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.html
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/lfx-one/src/app/modules/committees/committee-view/committee-view.component.html
- apps/lfx-one/src/app/modules/committees/committee-view/committee-view.component.ts
- apps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.ts
- apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
… findings - Revert canManageMembers to use API writer flag as sole authority (removes PersonaService, isBoardMember, isMaintainer signals) - Remove console.error from delete handler (toast handles UX) - Remove groupBehavioralClass input (never passed from parent, was dead code) - Simplify empty state to single generic message - Remove unnecessary target="_blank" from mailto button - Fix blank line artifacts in calendar attributes from onDateChange removal LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
- Remove direct ConfirmDialogModule PrimeNG import from committee-view - Remove <p-confirmDialog> from committee-view template - Fix middot entity rendering (· → ·) - Add Join Mode select field to committee-settings - Remove console.error from committee-manage error handlers (toast handles UX) - Fix handleCommitteeError signature (remove unused error param) - Remove console.error from createMemberOperation catchError Leadership dialog and interface changes deferred to follow-up when server-side enrichment exists upstream. LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-leadership-polish
branch
from
02f7a54 to
d3521ef
Compare
MRashad26
approved these changes
Mar 19, 2026
asithade
approved these changes
Mar 19, 2026
jordancclive
pushed a commit
that referenced
this pull request
Mar 19, 2026
* fix(committees): revert to permission-based access and address review findings - Revert canManageMembers to use API writer flag as sole authority (removes PersonaService, isBoardMember, isMaintainer signals) - Remove console.error from delete handler (toast handles UX) - Remove groupBehavioralClass input (never passed from parent, was dead code) - Simplify empty state to single generic message - Remove unnecessary target="_blank" from mailto button - Fix blank line artifacts in calendar attributes from onDateChange removal LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(committees): committee view polish and settings cleanup - Remove direct ConfirmDialogModule PrimeNG import from committee-view - Remove <p-confirmDialog> from committee-view template - Fix middot entity rendering (· → ·) - Add Join Mode select field to committee-settings - Remove console.error from committee-manage error handlers (toast handles UX) - Fix handleCommitteeError signature (remove unused error param) - Remove console.error from createMemberOperation catchError Leadership dialog and interface changes deferred to follow-up when server-side enrichment exists upstream. LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> --------- Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Jordan Clive <jclive@linuxfoundation.org>
manishdixitlfx
added a commit
that referenced
this pull request
Mar 31, 2026
* fix(committees): revert to permission-based access and address review findings - Revert canManageMembers to use API writer flag as sole authority (removes PersonaService, isBoardMember, isMaintainer signals) - Remove console.error from delete handler (toast handles UX) - Remove groupBehavioralClass input (never passed from parent, was dead code) - Simplify empty state to single generic message - Remove unnecessary target="_blank" from mailto button - Fix blank line artifacts in calendar attributes from onDateChange removal LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(committees): committee view polish and settings cleanup - Remove direct ConfirmDialogModule PrimeNG import from committee-view - Remove <p-confirmDialog> from committee-view template - Fix middot entity rendering (· → ·) - Add Join Mode select field to committee-settings - Remove console.error from committee-manage error handlers (toast handles UX) - Fix handleCommitteeError signature (remove unused error param) - Remove console.error from createMemberOperation catchError Leadership dialog and interface changes deferred to follow-up when server-side enrichment exists upstream. LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> --------- Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
MRashad26
pushed a commit
that referenced
this pull request
Apr 1, 2026
* fix(committees): revert to permission-based access and address review findings - Revert canManageMembers to use API writer flag as sole authority (removes PersonaService, isBoardMember, isMaintainer signals) - Remove console.error from delete handler (toast handles UX) - Remove groupBehavioralClass input (never passed from parent, was dead code) - Simplify empty state to single generic message - Remove unnecessary target="_blank" from mailto button - Fix blank line artifacts in calendar attributes from onDateChange removal LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(committees): committee view polish and settings cleanup - Remove direct ConfirmDialogModule PrimeNG import from committee-view - Remove <p-confirmDialog> from committee-view template - Fix middot entity rendering (· → ·) - Add Join Mode select field to committee-settings - Remove console.error from committee-manage error handlers (toast handles UX) - Fix handleCommitteeError signature (remove unused error param) - Remove console.error from createMemberOperation catchError Leadership dialog and interface changes deferred to follow-up when server-side enrichment exists upstream. LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> --------- Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rashad <mrashad@contractor.linuxfoundation.org>
MRashad26
pushed a commit
that referenced
this pull request
Apr 1, 2026
* fix(committees): revert to permission-based access and address review findings - Revert canManageMembers to use API writer flag as sole authority (removes PersonaService, isBoardMember, isMaintainer signals) - Remove console.error from delete handler (toast handles UX) - Remove groupBehavioralClass input (never passed from parent, was dead code) - Simplify empty state to single generic message - Remove unnecessary target="_blank" from mailto button - Fix blank line artifacts in calendar attributes from onDateChange removal LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(committees): committee view polish and settings cleanup - Remove direct ConfirmDialogModule PrimeNG import from committee-view - Remove <p-confirmDialog> from committee-view template - Fix middot entity rendering (· → ·) - Add Join Mode select field to committee-settings - Remove console.error from committee-manage error handlers (toast handles UX) - Fix handleCommitteeError signature (remove unused error param) - Remove console.error from createMemberOperation catchError Leadership dialog and interface changes deferred to follow-up when server-side enrichment exists upstream. LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> --------- Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rashad <mrashad@contractor.linuxfoundation.org>
MRashad26
pushed a commit
that referenced
this pull request
Apr 1, 2026
* fix(committees): revert to permission-based access and address review findings - Revert canManageMembers to use API writer flag as sole authority (removes PersonaService, isBoardMember, isMaintainer signals) - Remove console.error from delete handler (toast handles UX) - Remove groupBehavioralClass input (never passed from parent, was dead code) - Simplify empty state to single generic message - Remove unnecessary target="_blank" from mailto button - Fix blank line artifacts in calendar attributes from onDateChange removal LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(committees): committee view polish and settings cleanup - Remove direct ConfirmDialogModule PrimeNG import from committee-view - Remove <p-confirmDialog> from committee-view template - Fix middot entity rendering (· → ·) - Add Join Mode select field to committee-settings - Remove console.error from committee-manage error handlers (toast handles UX) - Fix handleCommitteeError signature (remove unused error param) - Remove console.error from createMemberOperation catchError Leadership dialog and interface changes deferred to follow-up when server-side enrichment exists upstream. LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> --------- Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rashad <mrashad@contractor.linuxfoundation.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ConfirmDialogModulePrimeNG import from committee-view (should use LFX wrappers)<p-confirmDialog>from committee-view template·→·)console.errorfrom committee-manage error handlers (toast handles UX, backend logs errors)handleCommitteeErrorsignature and remove unused error paramconsole.errorfromcreateMemberOperationcatchErrorWhat was removed from the original scope
Leadership dialog (
AssignLeadershipDialogComponent),CommitteeLeadership/LeadershipRoleinterfaces, andchair/co_chairfields onCommitteeare deferred to a follow-up PR when server-side enrichment exists upstream. ThecreateSurvey()dead code was also removed.Split Context
This is PR 2 of 2 splitting #335:
Test Plan
ConfirmDialogModuledirectlyyarn lint && yarn buildpasses ✅LFXV2-1190
🤖 Generated with Claude Code