-
Notifications
You must be signed in to change notification settings - Fork 0
fix(meetings): correct public meeting routes and registration handling #75
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
Conversation
Signed-off-by: Asitha de Silva <[email protected]>
- 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 <[email protected]>
- 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 <[email protected]>
- 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 <[email protected]>
- Fix meeting route path from /meeting/:id to /meetings/:id for consistency - Improve committee member registration counting logic - Add error handling for committee enrichment failures - Reduce access check logging verbosity to debug level - Update documentation to reflect correct route patterns - Clean up HTML formatting for meeting join buttons LFXV2-417 Signed-off-by: Asitha de Silva <[email protected]>
|
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. Caution Review failedThe pull request is closed. WalkthroughUpdated meeting page route from /meeting/:id to /meetings/:id across client and docs; adjusted return URL computation accordingly. Refactored template buttons to use label attribute. Fixed committee modal status comparison by flattening. Revised registrant counting logic in controller. Added guarded committee enrichment with warnings. Adjusted access-check logging details and level. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Client
participant MS as MeetingService
participant CS as CommitteeService
C->>MS: getMeetingCommittees(meetingId)
loop for each committee_uid
MS->>CS: fetchCommittee(committee_uid)
alt success
CS-->>MS: { uid, name }
MS->>MS: append { uid, name }
else failure
CS-->>MS: throws error
MS->>MS: log warn {operation, committee_uid, error}
MS->>MS: append { uid, name: undefined }
end
end
MS-->>C: committees[{ uid, name? }]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (7)
✨ Finishing Touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes critical meeting route consistency and improves registration counting logic while adding robust error handling for committee operations.
- Fixed public meeting route path from
/meeting/:idto/meetings/:idfor consistency across the application - Improved committee member registration counting to properly distinguish individual vs committee registrants
- Added comprehensive error handling for committee enrichment failures to prevent API crashes
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/meeting.interface.ts | Consolidated meeting committee interfaces and updated field types for consistency |
| packages/shared/src/constants/meeting.constants.ts | Updated platform values to use proper display names instead of lowercase values |
| docs/architecture/backend/public-meetings.md | Fixed route documentation to reflect correct /meetings/:id pattern |
| apps/lfx-pcc/src/server/services/meeting.service.ts | Added committee enrichment logic with robust error handling and improved query filtering |
| apps/lfx-pcc/src/server/services/access-check.service.ts | Reduced logging verbosity from info to debug level for access checks |
| apps/lfx-pcc/src/server/controllers/meeting.controller.ts | Fixed registration counting logic to properly calculate individual vs committee registrants |
| apps/lfx-pcc/src/app/shared/services/meeting.service.ts | Updated query tags to use proper project_uid: prefix format |
| Multiple frontend components | Updated to use consolidated committees interface and corrected form control names |
| Multiple test files | Enhanced test coverage with read-only user permissions and improved CI setup |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...oject/meetings/components/meeting-platform-features/meeting-platform-features.component.html
Show resolved
Hide resolved
...les/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts
Show resolved
Hide resolved
Signed-off-by: Asitha de Silva <[email protected]>
🧹 Deployment RemovedThe deployment for PR #75 has been removed. |
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
Summary
/meeting/:idto/meetings/:idfor consistency across the applicationTest plan
/meetings/:idpathResolves LFXV2-417