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

Fix last_enrolled_at for macOS devices when re-enrolling to MDM #20173

Merged
merged 7 commits into from
Jul 11, 2024

Conversation

lucasmrod
Copy link
Member

@lucasmrod lucasmrod commented Jul 2, 2024

#20059

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.
  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements)
  • If database migrations are included, checked table schema to confirm autoupdate
  • For database migrations:
    • Checked schema for all modified table for columns that will auto-update timestamps during migration.
    • Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects.
    • Ensured the correct collation is explicitly set for character columns (COLLATE utf8mb4_unicode_ci).
  • Added/updated tests
  • Manual QA for all new/changed functionality

roperzh
roperzh previously approved these changes Jul 3, 2024
gillespi314
gillespi314 previously approved these changes Jul 3, 2024
@@ -770,30 +770,37 @@ func updateMDMAppleHostDB(
) error {
refetchRequested, lastEnrolledAt := mdmHostEnrollFields(mdmHost)
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it would make things easier to maintain to split up this abstraction and calculate these two values separately when needed?

@sharon-fdm sharon-fdm added the :ai Request AI PR review label Jul 9, 2024
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

  • Added changes file changes/20059-fix-last_enrolled_at documenting the fix
  • Fixed last_enrolled_at field update for macOS hosts re-enrolling via MDM
  • Updated tests to cover the new functionality
  • Conducted manual QA for the changes

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings

@@ -0,0 +1 @@
* Fixed bug that set `Added to Fleet` to `Never` after macOS hosts re-enrolled to Fleet via MDM.
Copy link

Choose a reason for hiding this comment

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

ℹ️ info: Documented the fix for the last_enrolled_at field issue for macOS re-enrollment.

Copy link
Member

@getvictor getvictor left a comment

Choose a reason for hiding this comment

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

LGTM

@Sampfluger88
Copy link
Member

I'm assuming I was requested by mistake?

@lucasmrod lucasmrod merged commit b8479fa into main Jul 11, 2024
17 of 18 checks passed
@lucasmrod lucasmrod deleted the 20059-fix-last_enrolled_at branch July 11, 2024 00:40
@lucasmrod
Copy link
Member Author

I'm assuming I was requested by mistake?

Maybe because of the change to CODEOWNERS?

lucasmrod added a commit that referenced this pull request Jul 11, 2024
…0173)

- [X] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [X] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [X] If database migrations are included, checked table schema to
confirm autoupdate
- For database migrations:
- [X] Checked schema for all modified table for columns that will
auto-update timestamps during migration.
- [X] Confirmed that updating the timestamps is acceptable, and will not
cause unwanted side effects.
- [X] Ensured the correct collation is explicitly set for character
columns (`COLLATE utf8mb4_unicode_ci`).
- [X] Added/updated tests
- [X] Manual QA for all new/changed functionality
lucasmrod added a commit that referenced this pull request Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ai Request AI PR review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants