Conversation
…tab (#942 item 1)
…mpty (#942 item 3) Two narrow computes on res.partner (has_group_type_codes, has_group_membership_type_codes) plus a mirror compute on spp.group.membership for the standalone tree view. Group Type form field on the Group form is now invisible when the urn:openspp:vocab:group-type vocabulary has no active codes. Group Role column on the Members and Group Membership lists is now column_invisible when urn:openspp:vocab:group-membership-type has no active codes — the embedded lists read parent.has_group_membership_type_codes from res.partner; the standalone tree reads the field directly off the membership record. Both fields reappear automatically as soon as a code is added or re-activated.
![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 reorders fields in the registrant profile, renames 'Membership Types' to 'Group Role', and introduces logic to conditionally hide group-related fields and columns when their respective vocabularies are empty. Feedback is provided regarding the use of the limit argument in search_count calls within the group.py and group_membership.py models, as this argument is not supported by the method and will result in a TypeError at runtime.
| has_codes = bool( | ||
| self.env["spp.vocabulary.code"] # nosemgrep: odoo-sudo-without-context | ||
| .sudo() | ||
| .search_count( | ||
| [("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:group-membership-type")], | ||
| limit=1, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
The search_count method in Odoo does not accept a limit argument. Passing limit=1 will result in a TypeError at runtime. To efficiently check for the existence of at least one record, use search() with limit=1 instead. Note: This non-deterministic search (using limit=1 without order) is acceptable here but should be acknowledged as technical debt to be addressed if the context changes.
| has_codes = bool( | |
| self.env["spp.vocabulary.code"] # nosemgrep: odoo-sudo-without-context | |
| .sudo() | |
| .search_count( | |
| [("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:group-membership-type")], | |
| limit=1, | |
| ) | |
| ) | |
| has_codes = bool( | |
| self.env["spp.vocabulary.code"] # nosemgrep: odoo-sudo-without-context | |
| .sudo() | |
| .search( | |
| [("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:group-membership-type")], | |
| limit=1, | |
| ) | |
| ) |
References
- A non-deterministic database search (using search with limit=1 but no order) can be acceptable if the practical context makes it functionally deterministic, but it should be acknowledged as technical debt.
| has_codes = bool( | ||
| self.env["spp.vocabulary.code"] # nosemgrep: odoo-sudo-without-context | ||
| .sudo() | ||
| .search_count( | ||
| [("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:group-type")], | ||
| limit=1, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
The search_count method does not support the limit parameter. This will cause a TypeError. Use search() with limit=1 to perform an efficient existence check. This non-deterministic search is acceptable but should be acknowledged as technical debt to be addressed if the context changes.
| has_codes = bool( | |
| self.env["spp.vocabulary.code"] # nosemgrep: odoo-sudo-without-context | |
| .sudo() | |
| .search_count( | |
| [("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:group-type")], | |
| limit=1, | |
| ) | |
| ) | |
| has_codes = bool( | |
| self.env["spp.vocabulary.code"] # nosemgrep: odoo-sudo-without-context | |
| .sudo() | |
| .search( | |
| [("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:group-type")], | |
| limit=1, | |
| ) | |
| ) |
References
- A non-deterministic database search (using search with limit=1 but no order) can be acceptable if the practical context makes it functionally deterministic, but it should be acknowledged as technical debt.
| has_codes = bool( | ||
| self.env["spp.vocabulary.code"] # nosemgrep: odoo-sudo-without-context | ||
| .sudo() | ||
| .search_count( | ||
| [("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:group-membership-type")], | ||
| limit=1, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
In Odoo, search_count does not accept a limit argument. This implementation will fail with a TypeError. Please use search() with limit=1 for an optimized existence check. This approach is non-deterministic without an explicit order and should be acknowledged as technical debt to be addressed if the context changes.
| has_codes = bool( | |
| self.env["spp.vocabulary.code"] # nosemgrep: odoo-sudo-without-context | |
| .sudo() | |
| .search_count( | |
| [("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:group-membership-type")], | |
| limit=1, | |
| ) | |
| ) | |
| has_codes = bool( | |
| self.env["spp.vocabulary.code"] # nosemgrep: odoo-sudo-without-context | |
| .sudo() | |
| .search( | |
| [("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:group-membership-type")], | |
| limit=1, | |
| ) | |
| ) |
References
- A non-deterministic database search (using search with limit=1 but no order) can be acceptable if the practical context makes it functionally deterministic, but it should be acknowledged as technical debt.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #174 +/- ##
=======================================
Coverage 71.48% 71.48%
=======================================
Files 932 932
Lines 54840 54840
=======================================
Hits 39201 39201
Misses 15639 15639
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
1 participant
Why is this change needed?
OpenProject #942 — three small registry UX issues observed while reviewing the group + member overview:
Locationwas rendering beforeFinancial Informationon the Profile tab, contradicting the intended bottom-of-page placement.Membership Types; the agreed wording (already used in the Philippines demo override) isGroup Role.urn:openspp:vocab:group-typeorurn:openspp:vocab:group-membership-type, the corresponding form field (Group Type) and list column (Group Role) were still rendered as empty selectors / blank tag cells.The original ticket also included a registry role-based menu audit; that was split out into #951 so it can ship independently.
How was the change implemented?
Three commits, one per item:
ad51f8d8—spp_registrant_gis/views/res_partner_views.xml— change the xpath insertion target fromphone_section/group_phone_sectiontofinancial_section/group_financial_sectionso Location renders after Financial Information on both the individual and group profile tabs.09ed95dd—spp_registry/models/group_membership.py(masterstring=) plus 4 view files where the column was explicitly relabeled. Also bumped the inconsistentstring="Role:"inspp_registry_group_hierarchy/views/group_views.xmltoGroup Role:for consistency. Field namemembership_type_idsunchanged — only the user-visible label.0a708fba— two narrow computed booleans onres.partner(has_group_type_codes,has_group_membership_type_codes) plus a mirror compute onspp.group.membershipfor the standalone tree view. Each compute is a single sudo'dsearch_countagainst its vocabulary's namespace, returning a boolean (no vocab content leaks). Views gategroup_type_idwithinvisible="not has_group_type_codes"and themembership_type_idscolumn withcolumn_invisible="not parent.has_group_membership_type_codes"(orcolumn_invisible="not has_group_membership_type_codes"on the standalone tree).New unit tests
None — the changes are pure label edits and view conditionals, no new logic worth testing in isolation. Existing
spp_registrytest suite still passes (no regressions).Unit tests executed by the author
Both runs return
0 failed, 0 error(s)(spp_registry currently has no tagged tests; spp_registrant_gis has 4 — all pass).CI-local pre-commit (
./ci-local/run.sh OpenSPP2 --files …) passes on every modified file.How to test manually
QA guides for each item are posted as comments on #942:
spp_registrant_gisand check tab order on a registrant's Profile.Group Rolelabel everywhereMembership Typesused to appear.urn:openspp:vocab:group-type/urn:openspp:vocab:group-membership-typevia Settings → Vocabularies → Manage Vocabularies → Codes smart button, and verify both the Group Type field and the Group Role column disappear; re-activate one code and they reappear.Related links
spp_philippines_demo/views/registrant_form_override.xml) is now redundant; cleanup is queued as a follow-up.