feat(disability_registry): stabilization improvements for spp_disability_registry (#1047)#263
Merged
Merged
Conversation
- Show Devices smart button even at zero count - Remove has_disability prerequisite to add/view assistive devices - Make status editable inline (dropdown) with row-level color; surface provision date and provider when status is Provided - Drop inaccurate DCI CD.DR.04 / ISO 9999 claims from device vocabulary - Reword assistive-device readme bullet (status, not status workflow)
…rride (#1050) - Add Disability Registry settings with an 'Allow manual assessment type' toggle (res.config.settings + Configuration > Settings menu) - Default: assessment type is readonly and auto-determined from the registrant's age; a date of birth is required (validation) - When the toggle is enabled: assessment type is a manual dropdown and a date of birth is no longer required - Rename the 'WG Assessment' tab to 'WG/CFM Assessment' and show the WG-SS questionnaire only for the adult type; add a placeholder for the CFM forms (added by #1048 / #1049)
- Remove the standalone 'Response Information' tab and move proxy fields to the top of the WG/CFM Assessment tab - Drive the proxy response flag by assessment type: forced on for CFM 2-4 and (unless self-report is enabled) CFM 5-17; optional for WG-SS - Add config: allow self-report on CFM 5-17 (+ optional minimum age) and allow proxy report on WG-SS (default on) - Add a 'Reason for Proxy' dropdown shown for WG-SS proxy responses - Manage the 'allow proxy on WG-SS' parameter explicitly so unticking the default-on setting persists (config_parameter set_param(False) deletes the param and the field default would re-tick it)
- Add the WG/UNICEF Child Functioning Module 2-4 instrument (16 questions, CF1-CF16) as fields on the assessment, with the difficulty and behaviour response scales and the glasses/hearing-aid/walking-equipment branching - Compute has_disability per assessment type: CFM 2-4 uses the concluding answer per domain (standard threshold) plus the 'a lot more' behaviour threshold; WG-SS logic unchanged - Show the CFM 2-4 form only for the cfm_2_4 type; placeholder remains for cfm_5_17 (#1049) - Restyle both WG-SS and CFM 2-4 as carded questionnaires with horizontal radio options for clearer data entry
- Add the WG/UNICEF Child Functioning Module 5-17 instrument (24 questions, CF1-CF24) as fields on the assessment, with the difficulty scale, the frequency scale for anxiety/depression, and the glasses/hearing-aid/ walking-equipment branching (100 m / 500 m distances) - Score has_disability for CFM 5-17: standard threshold on concluding answers; mobility uses the with-aid answer (either distance) when equipment is used, otherwise the compared-with-peers answer; anxiety and depression use a 'daily' frequency threshold - Show the CFM 5-17 carded questionnaire for the cfm_5_17 type, replacing the placeholder; same horizontal-radio layout as CFM 2-4
…multi-row (#1054) - Add spp.disability.impairment line model (type + cause + severity per row) with ACLs; expose as impairment_line_ids on the assessment - Move Impairment Classification to its own tab with an editable list; Overview shows a read-only Classification Summary - Derive the assessment-level severity_level_id (most severe line) and impairment_type_ids (union) from the lines so the registrant propagation, list/badge decorations, filters, concept groups and CEL functions keep working unchanged; drop the per-assessment impairment_cause_id - Update demo data to use impairment lines (adult now has two) - Update tests to record severity via impairment lines; fix a stale test that called the _onchange_assessment_type removed in #1053
…#1060) - Make the assessment use a configured approval definition: override _get_approval_definition / _resolve_approval_definition to return the workflow selected in Disability Registry settings (same pattern as program cycles/entitlements and change-request types) - Add 'Assessment approval workflow' to Disability Registry settings, a reference to an spp.approval.definition for the assessment model (created in Approvals > Approval Definitions); no default is seeded - Hide Submit and show a guidance alert until a workflow is configured - Gate Approve/Reject by the computed can_approve/can_reject so the configured approvers drive the buttons instead of a fixed group
…estionnaire (#1022)
- Re-sync the registrant's disability status after approval/rejection/reset:
the approval mixin writes approval_state via raw SQL (optimistic locking),
which bypasses ORM dependency tracking, so the registrant's current
assessment / has_disability never recomputed. Override _on_approve,
_on_reject and _on_reset_to_draft to call modified(['approval_state'])
- Require the WG/CFM questionnaire to be complete enough to compute a result
before submitting: add questionnaire_complete, gate _check_can_submit and
hide Submit with a guidance alert until it is answered
- Correct the registrant message ('No approved assessment currently
identifies a disability' instead of claiming none exists)
- Make the severity in the disability smart button display-only (no_open)
so it no longer navigates to the vocabulary record
…0 r2) - Block assessments for registrants under 2 and when no date of birth is set (age-enforced mode): hide Create/New Assessment with a guidance message via can_create_disability_assessment, and add a constraint blocking under-2 - Hide 'Age at Assessment' on the Overview when the registrant has no DOB - Remove '(default)' wording from the manual-assessment-type setting help
- Exclude the registrant from the Proxy Respondent dropdown - Hide the proxy checkbox for WG-SS when proxy reporting is disabled (instead of showing it greyed out) - Require the minimum self-report age (5-17) when self-report on CFM 5-17 is enabled; validate the range on save - Remove '(default)'/'Disabled by default' wording from the proxy settings help
… r2) - Render the Yes/No gate questions (glasses, hearing aid, walking equipment) as Yes/No selections instead of a single checkbox; branch on 'yes' - Use the exact WG/UNICEF question wording from the ticket - Split the questionnaire into one section per domain (Vision, Hearing, Mobility, Dexterity, Communication, Learning, Playing, Controlling Behaviour) matching the instrument
- Convert glasses/hearing aid/walking-equipment gates from a single checkbox to explicit Yes/No options - Align all CFM 5-17 question labels with the exact instrument wording - Split combined sections into one card per functional domain (Vision, Hearing, Mobility, Self-care, Communication, Learning, Remembering, Concentrating, Accepting change, Controlling behaviour, Making friends, Anxiety, Depression)
…(#1068)
#1068 — Rearrange assessment tab:
- Config (Disability Registry settings): per-tab Show/Required toggles for
Impairment Classification / WG-CFM / Support Needs, plus Show Assistive
Devices on Support Needs (all default on)
- Overview: remove Has Disability? and the Classification Summary section
- Impairment tab: yes/no gate ("any impairments or functional limitations to
record?") — Yes shows the list + requires a row; No completes the tab
- Support Needs: assistive-device request list (Status "Needed", no provision
date); on approval each becomes a needed device on the registrant
- Submit gate is now config-driven: every displayed + required tab must be
complete (Support has no content gate)
- Registrant Disability Status card degrades gracefully for questionnaire-only
disabilities (no impairment severity / review)
BM rewordings:
- WG-SS communicating: '...in their usual language?'
- CFM 2-4 and 5-17 'without his/her equipment or assistance' walking questions
no longer offer 'No difficulty'
- Questionnaire editable in draft/revision, locked once submitted
- Proxy minimum self-report age (5-17) blank by default, cleared on disable
… (#1061) Bundle module installing the Disability Registry stack in one step: everything from spp_starter_social_registry except the DCI client modules (spp_dci_client, _crvs, _ibr, _dr), plus spp_disability_registry. Sets spp_starter.registry_type = disability_registry.
… submit gates (#1068 #1053) #1068 (overview assessment table): - Add a 'Disability according to WG/CFM' checkbox column (has_disability). - Show each impairment type with its own severity on a separate line via a new impairment_severity_display field, replacing the impairment-type tags and the single overall-severity badge in the history table. - Add a 'Review Schedule' configuration (Show / Required to submit, default on) following the existing assessment-tab toggle pattern; the section hides when off and review_category is gated at submit when required. #1053 (proxy information): - Add a 'Require proxy details on proxy responses' setting (default on). When on and a proxy responded, the respondent and relationship must be recorded before the assessment can be submitted for approval. Both submit gates fold into assessment_complete (draft saves still allowed). Tests: updated 2 gate tests for the now-required review schedule and added 3 new tests (review gate, proxy gate, per-line impairment display).
![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 significantly enhances the OpenSPP Disability Registry by introducing the WG/UNICEF Child Functioning Module (CFM) for children, implementing a multi-line impairment classification model, and adding configurable assessment tabs with submission completeness gates. It also introduces a new starter bundle module for one-click installation of the Disability Registry stack. Feedback on these changes includes resolving a persistence issue where the disability_support_show_devices parameter is missing from _DEFAULT_TRUE_PARAMS in the configuration settings, and optimizing the _materialize_device_requests method to batch database queries instead of executing them inside a loop.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+111
to
+122
| _DEFAULT_TRUE_PARAMS = { | ||
| "disability_allow_proxy_wg_ss": "spp_disability_registry.allow_proxy_wg_ss", | ||
| "disability_require_proxy_details": "spp_disability_registry.require_proxy_details", | ||
| "disability_display_impairment": "spp_disability_registry.display_impairment", | ||
| "disability_display_wg": "spp_disability_registry.display_wg", | ||
| "disability_display_support": "spp_disability_registry.display_support", | ||
| "disability_display_review": "spp_disability_registry.display_review", | ||
| "disability_require_impairment": "spp_disability_registry.require_impairment", | ||
| "disability_require_wg": "spp_disability_registry.require_wg", | ||
| "disability_require_support": "spp_disability_registry.require_support", | ||
| "disability_require_review": "spp_disability_registry.require_review", | ||
| } |
There was a problem hiding this comment.
The configuration parameter disability_support_show_devices is defined as a field on ResConfigSettings but is missing from _DEFAULT_TRUE_PARAMS. Because it does not have config_parameter set on the field definition, its value will not be persisted in the database and will reset to its default value every time the settings are loaded or saved. Adding it to _DEFAULT_TRUE_PARAMS ensures it is correctly saved and retrieved.
Suggested change
| _DEFAULT_TRUE_PARAMS = { | |
| "disability_allow_proxy_wg_ss": "spp_disability_registry.allow_proxy_wg_ss", | |
| "disability_require_proxy_details": "spp_disability_registry.require_proxy_details", | |
| "disability_display_impairment": "spp_disability_registry.display_impairment", | |
| "disability_display_wg": "spp_disability_registry.display_wg", | |
| "disability_display_support": "spp_disability_registry.display_support", | |
| "disability_display_review": "spp_disability_registry.display_review", | |
| "disability_require_impairment": "spp_disability_registry.require_impairment", | |
| "disability_require_wg": "spp_disability_registry.require_wg", | |
| "disability_require_support": "spp_disability_registry.require_support", | |
| "disability_require_review": "spp_disability_registry.require_review", | |
| } | |
| _DEFAULT_TRUE_PARAMS = { | |
| "disability_allow_proxy_wg_ss": "spp_disability_registry.allow_proxy_wg_ss", | |
| "disability_require_proxy_details": "spp_disability_registry.require_proxy_details", | |
| "disability_display_impairment": "spp_disability_registry.display_impairment", | |
| "disability_display_wg": "spp_disability_registry.display_wg", | |
| "disability_display_support": "spp_disability_registry.display_support", | |
| "disability_display_review": "spp_disability_registry.display_review", | |
| "disability_require_impairment": "spp_disability_registry.require_impairment", | |
| "disability_require_wg": "spp_disability_registry.require_wg", | |
| "disability_require_support": "spp_disability_registry.require_support", | |
| "disability_require_review": "spp_disability_registry.require_review", | |
| "disability_support_show_devices": "spp_disability_registry.support_show_devices", | |
| } |
Comment on lines
+883
to
+906
| def _materialize_device_requests(self): | ||
| """On approval, turn each Support-Needs device request into a real | ||
| spp.assistive.device (status 'needed') on the registrant so it shows on | ||
| the Overview / registrant (OP#1068). Skips a device type that already | ||
| has a pending 'needed' record to avoid duplicates on re-approval. | ||
| """ | ||
| Device = self.env["spp.assistive.device"] | ||
| for rec in self: | ||
| for req in rec.device_request_ids: | ||
| already = Device.search_count( | ||
| [ | ||
| ("registrant_id", "=", rec.registrant_id.id), | ||
| ("device_type_id", "=", req.device_type_id.id), | ||
| ("status", "=", "needed"), | ||
| ] | ||
| ) | ||
| if not already: | ||
| Device.create( | ||
| { | ||
| "registrant_id": rec.registrant_id.id, | ||
| "device_type_id": req.device_type_id.id, | ||
| "status": "needed", | ||
| } | ||
| ) |
There was a problem hiding this comment.
The _materialize_device_requests method executes a search_count query inside a nested loop over self and device_request_ids. This results in an
def _materialize_device_requests(self):
"""On approval, turn each Support-Needs device request into a real
spp.assistive.device (status 'needed') on the registrant so it shows on
the Overview / registrant (OP#1068). Skips a device type that already
has a pending 'needed' record to avoid duplicates on re-approval.
"""
Device = self.env["spp.assistive.device"]
req_data = []
for rec in self:
for req in rec.device_request_ids:
req_data.append((rec.registrant_id.id, req.device_type_id.id))
if not req_data:
return
registrant_ids = list({r_id for r_id, _ in req_data})
device_type_ids = list({dt_id for _, dt_id in req_data})
existing_devices = Device.search([
("registrant_id", "in", registrant_ids),
("device_type_id", "in", device_type_ids),
("status", "=", "needed"),
])
existing_set = {(d.registrant_id.id, d.device_type_id.id) for d in existing_devices}
devices_to_create = []
for rec in self:
for req in rec.device_request_ids:
key = (rec.registrant_id.id, req.device_type_id.id)
if key not in existing_set:
devices_to_create.append({
"registrant_id": rec.registrant_id.id,
"device_type_id": req.device_type_id.id,
"status": "needed",
})
existing_set.add(key)
if devices_to_create:
Device.create(devices_to_create)References
- When processing records in bulk, avoid evaluating filters or permissions individually per record to prevent O(N * M) database query patterns. Instead, perform individual checks (such as consent) first, and then evaluate filters in a single batched query.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 19.0 #263 +/- ##
==========================================
+ Coverage 71.68% 71.93% +0.25%
==========================================
Files 1010 1030 +20
Lines 60072 61239 +1167
==========================================
+ Hits 43062 44052 +990
- Misses 17010 17187 +177
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…ity, assistive devices, impairment (#1047)
emjay0921
added a commit
that referenced
this pull request
Jul 2, 2026
Revert "feat(disability_registry): stabilization improvements for spp_disability_registry (#1047)" (#263)
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
1 participant
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.
Why is this change needed?
Umbrella #1047 tracked the bugs and improvements needed to stabilise
spp_disability_registry. This branch consolidates all of that work — every child ticket is now resolved (10 Closed + #1051 Test pass), so the module is ready for review as a single reviewable unit.Tickets consolidated on this branch:
spp_starter_disability_registrybundleHow was the change implemented?
spp_disability_registry(enhanced):models/assessment.py): age-driven assessment-type selection with manual override; CFM 2-4 and CFM 5-17 questionnaires; WG-SS "Has disability" computation; proxy response captured per assessment type; configurable multi-tier approval workflow; review/proxy submit gates.models/impairment.py), with an overview table.models/assistive_device.py,data/vocabulary_device.xml).models/registrant.py).models/res_config_settings.py+ views): expose the approval-workflow and questionnaire configuration in the GUI.security/ir.model.access.csv; assessment/registrant view rework; menu additions.spp_starter_disability_registry(new): a starter bundle module (manifest, config parameters, description, icon) that packages the disability registry for deployment.New unit tests
spp_disability_registry/tests/test_assessment.py— expanded coverage for age-driven type selection, CFM questionnaires, WG-SS computation, proxy, and approval workflow.spp_disability_registry/tests/test_cel_functions.py,tests/test_registrant.py— updated.spp_starter_disability_registry/tests/test_starter_disability_registry.py— new bundle install test.Unit tests executed by the author
CI runs the
spp_disability_registryandspp_starter_disability_registrytest jobs on this PR.How to test manually
Install
spp_disability_registry(or thespp_starter_disability_registrybundle), create a disability assessment, and verify: the assessment type is auto-selected by the registrant's age (overridable); the CFM 2-4 / 5-17 / WG-SS questionnaires appear as appropriate; the "Has disability" flag computes correctly for WG-SS; proxy response is captured per type; the configured approval workflow gates submission; impairment classification is a multi-row tab with an overview table; and an approved assessment is recognised on the registrant.Related links
Parent: #1047 (Disability Registry - Bugs and improvements). Children: #1022, #1048, #1049, #1050, #1051, #1052, #1053, #1054, #1060, #1061, #1068.