feat(spp_vocabulary): manual codes UX on SYSTEM vocabularies (#954)#201
Open
emjay0921 wants to merge 4 commits into
Open
feat(spp_vocabulary): manual codes UX on SYSTEM vocabularies (#954)#201emjay0921 wants to merge 4 commits into
emjay0921 wants to merge 4 commits into
Conversation
System vocabularies are loaded via XML by modules / data files and shouldn't be reshaped by users at runtime — but until now the form allowed editing structural fields (name, namespace, domain, version, hierarchical flag, etc.) and the inline Codes list let users add or delete codes that downstream code paths key off of. This now matches the existing Python guards in `models/vocabulary_code.py` (line 191, 279, 437, 594) so users see what they can / can't change rather than hitting a server-side UserError after typing. Vocabulary form: - Hide the `is_system` toggle so it can no longer be flipped manually (the orange SYSTEM ribbon stays as the visual indicator). - Lock `name`, `namespace_uri`, `domain`, `version`, `is_hierarchical`, `reference_url`, `description`, and `active` when `is_system=True`. Codes tab on the vocabulary form: split into two `code_ids` blocks — the non-system one keeps the original full-edit list; the system one sets `create="false" delete="false"` on the inline list and locks identifying columns (`code`, `display`, `definition`, `parent_id`) while sequence handle / `deprecated` / `active` stay flippable. Local codes (`is_local=True`) keep their pre-existing edit behaviour via the `parent.is_system and not is_local` condition. Vocabulary code form: same per-field lock applied to `display`, `code`, `vocabulary_id`, `parent_id`, `target_type`, and `definition` — adds a hidden `is_local` field so the readonly expressions can reference it. Tests: spp_vocabulary suite still 0 failed, 0 errors of 193.
…(#954) The Vocabularies list view exposed `is_system` and `active` as boolean_toggle columns, so admins could flip a vocabulary to/from system status or archive it directly from the list. System status should only ever be set by a module's data files, and archiving should go through the form (where it's already gated to non-system vocabs after the previous round). Conditional readonly per row would be ideal but is out of scope for this round — making both columns unconditionally readonly is the safe interim fix.
System vocabularies already accepted is_local=True codes at the model layer
but the UI made the manual-vs-system distinction invisible:
- Add a stored, indexed `code_source` Selection field on spp.vocabulary.code
derived from `is_local` ('system' / 'manual'). No data migration — keeps
is_local as the source of truth (ADR-016).
- Add `action_add_manual_code` on spp.vocabulary. Opens the code form with
default_is_local=True + default_vocabulary_id pre-seeded so admins can
intentionally layer manual codes onto a SYSTEM vocab without tripping the
create-guard in vocabulary_code.py.
- Form view (vocabulary_code_views.xml): show Source as a coloured badge
next to the code title; drop the restrictive `[('is_system','=',False)]`
domain on vocabulary_id so manual codes can reference system vocabularies.
- Standalone code list + search: Source column with badge, System / Manual
filters, Group-by Source.
- Vocabulary form Codes tab (system case): explicit "Add Manual Code"
button above the embedded list; new Source column in the inline list so
admins can see at a glance which codes are module-shipped vs admin-added.
Inline create/delete stay disabled — manual codes go through the button
+ form so is_local is correctly set; system-code deletion is still
blocked at the backend.
Adds tests covering code_source compute, manual code create/edit/delete on
a system vocab, and action_add_manual_code context seeding.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #201 +/- ##
==========================================
+ Coverage 66.08% 72.08% +6.00%
==========================================
Files 86 356 +270
Lines 7501 21802 +14301
==========================================
+ Hits 4957 15716 +10759
- Misses 2544 6086 +3542
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![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 the ability to add manual codes to system vocabularies. It includes a new code_source field to distinguish between system-shipped and manually added codes, updates views to enforce read-only constraints on system data while allowing edits to manual overrides, and adds a dedicated action for creating manual codes. Feedback was provided to improve the list view by using conditional read-only logic for the active field, allowing users to archive custom vocabularies directly from the list while maintaining protection for system vocabularies.
Comment on lines
+14
to
+22
| <!-- Read-only in the list: system status is set by the | ||
| module's data files, not user-toggleable; and | ||
| archiving a vocabulary should go through the form | ||
| (where it's already gated to non-system vocabs). | ||
| Conditional readonly per row would be ideal here but | ||
| is not in scope for this round — see OP#954. --> | ||
| <field name="is_system" widget="boolean_toggle" readonly="1" /> | ||
| <field name="is_hierarchical" optional="hide" /> | ||
| <field name="active" widget="boolean_toggle" /> | ||
| <field name="active" widget="boolean_toggle" readonly="1" /> |
There was a problem hiding this comment.
In Odoo 17, you can implement the "ideal" conditional read-only behavior you mentioned by using readonly="is_system" on the active field. This allows users to archive custom vocabularies directly from the list view while keeping system vocabularies protected. I've also updated the comment to reflect this change.
Suggested change
| <!-- Read-only in the list: system status is set by the | |
| module's data files, not user-toggleable; and | |
| archiving a vocabulary should go through the form | |
| (where it's already gated to non-system vocabs). | |
| Conditional readonly per row would be ideal here but | |
| is not in scope for this round — see OP#954. --> | |
| <field name="is_system" widget="boolean_toggle" readonly="1" /> | |
| <field name="is_hierarchical" optional="hide" /> | |
| <field name="active" widget="boolean_toggle" /> | |
| <field name="active" widget="boolean_toggle" readonly="1" /> | |
| <!-- Read-only in the list: system status is set by the | |
| module's data files, not user-toggleable. | |
| Archiving a vocabulary is allowed in the list for | |
| custom vocabularies via conditional readonly. | |
| See OP#954. --> | |
| <field name="is_system" widget="boolean_toggle" readonly="1" /> | |
| <field name="is_hierarchical" optional="hide" /> | |
| <field name="active" widget="boolean_toggle" readonly="is_system" /> |
Contributor
Author
|
@gonzalesedwin1123 — ready for review. All CI checks are green. Clean merge with OP#954: https://projects.acn.fr/wp/954 |
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?
OP#954 — SYSTEM vocabularies (
spp.vocabularywithis_system=True) ship immutable system codes via module data files. The backend already enforced this lock at the model layer (create/write/unlinkguards invocabulary_code.py), but the UI had three usability gaps:is_local=Trueflag (ADR-016) was hidden, and any naïve attempt to add a code to a system vocab hit the backendUserError. Per QA feedback on the round-2 PR, this needed to be reworked to allow CRUD on manual codes added to a system vocab while keeping the lock on system codes.How was the change implemented?
Rounds 1–2 already locked the form / list toggles. This PR is round 3 — the manual-codes UX.
Model
code_sourceSelection field onspp.vocabulary.codecomputed fromis_local(system/manual). Reusesis_localas the source of truth — no data migration; the new field is just the readable label.action_add_manual_codemethod onspp.vocabularythat opens the code form withdefault_is_local=True+default_vocabulary_idpre-seeded, so admins don't need to know about theis_localflag.Views
vocabulary_code_views.xml:[('is_system','=',False)]domain onvocabulary_id— manual codes can now reference system vocabularies.vocabulary_views.xml(system vocab Codes tab):action_add_manual_code.is_local=Trueis seeded consistently) and the standalone form (where the backend rules apply).New unit tests
spp_vocabulary/tests/test_manual_codes.py— 5 focused tests:test_code_source_computed_from_is_local— selection field derives correctly fromis_local.test_manual_code_create_allowed_on_system_vocab—is_local=Truecodes can be created on a system vocab.test_manual_code_identifying_fields_editable—code,display,definitionstay editable on manual codes.test_manual_code_can_be_deleted— manual codes can be unlinked.test_action_add_manual_code_seeds_context— action context hasdefault_is_local=True+default_vocabulary_id.Unit tests executed by the author
Full
spp_vocabularysuite ran locally in Docker: 0 failed, 0 error(s) of 198 tests.How to test manually
See the QA round-3 comment on OP#954 for the full guide (https://projects.acn.fr/wp/954). Quick path:
spp_vocabulary.households_local/Households (Local), save.household) — identifying fields are readonly, onlyactive/deprecated/sequenceeditable.UserError. Try to delete the manual code — succeeds.Related links