Skip to content

Fixing No team issues with Android certs. - #37483

Merged
getvictor merged 1 commit into
mainfrom
victor/37439-no-team
Dec 18, 2025
Merged

Fixing No team issues with Android certs.#37483
getvictor merged 1 commit into
mainfrom
victor/37439-no-team

Conversation

@getvictor

@getvictor getvictor commented Dec 18, 2025

Copy link
Copy Markdown
Member

Related issue: Resolves #37439

Testing

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

For unreleased bug fixes in a release candidate, one of:

  • Confirmed that the fix is not expected to adversely impact load test results

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Fixed certificate template matching for hosts without team assignment to properly associate with global certificate templates.
  • Tests

    • Added comprehensive test coverage for certificate template scenarios involving hosts without team assignments.

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

@getvictor

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Dec 18, 2025

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@coderabbitai

coderabbitai Bot commented Dec 18, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Modified SQL JOIN conditions in three certificate template query functions to treat hosts with NULL team_id as matching templates with team_id = 0, enabling certificate delivery to no-team hosts. Added comprehensive test coverage for no-team scenarios.

Changes

Cohort / File(s) Summary
Query Logic Updates
server/datastore/mysql/host_certificate_templates.go
Modified JOIN conditions in three functions (ListAndroidHostUUIDsWithDeliverableCertificateTemplates, ListCertificateTemplatesForHosts, GetCertificateTemplateForHost) by adding OR (certificate_templates.team_id = 0 AND hosts.team_id IS NULL) predicate to treat NULL host team_id as matching global templates (team_id = 0).
Test Coverage
server/datastore/mysql/host_certificate_templates_test.go
Added noTeamCertTemplateTestSetup struct and helper function for no-team fixture creation. Introduced new test function testGetCertificateTemplateForHostNoTeam and extended existing tests with subtests for no-team hosts/templates validation. Added table truncation between subtests and helper supporting nil TeamID.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • The change follows a consistent, repetitive pattern (same JOIN condition applied identically across three query functions), reducing cognitive load per occurrence
  • Test additions follow established patterns with structured setup and subtest organization
  • Narrow scope confined to two related files with a clear, focused purpose

Areas requiring attention:

  • Verify the OR condition logic correctly handles NULL vs. 0 team_id semantics across all three affected queries
  • Confirm test cases comprehensively cover edge cases (no-team hosts with no-team templates, host/template mismatch scenarios, non-existent template errors)
  • Validate error handling paths in testGetCertificateTemplateForHostNoTeam for both no-team and non-existent scenarios

Possibly related PRs

Suggested reviewers

  • ksykulev
  • mostlikelee

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 2 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Fixing No team issues with Android certs' is vague and uses non-specific terms like 'issues' without clarifying the specific problem being addressed. Improve the title to be more specific about the fix, such as 'Allow certificate delivery to no-team Android hosts' or 'Fix certificate template joining for no-team hosts'.
Description check ❓ Inconclusive The PR description is minimal and incomplete. It only includes a related issue reference and partial checklist items without detailed explanation of changes. Expand the description to include: what problem is being fixed, how the solution works, why the specific database/query changes were made, and clarify which checklist items are actually completed. Check boxes should be verified for accuracy.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code changes directly address issue #37439 by allowing hosts with NULL team_id to receive certificate templates with team_id=0, enabling certificate delivery to no-team Android hosts.
Out of Scope Changes check ✅ Passed All changes are scoped to certificate template matching logic and related test coverage, directly addressing the no-team certificate delivery issue.
✨ 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/37439-no-team

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

codecov Bot commented Dec 18, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.94%. Comparing base (c78c63b) to head (d1170b6).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #37483      +/-   ##
==========================================
- Coverage   65.95%   65.94%   -0.02%     
==========================================
  Files        2353     2355       +2     
  Lines      186464   186606     +142     
  Branches     7927     7927              
==========================================
+ Hits       122981   123049      +68     
- Misses      52235    52311      +76     
+ Partials    11248    11246       -2     
Flag Coverage Δ
backend 67.75% <100.00%> (-0.02%) ⬇️

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 18, 2025 17:45
@getvictor
getvictor requested a review from a team as a code owner December 18, 2025 17:45
@getvictor
getvictor merged commit 5f44195 into main Dec 18, 2025
45 checks passed
@getvictor
getvictor deleted the victor/37439-no-team branch December 18, 2025 23:28
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 certificates can not be delivered to a host on a no-team team

3 participants