Skip to content

fix(change_request): let a routing key be served by several mappings #462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gonzalesedwin1123 merged 3 commits into from
Aug 28, 2026
Merged

fix(change_request): let a routing key be served by several mappings #462

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
de9977c
fix(change_request): let a routing key be served by several mappings
kneckinator Aug 26, 2026
baa249f
fix(change_request): expose routing_field in the type form's mapping …
kneckinator Aug 28, 2026
2bbd2a4
Merge branch '19.0' into fix/dynamic-routing-key-mapping
gonzalesedwin1123 Aug 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions spp_change_request_v2/README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,22 @@ Before declaring a new CR type complete:
Changelog
=========

19.0.3.1.13
~~~~~~~~~~~

- fix(change_request): a selectable field on a dynamic-approval type may
now be applied through more than one mapping. Apply is narrowed to the
field a request was routed and approved on, matched against the
mapping's ``source_field`` — which assumed every selectable value is a
physical source field. They need not be: a name may be offered as one
choice but stored as separate components, so one selectable value
legitimately drives several mappings, and matching on ``source_field``
alone matched none of them, applying nothing at all. A mapping can now
declare the selectable value it serves via ``routing_field``,
defaulting to ``source_field``, so existing configurations are
unchanged. Narrowing still holds — a mapping belonging to another
routing key is still not applied.

19.0.3.1.10
~~~~~~~~~~~

