Hide empty self-service categories on My device page (#48614)#48619
Hide empty self-service categories on My device page (#48614)#48619marko-lisica wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48619 +/- ##
========================================
Coverage 68.01% 68.01%
========================================
Files 3678 3678
Lines 233764 233780 +16
Branches 12268 12419 +151
========================================
+ Hits 158999 159015 +16
Misses 60470 60470
Partials 14295 14295
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
WalkthroughThis change hides self-service software categories that have no available software from the category filter dropdown on the My device page. A new helper, Changes
Sequence Diagram(s)sequenceDiagram
participant SelfServiceCard
participant filterCategoriesWithSoftware
participant SelfServiceFilters
SelfServiceCard->>filterCategoriesWithSoftware: categories, enhancedSoftware
filterCategoriesWithSoftware-->>SelfServiceCard: visibleCategories
SelfServiceCard->>SelfServiceCard: filter softwareInSelectedCategory
SelfServiceCard->>SelfServiceCard: validate category_id vs visibleCategories
SelfServiceCard->>SelfServiceFilters: render with visibleCategories
Related Issues Suggested Labels: Suggested Reviewers: none identified from provided context Poem 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/pages/hosts/details/cards/Software/SelfService/helpers.ts (1)
103-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCorrect implementation; consider deduplicating category extraction.
The new helper correctly reuses the same case-insensitive matching semantics as
filterSoftwareByCustomCategory(lines 93-99 in this file). Both functions independently build[...software_package?.categories ?? [], ...app_store_app?.categories ?? []]— extracting this into a small shared helper (e.g.,getItemCategoryNames(item)) would keep the two matching rules from silently diverging if either is updated later.♻️ Proposed refactor
+const getItemCategoryNames = (item: IDeviceSoftwareWithUiStatus): string[] => [ + ...(item.software_package?.categories ?? []), + ...(item.app_store_app?.categories ?? []), +]; + export const filterSoftwareByCustomCategory = ( software: IDeviceSoftwareWithUiStatus[], categories: ISelfServiceCategory[], categoryId?: number ): IDeviceSoftwareWithUiStatus[] => { ... - return software.filter((item) => { - const itemCategories = [ - ...(item.software_package?.categories ?? []), - ...(item.app_store_app?.categories ?? []), - ]; - return itemCategories.some((c) => c.toLowerCase() === normalized); - }); + return software.filter((item) => + getItemCategoryNames(item).some((c) => c.toLowerCase() === normalized) + ); }; export const filterCategoriesWithSoftware = ( categories: ISelfServiceCategory[], software: IDeviceSoftwareWithUiStatus[] ): ISelfServiceCategory[] => { const categoryNamesInUse = new Set<string>(); - software.forEach((item) => { - [ - ...(item.software_package?.categories ?? []), - ...(item.app_store_app?.categories ?? []), - ].forEach((name) => categoryNamesInUse.add(name.toLowerCase())); - }); + software.forEach((item) => + getItemCategoryNames(item).forEach((name) => + categoryNamesInUse.add(name.toLowerCase()) + ) + ); return categories.filter((c) => categoryNamesInUse.has(c.name.toLowerCase())); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/pages/hosts/details/cards/Software/SelfService/helpers.ts` around lines 103 - 125, The category matching logic is duplicated between filterCategoriesWithSoftware and filterSoftwareByCustomCategory, which risks the two paths diverging later. Extract the shared category-name collection into a small helper (for example, a getItemCategoryNames-style function) and reuse it in both places so the lowercased matching behavior stays identical for software_package and app_store_app categories.frontend/pages/hosts/details/cards/Software/SelfService/SelfServiceCard/SelfServiceCard.tests.tsx (1)
196-218: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for the loading-window fix.
The PR's stated goal is to stop clearing a valid
category_idwhileselfServiceDatais still loading (categories resolved, software not yet). Existing tests only cover the "should clear" paths (empty/unknown categories); none assert that a validcategory_idsurvives whileselfServiceDataisundefinedandisCategoriesSuccessistrue. Given this behavior was explicitly called out as the bug being fixed, a direct test would guard against regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/pages/hosts/details/cards/Software/SelfService/SelfServiceCard/SelfServiceCard.tests.tsx` around lines 196 - 218, Add a regression test in SelfServiceCard.tests.tsx covering the loading-window case where categories have loaded successfully but selfServiceData is still undefined and a valid category_id is present. Use SelfServiceCard with createTestProps, createMockRouter, and the existing listDeviceSelfServiceCategoriesHandler setup, then assert that the router push logic does not remove category_id while loading and only preserves the valid filter state. This should complement the existing auto-clear test and specifically verify the behavior around SelfServiceCard's category handling when isCategoriesSuccess is true but data is still pending.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/pages/hosts/details/cards/Software/SelfService/helpers.ts`:
- Around line 103-125: The category matching logic is duplicated between
filterCategoriesWithSoftware and filterSoftwareByCustomCategory, which risks the
two paths diverging later. Extract the shared category-name collection into a
small helper (for example, a getItemCategoryNames-style function) and reuse it
in both places so the lowercased matching behavior stays identical for
software_package and app_store_app categories.
In
`@frontend/pages/hosts/details/cards/Software/SelfService/SelfServiceCard/SelfServiceCard.tests.tsx`:
- Around line 196-218: Add a regression test in SelfServiceCard.tests.tsx
covering the loading-window case where categories have loaded successfully but
selfServiceData is still undefined and a valid category_id is present. Use
SelfServiceCard with createTestProps, createMockRouter, and the existing
listDeviceSelfServiceCategoriesHandler setup, then assert that the router push
logic does not remove category_id while loading and only preserves the valid
filter state. This should complement the existing auto-clear test and
specifically verify the behavior around SelfServiceCard's category handling when
isCategoriesSuccess is true but data is still pending.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02983817-d78c-46a5-98aa-3d8c46b8216b
📒 Files selected for processing (5)
changes/48614-hide-empty-self-service-categoriesfrontend/pages/hosts/details/cards/Software/SelfService/SelfServiceCard/SelfServiceCard.tests.tsxfrontend/pages/hosts/details/cards/Software/SelfService/SelfServiceCard/SelfServiceCard.tsxfrontend/pages/hosts/details/cards/Software/SelfService/helpers.tests.tsfrontend/pages/hosts/details/cards/Software/SelfService/helpers.ts
Description
Resolves #48614.
On the My device page's Self-service tab, the category filter listed every category defined for the host's fleet — including categories that had no self-service software available for that host. Selecting such a category showed an empty list, which is confusing.
This change hides categories that have no self-service software items for the host, so users only see categories they can actually install from.
Implementation
The fix is client-side. The device self-service software endpoint is not paginated (
per_page: 9999), so the frontend already holds the host's complete self-service software list, with each item's categories. That list has already been resolved by the backend software query for MDM enrollment, label scoping, and platform — so filtering the category dropdown against it is both correct and self-consistent with the existing per-category filtering.filterCategoriesWithSoftwareinSelfService/helpers.ts, which keeps only categories whose (case-insensitive) name matches a category present on some software item. It resolves category membership the same wayfilterSoftwareByCustomCategoryalready does (acrosssoftware_packageandapp_store_appcategories).SelfServiceCardnow derivesvisibleCategoriesand uses it everywhere downstream (the filter dropdown, the selected-category software list, and the stale-link recovery effect), so an empty category behaves as if it doesn't exist.category_idisn't cleared during the load window.Related issue: Resolves #48614
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Testing
Note: this is a frontend-only change; no backend endpoints, database schema, or configuration settings were modified.
Summary by CodeRabbit
New Features
Bug Fixes