Fleet UI: Route software title names through getDisplayedSoftwareName (#47084)
This commit is contained in:
@@ -83,6 +83,9 @@ Use helpers from `frontend/utilities/strings/stringUtils.ts`:
|
||||
- `stripQuotes(str)`, `strToBool(str)` — input parsing
|
||||
- `enforceFleetSentenceCasing(str)` — respects Fleet stylization rules
|
||||
|
||||
## Software titles — display name
|
||||
Render software title names via `getDisplayedSoftwareName(name, display_name)` from `pages/SoftwarePage/helpers.tsx` — never raw `t.name` or open-coded `display_name || name`. See `frontend/docs/patterns.md`.
|
||||
|
||||
## Styling (SCSS + BEM)
|
||||
- Define `const baseClass = "component-name"` at the top of the component
|
||||
- Elements: `` className={`${baseClass}__element-name`} ``
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
- Fixed software titles displaying the raw package name instead of the admin-set display name in the policy automations list and edit modal, the patch automation CTA, the hosts software filter pill, and the setup experience software row.
|
||||
@@ -1,11 +1,16 @@
|
||||
import React from "react";
|
||||
import { waitFor } from "@testing-library/react";
|
||||
import { Command } from "cmdk";
|
||||
import { createCustomRenderer } from "test/test-utils";
|
||||
|
||||
import softwareAPI from "services/entities/software";
|
||||
import { createMockSoftwareTitle } from "__mocks__/softwareMock";
|
||||
|
||||
import SoftwarePicker from "./SoftwarePicker";
|
||||
|
||||
// cmdk uses scrollIntoView which JSDOM doesn't implement.
|
||||
Element.prototype.scrollIntoView = jest.fn();
|
||||
|
||||
jest.mock("services/entities/software", () => ({
|
||||
__esModule: true,
|
||||
default: { getSoftwareTitles: jest.fn() },
|
||||
@@ -14,6 +19,9 @@ jest.mock("services/entities/software", () => ({
|
||||
const mockedSoftware = softwareAPI as jest.Mocked<typeof softwareAPI>;
|
||||
|
||||
const renderPicker = createCustomRenderer({ withBackendMock: true });
|
||||
// Picker renders Command.Item, which needs a Command root in context.
|
||||
const renderPickerInCommand = (ui: React.ReactElement) =>
|
||||
renderPicker(<Command>{ui}</Command>);
|
||||
|
||||
beforeEach(() => {
|
||||
mockedSoftware.getSoftwareTitles.mockReset();
|
||||
@@ -120,6 +128,40 @@ describe("SoftwarePicker", () => {
|
||||
expect(await findByText(/^No software found\.$/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders title labels via getDisplayedSoftwareName (display_name and normalization)", async () => {
|
||||
mockedSoftware.getSoftwareTitles.mockResolvedValue({
|
||||
count: 2,
|
||||
counts_updated_at: null,
|
||||
software_titles: [
|
||||
createMockSoftwareTitle({
|
||||
id: 1,
|
||||
name: "Zoom.pkg",
|
||||
display_name: "Zoom Workplace",
|
||||
}),
|
||||
createMockSoftwareTitle({
|
||||
id: 2,
|
||||
name: "Microsoft.CompanyPortal",
|
||||
}),
|
||||
],
|
||||
meta: { has_next_results: false, has_previous_results: false },
|
||||
});
|
||||
|
||||
// Unique search value sidesteps React Query cache pollution from
|
||||
// earlier empty-state tests under queryKey [..., "<search>"].
|
||||
const { findByText, queryByText } = renderPickerInCommand(
|
||||
<SoftwarePicker
|
||||
search="display-name-test"
|
||||
currentTeam={{ id: 5, name: "Engineering" }}
|
||||
onSelect={jest.fn()}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(await findByText("Zoom Workplace")).toBeInTheDocument();
|
||||
expect(await findByText("Company Portal")).toBeInTheDocument();
|
||||
expect(queryByText("Zoom.pkg")).not.toBeInTheDocument();
|
||||
expect(queryByText("Microsoft.CompanyPortal")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("library empty state uses 'this fleet's library' for Unassigned", async () => {
|
||||
const { findByText } = renderPicker(
|
||||
<SoftwarePicker
|
||||
|
||||
@@ -14,7 +14,10 @@ import {
|
||||
import softwareAPI, {
|
||||
ISoftwareTitlesResponse,
|
||||
} from "services/entities/software";
|
||||
import { getAutomaticInstallPoliciesCount } from "pages/SoftwarePage/helpers";
|
||||
import {
|
||||
getAutomaticInstallPoliciesCount,
|
||||
getDisplayedSoftwareName,
|
||||
} from "pages/SoftwarePage/helpers";
|
||||
import { InstallIconWithTooltip } from "components/TableContainer/DataTable/SoftwareNameCell/SoftwareNameCell";
|
||||
|
||||
import getFleetSuffix from "./pickerCopy";
|
||||
@@ -118,7 +121,7 @@ const SoftwarePicker = ({
|
||||
return (
|
||||
<Command.Group className={`${baseClass}__group`}>
|
||||
{titles.map((title) => {
|
||||
const label = title.display_name || title.name;
|
||||
const label = getDisplayedSoftwareName(title.name, title.display_name);
|
||||
const typeLabel = formatSoftwareType(title);
|
||||
const installerProps = getInstallerProps(title);
|
||||
return (
|
||||
|
||||
@@ -146,6 +146,37 @@ export default {
|
||||
}
|
||||
```
|
||||
|
||||
### Display names for software titles
|
||||
|
||||
Software titles have two fields that look like a name:
|
||||
|
||||
- `name` — the raw title from the installer/package metadata (e.g. `Microsoft.CompanyPortal`)
|
||||
- `display_name` — an optional custom name set per fleet by an admin
|
||||
|
||||
**Never render `name` directly in the UI.** Always route software names through
|
||||
`getDisplayedSoftwareName(name, display_name)` from `pages/SoftwarePage/helpers.tsx`.
|
||||
It prefers `display_name`, normalizes known awkward titles (e.g.
|
||||
`microsoft.companyportal` → `Company Portal`), and falls back to a sensible
|
||||
default. This applies everywhere a software title is shown: table rows, dropdown
|
||||
options, modal text, activity feed entries, automation summaries, etc.
|
||||
|
||||
```tsx
|
||||
// good
|
||||
label: getDisplayedSoftwareName(title.name, title.display_name),
|
||||
|
||||
// bad — misses display_name and the WELL_KNOWN_SOFTWARE_TITLES normalization
|
||||
label: title.name,
|
||||
|
||||
// also bad — misses the WELL_KNOWN_SOFTWARE_TITLES normalization
|
||||
label: title.display_name || title.name,
|
||||
```
|
||||
|
||||
The same rule applies to any object shape that carries both fields
|
||||
(`ISoftwareTitle`, `ISoftwarePackage`, `IAppStoreApp`, `IHostSoftware`,
|
||||
`IPolicySoftwareToInstall`, etc.). The `ISoftwareTitle.name` JSDoc states the
|
||||
expectation: "All software names displayed by UI is ran through
|
||||
getDisplayedSoftwareName."
|
||||
|
||||
## Components
|
||||
|
||||
### React functional components
|
||||
|
||||
@@ -173,6 +173,14 @@ describe("getDisplayedSoftwareName", () => {
|
||||
expect(getDisplayedSoftwareName("Some App", "")).toBe("Some App");
|
||||
});
|
||||
|
||||
it("treats a whitespace-only display_name as absent and falls back to name", () => {
|
||||
expect(getDisplayedSoftwareName("Some App", " ")).toBe("Some App");
|
||||
});
|
||||
|
||||
it("returns the default when display_name and name are both whitespace-only", () => {
|
||||
expect(getDisplayedSoftwareName(" ", " ")).toBe("Software");
|
||||
});
|
||||
|
||||
it("returns a default when neither name nor display_name is provided", () => {
|
||||
expect(getDisplayedSoftwareName(undefined, undefined)).toBe("Software");
|
||||
expect(getDisplayedSoftwareName(null, null)).toBe("Software");
|
||||
|
||||
@@ -322,12 +322,13 @@ export const getDisplayedSoftwareName = (
|
||||
name?: string | null,
|
||||
display_name?: string | null
|
||||
): string => {
|
||||
// 1. End-user custom name always wins.
|
||||
if (display_name) {
|
||||
// 1. End-user custom name always wins. Treat whitespace-only as absent so
|
||||
// an inadvertent " " from the backend doesn't render a blank label.
|
||||
if (display_name?.trim()) {
|
||||
return display_name;
|
||||
}
|
||||
|
||||
if (name) {
|
||||
if (name?.trim()) {
|
||||
// 2. Normalize known titles only from the raw name.
|
||||
const key = name.toLowerCase();
|
||||
if (WELL_KNOWN_SOFTWARE_TITLES[key]) {
|
||||
|
||||
+2
-2
@@ -20,6 +20,7 @@ import {
|
||||
import { IMunkiIssuesAggregate } from "interfaces/macadmins";
|
||||
import { IPolicy } from "interfaces/policy";
|
||||
import { SoftwareAggregateStatus } from "interfaces/software";
|
||||
import { getDisplayedSoftwareName } from "pages/SoftwarePage/helpers";
|
||||
|
||||
import {
|
||||
HOSTS_QUERY_PARAMS,
|
||||
@@ -352,11 +353,10 @@ const HostsFilterBlock = ({
|
||||
if (!softwareDetails) return null;
|
||||
|
||||
const { name, display_name, version } = softwareDetails;
|
||||
let label = display_name || name;
|
||||
let label = getDisplayedSoftwareName(name, display_name);
|
||||
if (version) {
|
||||
label += ` ${version}`;
|
||||
}
|
||||
label = label.trim() || "Unknown software";
|
||||
|
||||
const clearParams = [
|
||||
"software_id",
|
||||
|
||||
+2
-1
@@ -3,6 +3,7 @@ import React from "react";
|
||||
import { CellProps, Column } from "react-table";
|
||||
|
||||
import { ISetupStep } from "interfaces/setup";
|
||||
import { getDisplayedSoftwareName } from "pages/SoftwarePage/helpers";
|
||||
|
||||
import SetupSoftwareProcessCell from "components/TableContainer/DataTable/SetupSoftwareProcessCell";
|
||||
import SetupSoftwareStatusCell from "components/TableContainer/DataTable/SetupSoftwareStatusCell";
|
||||
@@ -22,7 +23,7 @@ const generateColumnConfigs = (): ISetupStatusTableConfig[] => [
|
||||
if (type === "software_install") {
|
||||
return (
|
||||
<SetupSoftwareProcessCell
|
||||
name={display_name || name || "Unknown software"}
|
||||
name={getDisplayedSoftwareName(name, display_name)}
|
||||
url={icon_url}
|
||||
/>
|
||||
);
|
||||
|
||||
@@ -400,7 +400,7 @@ const InstallStatusCell = ({
|
||||
!!software.app_store_app && isAndroid(software.app_store_app.platform);
|
||||
const lastInstall = getLastInstall(software); // TODO (back end bug fix) - `software.app_store_app.last_install sometimes coming back `null` for VPP apps, currently falls back to displaying the `InventoryVersionsModal`
|
||||
const lastUninstall = getLastUninstall(software);
|
||||
const softwarePackageName = getSoftwarePackageName(software); // @RachelElysia I renamed this function and the variable name its return value is set to here because it is looking at the software_package.name, which has a suffix like ".pkg". software.name has the more human-readable version. Not sure how else this data is being used so I am not going to refactor anything. Please update if needed.
|
||||
const softwarePackageName = getSoftwarePackageName(software);
|
||||
const displayStatus = software.ui_status;
|
||||
|
||||
if (displayStatus === "uninstalled" || displayStatus === "never_ran_script") {
|
||||
|
||||
@@ -334,6 +334,17 @@ describe("getAutomationsForPolicy", () => {
|
||||
expect(result[0].name).toBe("Chrome.app");
|
||||
});
|
||||
|
||||
it("normalizes known awkward titles via getDisplayedSoftwareName", () => {
|
||||
const result = getAutomationsForPolicy({
|
||||
...basePolicy,
|
||||
install_software: {
|
||||
name: "Microsoft.CompanyPortal",
|
||||
software_title_id: 42,
|
||||
},
|
||||
});
|
||||
expect(result[0].name).toBe("Company Portal");
|
||||
});
|
||||
|
||||
it("returns script automation with file name", () => {
|
||||
const result = getAutomationsForPolicy({
|
||||
...basePolicy,
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import React from "react";
|
||||
|
||||
import { IPolicyStats, OtherAutomationType } from "interfaces/policy";
|
||||
import { getDisplayedSoftwareName } from "pages/SoftwarePage/helpers";
|
||||
|
||||
import { IInstallSoftwareFormData } from "./components/InstallSoftwareModal/InstallSoftwareModal";
|
||||
import { IPolicyRunScriptFormData } from "./components/PolicyRunScriptModal/PolicyRunScriptModal";
|
||||
@@ -47,8 +48,10 @@ export const getAutomationsForPolicy = (
|
||||
if (policy.install_software) {
|
||||
automations.push({
|
||||
type: "software",
|
||||
name:
|
||||
policy.install_software.display_name || policy.install_software.name,
|
||||
name: getDisplayedSoftwareName(
|
||||
policy.install_software.name,
|
||||
policy.install_software.display_name
|
||||
),
|
||||
softwareTitleId: policy.install_software.software_title_id,
|
||||
iconUrl: policy.install_software.icon_url,
|
||||
});
|
||||
|
||||
@@ -60,6 +60,45 @@ describe("PatchAutomationCta", () => {
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("prefers patch_software.display_name over name in the label", () => {
|
||||
renderWithAppContext(
|
||||
<PatchAutomationCta
|
||||
storedPolicy={createMockPatchPolicy({
|
||||
patch_software: {
|
||||
name: "Microsoft.CompanyPortal",
|
||||
display_name: "Company Portal (Corp)",
|
||||
software_title_id: 42,
|
||||
},
|
||||
})}
|
||||
canEditPolicy
|
||||
onAddAutomation={jest.fn()}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(
|
||||
screen.getByText(/Automatically patch Company Portal \(Corp\)/)
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("normalizes well-known patch_software names when display_name is absent", () => {
|
||||
renderWithAppContext(
|
||||
<PatchAutomationCta
|
||||
storedPolicy={createMockPatchPolicy({
|
||||
patch_software: {
|
||||
name: "Microsoft.CompanyPortal",
|
||||
software_title_id: 42,
|
||||
},
|
||||
})}
|
||||
canEditPolicy
|
||||
onAddAutomation={jest.fn()}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(
|
||||
screen.getByText(/Automatically patch Company Portal/)
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("calls onAddAutomation when the button is clicked", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onAddAutomation = jest.fn();
|
||||
|
||||
@@ -5,6 +5,7 @@ import { IPolicy } from "interfaces/policy";
|
||||
import Button from "components/buttons/Button";
|
||||
import GitOpsModeTooltipWrapper from "components/GitOpsModeTooltipWrapper";
|
||||
import Icon from "components/Icon";
|
||||
import { getDisplayedSoftwareName } from "pages/SoftwarePage/helpers";
|
||||
|
||||
const baseClass = "patch-automation-cta";
|
||||
|
||||
@@ -26,22 +27,21 @@ const PatchAutomationCta = ({
|
||||
isAddingAutomation,
|
||||
}: IPatchAutomationCtaProps): JSX.Element | null => {
|
||||
const isPatchPolicy = storedPolicy.type === "patch";
|
||||
const hasPatchSoftware = !!storedPolicy.patch_software;
|
||||
const hasSoftwareAutomation = !!storedPolicy.install_software;
|
||||
|
||||
if (
|
||||
!isPatchPolicy ||
|
||||
!hasPatchSoftware ||
|
||||
!storedPolicy.patch_software ||
|
||||
hasSoftwareAutomation ||
|
||||
!canEditPolicy
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const patchSoftwareName =
|
||||
storedPolicy.patch_software?.display_name ||
|
||||
storedPolicy.patch_software?.name ||
|
||||
"";
|
||||
const patchSoftwareName = getDisplayedSoftwareName(
|
||||
storedPolicy.patch_software.name,
|
||||
storedPolicy.patch_software.display_name
|
||||
);
|
||||
|
||||
return (
|
||||
<div className={baseClass}>
|
||||
|
||||
+2
-1
@@ -30,6 +30,7 @@ import {
|
||||
getTicketOrWebhookInfo,
|
||||
getTicketOrWebhookLabel,
|
||||
} from "pages/policies/helpers";
|
||||
import { getDisplayedSoftwareName } from "pages/SoftwarePage/helpers";
|
||||
|
||||
import { IPolicyAutomationUpdate } from "pages/policies/hooks";
|
||||
|
||||
@@ -218,7 +219,7 @@ const PolicyAutomationsFields = forwardRef<
|
||||
const softwareOptions: CustomOptionType[] = useMemo(
|
||||
() =>
|
||||
(softwareTitlesData?.software_titles ?? []).map((t) => ({
|
||||
label: t.name,
|
||||
label: getDisplayedSoftwareName(t.name, t.display_name),
|
||||
value: String(t.id),
|
||||
helpText: generateSoftwareOptionHelpText(t),
|
||||
})),
|
||||
|
||||
+37
@@ -61,6 +61,43 @@ describe("PolicyAutomationsList", () => {
|
||||
expect(screen.queryByText("No automations")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("prefers install_software.display_name over name", () => {
|
||||
render(
|
||||
<PolicyAutomationsList
|
||||
storedPolicy={createMockPolicy({
|
||||
install_software: {
|
||||
name: "Zoom.pkg",
|
||||
display_name: "Zoom Workplace",
|
||||
software_title_id: 42,
|
||||
},
|
||||
})}
|
||||
currentAutomatedPolicies={[]}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(screen.getByText("Zoom Workplace")).toBeInTheDocument();
|
||||
expect(screen.queryByText("Zoom.pkg")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("normalizes well-known software names via the display helper", () => {
|
||||
render(
|
||||
<PolicyAutomationsList
|
||||
storedPolicy={createMockPolicy({
|
||||
install_software: {
|
||||
name: "Microsoft.CompanyPortal",
|
||||
software_title_id: 42,
|
||||
},
|
||||
})}
|
||||
currentAutomatedPolicies={[]}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(screen.getByText("Company Portal")).toBeInTheDocument();
|
||||
expect(
|
||||
screen.queryByText("Microsoft.CompanyPortal")
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("forwards the custom software icon_url to SoftwareIcon", () => {
|
||||
const iconUrl = ENDPOINTS.SOFTWARE_ICON(42);
|
||||
render(
|
||||
|
||||
@@ -8,6 +8,7 @@ import { getPathWithQueryParams } from "utilities/url";
|
||||
import Graphic from "components/Graphic";
|
||||
import { GraphicNames } from "components/graphics";
|
||||
import SoftwareIcon from "pages/SoftwarePage/components/icons/SoftwareIcon";
|
||||
import { getDisplayedSoftwareName } from "pages/SoftwarePage/helpers";
|
||||
|
||||
const baseClass = "policy-automations-list";
|
||||
|
||||
@@ -44,8 +45,12 @@ const PolicyAutomationsList = ({
|
||||
const automationRows: IAutomationDisplayRow[] = [];
|
||||
|
||||
if (storedPolicy.install_software) {
|
||||
const displayedName = getDisplayedSoftwareName(
|
||||
storedPolicy.install_software.name,
|
||||
storedPolicy.install_software.display_name
|
||||
);
|
||||
automationRows.push({
|
||||
name: storedPolicy.install_software.name,
|
||||
name: displayedName,
|
||||
type: "Software",
|
||||
isSoftware: true,
|
||||
iconUrl: storedPolicy.install_software.icon_url,
|
||||
@@ -56,7 +61,7 @@ const PolicyAutomationsList = ({
|
||||
{ fleet_id: storedPolicy.team_id }
|
||||
),
|
||||
sortOrder: 0,
|
||||
sortName: storedPolicy.install_software.name.toLowerCase(),
|
||||
sortName: displayedName.toLowerCase(),
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user