UI – Uninstall features for host details, install/uninstall actions, activity feed, misc other items #21933
Conversation
jacobshandling
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
jacobshandling
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
jacobshandling
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
jacobshandling
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
jacobshandling
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
jacobshandling
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
jacobshandling
had a problem deploying
to
Docker Hub
GitHub Actions
Error
jacobshandling
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
jacobshandling
changed the title
jacobshandling
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
jacobshandling
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
| ); | ||
| }; | ||
|
|
||
| const SoftwareUninstallDetailsModal = ({ |
There was a problem hiding this comment.
I didn't notice any significant differences between uninstall and install details? Are we doing this as its own component for speed of development? (Not a problem, I just wanted to be sure I wasn't missing something.)
There was a problem hiding this comment.
The differences in data source (similar UI, from different fields of different APIs) for various elements were proving challenging to reconcile. I actually spent a few hours trying to use the existing modal at first, and this route proved simplest and clearest.
| isWindowsPackageType(pkgType) ? "PowerS" : "s" | ||
| }hell scripts are supported.`; |
There was a problem hiding this comment.
| isWindowsPackageType(pkgType) ? "PowerS" : "s" | |
| }hell scripts are supported.`; | |
| isWindowsPackageType(pkgType) ? "PowerShell" : "shell" | |
| } scripts are supported.`; |
I appreciate the character efficiency, but I think it would be a little easier to read if slightly more verbose.
| @@ -175,13 +176,13 @@ const HostSoftware = ({ | |||
| [isMyDevicePage, refetchDeviceSoftware, refetchHostSoftware] | |||
| ); | |||
|
|
|||
| const userHasSWInstallPermission = Boolean( | |||
| const userHasSWWritePermission = Boolean( | |||
There was a problem hiding this comment.
Maybe userCanWriteSoftware?
| @@ -105,7 +105,8 @@ const HostSoftware = ({ | |||
| isTeamMaintainer, | |||
| } = useContext(AppContext); | |||
|
|
|||
| const [installingSoftwareId, setInstallingSoftwareId] = useState< | |||
| // disables install/uninstall actions after click | |||
| const [softwareIdActionPending, setSoftwareIdActionPending] = useState< | |||
There was a problem hiding this comment.
Maybe just pendingSoftwareId and setPendingSoftwareId?
| "pending_uninstall", | ||
| "failed_uninstall", | ||
| "failed_install", | ||
| ...SOFTWARE_UNINSTALL_STATUSES, |
There was a problem hiding this comment.
If we have time for changes, I'd probably split this into three consts: install, uninstall, and a combined set of install and uninstall (which I'd just call SOFTWARE_STATUSES).
That way the composition is more clear: [...SOFTWARE_INSTALL_STATUSES, ...SOFTWARE_UNINSTALL_STATUSES].
| ISoftwareTitleDetails, | ||
| isSoftwarePackage, | ||
| } from "interfaces/software"; | ||
| import { DEFAULT_EMPTY_CELL_VALUE } from "utilities/constants"; | ||
|
|
||
| const mergePackageStatuses = (packageStatuses: ISoftwarePackage["status"]) => ({ | ||
| installed: packageStatuses.installed, | ||
| pending: packageStatuses.pending_install + packageStatuses.pending_uninstall, |
There was a problem hiding this comment.
How is this used? Is there someplace where we are checking for the potential string "pending_installpending_uninstall"? Are we counting on the backend to ensure that never happens?
There was a problem hiding this comment.
If I'm understanding your question, those fields actually contain counts of hosts in their respective statuses, not strings representing those statuses, so this gives us the total number of hosts pending install or pending uninstall.
| SoftwareInstallStatus | "pending", | ||
| string | ||
| > = { | ||
| pending: "pending", |
There was a problem hiding this comment.
Is pending being retained for for backwards compatibility? Does this still generate the expected text in the getInstallStatusPredicate function?
There was a problem hiding this comment.
I'll double check and clean this up a bit if it's not used anymore on an iterative PR, thanks for catching
jacobshandling
merged commit d9f7935
into
20320-feat-uninstall-packages
#21566 - Host details updates for Uninstall packages
details > software page. Full tasks outlined in the issue, Figma here
Updated install status tooltips:
Uninstall action:
Update install details:
#21931 - updated specs for install/uninstall states
#21568 - activity feed items for Uninstall
#21567 - Uninstall details modal
remaining TODO:
manually QA 'failed' states
determine where to source timestamp from for uninstall activities
Manual QA for all new/changed functionality