Repository navigation
Commit 43837fe
LT-22802: Apply a tool's slice filter list in the Avalonia detail view (#1146)
* test: pin the relevance rule with the most rows riding on it
Raised by Jason on #1135, after the merge. MoStemMsa.IsFieldRelevant withholds
FromPartsOfSpeech ("Attaches to Categories") unless the owning entry has a
proclitic or enclitic, and Morphology.fwlayout:39 declares that part
visibility="always" -- so before the relevance gate the row composed on every
stem MSA, and now it disappears for every entry without a clitic. That is the
gate's most visible consequence and nothing pinned it.
The test is Jason's, run and confirmed to bite: suppressing the gate fails its
negative half with "Expected: False, But was: True", while the positive control
-- add a proclitic allomorph, change nothing else -- keeps passing.
Also corrects two comments that claimed more than the code does. The composer
implements the SECOND of SliceFilter.IncludeSlice's two gates; the first looks
the slice id up in the tool's filter list, and the id never reaches the
composer, so that half is LT-22802. The fixture summary said it covered the
remainder of the overrides, which was untrue while this test was missing, and
still excludes the InflectionClass limb that withholds the row from a compound
rule's left/right MSA.
xWorksTests filter Avalonia 1642 passed, 0 failed.
* LT-22802: Apply a tool's slice filter list in the Avalonia detail view
Legacy SliceFilter.IncludeSlice has two gates: look the slice's authored id up
in the filter list the tool's filterPath names and withhold the row when it is
listed, then ask IsFieldRelevant. LT-22672 added the second. This adds the
first.
The id was discarded at import, so three pieces:
- XmlLayoutImporter carries the slice's id onto ViewNode.SliceId, and id leaves
the unhandled-attribute report.
- DetailComposer.Walk withholds a node whose SliceId the tool lists, checked
before the node kind is dispatched so a withheld node takes its subtree with
it -- where legacy checks it, at the top of ProcessSubpartNode.
- RecordEditView reads filterPath from its own configuration and parses the
same file into the id set, memoized per view. A tool with no filterPath, or
a file that cannot be read, filters nothing: an extra row beats a view that
will not open.
NOTHING VISIBLE CHANGES TODAY, and the ticket's symptom section overstates it.
The only shipped filter entries that still resolve to a real slice are the five
CmPossibility ids in basicPlusFilter.xml, and the only tool they reach is
Exception "Features" (ProdRestrictEdit) -- which is not in the Avalonia tool
catalog, so it renders legacy even under UIMode=New. The other 27 ids across
the six filter files name nodes that no longer exist. Category Edit is in the
catalog and carries the same filterPath, but its layout reaches none of those
ids.
So this is parity landed before it is needed: the day ProdRestrictEdit joins
LexiconFeatureCatalog, the view already withholds what legacy withholds instead
of growing five rows nobody looks for.
Tested at the mechanism, not the wiring. Suppressing the composer gate fails
the filter tests with the full row list -- Name, Abbreviation, Description,
Status, Discussion, Confidence, Researchers, Restrictions -- and the importer
tests pin the id. Reading filterPath in RecordEditView has NO test and cannot
be checked in the app while the tool renders legacy; it becomes verifiable, by
Jason's acceptance steps, when that tool is activated.
FwAvaloniaTests 751 passed, xWorksTests filter Avalonia 1645 passed, 0 failed.
* LT-22802: Cover the step that reads a tool's filter list
The filter-list read was the one part with no test, and the part that could
quietly make the whole feature a no-op: every other test in this area supplies
the id set by hand, so a broken read leaves them all green.
Demonstrated rather than assumed. Pointing the XPath at the wrong element name
fails only the new test; the three composer tests still pass.
The read moves to an internal static taking the tool's configuration node, so
it is reachable without a mediator or property table -- the same way the
reference-vector menu entry points are tested. The property keeps only its
memoization.
One test drives the whole chain against a shipped filter file: the attribute
name, the path resolution, the XPath and the id attribute. It asserts the ids
are PRESENT rather than exhaustive, so editing that file leaves it alone while
breaking any link in the chain does not.
xWorksTests filter Avalonia 1650 passed, 0 failed.
* Merge remote-tracking branch 'origin/main' into LT-22802-slice-filter-lists
ViewNode gained an optional parameter on each side -- sliceId here, and
helpTopicId on main -- with the same conflict at its one call site in
the importer. Both are kept; they are independent.
* Merge remote-tracking branch 'origin/main' into LT-22802-slice-filter-lists
Main now has #1145, which this branch also carried as its first commit.
Squash-merging gave it a different identity, so both files it touched
conflicted with that commit's own later edits there.
Resolved toward main, whose comments had been corrected, with one
change: the relevance doc named LT-22802 as covering the other gate.
This branch is that gate, so the doc now points at IsFilteredOutByTool,
in the same class. The fixture summary keeps its pointer to
DetailSliceFilterTests, where that gate is tested.
* LT-22802: Rejoin a doc line the comment wrapper split
The comment-hygiene wrapper broke the new pointer to IsFilteredOutByTool
so that one word sat alone on the last line. Reflowed.
Comments only.
* Merge remote-tracking branch 'origin/main' into LT-22802-slice-filter-lists
* LT-22802: Stop an override from shifting a node's help topic and id
sliceId was inserted ahead of helpTopicId in ViewNode's constructor,
when resolving a merge conflict between the two. Both override clone
helpers pass that tail positionally, and both parameters are strings,
so n.HelpTopicId bound to sliceId and helpTopicId fell back to null --
with no warning. Any layout with a Field Visibility or Move Field
override lost every authored help topic, and each of its rows took the
help id as its slice id.
The helpers also never carried SliceId at all. Moving the parameter
back alone would have restored help topics and left SliceId null
after every override, so a tool's filter list would stop withholding
rows in exactly the layouts users have customised.
sliceId goes last again, and both helpers name the tail and pass both
fields, so a parameter added ahead of them cannot shift either one.
The existing test for fields an override must preserve now covers both,
and a duplicated node is checked for them too. Rebuilding the merged
code fails both tests exactly as reported: help topic null, slice id
the help id. Dropping only the SliceId carry fails them on SliceId.
Reported in review.
* LT-22802: Carry a node's slice id through the canonical JSON
The serializer wrote and read helpTopicID but had no slice id, so a
layout loaded from canonical JSON would lose the id a tool's filter
list names. No shipped path loads JSON today; this keeps that path
from springing the same trap if it is enabled.
Written as "sliceId", not "id": "id" is already the stable id's key,
and sharing it would overwrite every node's stable id on the way out.
The round-trip test that sets every property now sets helpTopicId and
sliceId, which it had not been covering, and checks the two ids stay
apart. Dropping the write fails it on SliceId.
* LT-22802: Test that a tool's filter list reaches the composer
The file reader and the composer's gate were each tested, but not the
join between them: deleting the view's hiddenSliceIds argument left
the whole suite green. No shipped tool reaches a live filter id today,
so nothing else would have noticed.
The view's composition moves out of ShowAvaloniaEntry into
ComposeDetail, unchanged, so a test can reach it without the settle,
adapter and message work around it.
The test installs the shipped ProdRestrictEdit configuration, whose
filter list is basicPlusFilter.xml, and composes a production
restriction through the view. The same view with the filterPath removed
is the control, and shows the Status row. Removing the argument fails
the test, with Status among the composed rows.
Reported in review.
* LT-22802: Put each summary back on the member it describes
Extracting ComposeDetail left it under ShowAvaloniaEntry's summary as
well as its own, and ShowAvaloniaEntry with none. That summary is back
on its method.
The memoized filter list caches a failed read too, which reads like an
oversight; the comment now says it is deliberate, since a missing
filter file is an install fault and re-reading it would log the same
failure for every record shown.
The reading tests' summary argued for their own existence, and its
claim that every other test supplies the id set by hand stopped being
true with the wiring test. It now says what the fixture reads.
Reported in review. Comments only.
Co-Authored-By: Zachary Burnham <zachary_burnham@sil.org>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>1 parent b18c601 commit 43837fe
13 files changed
Lines changed: 506 additions & 34 deletions
File tree
- Src
- Common/FwAvalonia
- FwAvaloniaTests
- ViewDefinition
- xWorks
- Avalonia
- Composer
- Hosting
- xWorksTests/Avalonia
- Composer
- Hosting
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
111 | 111 | | |
112 | 112 | | |
113 | 113 | | |
114 | | - | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
115 | 117 | | |
116 | 118 | | |
117 | 119 | | |
| |||
172 | 174 | | |
173 | 175 | | |
174 | 176 | | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
175 | 181 | | |
176 | 182 | | |
177 | 183 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
51 | | - | |
52 | | - | |
| 51 | + | |
| 52 | + | |
53 | 53 | | |
54 | 54 | | |
55 | 55 | | |
| |||
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
62 | | - | |
| 62 | + | |
| 63 | + | |
63 | 64 | | |
64 | 65 | | |
65 | 66 | | |
| |||
76 | 77 | | |
77 | 78 | | |
78 | 79 | | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
79 | 85 | | |
80 | 86 | | |
81 | 87 | | |
| |||
155 | 161 | | |
156 | 162 | | |
157 | 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 | + | |
158 | 188 | | |
159 | 189 | | |
160 | 190 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
90 | 90 | | |
91 | 91 | | |
92 | 92 | | |
| 93 | + | |
93 | 94 | | |
94 | 95 | | |
95 | 96 | | |
| |||
214 | 215 | | |
215 | 216 | | |
216 | 217 | | |
217 | | - | |
| 218 | + | |
| 219 | + | |
218 | 220 | | |
219 | 221 | | |
220 | 222 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
387 | 387 | | |
388 | 388 | | |
389 | 389 | | |
390 | | - | |
| 390 | + | |
| 391 | + | |
391 | 392 | | |
| 393 | + | |
392 | 394 | | |
393 | 395 | | |
394 | 396 | | |
| |||
432 | 434 | | |
433 | 435 | | |
434 | 436 | | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
435 | 444 | | |
436 | 445 | | |
437 | 446 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
299 | 299 | | |
300 | 300 | | |
301 | 301 | | |
302 | | - | |
303 | | - | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
304 | 305 | | |
305 | 306 | | |
306 | 307 | | |
| |||
310 | 311 | | |
311 | 312 | | |
312 | 313 | | |
313 | | - | |
| 314 | + | |
314 | 315 | | |
315 | 316 | | |
316 | 317 | | |
| |||
322 | 323 | | |
323 | 324 | | |
324 | 325 | | |
325 | | - | |
| 326 | + | |
326 | 327 | | |
327 | 328 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
37 | | - | |
| 37 | + | |
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
| |||
395 | 395 | | |
396 | 396 | | |
397 | 397 | | |
| 398 | + | |
398 | 399 | | |
399 | 400 | | |
400 | 401 | | |
| |||
412 | 413 | | |
413 | 414 | | |
414 | 415 | | |
| 416 | + | |
| 417 | + | |
415 | 418 | | |
416 | 419 | | |
417 | 420 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
117 | 117 | | |
118 | 118 | | |
119 | 119 | | |
120 | | - | |
| 120 | + | |
| 121 | + | |
121 | 122 | | |
122 | 123 | | |
123 | | - | |
| 124 | + | |
| 125 | + | |
124 | 126 | | |
125 | 127 | | |
126 | 128 | | |
| |||
141 | 143 | | |
142 | 144 | | |
143 | 145 | | |
144 | | - | |
| 146 | + | |
| 147 | + | |
145 | 148 | | |
146 | 149 | | |
147 | 150 | | |
| |||
162 | 165 | | |
163 | 166 | | |
164 | 167 | | |
165 | | - | |
| 168 | + | |
166 | 169 | | |
167 | 170 | | |
168 | 171 | | |
| |||
352 | 355 | | |
353 | 356 | | |
354 | 357 | | |
355 | | - | |
| 358 | + | |
| 359 | + | |
356 | 360 | | |
| 361 | + | |
357 | 362 | | |
358 | 363 | | |
359 | 364 | | |
| |||
572 | 577 | | |
573 | 578 | | |
574 | 579 | | |
575 | | - | |
576 | | - | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
577 | 583 | | |
578 | 584 | | |
579 | 585 | | |
| |||
601 | 607 | | |
602 | 608 | | |
603 | 609 | | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
604 | 623 | | |
605 | 624 | | |
606 | | - | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
607 | 628 | | |
| 629 | + | |
608 | 630 | | |
609 | 631 | | |
610 | 632 | | |
| |||
0 commit comments