Skip to content

Conversation

@KirschbaumP
Copy link
Collaborator

No description provided.

@KirschbaumP KirschbaumP requested a review from Copilot July 2, 2025 14:59
@KirschbaumP KirschbaumP merged commit 48c11f9 into main Jul 2, 2025
1 check passed
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

Adds search functionality to list endpoints and UIs by propagating an optional searchTerm through the backend API, OpenAPI spec, generated client, and frontend components/services.

  • Introduce SearchDto and update controllers/services in backend to accept and handle searchTerm
  • Extend OpenAPI spec and codegen to use single request parameter for all endpoints
  • Update frontend: add injected SEARCH_DEBOUNCE_TIME and SELECT_ITEMS_COUNT, BehaviorSubject-based search streams, and search inputs with debounce in multiple list views

Reviewed Changes

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

Show a summary per file
File Description
openapi/backend.yml Add searchTerm query parameter to count/list paths
backend/src/shared/dto/search.dto.ts Define SearchDto for optional search term
backend controller.ts & service.ts Update all controllers/services to accept searchTerm
backend *-db.service.ts Implement query-builder filtering by searchTerm
frontend/app.configs.ts Introduce SEARCH_DEBOUNCE_TIME & SELECT_ITEMS_COUNT tokens
frontend/src/app/config & bootstrap files Provide new tokens to the application
frontend//pages//list.service.ts Use BehaviorSubject + debounce streams for search
frontend//pages//list.component.html Add search input UIs with suffix icons
frontend/package.json Enable useSingleRequestParameter in OpenAPI generator
Comments suppressed due to low confidence (1)

backend/src/inventory/device/device-db.service.ts:14

  • There’s no unit test for searchQueryBuilder or its integration in getCount/findAll. Consider adding tests to verify filtering behavior for searchTerm.
  private searchQueryBuilder(

Comment on lines +3 to +4
export const SEARCH_DEBOUNCE_TIME = new InjectionToken<string>('SEARCH_DEBOUNCE_TIME');
export const SELECT_ITEMS_COUNT = new InjectionToken<string>('SELECT_ITEMS_COUNT');
Copy link

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

The token is typed as string but you inject a number (300). Consider changing to InjectionToken<number> for type safety.

Suggested change
export const SEARCH_DEBOUNCE_TIME = new InjectionToken<string>('SEARCH_DEBOUNCE_TIME');
export const SELECT_ITEMS_COUNT = new InjectionToken<string>('SELECT_ITEMS_COUNT');
export const SEARCH_DEBOUNCE_TIME = new InjectionToken<number>('SEARCH_DEBOUNCE_TIME');
export const SELECT_ITEMS_COUNT = new InjectionToken<number>('SELECT_ITEMS_COUNT');

Copilot uses AI. Check for mistakes.
entitiesLoadError = signal(false);
sortCol?: string;
sortDir?: string;
searchTerm$ = new BehaviorSubject<{ propagate: boolean, value: string }>({propagate: true, value: ''});
Copy link

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

[nitpick] The same search‐stream setup is duplicated across many list services. Consider extracting a reusable search+debounce utility or base class to reduce boilerplate.

Copilot uses AI. Check for mistakes.
.pipe(
filter(x => x.propagate),
debounceTime(time),
distinctUntilChanged(),
Copy link

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

Using distinctUntilChanged() on objects will compare by reference, not by value. Use distinctUntilChanged((a, b) => a.value === b.value) to prevent redundant API calls when the search string hasn’t actually changed.

Suggested change
distinctUntilChanged(),
distinctUntilChanged((a, b) => a.value === b.value),

Copilot uses AI. Check for mistakes.
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.

2 participants