fix(meetings): query service tag filtering and committee data handling #74
Conversation
Signed-off-by: Asitha de Silva <asithade@gmail.com>
- Consolidate MeetingCommittee and MeetingCommitteePayload interfaces - Add committee name enrichment when fetching meetings - Update committee modal to handle voting status configurations - Fix various UI components to handle updated committee structure - Improve committee data display across meeting components LFXV2-448 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRefactors meeting committee data from meeting_committees → committees across frontend and server, adds voting-status selection and filtering to the meeting-committee modal, renames meetingTool → platform in meeting forms, enriches meetings with committee names and registrant counts server-side, updates query tag formats, adjusts shared types/constants, revises E2E mocks/tests for writer permissions, and adds a Playwright setup action and workflow changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Client UI
participant SVC as MeetingService (server)
participant COM as CommitteeService
participant ACL as AccessControl
UI->>SVC: GET /meetings?tags=project_uid:${projectId}
SVC->>SVC: Convert resources → meetings
alt meetings contain committee UIDs
SVC->>COM: fetch committees for unique UIDs
COM-->>SVC: Committee { uid, name, allowed_voting_statuses? }
SVC->>SVC: Enrich meetings' committees with names & voting info
end
SVC->>ACL: apply access control
ACL-->>SVC: filtered/augmented meetings
SVC-->>UI: return meetings (committees populated, registrant counts)
sequenceDiagram
autonumber
participant User as U
participant Modal as MeetingCommitteeModalComponent
participant API as Meeting API
U->>Modal: open modal, select committees
Modal->>Modal: check if any selected committee supports voting
alt voting enabled
Modal->>U: show Voting Status multi-select (votingStatusOptions)
U->>Modal: choose voting statuses
Modal->>Modal: compute filteredCommitteeMembers (by selected statuses)
else no voting
Modal->>Modal: filteredCommitteeMembers = all members
end
U->>Modal: Save
Modal->>API: PUT /meetings/{id} with committees: [{uid, allowed_voting_statuses?}, ...]
API-->>Modal: success/failure
Modal-->>U: close or show error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests
Comment |
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
Fix issues with query service tag filtering and improve committee data handling in the meetings functionality.
- Merged MeetingCommittee and MeetingCommitteePayload interfaces for better type safety
- Enhanced meeting service to properly enrich committee data during retrieval
- Updated UI components to use the consolidated committee structure
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/meeting.interface.ts | Consolidates committee interfaces and improves type definitions |
| packages/shared/src/constants/meeting.constants.ts | Updates platform values to use proper casing |
| apps/lfx-pcc/src/server/services/meeting.service.ts | Adds committee enrichment and fixes query service tag filtering |
| apps/lfx-pcc/src/server/controllers/meeting.controller.ts | Improves registrant counting logic |
| apps/lfx-pcc/src/app/shared/services/meeting.service.ts | Fixes tag filtering with proper prefixes |
| apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts | Updates to use consolidated committee structure |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.html | Fixes form control reference |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-platform-features/meeting-platform-features.component.html | Updates form control references |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts | Updates form control names and validation |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.html | Fixes tab panel value |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts | Enhances committee modal with voting status handling |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.html | Adds voting status selection UI |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts | Updates to use committees property |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html | Updates template to use committees property |
| apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts | Updates to use committees property |
| apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.html | Updates template to use committees property |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...les/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts
Show resolved
Hide resolved
...s/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.html
Show resolved
Hide resolved
...pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.html
Show resolved
Hide resolved
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts (1)
47-53: Avoid calling undefined signal; initialize committees after upcomingMeeting is created.computed() currently calls this.upcomingMeeting() which is undefined until ngOnInit, risking a runtime error. Initialize committees after upcomingMeeting is assigned and guard against missing names.
Apply:
export class UpcomingCommitteeMeetingComponent implements OnInit { @@ public committees!: Signal<string>; public constructor() { - this.committees = this.initializeCommittees(); + // Defer committees initialization until upcomingMeeting is created in ngOnInit. } public ngOnInit() { runInInjectionContext(this.injector, () => { this.upcomingMeeting = this.initializeUpcomingMeeting(); }); + this.committees = this.initializeCommittees(); } @@ private initializeCommittees() { - return computed( - () => - this.upcomingMeeting() - ?.committees?.map((committee) => committee.name) - .join(', ') ?? '' - ); + return computed(() => { + const meetingSignal = this.upcomingMeeting as Signal<Meeting | null> | undefined; + const meeting = meetingSignal ? meetingSignal() : null; + return ( + meeting?.committees + ?.map((c) => c.name) + ?.filter(Boolean) + ?.join(', ') ?? '' + ); + }); }Also applies to: 31-39
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (1)
257-269: Committee registrants in the list lack uid (breaks trackBy and data-testid).Mapped committee members set id, but the template uses registrant.uid for tracking and test ids. This yields undefined keys and unstable DOM.
- return c.map((m) => ({ - id: m.uid, + return c.map((m) => ({ + uid: m.uid, meeting_id: this.meeting().uid, first_name: m.first_name, last_name: m.last_name, email: m.email, organization: m.organization?.name, is_host: false, type: 'committee', invite_accepted: null, attended: true, }));apps/lfx-pcc/src/server/services/meeting.service.ts (1)
213-216: Bug: wrong tag filter when fetching registrants (will return incorrect data).Query service expects a key:value tag (e.g., meeting_uid:), not a bare UID.
- tags: meetingUid, + tags: `meeting_uid:${meetingUid}`,apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts (1)
501-519: Editing flow misses populating platform (silent data drift on update).populateFormWithMeetingData doesn’t patch platform, so editing a meeting can unintentionally reset the platform to the default. Patch the form with meeting.platform.
Outside selected lines; add:
// inside populateFormWithMeetingData patchValue(...) platform: (meeting as any).platform || DEFAULT_MEETING_TOOL,
🧹 Nitpick comments (23)
.vscode/settings.json (1)
30-30: Prefer consistent acronym casing in cSpell entries.Consider "UID" and "UIDs" (or lowercased "uid", "uids") instead of "Uids" to match common usage across the codebase and avoid adding multiple variants later.
apps/lfx-pcc/src/app/shared/services/meeting.service.ts (1)
42-42: Good tag filter fix; add ordering and date filters for correctness.
- Upcoming/past queries currently lack ordering/temporal filters; results may not reflect “next”/“recent past.”
- At minimum, sort upcoming by start_time.asc and past by start_time.desc. Add server-side date filters when available.
Apply this minimal ordering tweak:
public getUpcomingMeetingsByProject(projectId: string, limit: number = 3): Observable<Meeting[]> { - let params = new HttpParams().set('tags', `project_uid:${projectId}`); + let params = new HttpParams().set('tags', `project_uid:${projectId}`).set('order', 'start_time.asc'); // TODO: Add filter for upcoming meetings if (limit) { params = params.set('limit', limit.toString()); } return this.getMeetings(params); } public getPastMeetingsByProject(projectId: string, limit: number = 3): Observable<Meeting[]> { - let params = new HttpParams().set('tags', `project_uid:${projectId}`); + let params = new HttpParams().set('tags', `project_uid:${projectId}`).set('order', 'start_time.desc'); // TODO: Add filter for past meetings if (limit) { params = params.set('limit', limit.toString()); } return this.getMeetings(params); }Also confirm the tags format project_uid:${projectId} matches the API contract.
Also applies to: 60-63, 71-74
apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts (1)
56-86: Ensure the “next upcoming” meeting is actually the earliest future one.When no committeeId is provided, the code takes the first future meeting without sorting. Sort by start_time ascending before selecting the first.
Apply:
- // Return the next upcoming meeting by date in the future - return meetings.filter((meeting) => new Date(meeting.start_time).getTime() > new Date().getTime())[0]; + // Return the next upcoming meeting by earliest future start time + const now = Date.now(); + return ( + meetings + .filter((m) => new Date(m.start_time).getTime() > now) + .sort((a, b) => new Date(a.start_time).getTime() - new Date(b.start_time).getTime())[0] ?? null + );apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.html (1)
21-26: Add a test hook for committee info (optional).For e2e stability, consider a data-testid on the committee display container.
Example:
- <div class="text-xs text-gray-500"> + <div class="text-xs text-gray-500" data-testid="upcoming-committee-committee-info">apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (2)
142-146: Replace non-null assertions with optional chaining and avoid recomputation.Safer and clearer to compute once and reuse the boolean.
-const header = this.meeting().committees && this.meeting().committees!.length > 0 ? 'Manage Committees' : 'Connect Committees'; +const hasCommittees = (this.meeting().committees?.length ?? 0) > 0; +const header = hasCommittees ? 'Manage Committees' : 'Connect Committees';
323-326: Deduplicate the “Manage/Connect Committees” label logic.Use optional chaining; consider extracting a tiny helper (hasCommittees or committeeActionLabel) to keep TS/HTML consistent.
- label: this.meeting().committees && this.meeting().committees!.length > 0 ? 'Manage Committees' : 'Connect Committees', + label: (this.meeting().committees?.length ?? 0) > 0 ? 'Manage Committees' : 'Connect Committees',Optionally:
private hasCommittees(): boolean { return (this.meeting().committees?.length ?? 0) > 0; }apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html (1)
256-263: Use optional chaining instead of non-null assertions; add a test id hook.Prevents potential runtime null issues and improves test targeting.
-@if (meeting().committees && meeting().committees!.length > 0) { - <div class="flex items-start gap-2 mb-4"> +@if (meeting().committees?.length > 0) { + <div class="flex items-start gap-2 mb-4" data-testid="meeting-committees"> <i class="fa-light fa-people-group text-green-500 mt-0.5"></i> <div class="flex flex-wrap gap-1"> <div class="text-sm text-gray-600"> - @for (committee of meeting().committees!; track committee.uid; let isLast = $last) { + @for (committee of meeting().committees; track committee.uid; let isLast = $last) { <a [routerLink]="['/project', project()?.slug, 'committees', committee.uid]" class="text-primary hover:text-primary-600"> {{ committee.name }} </a>apps/lfx-pcc/src/server/controllers/meeting.controller.ts (1)
45-47: Nit: redundant nullish coalescing on .length.length is always a number. Remove ?? 0.
apps/lfx-pcc/src/server/services/meeting.service.ts (2)
49-55: Committee enrichment is fine; avoid logging full meeting payloads at debug in prod.Consider logging counts/ids only to reduce log volume/PII surface.
90-98: Simplify single-meeting handling; avoid array juggling.Reduces cognitive load and minor allocations.
- let meeting = resources.map((resource) => resource.data); - - if (meeting[0].committees && meeting[0].committees.length > 0) { - meeting = await this.getMeetingCommittees(req, meeting); - } - - return await this.accessCheckService.addAccessToResource(req, meeting[0], 'meeting', 'organizer'); + let m = resources[0].data; + if (m.committees?.length) { + [m] = await this.getMeetingCommittees(req, [m]); + } + return await this.accessCheckService.addAccessToResource(req, m, 'meeting', 'organizer');apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts (3)
209-211: Committee options: guard missing names.Some committees may lack names; consider a safer label.
- committeeMap.set(committee.uid, committee.name || ''); + committeeMap.set(committee.uid, committee.name?.trim() || 'Unknown Committee');
321-323: Calendar title shows only first committee.Optional: include multiple committees for clarity when applicable.
- const committeeName = meeting.committees?.[0]?.name; - const displayTitle = committeeName ? `${meeting.title || 'Meeting'} (${committeeName})` : meeting.title || 'Meeting'; + const committeeNames = (meeting.committees || []).map(c => c.name).filter(Boolean); + const suffix = committeeNames.length ? ` (${committeeNames.slice(0,2).join(', ')}${committeeNames.length>2?'…':''})` : ''; + const displayTitle = `${meeting.title || 'Meeting'}${suffix}`;
336-337: extendedProps: expose all committees (optional).If consumers need more than the first committee, add an array.
- committee: meeting.committees?.[0]?.name, + committee: meeting.committees?.[0]?.name, + committees: (meeting.committees || []).map(c => c.name).filter(Boolean),apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.html (2)
33-41: Prefer LFX tooltip wrapper over PrimeNG Tooltip.Project guideline: use LFX wrappers instead of PrimeNG directly. Replace pTooltip with the LFX equivalent, or confirm no wrapper exists.
44-56: data-testid naming: align with convention.Use [section]-[component]-[element]. Current id omits component name.
- data-testid="meeting-voting-status-multi-select" + data-testid="meeting-committee-modal-voting-status-multi-select"apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts (5)
11-11: Avoid barrel for constants if discouraged; keep consistent with repo.If barrels are discouraged here, import VOTING_STATUSES from its constants file; otherwise keep as-is for consistency.
- import { VOTING_STATUSES } from '@lfx-pcc/shared'; + import { VOTING_STATUSES } from '@lfx-pcc/shared/constants/committees.constants';
52-53: Type the signal to avoid any[].Minor typing improvement.
- public filteredCommitteeMembers: Signal<CommitteeMemberDisplay[]> = signal([]); + public filteredCommitteeMembers: Signal<CommitteeMemberDisplay[]> = signal<CommitteeMemberDisplay[]>([]);
112-128: Initial voting-status seeding from meeting.Deduping via Set is good; consider only seeding when any selected committee has voting enabled.
180-184: allowedVotingStatuses computed once for all committees.Reasonable simplification; backend should accept empty array when voting disabled.
251-253: Re-initializing filtered signal inside load path.Initialize the computed once; let it react to dependencies instead of reassigning in tap.
- tap(() => { - this.filteredCommitteeMembers = this.initializeFilteredCommitteeMembers(); - }), + tap(() => void 0),Then in constructor (outside selected lines), add:
this.filteredCommitteeMembers = this.initializeFilteredCommitteeMembers();packages/shared/src/interfaces/meeting.interface.ts (3)
66-73: Type-safe voting statuses and clarify response-only name
- Consider a shared enum (e.g., CommitteeVotingStatus) instead of string[] for
allowed_voting_statusesto enforce valid values across UI/server.- Clarify
nameas response-only to avoid clients posting it.Apply this doc tweak while you wire in the enum:
/** Allowed voting statuses for committee members */ - allowed_voting_statuses?: string[]; + allowed_voting_statuses?: string[]; // TODO: restrict to shared enum (CommitteeVotingStatus) - /** Committee name */ - name?: string; + /** Committee name (response-only; server enriches) */ + name?: string;I can help add the enum to
packages/shared/src/enums. Want me to open a patch for that?
127-127: Avoid optional + nullable onzoom_configin the response shapePrefer a stable key with null when absent:
zoom_config: ZoomConfig | null. This simplifies consumers vs handlingundefined | null.- zoom_config?: ZoomConfig | null; + zoom_config: ZoomConfig | null;
156-157: Input type uses consolidated committee shape—document ignored fieldsIf the backend ignores
nameon input, note it explicitly to prevent clients from attempting to set it.- committees?: MeetingCommittee[]; // Associated committees with voting statuses + committees?: MeetingCommittee[]; // Associated committees (uid + allowed_voting_statuses); name is ignored on input
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
.vscode/settings.json(1 hunks)apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts(2 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.html(4 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts(7 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts(3 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-platform-features/meeting-platform-features.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts(5 hunks)apps/lfx-pcc/src/app/shared/services/meeting.service.ts(3 hunks)apps/lfx-pcc/src/server/controllers/meeting.controller.ts(2 hunks)apps/lfx-pcc/src/server/services/meeting.service.ts(6 hunks)packages/shared/src/constants/meeting.constants.ts(1 hunks)packages/shared/src/interfaces/meeting.interface.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
apps/lfx-pcc/src/**/*.{ts,html}
📄 CodeRabbit inference engine (CLAUDE.md)
Use LFX wrapper components instead of importing PrimeNG components directly
Files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.htmlapps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.htmlapps/lfx-pcc/src/app/shared/services/meeting.service.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsapps/lfx-pcc/src/server/controllers/meeting.controller.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.tsapps/lfx-pcc/src/server/services/meeting.service.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-platform-features/meeting-platform-features.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.html
apps/lfx-pcc/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-pcc/src/**/*.html: Add data-testid attributes to new components for reliable test targeting
Follow data-testid naming convention: [section]-[component]-[element]
Files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.htmlapps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-platform-features/meeting-platform-features.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.html
**/*.{ts,tsx,js,jsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.htmlapps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.htmlapps/lfx-pcc/src/app/shared/services/meeting.service.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tspackages/shared/src/constants/meeting.constants.tsapps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsapps/lfx-pcc/src/server/controllers/meeting.controller.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.tsapps/lfx-pcc/src/server/services/meeting.service.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-platform-features/meeting-platform-features.component.htmlpackages/shared/src/interfaces/meeting.interface.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.html
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer TypeScript interfaces over union types when modeling shapes and contracts
Files:
apps/lfx-pcc/src/app/shared/services/meeting.service.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tspackages/shared/src/constants/meeting.constants.tsapps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsapps/lfx-pcc/src/server/controllers/meeting.controller.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.tsapps/lfx-pcc/src/server/services/meeting.service.tspackages/shared/src/interfaces/meeting.interface.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-pcc/src/app/shared/services/meeting.service.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tspackages/shared/src/constants/meeting.constants.tsapps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsapps/lfx-pcc/src/server/controllers/meeting.controller.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.tsapps/lfx-pcc/src/server/services/meeting.service.tspackages/shared/src/interfaces/meeting.interface.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts
apps/lfx-pcc/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-pcc/src/**/*.ts: In the Angular app, use direct imports for standalone components (no barrel imports)
When defining types for PrimeNG usage, reference PrimeNG’s component interfaces
Files:
apps/lfx-pcc/src/app/shared/services/meeting.service.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsapps/lfx-pcc/src/server/controllers/meeting.controller.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.tsapps/lfx-pcc/src/server/services/meeting.service.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts
packages/shared/src/constants/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all reusable constants (design tokens, etc.) in the shared package
Files:
packages/shared/src/constants/meeting.constants.ts
packages/shared/src/interfaces/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all shared interfaces in the shared package
Files:
packages/shared/src/interfaces/meeting.interface.ts
🧠 Learnings (1)
📚 Learning: 2025-09-05T18:09:48.792Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.792Z
Learning: Applies to apps/lfx-pcc/src/**/*.{ts,html} : Use LFX wrapper components instead of importing PrimeNG components directly
Applied to files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts
🧬 Code graph analysis (4)
apps/lfx-pcc/src/server/controllers/meeting.controller.ts (1)
apps/lfx-pcc/src/server/helpers/logger.ts (2)
error(52-65)Logger(10-129)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts (1)
packages/shared/src/constants/meeting.constants.ts (1)
DEFAULT_MEETING_TOOL(160-160)
apps/lfx-pcc/src/server/services/meeting.service.ts (3)
apps/lfx-pcc/src/server/services/committee.service.ts (1)
CommitteeService(24-351)apps/lfx-pcc/src/server/services/access-check.service.ts (1)
AccessCheckService(13-189)packages/shared/src/interfaces/meeting.interface.ts (1)
Meeting(75-142)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts (1)
packages/shared/src/constants/committees.constants.ts (1)
VOTING_STATUSES(66-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests / Playwright E2E Tests
🔇 Additional comments (24)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.html (1)
191-191: Verify platform value vs. display label.Now that the form binds to platform, ensure the value stored/sent matches backend expectations (slug vs. label). If the API expects slugs, map to labels only for display here.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-platform-features/meeting-platform-features.component.html (1)
17-17: LGTM: control rename to platform is consistent.Binding and validation updated correctly.
Also applies to: 25-26
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.html (1)
121-121: LGTM: tab value corrected.Fix aligns tabpanel with tablist index; resolves mis-selection in edit mode.
apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.html (1)
21-26: LGTM: committees migration reflected in template.Bindings and tooltip updated correctly to committees.
apps/lfx-pcc/src/server/services/meeting.service.ts (2)
19-20: Good: CommitteeService integration is localized and constructor-initialized.Also applies to: 30-36
67-68: Good: switch to tag-based lookup for meeting by UID.apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts (3)
303-304: Platform mapped correctly in payload.Looks good switching to platform with DEFAULT_MEETING_TOOL fallback.
563-564: Step 3 validation updated to platform.LGTM; validation now targets the correct control.
598-604: Form control renamed to platform with required validator.Good alignment with payload and validation.
apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts (2)
238-239: Search includes committee names.Good null-safe match on committee.name.
251-252: Committee filter updated to committees[].uid.Correct filter against new shape.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.html (5)
29-31: Helpful guidance text.Clear instruction for users.
59-66: Concise selection summary.Nice UX; reflects filtered count.
87-92: Table bound to filteredCommitteeMembers.Correctly ties visibility to filtered set.
142-145: Voting status cell is robust.Titlecase and fallback to “-” are fine.
165-174: Empty-state and counts match filtering.Good conditional messaging for voting-status filters.
Also applies to: 168-184
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts (4)
101-106: Form control for votingStatuses added.LGTM; default empty array is correct.
141-148: Auto-clear statuses when no voting-enabled committees are selected.Good UX guard.
187-192: Update payload structure matches new API shape.LGTM.
304-320: Filtering logic by selected voting statuses.Implementation is clean and reactive.
packages/shared/src/interfaces/meeting.interface.ts (4)
180-180: Allowingrecurrenceto be null in Update = good for clearing scheduleThis enables explicit removal of recurrence. LGTM.
125-125: Audit consumer UI/forms for null-safe password handling
No direct.trim()or other string operations onpasswordwere found in the shared package; manually verify that all UI/form components consuming this interface (e.g. in webapp packages) guard againstnullbefore invoking string methods.
105-106: Initializecommitteesto an empty array for all meetings
In meeting.service.tsgetMeetingCommittees, before returning, add:meetings.forEach(m => { m.committees = m.committees ?? []; });to guarantee
Meeting.committeesis nevernull/undefined.
182-188: Retain| nullfor UpdateMeetingRequest flags
Null distinguishes an explicit “clear” from omitting a field—removing it breaks the intended update semantics.Likely an incorrect or invalid review comment.
...les/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts
Show resolved
Hide resolved
- Add writer permission override option to ApiMockHelper.setupProjectSlugMock() - Add comprehensive tests for users without write permissions - Add writer property to mock project data for permission testing - Restructure tests with permission-based describe blocks - Fix GitHub Actions e2e workflow to use exit codes instead of .last-run.json - Test Quick Actions menu visibility based on permissions - Test empty state buttons (Create Meeting, Add Committee) are hidden for read-only - Test read-only users can still access all view-only functionality - Test responsive design for read-only users 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/lfx-pcc/e2e/project-dashboard.spec.ts (1)
1-3: Add missing structural E2E test file
The project-dashboard suite currently only has the content-based test (project-dashboard.spec.ts); add its structural counterpart (project-dashboard-robust.spec.ts) under apps/lfx-pcc/e2e to satisfy dual E2E guidelines.
🧹 Nitpick comments (15)
.github/workflows/e2e-tests.yml (4)
246-263: Capture the exit code into a variable before writing it (minor robustness).Storing
$?first avoids accidental changes if more commands get added between the test run and the output write.- yarn ${{ inputs.test-command }} --reporter=list - echo "test_exit_code=$?" >> $GITHUB_OUTPUT + yarn ${{ inputs.test-command }} --reporter=list + EXIT_CODE=$? + echo "test_exit_code=$EXIT_CODE" >> $GITHUB_OUTPUT
265-283: Same nit: assign$?before echoing output.Keeps behavior stable if the step grows.
- yarn ${{ inputs.test-command }} --project=${{ inputs.browser }} --reporter=list - echo "test_exit_code=$?" >> $GITHUB_OUTPUT + yarn ${{ inputs.test-command }} --project=${{ inputs.browser }} --reporter=list + EXIT_CODE=$? + echo "test_exit_code=$EXIT_CODE" >> $GITHUB_OUTPUT
299-329: Fix YAML lint errors and prefer numeric comparison for exit code.
- Remove trailing whitespace flagged by yamllint (Lines 314 and 321).
- Use
-eqfor numeric comparison.- ALL_EXIT_CODE="${{ steps.run-tests-all.outputs.test_exit_code }}" - SPECIFIC_EXIT_CODE="${{ steps.run-tests-specific.outputs.test_exit_code }}" - + ALL_EXIT_CODE="${{ steps.run-tests-all.outputs.test_exit_code }}" + SPECIFIC_EXIT_CODE="${{ steps.run-tests-specific.outputs.test_exit_code }}" + # Determine which test was run and check its exit code if [ "${{ inputs.browser }}" == "all" ]; then EXIT_CODE="$ALL_EXIT_CODE" else EXIT_CODE="$SPECIFIC_EXIT_CODE" fi - - if [ "$EXIT_CODE" == "0" ]; then + + if [ "$EXIT_CODE" -eq 0 ]; then echo "✅ E2E tests completed successfully" echo "results=success" >> $GITHUB_OUTPUT else echo "❌ E2E tests failed (exit code: $EXIT_CODE)" echo "results=failure" >> $GITHUB_OUTPUT fi
331-404: PR comment step won’t run in a reusable workflow triggered via workflow_call.In called workflows,
github.event_nameisworkflow_call, so this step won’t execute even when the caller is a pull_request workflow. Consider moving this step to the caller, or passpr-numberas an input and gate on that.Happy to propose a caller-side snippet if you share the calling workflow.
apps/lfx-pcc/e2e/fixtures/mock-data/projects.mock.ts (1)
32-32: Avoid nondeterministic dates in E2E fixtures.updated_at: new Date().toISOString() can introduce flaky tests if any assertions or snapshots include it. Prefer a fixed ISO string or a helper constant.
Apply this diff:
- updated_at: new Date().toISOString(), + // Fixed timestamp to keep E2E deterministic + updated_at: '2025-01-01T00:00:00.000Z',Also applies to: 59-59, 85-85
apps/lfx-pcc/e2e/helpers/api-mock.helper.ts (2)
30-33: More robust slug parsing (handles trailing slashes and query).Using URL + pathname filtering avoids edge cases like trailing slash or extra segments.
Apply this diff:
- const pathSegments = url.split('/'); - const slug = pathSegments[pathSegments.length - 1].split('?')[0]; // Remove query params + const { pathname } = new URL(url); + // Split, drop empty segments (e.g., trailing slash), take last + const parts = pathname.split('/').filter(Boolean); + const slug = parts[parts.length - 1];
21-27: Tighten route matcher; remove manual excludes.Match only direct /api/projects/:slug to eliminate the need for skip checks.
Apply this diff:
- await page.route('**/api/projects/*', async (route) => { - const url = route.request().url(); - - // Skip other endpoints - only handle direct slug requests - if (url.includes('/search') || url.includes('/recent-activity')) { - await route.continue(); - return; - } + await page.route(/\/api\/projects\/[^/]+(\?.*)?$/i, async (route) => { + const url = route.request().url();apps/lfx-pcc/e2e/project-dashboard.spec.ts (8)
8-30: Type page parameter; avoid any.Use Playwright’s Page type for navigateToDashboard.
Apply this diff:
-import { expect, test } from '@playwright/test'; +import { expect, test } from '@playwright/test'; +import type { Page } from '@playwright/test'; @@ -async function navigateToDashboard(page: any) { +async function navigateToDashboard(page: Page) {
41-45: Relax page title assertion to reduce brittleness.Titles often change subtly; use a case-insensitive regex.
Apply this diff:
- await expect(page).toHaveTitle('LFX Projects'); + await expect(page).toHaveTitle(/lfx projects/i);
75-79: Prefer ARIA state over CSS class for active tab.Class names are volatile; aria-current is more stable.
Apply this diff:
- const dashboardTab = page.getByRole('link', { name: 'Dashboard' }); - await expect(dashboardTab).toHaveClass(/bg-blue-50/); + const dashboardTab = page.getByRole('link', { name: 'Dashboard' }); + await expect(dashboardTab).toHaveAttribute('aria-current', /page|true/);
95-99: Target project logo more robustly.img().first() is brittle; prefer alt text or data-testid on the project logo.
Apply this diff (if alt text is available):
- const projectImage = page.locator('img').first(); + const projectImage = page.getByAltText(/academy software foundation|aswf/i);If alt text isn’t available, consider adding data-testid="project-logo" to the component and assert on that.
237-242: Future-proof footer year assertion.Hard-coding 2025 will break next year. Use a year-agnostic regex.
Apply this diff:
- await expect(page.getByText(/Copyright © 2025 The Linux Foundation/)).toBeVisible(); + await expect(page.getByText(/Copyright © \d{4} The Linux Foundation/)).toBeVisible();Also applies to: 245-258
345-351: Assert absence strictly.not.toBeVisible can pass with zero matches; assert count for stricter guarantees.
Apply this diff:
- await expect(page.getByRole('menuitem', { name: 'Create Meeting' })).not.toBeVisible(); - await expect(page.getByRole('menuitem', { name: 'Create Committee' })).not.toBeVisible(); + await expect(page.getByRole('menuitem', { name: 'Create Meeting' })).toHaveCount(0); + await expect(page.getByRole('menuitem', { name: 'Create Committee' })).toHaveCount(0);
433-459: Avoid re-registering the same route.setupProjectSlugMock is already called in beforeEach for this describe; calling it again can stack handlers. Remove the extra call.
Apply this diff:
- await ApiMockHelper.setupProjectSlugMock(page, { writer: false });
282-332: DRY navigation in responsive tests.Reuse navigateToDashboard after setting viewport to reduce duplication and keep flows consistent.
Example:
- // Navigate to homepage and search for a project after viewport change - await page.goto('/'); - await expect(page.getByRole('textbox', { name: 'Search projects, committees, meetings, or mailing lists...' })).toBeVisible({ timeout: 10000 }); - await page.getByRole('textbox', { name: 'Search projects, committees, meetings, or mailing lists...' }).fill('aswf'); - await page.keyboard.press('Enter'); - await expect(page.locator('lfx-project-card').first()).toBeVisible({ timeout: 10000 }); - await page.locator('lfx-project-card').filter({ hasText: 'Academy Software Foundation' }).click(); - await expect(page).toHaveURL(/\/project\/[\w-]+$/, { timeout: 10000 }); - await expect(page.getByRole('link', { name: 'Dashboard' })).toBeVisible(); - await page.getByRole('link', { name: 'Dashboard' }).click(); + await navigateToDashboard(page);Note: If header search is hidden on mobile but a hero search remains, navigateToDashboard should still work. If not, consider parameterizing the helper for mobile vs desktop search boxes.
Also applies to: 433-459
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/e2e-tests.yml(2 hunks)apps/lfx-pcc/e2e/fixtures/mock-data/projects.mock.ts(3 hunks)apps/lfx-pcc/e2e/helpers/api-mock.helper.ts(2 hunks)apps/lfx-pcc/e2e/project-dashboard.spec.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer TypeScript interfaces over union types when modeling shapes and contracts
Files:
apps/lfx-pcc/e2e/fixtures/mock-data/projects.mock.tsapps/lfx-pcc/e2e/helpers/api-mock.helper.tsapps/lfx-pcc/e2e/project-dashboard.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-pcc/e2e/fixtures/mock-data/projects.mock.tsapps/lfx-pcc/e2e/helpers/api-mock.helper.tsapps/lfx-pcc/e2e/project-dashboard.spec.ts
**/*.{ts,tsx,js,jsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-pcc/e2e/fixtures/mock-data/projects.mock.tsapps/lfx-pcc/e2e/helpers/api-mock.helper.tsapps/lfx-pcc/e2e/project-dashboard.spec.ts
**/{*.spec.ts,*-robust.spec.ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain dual E2E test architecture: content-based (.spec.ts) and structural (-robust.spec.ts) tests
Files:
apps/lfx-pcc/e2e/project-dashboard.spec.ts
🧠 Learnings (3)
📚 Learning: 2025-09-05T18:09:48.792Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.792Z
Learning: Applies to **/{*.spec.ts,*-robust.spec.ts} : Maintain dual E2E test architecture: content-based (*.spec.ts) and structural (*-robust.spec.ts) tests
Applied to files:
apps/lfx-pcc/e2e/project-dashboard.spec.ts.github/workflows/e2e-tests.yml
📚 Learning: 2025-09-05T18:09:48.791Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.791Z
Learning: Test responsive behavior across mobile, tablet, and desktop viewports
Applied to files:
apps/lfx-pcc/e2e/project-dashboard.spec.ts
📚 Learning: 2025-09-05T18:09:48.791Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.791Z
Learning: Always run yarn e2e before major changes to ensure the full suite passes
Applied to files:
.github/workflows/e2e-tests.yml
🧬 Code graph analysis (1)
apps/lfx-pcc/e2e/project-dashboard.spec.ts (1)
apps/lfx-pcc/e2e/helpers/api-mock.helper.ts (1)
ApiMockHelper(12-62)
🪛 YAMLlint (1.37.1)
.github/workflows/e2e-tests.yml
[error] 314-314: trailing spaces
(trailing-spaces)
[error] 321-321: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests / Playwright E2E Tests
🔇 Additional comments (2)
apps/lfx-pcc/e2e/fixtures/mock-data/projects.mock.ts (1)
36-36: Writer flag in mocks — looks good.The added writer property aligns with the permission-driven tests and the new mock override. No issues found.
Also applies to: 63-63, 89-89
apps/lfx-pcc/e2e/helpers/api-mock.helper.ts (1)
19-19: Options-based writer override is clean and contained.Signature + override logic are straightforward and safe; response body uses the merged object as expected.
Also applies to: 49-54, 58-58
- Add composite action .github/actions/setup-playwright/action.yml - Cache browser binaries based on OS and Playwright version - Install browsers only on cache miss, OS deps only on cache hit - Support selective browser installation (all, chromium, firefox, mobile-chrome) - Update e2e-tests.yml workflow to use new composite action - Reduce CI/CD pipeline time by avoiding redundant browser downloads Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
.github/actions/setup-playwright/action.yml (5)
14-14: Comment is misleading.The step doesn’t run “npm ci”. Trim the comment to avoid confusion.
Apply:
- # Run npm ci and get Playwright version + # Get Playwright version
26-30: Cache path isn’t Windows-safe.The Playwright cache lives at ~/.cache/ms-playwright on Linux/macOS but at %LOCALAPPDATA%\ms-playwright on Windows. Use an OS-aware path to actually cache on Windows runners.
Apply:
- path: "~/.cache/ms-playwright" + path: ${{ runner.os == 'Windows' && format('{0}\\ms-playwright', env.LOCALAPPDATA) || format('{0}/.cache/ms-playwright', env.HOME) }}
31-41: Harden shell execution.Add strict mode to fail fast on errors.
Apply:
run: | + set -euo pipefail if [ "${{ inputs.browser }}" == "all" ]; then npm exec -- playwright install --with-deps else npm exec -- playwright install --with-deps ${{ inputs.browser }} fi
42-51: Skip install-deps on Windows; add strict mode.
playwright install-depsisn’t supported on Windows and produces noise. Gate by OS and harden the script.Apply:
- - name: 🏗 Install Playwright OS dependencies - if: steps.playwright-cache.outputs.cache-hit == 'true' + - name: 🏗 Install Playwright OS dependencies + if: runner.os != 'Windows' && steps.playwright-cache.outputs.cache-hit == 'true' shell: bash run: | + set -euo pipefail if [ "${{ inputs.browser }}" == "all" ]; then npm exec -- playwright install-deps else npm exec -- playwright install-deps ${{ inputs.browser }} fi
51-51: Add a trailing newline.Satisfies linters and POSIX conventions.
- fi + fi +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/actions/setup-playwright/action.yml(1 hunks).github/workflows/e2e-tests.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/e2e-tests.yml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/actions/setup-playwright/action.yml
[error] 51-51: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests / Playwright E2E Tests
🧹 Deployment RemovedThe deployment for PR #74 has been removed. |
Summary
Fix issues with query service tag filtering and improve committee data handling in the meetings functionality.
Changes Made
Files Modified (16 files)
Testing
JIRA Ticket
LFXV2-448
Generated with Claude Code