-
Notifications
You must be signed in to change notification settings - Fork 0
List enhancements #3
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
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
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
SearchDtoand update controllers/services in backend to accept and handlesearchTerm - Extend OpenAPI spec and codegen to use single request parameter for all endpoints
- Update frontend: add injected
SEARCH_DEBOUNCE_TIMEandSELECT_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
searchQueryBuilderor its integration ingetCount/findAll. Consider adding tests to verify filtering behavior forsearchTerm.
private searchQueryBuilder(
| export const SEARCH_DEBOUNCE_TIME = new InjectionToken<string>('SEARCH_DEBOUNCE_TIME'); | ||
| export const SELECT_ITEMS_COUNT = new InjectionToken<string>('SELECT_ITEMS_COUNT'); |
Copilot
AI
Jul 2, 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 token is typed as string but you inject a number (300). Consider changing to InjectionToken<number> for type safety.
| 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'); |
| entitiesLoadError = signal(false); | ||
| sortCol?: string; | ||
| sortDir?: string; | ||
| searchTerm$ = new BehaviorSubject<{ propagate: boolean, value: string }>({propagate: true, value: ''}); |
Copilot
AI
Jul 2, 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.
[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.
| .pipe( | ||
| filter(x => x.propagate), | ||
| debounceTime(time), | ||
| distinctUntilChanged(), |
Copilot
AI
Jul 2, 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.
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.
| distinctUntilChanged(), | |
| distinctUntilChanged((a, b) => a.value === b.value), |
No description provided.