-
Notifications
You must be signed in to change notification settings - Fork 0
fix(ui): query service tag filtering #73
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 <asithade@gmail.com>
|
Caution Review failedThe pull request is closed. WalkthroughMeeting query tag parameters were standardized: client and server services now pass formatted tags ("project_uid:${projectId}" and "meeting_uid:${meetingUid}") to resource queries. Client service also updates TODOs, adding notes to implement upcoming/past filters. No public method signatures or control flow changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 (2)
✨ 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 pull request updates the meeting query service to use explicit UID-based tag formatting for filtering operations. The changes standardize how meetings are queried by modifying the tags parameter format to include explicit prefixes that identify the type of resource being filtered.
- Updated client-side meeting service methods to use
project_uid:prefix for project-based queries - Updated server-side meeting service method to use
meeting_uid:prefix for meeting ID queries - Removed TODO comments that were related to the old tag format
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/lfx-pcc/src/server/services/meeting.service.ts | Updated getMeetingById method to use meeting_uid: prefix in tags parameter |
| apps/lfx-pcc/src/app/shared/services/meeting.service.ts | Updated project-based query methods to use project_uid: prefix and removed outdated TODO comments |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let params = new HttpParams().set('tags', `${projectId}`); | ||
| let params = new HttpParams().set('tags', `project_uid:${projectId}`); | ||
|
|
||
| // TODO: Add filter for upcoming meetings |
Copilot
AI
Sep 8, 2025
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.
The TODO comment is too vague and doesn't provide specific implementation guidance. Consider adding more details about what type of filter should be implemented (e.g., 'TODO: Add start_time_gte filter to only return meetings with start times after current date').
| // TODO: Add filter for upcoming meetings | |
| // TODO: Add start_time_gte filter to only return meetings with start times after the current date/time (i.e., upcoming meetings) |
| let params = new HttpParams().set('tags', `${projectId}`); | ||
| let params = new HttpParams().set('tags', `project_uid:${projectId}`); | ||
|
|
||
| // TODO: Add filter for past meetings |
Copilot
AI
Sep 8, 2025
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.
The TODO comment is too vague and doesn't provide specific implementation guidance. Consider adding more details about what type of filter should be implemented (e.g., 'TODO: Add start_time_lte filter to only return meetings with start times before current date').
| // TODO: Add filter for past meetings | |
| // TODO: Add end_time_lte filter to only return meetings with end times before the current date/time |
🧹 Deployment RemovedThe deployment for PR #73 has been removed. |
⏭️ E2E Tests SkippedBrowser: chromium E2E tests were skipped (no test credentials provided). Test Configuration
|
This pull request updates how meetings are queried by changing the format of the
tagsparameter to use explicit UID-based keys, improving clarity and future-proofing the API calls. The changes affect both client and server meeting service methods for fetching meetings by project or by meeting ID.Meeting query parameter updates:
tagsparameter ingetMeetingsByProject,getUpcomingMeetingsByProject, andgetPastMeetingsByProjectmethods inmeeting.service.tsto use the formatproject_uid:<projectId>instead of just the project ID. [1] [2] [3]tagsparameter in the server-sidegetMeetingByIdmethod to use the formatmeeting_uid:<meetingUid>instead of just the meeting UID.