test(spp_registry): add test suite with 83% line+branch coverage#214
Merged
Conversation
The module shipped with no tests; this adds 240 tests across 14 files covering constraints, computes, onchanges, the CRUD-override metric invalidation chain, the disable-registrant wizard, the mail controller overrides, the SQL aggregation builder, and the unlink permission guard. Tests surfaced and pinned several real issues for follow-up: - mail.py::mail_message_update_content calls ir.attachment._check_attachments_access, which no longer exists in Odoo 19; the endpoint is broken. - _query_members_aggregate uses the deprecated expression.expression() API, which silently produces empty WHERE clauses on Odoo 19. As a result the disabled/ended-membership filters never apply and count_individuals() over-counts. Affects all callers that build group metrics off the count. - _onchange_source_destination_clear_relation clears a relation type even when it's valid for the new partner-type pair (e.g. 'head' is valid for both i2i and mixed). - _compute_phone_sanitized stores the unsanitized fallback value when parsing fails, so phone_sanitized can hold non-E164 strings. - _check_age_is_integer is dead code (constrains on a non-stored compute). - _check_date_collected is onchange-only and bypassed by programmatic create. - disable_registrant wizard has no idempotency guard, unlike its siblings. - count_individuals(relationship_kinds=...) expects display strings, not vocab code slugs — silent footgun. Each finding has a skipped test with a detailed TODO documenting the discovery and the remediation path, so the value isn't lost.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive suite of unit and integration tests for the spp_registry module, covering constraints, controllers, wizards, and model logic. The review feedback suggests cleaning up unused imports in test_group_aggregation.py and making the empty phone sanitization assertion in test_phone_number.py more robust by including False in the expected values.
Comment on lines
+39
to
+42
| from datetime import datetime, timedelta | ||
|
|
||
| from odoo import fields | ||
| from odoo.tests import tagged |
There was a problem hiding this comment.
The imports datetime, timedelta, and fields are imported but never used in this test file. Removing unused imports keeps the codebase clean and maintainable.
Suggested change
| from datetime import datetime, timedelta | |
| from odoo import fields | |
| from odoo.tests import tagged | |
| from odoo.tests import tagged |
| # A whitespace-only string is truthy in Python, so this goes | ||
| # through phone_format, which returns either the formatted | ||
| # value or empty. | ||
| self.assertIn(rec.phone_sanitized, ("", None, " ")) |
There was a problem hiding this comment.
In Odoo, empty or null Char fields are typically represented as False in Python when read from the database. To ensure the test is robust and does not fail if the computed field returns False instead of "" or None, it is safer to include False in the allowed values list.
Suggested change
| self.assertIn(rec.phone_sanitized, ("", None, " ")) | |
| self.assertIn(rec.phone_sanitized, ("", False, None, " ")) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #214 +/- ##
==========================================
+ Coverage 71.27% 71.49% +0.22%
==========================================
Files 976 993 +17
Lines 57736 58564 +828
==========================================
+ Hits 41150 41869 +719
- Misses 16586 16695 +109
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Replace blind `assertRaises(Exception)` with the actual `UserError` raised by `phone_validation.phone_format`, drop unused imports, and apply ruff format to the new test files so pre-commit passes on CI.
gonzalesedwin1123
approved these changes
May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
spp_registryshipped with no tests. This PR adds 240 tests across 14 files, taking the module from 0% to 83% line+branch coverage.models/registrant.pymodels/reg_id.pymodels/phone_number.pymodels/individual.pymodels/group.pymodels/reg_relationship.pymodels/group_membership.pycontrollers/mail.pywizard/disable_registrant.pyFindings surfaced by the tests
These are real issues in the module, pinned as skipped tests with detailed TODOs so the discovery isn't lost:
controllers/mail.py::mail_message_update_contentis broken on Odoo 19. It callsir.attachment._check_attachments_access, which was removed upstream. Every call to the endpoint raisesAttributeError._query_members_aggregatefilters don't apply on Odoo 19. The impl uses the deprecatedexpression.expression()API, which produces empty WHERE clauses. As a result thedisabled IS NULLandis_ended = Falsefilters never apply —count_individuals()over-counts. Affects every caller that builds group metrics off the result._onchange_source_destination_clear_relationclears valid relations. Switching destination from individual to group wipeshead, even thoughheadis in the mixed concept group._compute_phone_sanitizedstores the unsanitized fallback. When parsing fails, the original (non-E164) string lands inphone_sanitized._check_age_is_integeris dead code.@api.constrainson a non-stored computed Char field doesn't fire._check_date_collectedis onchange-only. Programmaticcreate()with a future date bypasses the guard.disable_registrantwizard has no idempotency guard (unlikedisable_phone/disable_relationship).count_individuals(relationship_kinds=...)expects display strings, not code slugs. Silent footgun: passing["head"]returns no matches;["Head"]works.What's tested, by file
test_constraints.py— group head-uniqueness, registration-date, SPPIDType ADR-007 URI formattest_unlink_permissions.py— officer/registrar/manager/admin gates onregistrant.unlinktest_mail_controllers.py— author/admin/bystander/unauth matrices on both mail endpoints (HttpCase)test_relationships.py— partner-type filter, date constraints, overlap rules, disable/enabletest_metric_invalidation.py— group/individual/membership write → invalidation funneltest_wizard_disable_registrant.py— context active_id, audit field stampingtest_individual_name.py— name composition, age, future-birthdate onchangetest_phone_number.py— date_collected, format fallbacks, disable/enable, registrant synctest_reg_id.py— ID-type picklist, is_verified compute, regex validation, display name, name_searchtest_membership_constraints.py— duplicate member, ended-date constraint, status/active togglestest_registrant_misc.py— enable, negative-income onchange, company-domain override, ID/relationship countstest_group_aggregation.py—count_individuals+_query_members_aggregate(the SQL builder)Test plan
./scripts/test_single_module.sh spp_registry