Commit 4f07be2
* Clarin9/Port per-field type binding (submit.type-bind.field "A=>B") (#876)
Upstream type-bind supports exactly one global controlling field. LINDAT needs
per-field control, expressed as `submit.type-bind.field = dc.type,
dc.language.iso=>edm.type`, and none of that machinery was ported to the v9
branch: `FormFieldModel` had no `typeBindField` (so cerialize dropped the REST
value), the parser always stamped the relation with the global type field, and
`FormBuilderService` kept a single `typeField` string and a single type bind
model. Selecting `edm.type = TEXT` therefore evaluated the relation against the
empty `dc.type` model and the dependent language field kept its `d-none` class.
- `FormFieldModel.typeBindField` is deserialized again.
- `FormBuilderService` keeps a map of controlling fields (default + one entry
per `A=>B` override, order-independent, duplicate-safe, trimmed) and a map of
registered controlling models, plus a subject that emits every registration.
`getTypeBindModel(ref?)` takes an optional ref so all existing call sites and
mocks keep working.
- `FieldParser.getTypeBindFieldRef()` stamps the relation with the controlling
model id when `<type-bind field="...">` is declared, and otherwise with the
field's own metadata name, which is resolved against the map later - the
property arrives over REST asynchronously.
- `DsDynamicTypeBindRelationService` passes the relation id through, no longer
dereferences a missing bind model, and attaches to a controlling model that is
only registered by a later `modelFromConfiguration()` call.
Deliberate deviations from the 7.x implementation (documented in the PR):
`findById` and `row-parser` are left untouched, `typeBindField` is not carried
on control models, and the inverted second clause of the 7.x `getTypeBindModel`
guard is dropped - it is a no-op for the LINDAT config and would otherwise fall
back to `dc_type`, reproducing this very bug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Review feedback: fix the subscription hand-off, scope the registry, harden parsing
Behaviour:
- subscribeRelations now returns a single owning Subscription. The caller spreads
the returned array into its own, so a child created after that snapshot - which
is exactly what the late-registration listener does - could never be torn down
and kept mutating hidden/disabled on a destroyed model.
- Always listen for type bind model registrations, not only when nothing was
attached: until the real controlling model exists the field is temporarily
attached to the default one, and that case was never re-attached (Copilot).
- The type bind registry is now dropped when the submission changes. Sections of
one submission still share it (a controlling field may live in another section),
but a model from the previously opened collection's form can no longer answer
lookups and defeat the fall-back-to-default behaviour.
- Controlling models are registered again once submit.type-bind.field arrives, so
an override that exists only in the property - with no <type-bind field="..."> in
the XML - still resolves if the config lands after the form was parsed.
- A self-referencing type bind is now a console.warn + skip instead of a throw: it
can be raised from the registration callback, and one misconfigured field should
not take down the whole submission section.
- Tolerate blank/whitespace/malformed values in the property (a null entry used to
throw inside the subscribe) and trim typeBindField before building the model id
(Copilot). getTypeBindModel is typed `| undefined` (Copilot).
Tests: late registration now asserts the relation is really re-evaluated and stops
on unsubscribe; added the attached-to-default case, cross-submission isolation, a
delayed configuration response, malformed values, and a cerialize round-trip
proving typeBindField survives deserialization.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Copilot follow-up: bound the parsed-rows cache and fix a misleading comment
- typeBindParsedRows is only filled until submit.type-bind.field has been
processed, and is dropped once it has. A section form re-parses on every data
update, so the cache would otherwise keep growing and retain the model graph of
every re-parse for the lifetime of the submission. Without a config service the
map can never change, so nothing is cached at all.
- Reword the matchesCondition guard comment: getTypeBindModel falls back to the
default model, so no model at all means neither the field's controlling model
nor the default one has been registered yet - not that the controlling field is
permanently absent from the form.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Copilot follow-up: trim typeBindField in getTypeBindModelIds too
isNotEmpty(' ') is true in this codebase, so a padded `<type-bind field=" edm.type ">`
would have registered the controlling model as ' edm_type ' while
FieldParser.getTypeBindFieldRef - which does trim - stamps the relations with
'edm_type'. The lookup would then miss, fall back to dc_type and leave the
dependent field permanently hidden, i.e. reproduce the very bug this PR fixes.
The existing end-to-end spec now uses a padded value, so it fails without the trim.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Follow the controlling model when its section is re-parsed
Attachment was deduped by model id, but setTypeBindModel emits on identity: when
the section holding the controlling field is re-parsed (section forms re-parse on
every data update) a NEW instance is registered under the same id, and a bound
field in another section kept listening to the dead one - so the dependent field
stopped reacting, which is the A3 symptom again in a narrower form.
Attachment is now keyed by id but compared by identity, and the stale child
subscription is removed from the owning Subscription and unsubscribed, so nothing
accumulates across re-parses either.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Copilot follow-up: do not evaluate a self-bound relation at all
Skipping the self-reference in getRelatedFormModel only stopped it from being
subscribed to; evaluateRelations still ran matchesCondition against the field's
own (empty) value on the initial pass, hid the field, and - with nothing attached
- never re-evaluated it, so a misconfigured <type-bind field="..."> pointing at
its own metadata field made that field permanently unreachable.
subscribeRelations now detects the self-reference up front, warns once and returns
without evaluating or attaching anything, leaving the field exactly as rendered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Distinguish a configured self-reference from a not-yet-parsed controlling model
The self-reference guard used the runtime lookup, which falls back to the default
model. For a field whose own id IS the default model id, a relation whose real
target simply had not been parsed yet therefore looked like a misconfiguration:
subscribeRelations bailed out before wiring the registration listener, so the
field never picked up its controlling model.
Resolve the reference from configuration only (new
FormBuilderService.resolveTypeBindModelId, order-independent) to decide whether
the field really is bound to itself; the transient case now just skips the
initial evaluation - so the field is not hidden for no reason - and still attaches
when the real model is registered.
Also add the spec that actually pins the getTypeBindModelIds trim. The one added
in 79f94f1 did not: the suite's config already contributes 'edm_type' through
its A=>B entry, so the padded id was merely an extra miss. The new case configures
only 'dc.type', making the padded <type-bind field=" edm.type "> the sole source
of the controlling id - verified to fail with the trim removed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Copilot follow-up: make the default type bind model id explicit
resolveTypeBindModelId declared `string` but ended in a bare Map.get, and
getTypeBindModel fed the same possibly-undefined value into its fallback lookup.
Both now go through getDefaultTypeBindModelId(), which falls back to the
TYPE_BIND_DEFAULT_MODEL_ID constant that also replaces the four scattered
'dc_type' literals. Also fix a comment still naming the pre-rename
dependsOnItself().
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Copilot follow-up: route getTypeField through the guaranteed default too
It was the one remaining place that returned a bare Map.get for a method declared
to return string.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Copilot follow-up: honest mock return value and an optional typeBindField
- The shared FormBuilderService mock returned undefined from
resolveTypeBindModelId while the real one always returns a model id. It now
mirrors the real implementation for a reference the type field map does not
remap, so a caller that starts using the value does not silently get undefined.
- typeBindField is genuinely optional in the REST payload and the code already
treats it that way (`?.trim()` in FieldParser and in getTypeBindModelIds, and a
spec asserting it stays undefined when the attribute is absent). I argued for
consistency with the other non-optional @autoserialize members earlier; the
usage asymmetry is the stronger argument, so it is now declared optional.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Copilot follow-up: align the mock signature with the optional parameter
resolveTypeBindModelId takes an optional ref, which is what the `??` fallback in
the fake is there for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Shorten the type bind comments
Keep the non-obvious reasoning, drop the prose around it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Shorten the type bind spec comments
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d513553 commit 4f07be2
9 files changed
Lines changed: 764 additions & 87 deletions
File tree
Lines changed: 143 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| 17 | + | |
17 | 18 | | |
18 | 19 | | |
19 | 20 | | |
| 21 | + | |
20 | 22 | | |
21 | 23 | | |
22 | 24 | | |
23 | 25 | | |
24 | 26 | | |
25 | 27 | | |
| 28 | + | |
26 | 29 | | |
27 | 30 | | |
28 | 31 | | |
| |||
87 | 90 | | |
88 | 91 | | |
89 | 92 | | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
90 | 99 | | |
91 | 100 | | |
92 | 101 | | |
| |||
129 | 138 | | |
130 | 139 | | |
131 | 140 | | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
132 | 275 | | |
133 | 276 | | |
134 | 277 | | |
Lines changed: 112 additions & 37 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
72 | | - | |
73 | | - | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
74 | 76 | | |
75 | 77 | | |
76 | | - | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
77 | 82 | | |
78 | | - | |
| 83 | + | |
79 | 84 | | |
80 | 85 | | |
81 | 86 | | |
82 | 87 | | |
83 | 88 | | |
84 | 89 | | |
85 | 90 | | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
86 | 110 | | |
87 | 111 | | |
88 | 112 | | |
| |||
102 | 126 | | |
103 | 127 | | |
104 | 128 | | |
105 | | - | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
106 | 136 | | |
107 | 137 | | |
108 | 138 | | |
| |||
174 | 204 | | |
175 | 205 | | |
176 | 206 | | |
177 | | - | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
178 | 212 | | |
179 | 213 | | |
180 | 214 | | |
181 | 215 | | |
182 | 216 | | |
183 | | - | |
184 | | - | |
185 | | - | |
186 | | - | |
187 | | - | |
188 | | - | |
189 | | - | |
190 | | - | |
191 | | - | |
192 | | - | |
193 | | - | |
194 | | - | |
195 | | - | |
196 | | - | |
197 | | - | |
198 | | - | |
199 | | - | |
200 | | - | |
201 | | - | |
202 | | - | |
203 | | - | |
204 | | - | |
205 | | - | |
206 | | - | |
207 | | - | |
208 | | - | |
209 | | - | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
210 | 238 | | |
211 | | - | |
212 | | - | |
213 | | - | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
214 | 265 | | |
215 | | - | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
216 | 291 | | |
217 | 292 | | |
218 | 293 | | |
0 commit comments