Skip to content

Added operation_type to host_certificate_templates#36926

Merged
getvictor merged 2 commits intomainfrom
victor/36684-android-status
Dec 9, 2025
Merged

Added operation_type to host_certificate_templates#36926
getvictor merged 2 commits intomainfrom
victor/36684-android-status

Conversation

@getvictor
Copy link
Copy Markdown
Member

@getvictor getvictor commented Dec 8, 2025

Related issue: Resolves #36684

This is a DB migration change as the first part of this story #36684

Checklist for submitter

Testing

  • Added/updated automated tests
  • QA'd all new/changed functionality manually

Database migrations

  • Checked schema for all modified table for columns that will auto-update timestamps during migration.
  • Ensured the correct collation is explicitly set for character columns (COLLATE utf8mb4_unicode_ci).

Summary by CodeRabbit

  • New Features
    • Added operation type tracking to host certificate templates, enabling the system to better manage certificate operations during mobile device management workflows.

✏️ Tip: You can customize this high-level summary in your review settings.

@getvictor
Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 8, 2025

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 8, 2025

Walkthrough

This PR adds an operation_type column to the host_certificate_templates database table and introduces a corresponding OperationType field to the HostCertificateTemplate struct. The field tracks the operation type (install/remove) for certificate operations, with new records defaulting to install. Changes span database migrations, schema definitions, datastore operations, and domain models.

Changes

Cohort / File(s) Summary
Database Migration
server/datastore/mysql/migrations/tables/20251208215800_AddOperationTypeToHostCertificateTemplates.go
New migration file that adds operation_type column (VARCHAR(20)) to host_certificate_templates with default 'install', foreign key constraint to mdm_operation_types, and cascading updates. Down migration is a no-op.
Schema Definition
server/datastore/mysql/schema.sql
Updated host_certificate_templates table schema to include operation_type column with default 'install', foreign key constraint, and index. Updated unique key to include operation_type. Incremented migration_status_tables AUTO_INCREMENT from 457 to 458.
Domain Models
server/fleet/host_certificate_template.go, server/fleet/certificate_templates_test.go
Added OperationType field (type MDMOperationType) to HostCertificateTemplate struct and updated ToHostMDMProfile mapping to propagate the field to HostMDMProfile. Tests updated to set and verify the new field.
Datastore Operations
server/datastore/mysql/host_certificate_templates.go, server/datastore/mysql/host_certificate_templates_test.go, server/datastore/mysql/certificate_templates_test.go
Updated BulkInsertHostCertificateTemplates to include operation_type in insert columns and values. Updated UpsertCertificateStatus to include operation_type with default MDMOperationTypeInstall. Test fixtures updated to provide OperationType field.
Service Layer
server/mdm/android/service/service.go
Updated BuildAndSendFleetAgentConfig to set OperationType to MDMOperationTypeInstall when registering newly-created certificate templates for hosts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Database migration and schema changes: Verify foreign key constraint syntax, collation, default values, and unique key modifications
  • Datastore operations: Ensure all insert/upsert paths correctly populate and handle the new operation_type field across multiple functions
  • Field consistency: Confirm OperationType field is properly propagated between HostCertificateTemplate and HostMDMProfile structs
  • Test coverage: Validate that test fixtures in multiple files consistently set the new field with appropriate values

Possibly related PRs

Suggested reviewers

  • ksykulev
  • dantecatalfamo

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ⚠️ Warning The PR description is incomplete; it covers only Database migrations section but omits multiple required checklist items including changes files, input validation, endpoint compatibility, and testing details. Complete the PR description by filling out all applicable checklist sections: verify changes files were added, confirm input validation and SQL injection prevention, address endpoint compatibility if applicable, and provide details on automated and manual testing performed.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the primary change: adding an operation_type column to the host_certificate_templates table.
Linked Issues check ✅ Passed The PR adds operation_type field to host_certificate_templates as required by #36684, with a default value of 'install' and proper database constraints, supporting the two-column design pattern (operation_type and delivery_status).
Out of Scope Changes check ✅ Passed All changes are scoped to adding the operation_type field to host_certificate_templates and related code: migrations, schema, datastore operations, and struct definitions. No unrelated modifications are present.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch victor/36684-android-status

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 8, 2025

Codecov Report

❌ Patch coverage is 87.87879% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.92%. Comparing base (0813bfb) to head (24e9732).
⚠️ Report is 34 commits behind head on main.

Files with missing lines Patch % Lines
...5800_AddOperationTypeToHostCertificateTemplates.go 71.42% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #36926   +/-   ##
=======================================
  Coverage   65.92%   65.92%           
=======================================
  Files        2263     2264    +1     
  Lines      184333   184351   +18     
  Branches     7750     7750           
=======================================
+ Hits       121514   121528   +14     
+ Misses      51716    51712    -4     
- Partials    11103    11111    +8     
Flag Coverage Δ
backend 67.72% <87.87%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@getvictor getvictor marked this pull request as ready for review December 8, 2025 23:23
@getvictor getvictor requested a review from a team as a code owner December 8, 2025 23:23
Copy link
Copy Markdown
Contributor

@mostlikelee mostlikelee left a comment

Choose a reason for hiding this comment

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

LGTM

@getvictor getvictor merged commit ec61f2c into main Dec 9, 2025
86 of 90 checks passed
@getvictor getvictor deleted the victor/36684-android-status branch December 9, 2025 16:57
@coderabbitai coderabbitai bot mentioned this pull request Dec 10, 2025
5 tasks
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.

Android certs: immediate status

2 participants