Skip to content

Conversation

@LukasHirt
Copy link
Collaborator

Description

This changes the search behaviour in the admin settings. Instead of filtering the groups in the client, we now use the search parameter of the list groups API endpoint.

Motivation and Context

Leave the search to the API and don't slow down the browser.

How Has This Been Tested?

  • test environment: macos, chrome
  • test case 1: search groups

Screenshots (if appropriate):

Zaznam.obrazovky.2025-10-23.v.11.53.50.mov

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

This changes the search behaviour in the admin settings.
Instead of filtering the groups in the client, we now use the search
parameter of the list groups API endpoint.
@LukasHirt LukasHirt requested review from Copilot and mzner October 23, 2025 09:55
@LukasHirt LukasHirt self-assigned this Oct 23, 2025
Copy link

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 PR implements server-side group search in the admin settings, replacing client-side filtering. When users search for groups, the search term is now sent directly to the API endpoint rather than filtering results in the browser.

Key changes:

  • Search now uses the API's search parameter instead of client-side filtering
  • Search term is stored in URL query parameters (q_displayName)
  • Removed client-side filtering logic and associated footer text

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/web-app-admin-settings/src/views/Groups.vue Added search UI components and filterGroups function to handle API-based search
packages/web-app-admin-settings/src/components/Groups/GroupsList.vue Removed client-side filtering logic, converted filter input to a slot, and updated search highlighting to use query parameters
packages/web-app-admin-settings/tests/unit/views/Groups.spec.ts Added test for search functionality with mocked router
packages/web-app-admin-settings/tests/unit/components/Groups/GroupsList.spec.ts Added router mocks to support component changes
changelog/unreleased/enhancement-use-api-groups-search-in-admin.md Documented the enhancement

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +316 to +320
if (!unref(filterTerm)) {
return
}
unref(markInstance).mark(unref(filterTerm), {
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unref() call on line 316 is incorrect. filterTerm is already a string from queryItemAsString() on line 315, not a ref. Remove the unref() wrapper.

Suggested change
if (!unref(filterTerm)) {
return
}
unref(markInstance).mark(unref(filterTerm), {
if (!filterTerm) {
return
}
unref(markInstance).mark(filterTerm, {

Copilot uses AI. Check for mistakes.
<div class="oc-flex oc-flex-middle">
<oc-text-input
id="groups-filter"
v-model.trim="filterTerm"
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The v-model.trim modifier removes whitespace from the input value. However, when the search is triggered, the function uses unref(filterTerm) which will include the already-trimmed value. Consider whether .trim is necessary here or if validation should occur in the filterGroups function.

Copilot uses AI. Check for mistakes.
...unref(route),
query: {
...omit(unref(route).query, 'q_displayName'),
q_displayName: unref(filterTerm),
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When filterTerm is empty, this code still adds an empty q_displayName parameter to the query. This should conditionally add the parameter only when filterTerm has a value, or explicitly remove it when empty.

Suggested change
q_displayName: unref(filterTerm),
...(unref(filterTerm) ? { q_displayName: unref(filterTerm) } : {}),

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link

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.

1 participant