Skip to content

Add UI for Fleet Sandbox to download prepackaged installers#6721

Merged
gillespi314 merged 9 commits into
mainfrom
issue-5757-download-orbit-installer
Jul 19, 2022
Merged

Add UI for Fleet Sandbox to download prepackaged installers#6721
gillespi314 merged 9 commits into
mainfrom
issue-5757-download-orbit-installer

Conversation

@gillespi314
Copy link
Copy Markdown
Contributor

@gillespi314 gillespi314 commented Jul 18, 2022

Issue #5757

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Changes file added for user-visible changes (in changes/ and/or orbit/changes/).
  • Added/updated tests
  • Manual QA for all new/changed functionality

TODO: QA on Windows and Linux

@gillespi314 gillespi314 temporarily deployed to Docker Hub July 18, 2022 16:16 Inactive
@gillespi314 gillespi314 temporarily deployed to Docker Hub July 18, 2022 23:41 Inactive
Copy link
Copy Markdown
Contributor

@roperzh roperzh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the doc changes, they LGTM! I also pulled and tested and everything works great 👍

Leaving the code / style review for the people that really know about that.

Copy link
Copy Markdown
Member

@lukeheath lukeheath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 Looks great! A couple of minor nits.

As for styling, I think a hover effect on the buttons would be nice. We don't have direction on hover style, so what do you think of changing the button border on hover, then highlight the rest on select?

Comment thread docs/Contributing/Testing.md
Comment thread frontend/components/AddHostsModal/DownloadInstallers/DownloadInstallers.tsx Outdated
Comment thread frontend/components/AddHostsModal/DownloadInstallers/DownloadInstallers.tsx Outdated
const path = `${ENDPOINTS.DOWNLOAD_INSTALLER}/${encodeURIComponent(
enrollSecret
)}/${installerType}?desktop=${includeDesktop}`;
console.log("path: ", path);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray log.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukeheath is there any logging we might want here in case of sandbox support requests?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the plan is to monitor the S3 logs so we're notified if there are any 404s. Beyond that I'm not thinking we need any extra logging in the frontend for now.

@gillespi314
Copy link
Copy Markdown
Contributor Author

Thank you!

@gillespi314 gillespi314 temporarily deployed to Docker Hub July 19, 2022 15:47 Inactive
@gillespi314 gillespi314 requested a review from lukeheath July 19, 2022 15:47
@gillespi314 gillespi314 temporarily deployed to Docker Hub July 19, 2022 15:51 Inactive
Copy link
Copy Markdown
Member

@lukeheath lukeheath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@gillespi314 gillespi314 merged commit 4792d7a into main Jul 19, 2022
@gillespi314 gillespi314 deleted the issue-5757-download-orbit-installer branch July 19, 2022 19:28
@gillespi314
Copy link
Copy Markdown
Contributor Author

@xpkoala, quick QA note for whenever you turn to the Sandbox features. We should probably smoke test the download installer flow on all platforms (including verifying that Fleet Desktop works as expected on the device).

@Desmi-Dizney
Copy link
Copy Markdown
Contributor

Desmi-Dizney added a commit that referenced this pull request Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants