Fleet UI: 'Latest' pin only shows on Fleet-maintained apps - #48871
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
This PR fixes misleading UI behavior in the Software Library accordion by ensuring the “Latest” status pin only appears for Fleet-maintained apps (FMAs), where it has defined semantics and (optionally) a click handler.
Changes:
- Removed
badgeState="latest"from non-FMA rows (App Store and the non-FMA fallback row built bybuildLibraryVersionRows). - Refactored
LibraryItemAccordionto centralize status-badge rendering and to render a non-interactive<span>whenonBadgeClickis not provided. - Updated unit tests and Storybook stories to cover/button-vs-static rendering and the “All hosts” fallback behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/SoftwareTitleDetailsPage.tsx | Stops forcing the “Latest” badge on the App Store branch. |
| frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/LibraryItemAccordion/LibraryItemAccordion.tsx | Introduces renderStatusBadge and renders status badges as static spans when no click handler is wired. |
| frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/LibraryItemAccordion/LibraryItemAccordion.tests.tsx | Updates badge tests to assert button vs static behavior and adjusts “All hosts” expectations. |
| frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/LibraryItemAccordion/LibraryItemAccordion.stories.tsx | Adds story args to demonstrate clickable FMA badges vs decorative non-FMA badges. |
| frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/helpers.ts | Removes “latest” badge emission from the non-FMA fallback row and clarifies semantics in comments. |
| frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/helpers.tests.ts | Updates expected rows for non-FMA fallback to be un-badged and adds clarifying test commentary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 97b8c4084f46a6ed1c59bc00f17bfec0784c4424 and 409ded1. 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughThis change updates Possibly related PRs
🚥 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 (1)
frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/LibraryItemAccordion/LibraryItemAccordion.tsx (1)
187-269: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLabel-count/All-hosts badges still duplicate the Button-vs-span pattern
renderStatusBadgewas meant to consolidate.
renderStatusBadge(lines 190-212) removes duplication for the three status badges, but the label-count block (223-250) and the All-hosts block (251-269) repeat the identicalcanEditSoftware ? <Button>… : <span>…pattern inline. Consider extendingrenderStatusBadge(or a sibling generic helper) to also cover these two cases for consistency and to avoid future divergence.♻️ Possible consolidation
- const renderStatusBadge = (iconName: IconNames, label: string) => { - if (onBadgeClick) { + const renderInteractiveBadge = ( + iconName: IconNames, + label: React.ReactNode, + onClick?: () => void + ) => { + if (onClick) { return ( <Button variant="inverse" size="small" - onClick={handleBadgeClick(onBadgeClick)} + onClick={handleBadgeClick(onClick)} className={`${baseClass}__badge-button`} > <Icon name={iconName} color="ui-fleet-black-75" /> <span>{label}</span> </Button> ); } return ( <span className={`${baseClass}__badge-button ${baseClass}__badge-button--static`} > <Icon name={iconName} color="ui-fleet-black-75" /> <span>{label}</span> </span> ); };Then call
renderInteractiveBadge("tag", labelCount, onLabelCountClick)whencanEditSoftware, gated byhasLabelScope/showAllHostsBadgeas before.🤖 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/SoftwarePage/SoftwareTitleDetailsPage/LibraryItemAccordion/LibraryItemAccordion.tsx` around lines 187 - 269, The label-count and All-hosts badges still repeat the same Button-versus-span rendering logic that renderStatusBadge already centralized for status badges. Extend renderStatusBadge or add a small shared helper in LibraryItemAccordion so the label-count and All-hosts branches reuse the same interactive/static badge rendering path, keeping the existing hasLabelScope, showAllHostsBadge, and canEditSoftware gating unchanged.
🤖 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/SoftwarePage/SoftwareTitleDetailsPage/LibraryItemAccordion/LibraryItemAccordion.tsx`:
- Around line 187-269: The label-count and All-hosts badges still repeat the
same Button-versus-span rendering logic that renderStatusBadge already
centralized for status badges. Extend renderStatusBadge or add a small shared
helper in LibraryItemAccordion so the label-count and All-hosts branches reuse
the same interactive/static badge rendering path, keeping the existing
hasLabelScope, showAllHostsBadge, and canEditSoftware gating unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 27df0785-7398-4508-8846-da8ff8866c3e
📥 Commits
Reviewing files that changed from the base of the PR and between 28f4da0 and 75ea03f2e9ec97c98f2e7f0f60c8b1a139712986.
📒 Files selected for processing (6)
frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/LibraryItemAccordion/LibraryItemAccordion.stories.tsxfrontend/pages/SoftwarePage/SoftwareTitleDetailsPage/LibraryItemAccordion/LibraryItemAccordion.tests.tsxfrontend/pages/SoftwarePage/SoftwareTitleDetailsPage/LibraryItemAccordion/LibraryItemAccordion.tsxfrontend/pages/SoftwarePage/SoftwareTitleDetailsPage/SoftwareTitleDetailsPage.tsxfrontend/pages/SoftwarePage/SoftwareTitleDetailsPage/helpers.tests.tsfrontend/pages/SoftwarePage/SoftwareTitleDetailsPage/helpers.ts
💤 Files with no reviewable changes (1)
- frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/SoftwareTitleDetailsPage.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48871 +/- ##
========================================
Coverage 68.06% 68.06%
========================================
Files 3697 3697
Lines 234703 234705 +2
Branches 12500 12353 -147
========================================
+ Hits 159759 159761 +2
Misses 60599 60599
Partials 14345 14345
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:
|
|
@claude review |
Bug fix (root cause): the "Latest" pin was rendered on every active library-accordion row regardless of installer type. `buildLibraryVersionRows` stamped `badgeState: "latest"` on the non-FMA fallback row and `SoftwareTitleDetailsPage` hard-coded it on the App Store branch. But the pin is FMA-only semantics (introduced with the FMA versioning feature in #47944) - for custom packages and App Store apps it had no meaning and no click handler, so it looked interactive (pointer cursor, hover state) without doing anything. Fix: drop `badgeState` on custom and App Store rows so no pin renders. LibraryItemAccordion: extracted the three duplicated badge blocks (latest / pinned / major-version) into one `renderStatusBadge` helper that emits a `<Button>` when `onBadgeClick` is wired and a static `<span>` (reusing the existing `--static` modifier) when it isn't. Covers the observer-viewing-FMA case, where the pin is info-only. Kept the label-count and "All hosts" fallback badges on non-FMA rows - labels can be scoped on any installer type.
Leftover from the earlier iteration that rendered a static Latest pin on custom packages. Since the pin is now hidden entirely for non-FMA rows, no production call site matches that story. The static-span code path is still exercised by ActiveCannotEditSoftware (FMA + observer).
The onBadgeClick JSDoc, the renderStatusBadge inline comment, and the static-span test comment all still referenced the earlier plan of rendering a decorative pin on custom / App Store rows. Since those rows no longer receive a badgeState, the static-span path only fires for observers viewing FMA rows — updated the wording to reflect that.
97b8c40 to
409ded1
Compare
Issue
Closes #48480
Description
buildLibraryVersionRowsstampedbadgeState: "latest"on the non-FMA fallback row andSoftwareTitleDetailsPagehard-coded it on the App Store branch. But the pin is FMA-only semantics (introduced with the FMA versioning feature in Fleet UI: APRF library item accordion component #47944) — for custom packages and App Store apps it had no meaning and no click handler, so it looked interactive (pointer cursor, hover state) without doing anything.badgeStateon custom and App Store rows so no pin renders. FMA rows still show it — clickable for admins/maintainers via the versions modal, static for observers.LibraryItemAccordion: extracted the three duplicated badge blocks (latest/pinned/majorVersion) into onerenderStatusBadgehelper that emits a<Button>whenonBadgeClickis wired and a static<span>(reusing the existing--staticmodifier) when it isn't. Covers the observer-viewing-FMA case, where the pin is info-only.Screenrecording
Screen.Recording.2026-07-07.at.9.58.09.AM.mov
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation