Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "Include Fleet Desktop" option to "Add hosts" modal #5866

Merged

Conversation

noahtalerman
Copy link
Member

@noahtalerman noahtalerman commented May 23, 2022

Related to #5663
Screen Shot 2022-05-23 at 3 09 33 PM

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/).
  • Documented any API changes (docs/Using-Fleet/REST-API.md)
  • Documented any permissions changes
  • Ensured that input data is properly validated, SQL injection is prevented (using placeholders for values in statements)
  • Added support on fleet's osquery simulator cmd/osquery-perf for new osquery data ingestion features.
  • Added/updated tests
  • Manual QA for all new/changed functionality

@noahtalerman noahtalerman requested a review from lukeheath May 23, 2022 19:09
@lucasmrod
Copy link
Member

lucasmrod commented May 23, 2022

Do we explain somewhere in the UI that "Fleet Desktop" is in beta? (If so, shouldn't it be unchecked by default?)

@noahtalerman noahtalerman requested a review from gillespi314 May 23, 2022 20:07
@noahtalerman
Copy link
Member Author

noahtalerman commented May 23, 2022

Do we explain somewhere in the UI that "Fleet Desktop" is in beta?

@lucasmrod there isn't any explanation in the UI. However, the release blog post and release notes will be loud about "Beta support for Fleet Desktop" and link to this issue that includes the work to bring Fleet Desktop out of beta: #5684

What do you think? Do you have any concerns with this approach?

shouldn't it be unchecked by default?

Yes, the option is unchecked by default. I checked the checkbox before taking the screenshot that's in this issue's description.

@lucasmrod
Copy link
Member

What do you think? Do you have any concerns with this approach?

Sounds good. (Though there will be some users like me that won't read the docs/release-notes and will try it out :)

Yes, the option is unchecked by default. I checked the checkbox before taking the screenshot that's in this issue's description.

Awesome.

Copy link
Contributor

@gillespi314 gillespi314 left a comment

Choose a reason for hiding this comment

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

Looks good. @noahtalerman were there any integration tests previously written that should also be enabled?

@lukeheath
Copy link
Member

@noahtalerman Thanks for grabbing this!

@lukeheath
Copy link
Member

@gillespi314 @noahtalerman

were there any integration tests previously written that should also be enabled?

I checked and I'm not seeing any.

@noahtalerman noahtalerman merged commit 6d4e64c into fleetdm:main May 24, 2022
@noahtalerman noahtalerman deleted the reveal-fleet-desktop-checkbox branch May 24, 2022 13:24
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