Expand Down
2 changes: 1 addition & 1 deletion spp_change_request_v2/__manifest__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "OpenSPP Change Request V2",
"version": "19.0.3.1.10",
"version": "19.0.3.1.13",
"sequence": 50,
"category": "OpenSPP",
"summary": "Configuration-driven change request system with UX improvements, conflict detection and duplicate prevention",
Expand Down
9 changes: 9 additions & 0 deletions spp_change_request_v2/models/change_request_type_mapping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ class SPPChangeRequestTypeMapping(models.Model):
required=True,
help="Field name on detail model",
)
routing_field = fields.Char(

@gonzalesedwin1123 gonzalesedwin1123 Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must fix: routing_field is not exposed in any view. The inline mapping list in change_request_type_views.xml (~line 161) shows sequence / source_field / target_field / transform / transform_expression — an admin configuring a type through the UI can neither set this field nor see it. That cuts both ways: the feature is XML/API-only, and — per your own test_routing_field_shadows_source_field_for_matching — a value set via API makes source_field silently stop matching with nothing in the form explaining why apply now refuses. One line fixes it:

<field name="routing_field" optional="hide" />

(optional="hide" keeps the common case uncluttered while making the shadowing visible to whoever goes looking.)

@kneckinator kneckinator Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in baa249f — exactly the one line suggested, <field name="routing_field" optional="hide" />, placed right after source_field since that is the field it shadows. Also pinned with a test (test_routing_field_is_exposed_in_the_type_form_view) so it cannot silently drop out of the view again. Full module suite: 401 tests, 0 failures.

help=(
"For dynamic-approval types, the selectable value this mapping serves. "
"Defaults to source_field. Set it when one selectable field is applied "
"through several mappings -- e.g. a name captured as one choice but "
"stored as separate components -- so apply still writes exactly what "
"was routed and approved."
),
)
target_field = fields.Char(
required=True,
help="Field name on registrant (res.partner)",
Expand Down
4 changes: 4 additions & 0 deletions spp_change_request_v2/readme/HISTORY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
### 19.0.3.1.13

- fix(change_request): a selectable field on a dynamic-approval type may now be applied through more than one mapping. Apply is narrowed to the field a request was routed and approved on, matched against the mapping's `source_field` — which assumed every selectable value is a physical source field. They need not be: a name may be offered as one choice but stored as separate components, so one selectable value legitimately drives several mappings, and matching on `source_field` alone matched none of them, applying nothing at all. A mapping can now declare the selectable value it serves via `routing_field`, defaulting to `source_field`, so existing configurations are unchanged. Narrowing still holds — a mapping belonging to another routing key is still not applied.

### 19.0.3.1.10

- fix(security): conflict and duplicate detection now decide whether a mapped field changed using the same comparison the apply strategy uses. Detection compared through a helper that lowercases and strips strings while apply compares raw, so a case- or whitespace-only edit was invisible to detection yet still written to the registrant — enough to sidestep a field-scoped conflict rule with a cosmetic edit. Detection also ignored transform expressions, which apply evaluates before comparing. Similarity scoring is unchanged and stays case-insensitive, since that is the point of a fuzzy match.
Expand Down
55 changes: 36 additions & 19 deletions spp_change_request_v2/static/description/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1339,6 +1339,23 @@ <h2>Changelog</h2>
</div>
</div>
<div class="section" id="section-1">
<h1>19.0.3.1.13</h1>
<ul class="simple">
<li>fix(change_request): a selectable field on a dynamic-approval type may
now be applied through more than one mapping. Apply is narrowed to the
field a request was routed and approved on, matched against the
mapping’s <tt class="docutils literal">source_field</tt> — which assumed every selectable value is a
physical source field. They need not be: a name may be offered as one
choice but stored as separate components, so one selectable value
legitimately drives several mappings, and matching on <tt class="docutils literal">source_field</tt>
alone matched none of them, applying nothing at all. A mapping can now
declare the selectable value it serves via <tt class="docutils literal">routing_field</tt>,
defaulting to <tt class="docutils literal">source_field</tt>, so existing configurations are
unchanged. Narrowing still holds — a mapping belonging to another
routing key is still not applied.</li>
</ul>
</div>
<div class="section" id="section-2">
<h1>19.0.3.1.10</h1>
<ul class="simple">
<li>fix(security): conflict and duplicate detection now decide whether a
Expand Down Expand Up @@ -1378,7 +1395,7 @@ <h1>19.0.3.1.10</h1>
configured mapping.</li>
</ul>
</div>
<div class="section" id="section-2">
<div class="section" id="section-3">
<h1>19.0.3.1.9</h1>
<ul class="simple">
<li>fix(security): duplicate detection now scores the fields both change
Expand All @@ -1395,7 +1412,7 @@ <h1>19.0.3.1.9</h1>
requester-writable <tt class="docutils literal">selected_field_name</tt> / <tt class="docutils literal">field_to_modify</tt>.</li>
</ul>
</div>
<div class="section" id="section-3">
<div class="section" id="section-4">
<h1>19.0.3.1.8</h1>
<ul class="simple">
<li>fix(security): scope the Create-Group member wizards to the parent
Expand All @@ -1413,7 +1430,7 @@ <h1>19.0.3.1.8</h1>
access-control entry grants.</li>
</ul>
</div>
<div class="section" id="section-4">
<div class="section" id="section-5">
<h1>19.0.3.1.7</h1>
<ul class="simple">
<li>fix(security): require change-request manager rights to apply a change
Expand All @@ -1428,7 +1445,7 @@ <h1>19.0.3.1.7</h1>
endpoint.</strong></li>
</ul>
</div>
<div class="section" id="section-5">
<div class="section" id="section-6">
<h1>19.0.3.1.6</h1>
<ul class="simple">
<li>fix(security): derive conflict and duplicate detection from the change
Expand All @@ -1442,7 +1459,7 @@ <h1>19.0.3.1.6</h1>
an empty one, so detection cannot silently disable itself.</li>
</ul>
</div>
<div class="section" id="section-6">
<div class="section" id="section-7">
<h1>19.0.3.1.5</h1>
<ul class="simple">
<li>fix(security): scope the CR Requestor, Local Validator and HQ
Expand All @@ -1454,7 +1471,7 @@ <h1>19.0.3.1.5</h1>
are <tt class="docutils literal">noupdate</tt>.</li>
</ul>
</div>
<div class="section" id="section-7">
<div class="section" id="section-8">
<h1>19.0.3.1.4</h1>
<ul class="simple">
<li>fix(security): add ownership and area record rules to every concrete
Expand All @@ -1471,7 +1488,7 @@ <h1>19.0.3.1.4</h1>
unrestricted delete their access-control entries grant.</li>
</ul>
</div>
<div class="section" id="section-8">
<div class="section" id="section-9">
<h1>19.0.3.1.3</h1>
<ul class="simple">
<li>fix(security): route and apply the same single field for
Expand All @@ -1484,7 +1501,7 @@ <h1>19.0.3.1.3</h1>
the routing selector.</li>
</ul>
</div>
<div class="section" id="section-9">
<div class="section" id="section-10">
<h1>19.0.3.1.2</h1>
<ul class="simple">
<li>fix(change_request_v2): adding an ID now looks for a live one of that
Expand All @@ -1493,7 +1510,7 @@ <h1>19.0.3.1.2</h1>
(#1136)</li>
</ul>
</div>
<div class="section" id="section-10">
<div class="section" id="section-11">
<h1>19.0.3.1.1</h1>
<ul class="simple">
<li>fix(change_request): enforce the <tt class="docutils literal">(cr_type_id, reason)</tt> uniqueness
Expand All @@ -1507,7 +1524,7 @@ <h1>19.0.3.1.1</h1>
applied) so the constraint applies cleanly on upgrade.</li>
</ul>
</div>
<div class="section" id="section-11">
<div class="section" id="section-12">
<h1>19.0.3.1.0</h1>
<ul class="simple">
<li>revert(change_request): restore the create-a-new-individual <strong>Add
Expand All @@ -1525,7 +1542,7 @@ <h1>19.0.3.1.0</h1>
<strong>not</strong> restored here; reinstate separately if needed.</li>
</ul>
</div>
<div class="section" id="section-12">
<div class="section" id="section-13">
<h1>19.0.3.0.0</h1>
<ul class="simple">
<li>feat(change_request): redesign the group/membership CR flows (#242) —
Expand All @@ -1547,7 +1564,7 @@ <h1>19.0.3.0.0</h1>
must adapt (see #1133).</li>
</ul>
</div>
<div class="section" id="section-13">
<div class="section" id="section-14">
<h1>19.0.2.0.8</h1>
<ul class="simple">
<li>fix(views): disable inline creation of CR document types on the Change
Expand All @@ -1558,7 +1575,7 @@ <h1>19.0.2.0.8</h1>
Documents” modal (missing Name field) that blocked saving (#1125)</li>
</ul>
</div>
<div class="section" id="section-14">
<div class="section" id="section-15">
<h1>19.0.2.0.7</h1>
<ul class="simple">
<li>fix(security): align CR Requestor / CR Local Validator / CR HQ
Expand All @@ -1570,7 +1587,7 @@ <h1>19.0.2.0.7</h1>
dependencies.</li>
</ul>
</div>
<div class="section" id="section-15">
<div class="section" id="section-16">
<h1>19.0.2.0.6</h1>
<ul class="simple">
<li>fix(views): route post-submit CRs (pending / approved / applied /
Expand All @@ -1585,7 +1602,7 @@ <h1>19.0.2.0.6</h1>
list so row-click goes through the stage router.</li>
</ul>
</div>
<div class="section" id="section-16">
<div class="section" id="section-17">
<h1>19.0.2.0.5</h1>
<ul class="simple">
<li>fix(security): add a global <tt class="docutils literal">ir.rule</tt> on <tt class="docutils literal">spp.change.request</tt> that
Expand All @@ -1598,27 +1615,27 @@ <h1>19.0.2.0.5</h1>
roles).</li>
</ul>
</div>
<div class="section" id="section-17">
<div class="section" id="section-18">
<h1>19.0.2.0.3</h1>
<ul class="simple">
<li>fix: add HTML escaping to all computed Html fields with
<tt class="docutils literal">sanitize=False</tt> to prevent stored XSS (#50)</li>
</ul>
</div>
<div class="section" id="section-18">
<div class="section" id="section-19">
<h1>19.0.2.0.2</h1>
<ul class="simple">
<li>fix: fix batch approval wizard line deletion (#130)</li>
</ul>
</div>
<div class="section" id="section-19">
<div class="section" id="section-20">
<h1>19.0.2.0.1</h1>
<ul class="simple">
<li>fix: skip field types before getattr and isolate detail prefetch
(#129)</li>
</ul>
</div>
<div class="section" id="section-20">
<div class="section" id="section-21">
<h1>19.0.2.0.0</h1>
<ul class="simple">
<li>Initial migration to OpenSPP2</li>
Expand Down
10 changes: 9 additions & 1 deletion spp_change_request_v2/strategies/field_mapping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ def _effective_mappings(self, change_request):
fields that were also changed. This keeps the applied change in lockstep
with what was actually approved. Fail closed: if no field is selected, or
the selection maps to no configured field, nothing is applied.

A mapping matches on ``routing_field`` where set, else on
``source_field``. The selectable values come from the detail model's
``_get_field_to_modify_selection()`` and need not be physical source
fields: one selectable value may be applied through several mappings --
a name offered as a single choice but stored as separate components,
say. Matching on ``source_field`` alone could not express that, and
matched nothing, so such a request applied nothing at all.
"""
cr_type = change_request.request_type_id
mappings = cr_type.apply_mapping_ids
Expand All @@ -32,7 +40,7 @@ def _effective_mappings(self, change_request):
selected = change_request.selected_field_name
if not selected:
return mappings.browse()
return mappings.filtered(lambda m: m.source_field == selected)
return mappings.filtered(lambda m: (m.routing_field or m.source_field) == selected)

def current_target_value(self, mapping, registrant):
"""The registrant's current value for ``mapping``, as apply compares it."""
Expand Down
1 change: 1 addition & 0 deletions spp_change_request_v2/tests/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@
from . import test_frozen_value_normalisation
from . import test_frozen_detail_binding
from . import test_detection_matches_apply
from . import test_routing_field
Loading
Loading