Skip to content

Conversation

@asithade
Copy link
Contributor

@asithade asithade commented Sep 8, 2025

This pull request updates how meetings are queried by changing the format of the tags parameter 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:

  • Updated the tags parameter in getMeetingsByProject, getUpcomingMeetingsByProject, and getPastMeetingsByProject methods in meeting.service.ts to use the format project_uid:<projectId> instead of just the project ID. [1] [2] [3]
  • Updated the tags parameter in the server-side getMeetingById method to use the format meeting_uid:<meetingUid> instead of just the meeting UID.

Signed-off-by: Asitha de Silva <asithade@gmail.com>
Copilot AI review requested due to automatic review settings September 8, 2025 18:43
@asithade asithade requested a review from jordane as a code owner September 8, 2025 18:43
@coderabbitai
Copy link

coderabbitai bot commented Sep 8, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Meeting 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

Cohort / File(s) Summary of Changes
Client meeting service tag updates and TODO adjustments
apps/lfx-pcc/src/app/shared/services/meeting.service.ts
Updated tag values to "project_uid:${projectId}" in getMeetingsByProject, getUpcomingMeetingsByProject, and getPastMeetingsByProject; removed older TODOs; added TODOs to implement upcoming/past filters. No API/signature changes.
Server meeting service tag update
apps/lfx-pcc/src/server/services/meeting.service.ts
Updated getMeetingById to use tag "meeting_uid:${meetingUid}" when querying resources. No API/signature or control-flow changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs


📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 95953ea and 8955ff0.

📒 Files selected for processing (2)
  • apps/lfx-pcc/src/app/shared/services/meeting.service.ts (3 hunks)
  • apps/lfx-pcc/src/server/services/meeting.service.ts (1 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/query-service-tags

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a 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
Copy link

Copilot AI Sep 8, 2025

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').

Suggested change
// 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)

Copilot uses AI. Check for mistakes.
let params = new HttpParams().set('tags', `${projectId}`);
let params = new HttpParams().set('tags', `project_uid:${projectId}`);

// TODO: Add filter for past meetings
Copy link

Copilot AI Sep 8, 2025

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').

Suggested change
// TODO: Add filter for past meetings
// TODO: Add end_time_lte filter to only return meetings with end times before the current date/time

Copilot uses AI. Check for mistakes.
@asithade asithade merged commit 1a3772e into main Sep 8, 2025
5 of 6 checks passed
@asithade asithade deleted the fix/query-service-tags branch September 8, 2025 18:44
@github-actions
Copy link

github-actions bot commented Sep 8, 2025

🧹 Deployment Removed

The deployment for PR #73 has been removed.

@github-actions
Copy link

github-actions bot commented Sep 8, 2025

⏭️ E2E Tests Skipped

Browser: chromium
Status: skipped

E2E tests were skipped (no test credentials provided).

Test Configuration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants