From c03c95a3b35e7527fd71428b3e0eedd29122dbc8 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Thu, 17 Sep 2026 07:49:35 -0400 Subject: [PATCH 1/8] 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. Co-Authored-By: Claude Opus 5 --- .../Avalonia/Composer/DetailComposer.cs | 13 +++-- .../Composer/DetailFieldRelevanceTests.cs | 47 +++++++++++++++++-- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/Src/xWorks/Avalonia/Composer/DetailComposer.cs b/Src/xWorks/Avalonia/Composer/DetailComposer.cs index d748be529a..a818a74f60 100644 --- a/Src/xWorks/Avalonia/Composer/DetailComposer.cs +++ b/Src/xWorks/Avalonia/Composer/DetailComposer.cs @@ -536,10 +536,15 @@ private ViewDefinitionModel CompileForObjectWithOverrides(ICmObject obj, string private bool HideWhenEmpty(ViewNode node) => node.Visibility == ViewVisibility.IfData && !_showHidden; /// - /// Whether the DOMAIN says this field does not apply to this object, which legacy - /// asks before building a slice (SliceFilter -> ICmObject.IsFieldRelevant). StemName - /// is irrelevant on a clitic or particle, Position on a non-infix, InflectionClasses - /// on some affix forms. + /// Whether the DOMAIN says this field does not apply to this object. StemName is + /// irrelevant on a clitic or particle, Position on a non-infix, InflectionClasses on + /// some affix forms, FromPartsOfSpeech on an entry with no clitic. + /// + /// This is the SECOND of the two gates legacy's SliceFilter.IncludeSlice applies, not + /// the whole of it. The first looks the slice's id up in the tool's filter list and + /// withholds the row when it is listed; that one is NOT implemented here, because + /// the id never reaches the composer -- the importer does not carry it, and + /// ViewNode has no Id. Tracked as LT-22802. /// /// Not the same as hidden: show-hidden-fields does NOT reveal an irrelevant field, so /// this is checked whatever _showHidden says. Legacy's propsToMonitor set is diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailFieldRelevanceTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailFieldRelevanceTests.cs index 9883332317..a500d43f06 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailFieldRelevanceTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailFieldRelevanceTests.cs @@ -12,12 +12,13 @@ namespace SIL.FieldWorks.XWorks { /// /// The composer asks the DOMAIN whether a field applies to an object before emitting a row, - /// which legacy does through SliceFilter -> ICmObject.IsFieldRelevant. + /// through ICmObject.IsFieldRelevant -- the second of the two gates legacy's + /// SliceFilter.IncludeSlice applies (the id/filter-list gate is LT-22802). /// /// Five classes override it in liblcm. MoStemAllomorph.StemName is covered by - /// AllomorphSectionCompositionTests; the detail-view relevant remainder is covered here. - /// VirtualOrdering also overrides it, but that class is not shown in a detail view, so there - /// is nothing for the composer to gate. + /// AllomorphSectionCompositionTests; VirtualOrdering is not shown in a detail view, so there + /// is nothing to gate. The rest are here, EXCEPT the InflectionClass limb that withholds the + /// row from a compound rule's left/right MSA, which no test reaches. /// /// Every test runs with showHiddenFields TRUE. Relevance is not a hidden field -- legacy /// withholds an irrelevant row even with Show Hidden Fields on -- and the flag also keeps an @@ -130,6 +131,44 @@ public void Compose_AffixInflectionClasses_OnlyWhenTheEntrySupportsThem() "now there is something to choose from, so the row composes"); } + /// + /// MoStemMsa.FromPartsOfSpeech ("Attaches to Categories") is relevant only when the + /// owning entry has a proclitic or enclitic allomorph. The layout declares that part + /// visibility="always", so before the relevance gate it composed on EVERY stem MSA -- + /// making this the gate's most visible consequence, and the one with the most rows + /// riding on it. + /// + [Test] + public void Compose_FromPartsOfSpeech_OnlyForAnEntryWithAClitic() + { + ILexEntry entry = null; + IMoStemMsa msa = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + entry = Cache.ServiceLocator.GetInstance().Create(); + msa = Cache.ServiceLocator.GetInstance().Create(); + entry.MorphoSyntaxAnalysesOC.Add(msa); + }); + + var fields = DetailComposer.Compose(entry, Cache, showHiddenFields: true).Model.Fields; + Assert.That(HasRow(fields, "FromPartsOfSpeech", msa.Hvo), Is.False, + "no clitic on the entry, so the domain says the row does not apply"); + + // The positive control: add a proclitic allomorph, change nothing else. + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + var clitic = Cache.ServiceLocator.GetInstance().Create(); + entry.AlternateFormsOS.Add(clitic); + clitic.Form.set_String(Cache.DefaultVernWs, + TsStringUtils.MakeString("clitico", Cache.DefaultVernWs)); + clitic.MorphTypeRA = MorphTypes.GetObject(MoMorphTypeTags.kguidMorphProclitic); + }); + + fields = DetailComposer.Compose(entry, Cache, showHiddenFields: true).Model.Fields; + Assert.That(HasRow(fields, "FromPartsOfSpeech", msa.Hvo), Is.True, + "the same row on the same object composes once the entry has a clitic"); + } + /// /// MoStemMsa.InflectionClass is irrelevant until a part of speech is chosen -- there is /// no inflection class to pick without one. This one is NOT an allomorph field; it is the From e8e149471d9281ad17d6e488fcc9a1581f9d5e6e Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Thu, 17 Sep 2026 10:21:35 -0400 Subject: [PATCH 2/8] 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. Co-Authored-By: Claude Opus 5 --- .../FwAvaloniaTests/SliceIdImportTests.cs | 84 ++++++++++++++++ .../ViewDefinition/ViewDefinitionModel.cs | 11 ++- .../ViewDefinition/XmlLayoutImporter.cs | 7 +- .../Avalonia/Composer/DetailComposer.cs | 39 +++++--- .../Hosting/RecordEditView.Avalonia.cs | 52 +++++++++- .../Composer/DetailFieldRelevanceTests.cs | 4 +- .../Composer/DetailSliceFilterTests.cs | 97 +++++++++++++++++++ 7 files changed, 275 insertions(+), 19 deletions(-) create mode 100644 Src/Common/FwAvalonia/FwAvaloniaTests/SliceIdImportTests.cs create mode 100644 Src/xWorks/xWorksTests/Avalonia/Composer/DetailSliceFilterTests.cs diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/SliceIdImportTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/SliceIdImportTests.cs new file mode 100644 index 0000000000..97d30f64aa --- /dev/null +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/SliceIdImportTests.cs @@ -0,0 +1,84 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Collections.Generic; +using System.Linq; +using System.Xml.Linq; +using NUnit.Framework; +using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; + +namespace FwAvaloniaTests +{ + /// + /// A slice's authored id= is the name a tool's filter list uses to withhold the row. + /// It has to survive import, or the composer has nothing to match a filter against + /// (LT-22802). + /// + [TestFixture] + public class SliceIdImportTests + { + private const string PartsXml = @" + + + + + + + +"; + + private static ViewDefinitionModel Import(string layoutXml) + { + var parts = new DictionaryPartResolver(XElement.Parse(PartsXml)); + return new XmlLayoutImporter().Import(XElement.Parse(layoutXml), parts); + } + + private static IEnumerable Flatten(ViewNode n) + { + yield return n; + foreach (var c in n.Children) + foreach (var d in Flatten(c)) + yield return d; + } + + private static ViewDefinitionModel BothRows() => Import(@" + + + +"); + + [Test] + public void Slice_WithAnId_CarriesItOntoTheNode() + { + var nodes = BothRows().Roots.SelectMany(Flatten).ToList(); + + Assert.That(nodes.Select(n => n.SliceId), Does.Contain("CmPossibilityStatus"), + "the filter list names rows by this id; dropping it at import leaves the " + + "composer unable to apply the tool's filter at all"); + } + + [Test] + public void Slice_WithoutAnId_LeavesItNull() + { + var name = BothRows().Roots.SelectMany(Flatten) + .Single(n => n.Field == "Name"); + + Assert.That(name.SliceId, Is.Null, + "most slices author no id, and a synthesized one could collide with a real " + + "entry in some tool's filter list"); + } + + [Test] + public void SliceId_IsNotReportedAsAnUnhandledAttribute() + { + var unhandled = BothRows().Diagnostics + .Where(d => d.Code == "unhandled-attribute" && d.Message.Contains("id")) + .ToList(); + + Assert.That(unhandled, Is.Empty, + "id is consumed now, so it must leave the unhandled-attribute report -- that " + + "report is the list of things the Avalonia view still ignores"); + } + } +} diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs index 22730bc522..7bbbe57ca6 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs @@ -386,8 +386,10 @@ public ViewNode( ViewStringList enumStringList = null, IReadOnlyList visibleWritingSystems = null, bool toggleValue = false, - bool reorder = false) + bool reorder = false, + string sliceId = null) { + SliceId = sliceId; Reorder = reorder; ToggleValue = toggleValue; VisibleWritingSystems = visibleWritingSystems; @@ -430,6 +432,13 @@ public ViewNode( public ViewNodeKind Kind { get; } + /// + /// The slice's authored id=, the name a tool's filter list uses to withhold the + /// row. Null on the nodes that author none, which is most of them. NOT + /// , which is synthesized and always present. + /// + public string SliceId { get; } + public string Label { get; } public string Abbreviation { get; } diff --git a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs index 9054b94462..889e7f7611 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs @@ -34,7 +34,7 @@ public sealed class XmlLayoutImporter : IViewDefinitionImporter public static readonly HashSet HandledSliceAttributes = new HashSet(System.StringComparer.Ordinal) { - "label", "abbr", "field", "ws", "editor", "visibility", "expansion", + "id", "label", "abbr", "field", "ws", "editor", "visibility", "expansion", "localizationKey", "labelId", "automationId", "routing", "menu", "contextMenu", "hotlinks", "forVariant", "visibleWritingSystems", "reorder" }; @@ -394,7 +394,8 @@ private ViewNode CreateNode( localizationKey, automationId, routing, boldEmphasis, fontScalePercent, menuId, contextMenuId, hotlinksId, chooserLinks: chooserLinks.Count > 0 ? chooserLinks : null, - visibleWritingSystems: visibleWss); + visibleWritingSystems: visibleWss, + sliceId: Attr(contentEl, "id")); } // Dynamic custom slices keep their legacy class/assembly identity so the host can @@ -411,6 +412,8 @@ private ViewNode CreateNode( chooserLinks: chooserLinks.Count > 0 ? chooserLinks : null, enumStringList: enumStringList, visibleWritingSystems: visibleWss, + // A tool's filter list withholds rows by this authored id. + sliceId: Attr(contentEl, "id"), // Legacy toggleValue= on a boolean slice (the displayed checkbox is the // logical inverse of the stored property); carried so the composer inverts read+write. toggleValue: ParseOptionalBool(Attr(contentEl, "toggleValue")) ?? false, diff --git a/Src/xWorks/Avalonia/Composer/DetailComposer.cs b/Src/xWorks/Avalonia/Composer/DetailComposer.cs index a818a74f60..091bff15ae 100644 --- a/Src/xWorks/Avalonia/Composer/DetailComposer.cs +++ b/Src/xWorks/Avalonia/Composer/DetailComposer.cs @@ -117,10 +117,12 @@ public static ComposedDetail Compose(ILexEntry entry, LcmCache cache, bool showH SlicePluginRegistry plugins = null, ViewDefinitionOverrideResolver overrides = null, ISet showAllWritingSystemsFields = null, - Action writingSystemFocused = null) + Action writingSystemFocused = null, + ISet hiddenSliceIds = null) => Compose((ICmObject)entry, cache, "Normal", showHiddenFields, plugins, overrides, showAllWritingSystemsFields: showAllWritingSystemsFields, - writingSystemFocused: writingSystemFocused); + writingSystemFocused: writingSystemFocused, + hiddenSliceIds: hiddenSliceIds); /// /// Compose the structured detail view for ANY record root + starting layout -- the @@ -141,7 +143,8 @@ public static ComposedDetail Compose(ICmObject obj, LcmCache cache, string layou ViewDefinitionOverrideResolver overrides = null, string layoutChoiceField = null, ISet showAllWritingSystemsFields = null, - Action writingSystemFocused = null) + Action writingSystemFocused = null, + ISet hiddenSliceIds = null) { if (obj == null) throw new ArgumentNullException(nameof(obj)); if (cache == null) throw new ArgumentNullException(nameof(cache)); @@ -162,7 +165,7 @@ public static ComposedDetail Compose(ICmObject obj, LcmCache cache, string layou IDetailEditContext composedContext = null; var state = new ComposeState(cache, showHiddenFields, plugins ?? SlicePluginRegistry.Default, () => composedContext, overrides, - showAllWritingSystemsFields, writingSystemFocused); + showAllWritingSystemsFields, writingSystemFocused, hiddenSliceIds); state.EnterModel(root); foreach (var node in root.Roots) state.Walk(node, obj, 0); @@ -352,8 +355,10 @@ public ComposeState(LcmCache cache, bool showHiddenFields, SlicePluginRegistry plugins, Func editContextAccessor, ViewDefinitionOverrideResolver overrides = null, ISet showAllWritingSystemsFields = null, - Action writingSystemFocused = null) + Action writingSystemFocused = null, + ISet hiddenSliceIds = null) { + _hiddenSliceIds = hiddenSliceIds; _cache = cache; _showHidden = showHiddenFields; _plugins = plugins; @@ -540,12 +545,6 @@ private ViewDefinitionModel CompileForObjectWithOverrides(ICmObject obj, string /// irrelevant on a clitic or particle, Position on a non-infix, InflectionClasses on /// some affix forms, FromPartsOfSpeech on an entry with no clitic. /// - /// This is the SECOND of the two gates legacy's SliceFilter.IncludeSlice applies, not - /// the whole of it. The first looks the slice's id up in the tool's filter list and - /// withholds the row when it is listed; that one is NOT implemented here, because - /// the id never reaches the composer -- the importer does not carry it, and - /// ViewNode has no Id. Tracked as LT-22802. - /// /// Not the same as hidden: show-hidden-fields does NOT reveal an irrelevant field, so /// this is checked whatever _showHidden says. Legacy's propsToMonitor set is /// discarded -- it exists so a live slice can re-evaluate when the property it @@ -577,10 +576,26 @@ private bool IsIrrelevantForObject(ViewNode node, ICmObject obj) return !obj.IsFieldRelevant(flid, _propsToMonitor); } + // The tool's filter list, by authored slice id; null when the tool configures none. + private readonly ISet _hiddenSliceIds; + + /// + /// Whether the TOOL withholds this row: a tool's configuration can name slice ids + /// to leave out, and a node carrying one of them is dropped. Checked before the + /// node kind is dispatched, so a withheld node takes its subtree with it. + /// + private bool IsFilteredOutByTool(ViewNode node) + => _hiddenSliceIds != null + && !string.IsNullOrEmpty(node?.SliceId) + && _hiddenSliceIds.Contains(node.SliceId); + public void Walk(ViewNode node, ICmObject obj, int depth) { - if (IsHidden(node) || depth > MaxDepth || IsIrrelevantForObject(node, obj)) + if (IsHidden(node) || depth > MaxDepth || IsFilteredOutByTool(node) + || IsIrrelevantForObject(node, obj)) + { return; + } switch (node.Kind) { diff --git a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs index b98840e967..c1383320f0 100644 --- a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs +++ b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs @@ -85,6 +85,52 @@ private bool ShouldUseAvaloniaLexiconEdit get { return m_activeUIFramework == UIFramework.Avalonia; } } + // Memoized: the tool's configuration cannot change while the view lives. Null until read, + // then either the id set or an empty set (which the composer treats as "filter nothing"). + private ISet m_hiddenSliceIds; + + /// + /// The slice ids this tool's filter list withholds, read from the filterPath its own + /// configuration names. Empty for a tool that configures no filterPath, and empty when + /// the file cannot be read: a detail view showing an extra row beats one that will not + /// open. + /// + private ISet HiddenSliceIds + { + get + { + if (m_hiddenSliceIds != null) + return m_hiddenSliceIds; + + m_hiddenSliceIds = new HashSet(StringComparer.Ordinal); + try + { + var filterPath = XmlUtils.GetOptionalAttributeValue( + m_configurationParameters, "filterPath"); + if (string.IsNullOrEmpty(filterPath)) + return m_hiddenSliceIds; + if (!Platform.IsWindows) + filterPath = filterPath.Replace(@"\", "/"); + + var document = new XmlDocument(); + document.Load(FwDirectoryFinder.GetCodeFile(filterPath)); + foreach (XmlNode node in document.SelectNodes("SliceFilter/node")) + { + var id = XmlUtils.GetOptionalAttributeValue(node, "id"); + if (!string.IsNullOrEmpty(id)) + m_hiddenSliceIds.Add(id); + } + } + catch (Exception e) + { + Logger.WriteError("Reading the tool's slice filter failed; no row is " + + "withheld by it.", e); + } + + return m_hiddenSliceIds; + } + } + /// /// Auto-save: settles any open fenced edit session -- commit when validation is /// clean, roll back otherwise. The holder guards internally (no-op when nothing is open), @@ -360,7 +406,8 @@ private void ShowAvaloniaEntry(ICmObject obj) ? DetailComposer.Compose(lexEntry, Cache, showHidden, overrides: ResolveViewOverride, showAllWritingSystemsFields: m_showAllWsFields, - writingSystemFocused: OnDetailWritingSystemFocused) + writingSystemFocused: OnDetailWritingSystemFocused, + hiddenSliceIds: HiddenSliceIds) // Non-entry roots compose against the tool's configured layout // (m_layoutName, default "Normal"); a type-selected layout (m_layoutChoiceField, e.g. // Notebook RnGenericRec keyed on "Type") resolves to the right variant inside Compose. @@ -369,7 +416,8 @@ private void ShowAvaloniaEntry(ICmObject obj) overrides: ResolveViewOverride, layoutChoiceField: m_layoutChoiceField, showAllWritingSystemsFields: m_showAllWsFields, - writingSystemFocused: OnDetailWritingSystemFocused); + writingSystemFocused: OnDetailWritingSystemFocused, + hiddenSliceIds: HiddenSliceIds); if (composed != null) { detail = composed.Model; diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailFieldRelevanceTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailFieldRelevanceTests.cs index a500d43f06..7a88d7066e 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailFieldRelevanceTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailFieldRelevanceTests.cs @@ -12,8 +12,8 @@ namespace SIL.FieldWorks.XWorks { /// /// The composer asks the DOMAIN whether a field applies to an object before emitting a row, - /// through ICmObject.IsFieldRelevant -- the second of the two gates legacy's - /// SliceFilter.IncludeSlice applies (the id/filter-list gate is LT-22802). + /// through ICmObject.IsFieldRelevant. A tool's own filter list is a separate gate, covered + /// by DetailSliceFilterTests. /// /// Five classes override it in liblcm. MoStemAllomorph.StemName is covered by /// AllomorphSectionCompositionTests; VirtualOrdering is not shown in a detail view, so there diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSliceFilterTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSliceFilterTests.cs new file mode 100644 index 0000000000..4c346768a8 --- /dev/null +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSliceFilterTests.cs @@ -0,0 +1,97 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using SIL.FieldWorks.Common.FwAvalonia.Detail; +using SIL.LCModel; +using SIL.LCModel.Core.Text; +using SIL.LCModel.Infrastructure; + +namespace SIL.FieldWorks.XWorks +{ + /// + /// The TOOL's gate on a row: a slice whose authored id appears in the filter list the + /// tool's filterPath names is withheld, whatever the domain says about it (LT-22802). The + /// domain's own gate is separate, and is covered by DetailFieldRelevanceTests. + /// + /// Composed against CmPossibility because that is the only class any shipped filter list + /// reaches: the five CmPossibility slice ids in basicPlusFilter.xml are the only entries + /// across the six filter files that name a slice the parts inventory defines. + /// + [TestFixture] + public class DetailSliceFilterTests : MemoryOnlyBackendProviderTestBase + { + private ICmPossibility m_possibility; + + [SetUp] + public void CreateProductionRestriction() + { + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + var list = Cache.LangProject.MorphologicalDataOA.ProdRestrictOA; + m_possibility = Cache.ServiceLocator.GetInstance() + .Create(); + list.PossibilitiesOS.Add(m_possibility); + m_possibility.Name.set_String(Cache.DefaultAnalWs, + TsStringUtils.MakeString("restriction", Cache.DefaultAnalWs)); + }); + } + + private IReadOnlyList Compose(params string[] hiddenSliceIds) + => DetailComposer.Compose(m_possibility, Cache, "default", showHiddenFields: true, + hiddenSliceIds: hiddenSliceIds.Length == 0 + ? null + : new HashSet(hiddenSliceIds)) + .Model.Fields; + + [Test] + public void AFilteredSliceId_WithholdsThatRow_AndNothingElse() + { + var unfiltered = Compose(); + Assume.That(unfiltered.Any(f => f.Field == "Status"), Is.True, + "fixture check: unfiltered, the Status row composes"); + + var filtered = Compose("CmPossibilityStatus"); + + Assert.That(filtered.Any(f => f.Field == "Status"), Is.False, + "the tool's filter list names this row, so it is withheld"); + Assert.That(filtered.Count, Is.EqualTo(unfiltered.Count - 1), + "and ONLY that row: a withheld node must not take unrelated rows with it"); + } + + /// + /// A whole shipped filter list at once: every id it names goes, and the rows it does + /// not name stay. + /// + [Test] + public void AWholeFilterList_WithholdsEveryIdItNames_AndNothingElse() + { + var filtered = Compose("CmPossibilityStatus", "CmPossibilityDiscussion", + "CmPossibilityConfidence", "CmPossibilityResearchers", "CmPossibilityRestrictions"); + var fields = filtered.Select(f => f.Field).ToList(); + + Assert.That(fields, Has.No.Member("Status").And.No.Member("Discussion") + .And.No.Member("Confidence").And.No.Member("Researchers") + .And.No.Member("Restrictions"), + "every id the filter lists is withheld. Composed:\n " + + string.Join("\n ", fields)); + Assert.That(fields, Does.Contain("Name").And.Contains("Abbreviation"), + "and the rows the filter does not name are untouched"); + } + + [Test] + public void AnIdInNoFilterList_LeavesEveryRowAlone() + { + var unfiltered = Compose(); + + var filtered = Compose("NotAnIdAnySliceAuthors"); + + Assert.That(filtered.Count, Is.EqualTo(unfiltered.Count), + "a filter naming nothing withholds nothing -- most shipped filter entries are " + + "stale ids that resolve to no slice, and they must stay harmless"); + } + } +} From 9561a78a70ce0c3cc8631898340aa7ce3a8b55cc Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Thu, 17 Sep 2026 16:50:03 -0400 Subject: [PATCH 3/8] 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. Co-Authored-By: Claude Opus 5 --- .../Hosting/RecordEditView.Avalonia.cs | 67 ++++++++-------- .../Hosting/SliceFilterListReadingTests.cs | 76 +++++++++++++++++++ 2 files changed, 108 insertions(+), 35 deletions(-) create mode 100644 Src/xWorks/xWorksTests/Avalonia/Hosting/SliceFilterListReadingTests.cs diff --git a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs index c1383320f0..2bfb81d8f3 100644 --- a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs +++ b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs @@ -85,50 +85,47 @@ private bool ShouldUseAvaloniaLexiconEdit get { return m_activeUIFramework == UIFramework.Avalonia; } } - // Memoized: the tool's configuration cannot change while the view lives. Null until read, - // then either the id set or an empty set (which the composer treats as "filter nothing"). + // Memoized: the tool's configuration cannot change while the view lives. private ISet m_hiddenSliceIds; + /// The slice ids this tool's filter list withholds. + private ISet HiddenSliceIds + => m_hiddenSliceIds ?? (m_hiddenSliceIds = ReadSliceFilterIds(m_configurationParameters)); + /// - /// The slice ids this tool's filter list withholds, read from the filterPath its own - /// configuration names. Empty for a tool that configures no filterPath, and empty when - /// the file cannot be read: a detail view showing an extra row beats one that will not - /// open. + /// The slice ids named by the filter list a tool's configuration points at through its + /// filterPath. Empty for a configuration that names none, and empty when the file cannot + /// be read: a detail view showing an extra row beats one that will not open. /// - private ISet HiddenSliceIds + /// The tool's configuration parameters; null yields an empty + /// set. + internal static ISet ReadSliceFilterIds(XmlNode configuration) { - get + var ids = new HashSet(StringComparer.Ordinal); + try { - if (m_hiddenSliceIds != null) - return m_hiddenSliceIds; - - m_hiddenSliceIds = new HashSet(StringComparer.Ordinal); - try - { - var filterPath = XmlUtils.GetOptionalAttributeValue( - m_configurationParameters, "filterPath"); - if (string.IsNullOrEmpty(filterPath)) - return m_hiddenSliceIds; - if (!Platform.IsWindows) - filterPath = filterPath.Replace(@"\", "/"); - - var document = new XmlDocument(); - document.Load(FwDirectoryFinder.GetCodeFile(filterPath)); - foreach (XmlNode node in document.SelectNodes("SliceFilter/node")) - { - var id = XmlUtils.GetOptionalAttributeValue(node, "id"); - if (!string.IsNullOrEmpty(id)) - m_hiddenSliceIds.Add(id); - } - } - catch (Exception e) + var filterPath = XmlUtils.GetOptionalAttributeValue(configuration, "filterPath"); + if (string.IsNullOrEmpty(filterPath)) + return ids; + if (!Platform.IsWindows) + filterPath = filterPath.Replace(@"\", "/"); + + var document = new XmlDocument(); + document.Load(FwDirectoryFinder.GetCodeFile(filterPath)); + foreach (XmlNode node in document.SelectNodes("SliceFilter/node")) { - Logger.WriteError("Reading the tool's slice filter failed; no row is " - + "withheld by it.", e); + var id = XmlUtils.GetOptionalAttributeValue(node, "id"); + if (!string.IsNullOrEmpty(id)) + ids.Add(id); } - - return m_hiddenSliceIds; } + catch (Exception e) + { + Logger.WriteError("Reading the tool's slice filter failed; no row is withheld " + + "by it.", e); + } + + return ids; } /// diff --git a/Src/xWorks/xWorksTests/Avalonia/Hosting/SliceFilterListReadingTests.cs b/Src/xWorks/xWorksTests/Avalonia/Hosting/SliceFilterListReadingTests.cs new file mode 100644 index 0000000000..0f61bb4a41 --- /dev/null +++ b/Src/xWorks/xWorksTests/Avalonia/Hosting/SliceFilterListReadingTests.cs @@ -0,0 +1,76 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Collections.Generic; +using System.Xml; +using NUnit.Framework; + +namespace SIL.FieldWorks.XWorks +{ + /// + /// Reading a tool's filter list off its configuration: the step between the file on disk and + /// the ids the composer withholds rows by. Every other test in this area supplies that set by + /// hand, so without these the whole feature could be a silent no-op and still look green. + /// + [TestFixture] + public class SliceFilterListReadingTests + { + // The filterPath Grammar's Category Edit and Exception "Features" both configure. + private const string ShippedFilterPath = + @"Language Explorer\Configuration\Grammar\Edit\DataEntryFilters\basicPlusFilter.xml"; + + private static XmlNode Configuration(string attributes) + { + var document = new XmlDocument(); + document.LoadXml(""); + return document.DocumentElement; + } + + /// + /// The whole chain against a real shipped filter file: the attribute name, the path + /// resolution, the XPath and the id attribute. Asserts CONTAINMENT, so editing that file + /// does not break the test, but breaking any link in the chain does. + /// + [Test] + public void AConfiguredFilterPath_YieldsTheIdsThatFileNames() + { + var ids = RecordEditView.ReadSliceFilterIds( + Configuration(@"filterPath=""" + ShippedFilterPath + @"""")); + + Assert.That(ids, Does.Contain("CmPossibilityStatus"), + "the ids the file names must come back, or the composer is handed an empty set " + + "and withholds nothing while every other test still passes"); + Assert.That(ids, Does.Contain("CmPossibilityDiscussion") + .And.Contains("CmPossibilityConfidence") + .And.Contains("CmPossibilityResearchers") + .And.Contains("CmPossibilityRestrictions")); + } + + [Test] + public void AConfigurationWithNoFilterPath_YieldsNoIds() + { + var ids = RecordEditView.ReadSliceFilterIds(Configuration(@"clerk=""entries""")); + + Assert.That(ids, Is.Empty, + "most tools configure no filter, and they must withhold nothing"); + } + + [Test] + public void AFilterPathThatResolvesToNothing_YieldsNoIds_WithoutThrowing() + { + ISet ids = null; + + Assert.DoesNotThrow(() => ids = RecordEditView.ReadSliceFilterIds( + Configuration(@"filterPath=""no\such\filter.xml""")), + "an unreadable filter must not stop the detail view opening"); + Assert.That(ids, Is.Empty); + } + + [Test] + public void ANullConfiguration_YieldsNoIds() + { + Assert.That(RecordEditView.ReadSliceFilterIds(null), Is.Empty); + } + } +} From 9fe3619dd192db444e14db30fa79a3bb688ab178 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Mon, 28 Sep 2026 07:49:25 -0400 Subject: [PATCH 4/8] 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. Co-Authored-By: Claude Opus 5.5 --- Src/xWorks/Avalonia/Composer/DetailComposer.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Src/xWorks/Avalonia/Composer/DetailComposer.cs b/Src/xWorks/Avalonia/Composer/DetailComposer.cs index 9e9864c12b..0ae621e60a 100644 --- a/Src/xWorks/Avalonia/Composer/DetailComposer.cs +++ b/Src/xWorks/Avalonia/Composer/DetailComposer.cs @@ -578,7 +578,8 @@ private readonly HashSet> _propsToMonitor /// some affix forms, FromPartsOfSpeech on an entry with no clitic. /// /// The relevance gate of the two SliceFilter.IncludeSlice applies. The other, - /// , looks the slice's id up in the tool's filter list. + /// , looks the slice's id up in the tool's + /// filter list. /// /// Not the same as hidden: show-hidden-fields leaves an irrelevant field withheld, /// so this is asked whatever _showHidden says. From 7f259ea01d63a906ccf45f9ecea8594fc99cdc2e Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Tue, 29 Sep 2026 13:09:30 -0400 Subject: [PATCH 5/8] 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. Co-Authored-By: Claude Opus 5.5 --- .../ViewDefinitionOverrideApplierTests.cs | 36 +++++++++++++++++-- .../ViewDefinition/ViewDefinitionModel.cs | 4 +-- .../ViewDefinitionOverrideApplier.cs | 9 ++--- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideApplierTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideApplierTests.cs index 36dd1f4363..fe9b6a6f38 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideApplierTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideApplierTests.cs @@ -48,8 +48,8 @@ public void Apply_EmptyPatch_ReproducesBaseExactly() } // Every node is rebuilt on apply, so a clone that omits a field strips it tree-wide once - // any override exists. These three fields are outside ToSnapshot() and aren't covered by - // the EmptyPatch test. + // any override exists. These fields are outside ToSnapshot() and aren't covered by the + // EmptyPatch test. [Test] public void Apply_PreservesNodeFieldsNoOperationTouches() { @@ -59,7 +59,8 @@ public void Apply_PreservesNodeFieldsNoOperationTouches() new ViewNode("g/a", ViewNodeKind.Field, "A", null, "F", "multistring", EditorClassification.Known, "vern", ViewVisibility.Always, ViewExpansion.NotApplicable, false, null, null, - enumStringList: options, visibleWritingSystems: writingSystems, toggleValue: true))); + enumStringList: options, visibleWritingSystems: writingSystems, toggleValue: true, + helpTopicId: "khtpField-LexEntry-Form", sliceId: "CmPossibilityStatus"))); var patch = new ViewDefinitionOverride("LexEntry", "detail", "jtview", new[] { @@ -76,6 +77,11 @@ public void Apply_PreservesNodeFieldsNoOperationTouches() Assert.That(rebuilt.ToggleValue, Is.True, "a toggle value survives the rebuild"); Assert.That(rebuilt.EnumStringList?.Ids, Is.EqualTo(options.Ids), "an enum option list survives the rebuild"); + Assert.That(rebuilt.HelpTopicId, Is.EqualTo("khtpField-LexEntry-Form"), + "an authored help topic survives the rebuild"); + Assert.That(rebuilt.SliceId, Is.EqualTo("CmPossibilityStatus"), + "and so does the slice id a tool's filter list names; losing it would show a " + + "withheld row in every layout carrying an override"); } // The writing-system subset a user picks for one field, recorded against its stable id. @@ -155,6 +161,30 @@ public void Apply_DuplicateNode_CopiesLeafUnderNewId() Assert.That(children[1].Field, Is.EqualTo("F")); } + // A duplicate is the same authored part, so a filter list that withholds the source + // withholds the copy, and its help is the source's. + [Test] + public void Apply_DuplicateNode_KeepsTheSourcesSliceIdAndHelpTopic() + { + var shipped = Model(GroupNode("g", "Group", + new ViewNode("g/a", ViewNodeKind.Field, "A", null, "F", "string", + EditorClassification.Known, "vern", ViewVisibility.Always, + ViewExpansion.NotApplicable, false, null, null, + helpTopicId: "khtpField-LexEntry-Form", sliceId: "CmPossibilityStatus"))); + var patch = new ViewDefinitionOverride("LexEntry", "detail", "jtview", + new[] + { + new ViewOverrideOperation(ViewOverrideOperationKind.DuplicateNode, "g/a-copy", + parentStableId: "g", index: 1, sourceStableId: "g/a") + }, null); + + var copy = ViewDefinitionOverrideApplier.Apply(shipped, patch).Roots[0].Children[1]; + + Assert.That(copy.StableId, Is.EqualTo("g/a-copy"), "precondition: this is the copy"); + Assert.That(copy.SliceId, Is.EqualTo("CmPossibilityStatus")); + Assert.That(copy.HelpTopicId, Is.EqualTo("khtpField-LexEntry-Form")); + } + [Test] public void Apply_DuplicateNode_SourceWithChildren_ReportsDiagnostic_AndDoesNotInsert() { diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs index 05b1e51c08..920b54228d 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs @@ -387,8 +387,8 @@ public ViewNode( IReadOnlyList visibleWritingSystems = null, bool toggleValue = false, bool reorder = false, - string sliceId = null, - string helpTopicId = null) + string helpTopicId = null, + string sliceId = null) { SliceId = sliceId; HelpTopicId = helpTopicId; diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs index fb6e5e9484..54c968eaed 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs @@ -299,8 +299,9 @@ void Visit(ViewNode node) return map; } - // Reconstruct an immutable node with the overridden fields, copying every other one. - // Every trailing optional constructor argument must be passed, or that field is stripped. + // Rebuild an immutable node with the overridden fields, copying every other one. Pass + // every trailing optional argument, by name, or that field is stripped or shifted into + // its neighbour. private static ViewNode CloneWith(ViewNode n, ViewVisibility visibility, string label, IReadOnlyList children, IReadOnlyList visibleWritingSystems) => new ViewNode( @@ -310,7 +311,7 @@ private static ViewNode CloneWith(ViewNode n, ViewVisibility visibility, string n.ContextMenuId, n.HotlinksId, n.GhostField, n.GhostWs, n.GhostClass, n.GhostLabel, n.ForVariant, n.CustomEditorClass, n.CustomEditorAssembly, n.GhostInitMethod, n.Condition, n.ChooserLinks, n.EnumStringList, visibleWritingSystems, n.ToggleValue, n.Reorder, - n.HelpTopicId); + helpTopicId: n.HelpTopicId, sliceId: n.SliceId); // Copy a (leaf) node under a new StableId; AutomationId is dropped so the duplicate gets a fresh, // non-colliding identity (the renderer derives one from the new StableId by convention). @@ -322,6 +323,6 @@ private static ViewNode CloneWithId(ViewNode n, string newId) n.ContextMenuId, n.HotlinksId, n.GhostField, n.GhostWs, n.GhostClass, n.GhostLabel, n.ForVariant, n.CustomEditorClass, n.CustomEditorAssembly, n.GhostInitMethod, n.Condition, n.ChooserLinks, n.EnumStringList, n.VisibleWritingSystems, n.ToggleValue, n.Reorder, - n.HelpTopicId); + helpTopicId: n.HelpTopicId, sliceId: n.SliceId); } } From d8896ad1d0cdb4758741cb54194eddb874a83a20 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Tue, 29 Sep 2026 13:09:31 -0400 Subject: [PATCH 6/8] 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. Co-Authored-By: Claude Opus 5.5 --- .../FwAvalonia/FwAvaloniaTests/CanonicalJsonTests.cs | 8 +++++++- .../ViewDefinition/ViewDefinitionJsonSerializer.cs | 4 +++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/CanonicalJsonTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/CanonicalJsonTests.cs index 1b18330748..52b9615f65 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/CanonicalJsonTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/CanonicalJsonTests.cs @@ -111,7 +111,9 @@ public void EveryViewNodeProperty_SurvivesRoundTrip() { new ViewChooserLink("goto", "Edit the Publications list", "publicationsEdit"), new ViewChooserLink("simple", "Add a slot", "MakeInflAffixSlotChooserCommand", "TopPOS") - }); + }, + helpTopicId: "khtpField-LexEntry-Senses", + sliceId: "CmPossibilityStatus"); var model = new ViewDefinitionModel("LexEntry", "Normal", "detail", new List { node }, new List()); @@ -172,6 +174,10 @@ public void EveryViewNodeProperty_SurvivesRoundTrip() Assert.That(r.ChooserLinks[0].Target, Is.Null, "ChooserLinks[0].Target"); Assert.That(r.ChooserLinks[1].Type, Is.EqualTo("simple"), "ChooserLinks[1].Type"); Assert.That(r.ChooserLinks[1].Target, Is.EqualTo("TopPOS"), "ChooserLinks[1].Target"); + Assert.That(r.HelpTopicId, Is.EqualTo("khtpField-LexEntry-Senses"), nameof(r.HelpTopicId)); + Assert.That(r.SliceId, Is.EqualTo("CmPossibilityStatus"), nameof(r.SliceId)); + Assert.That(r.StableId, Is.Not.EqualTo(r.SliceId), + "both are written as ids, and one must not overwrite the other"); }); } } diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs index bffd46bc19..2698e3ea5b 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs @@ -90,6 +90,7 @@ private static JObject WriteNode(ViewNode node) AddIfPresent(o, "contextMenu", node.ContextMenuId); AddIfPresent(o, "hotlinks", node.HotlinksId); AddIfPresent(o, "helpTopicID", node.HelpTopicId); + AddIfPresent(o, "sliceId", node.SliceId); AddIfPresent(o, "ghost", node.GhostField); AddIfPresent(o, "ghostWs", node.GhostWs); AddIfPresent(o, "ghostClass", node.GhostClass); @@ -214,7 +215,8 @@ private static ViewNode ReadNode(JToken token) ghostInitMethod: (string)o["ghostInitMethod"], condition: ReadCondition((JObject)o["condition"]), chooserLinks: ((JArray)o["chooserLinks"])?.Select(ReadChooserLink).ToList(), - helpTopicId: (string)o["helpTopicID"]); + helpTopicId: (string)o["helpTopicID"], + sliceId: (string)o["sliceId"]); } private static T ParseEnum(JObject o, string name, T fallback) where T : struct From 81e5143a125317b2dbf3346e78afa786dd6f08c7 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Tue, 29 Sep 2026 13:09:31 -0400 Subject: [PATCH 7/8] 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. Co-Authored-By: Claude Opus 5.5 --- .../Hosting/RecordEditView.Avalonia.cs | 42 +++++---- .../DetailObjectCommandExecutionTests.cs | 85 +++++++++++++++++++ 2 files changed, 111 insertions(+), 16 deletions(-) diff --git a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs index 31239d2dfd..818da9d250 100644 --- a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs +++ b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs @@ -353,6 +353,31 @@ private void ScheduleOnUiThread(Action runner) /// lexical entry (first-slice fallback if composition fails), or the resource-backed /// unsupported state otherwise. /// + /// + /// The detail this view composes for , under the tool's own + /// configuration: its layout, its view overrides and its slice filter list. + /// + internal ComposedDetail ComposeDetail(ICmObject obj, bool showHidden) + { + var lexEntry = obj as ILexEntry; + return lexEntry != null + ? DetailComposer.Compose(lexEntry, Cache, showHidden, + overrides: ResolveViewOverride, + showAllWritingSystemsFields: m_showAllWsFields, + writingSystemFocused: OnDetailWritingSystemFocused, + hiddenSliceIds: HiddenSliceIds) + // Other roots use the tool's layout (m_layoutName, default "Normal"); + // a type-selected one, such as RnGenericRec keyed on "Type", resolves + // inside Compose. + : DetailComposer.Compose(obj, Cache, + string.IsNullOrEmpty(m_layoutName) ? "Normal" : m_layoutName, showHidden, + overrides: ResolveViewOverride, + layoutChoiceField: m_layoutChoiceField, + showAllWritingSystemsFields: m_showAllWsFields, + writingSystemFocused: OnDetailWritingSystemFocused, + hiddenSliceIds: HiddenSliceIds); + } + private void ShowAvaloniaEntry(ICmObject obj) { // Auto-save: a session still open from the previous record/edit settles (commit @@ -400,22 +425,7 @@ private void ShowAvaloniaEntry(ICmObject obj) ComposedDetail composed = null; try { - composed = lexEntry != null - ? DetailComposer.Compose(lexEntry, Cache, showHidden, - overrides: ResolveViewOverride, - showAllWritingSystemsFields: m_showAllWsFields, - writingSystemFocused: OnDetailWritingSystemFocused, - hiddenSliceIds: HiddenSliceIds) - // Non-entry roots compose against the tool's configured layout - // (m_layoutName, default "Normal"); a type-selected layout (m_layoutChoiceField, e.g. - // Notebook RnGenericRec keyed on "Type") resolves to the right variant inside Compose. - : DetailComposer.Compose(obj, Cache, - string.IsNullOrEmpty(m_layoutName) ? "Normal" : m_layoutName, showHidden, - overrides: ResolveViewOverride, - layoutChoiceField: m_layoutChoiceField, - showAllWritingSystemsFields: m_showAllWsFields, - writingSystemFocused: OnDetailWritingSystemFocused, - hiddenSliceIds: HiddenSliceIds); + composed = ComposeDetail(obj, showHidden); if (composed != null) { detail = composed.Model; diff --git a/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs b/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs index 279709f03c..5f52eb2905 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs @@ -318,6 +318,91 @@ public void AnEnvironmentItem_ResolvesToTheMenuThatCarriesItsCommands() } } + /// + /// A tool's filter list reaches the composer through the view. The shipped Exception + /// "Features" configuration (ProdRestrictEdit) names basicPlusFilter.xml, which withholds + /// Status; composing through the view's own configuration, rather than a hand-built id + /// set, is what shows the file, the property and the composer are joined. + /// + [Test] + public void AToolsFilterList_ReachesTheComposer_ThroughTheView() + { + ICmPossibility restriction = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + restriction = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.MorphologicalDataOA.ProdRestrictOA.PossibilitiesOS.Add(restriction); + restriction.Name.set_String(Cache.DefaultAnalWs, + TsStringUtils.MakeString("restriction", Cache.DefaultAnalWs)); + }); + var original = SwapToolConfiguration(null); + try + { + var shipped = ProdRestrictEditParameters(); + Assert.That(shipped.Attributes["filterPath"], Is.Not.Null, + "precondition: the shipped configuration names a filter list"); + + var unfiltered = (XmlElement)shipped.Clone(); + unfiltered.RemoveAttribute("filterPath"); + SwapToolConfiguration(unfiltered); + Assert.That(ComposedFields(restriction), Does.Contain("Status"), + "control: without the filter list, the same view composes the Status row"); + + SwapToolConfiguration(shipped); + Assert.That(ComposedFields(restriction), Does.Not.Contain("Status"), + "the view's filter list reached the composer and withheld the row"); + } + finally + { + SwapToolConfiguration(original); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + if (restriction != null && restriction.IsValidObject) + restriction.Delete(); + }); + } + } + + private IReadOnlyList ComposedFields(ICmObject obj) + => m_view.ComposeDetail(obj, showHidden: true).Model.Fields.Select(f => f.Field).ToList(); + + // The record-edit parameters of the shipped ProdRestrictEdit tool: the node that carries + // its filterPath and its layout. + private static XmlElement ProdRestrictEditParameters() + { + var document = new XmlDocument(); + document.Load(Path.Combine(FwDirectoryFinder.CodeDirectory, "Language Explorer", + "Configuration", "Grammar", "Edit", "toolConfiguration.xml")); + var node = (XmlElement)document.SelectSingleNode( + "//tool[@value='ProdRestrictEdit']//parameters[@filterPath]"); + Assert.That(node, Is.Not.Null, "precondition: the shipped ProdRestrictEdit parameters"); + return node; + } + + // Installs a tool configuration on the view the way ReadParameters does, layout + // included, and clears the memoized filter list. Returns the one it replaced; null + // changes nothing. + private XmlNode SwapToolConfiguration(XmlNode configuration) + { + var parameters = typeof(XCoreUserControl).GetField("m_configurationParameters", + BindingFlags.Instance | BindingFlags.NonPublic); + var layout = typeof(RecordEditView).GetField("m_layoutName", + BindingFlags.Instance | BindingFlags.NonPublic); + var memo = typeof(RecordEditView).GetField("m_hiddenSliceIds", + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.That(parameters, Is.Not.Null); + Assert.That(layout, Is.Not.Null); + Assert.That(memo, Is.Not.Null); + var previous = (XmlNode)parameters.GetValue(m_view); + if (configuration != null) + { + parameters.SetValue(m_view, configuration); + layout.SetValue(m_view, configuration.Attributes?["layout"]?.Value); + memo.SetValue(m_view, null); + } + return previous; + } + [Test] public void SubentriesCtrlClick_ResolvesTheClickedEntry_AndRunsTheDefaultJumpPath() { From 7fda40d558b61a1f9d031f7adf684cf1e431da83 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Tue, 29 Sep 2026 16:08:01 -0400 Subject: [PATCH 8/8] 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: Claude Opus 5.5 --- .../Avalonia/Hosting/RecordEditView.Avalonia.cs | 13 +++++++------ .../Avalonia/Hosting/SliceFilterListReadingTests.cs | 6 +++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs index 818da9d250..ab87bd433b 100644 --- a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs +++ b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs @@ -86,7 +86,8 @@ private bool ShouldUseAvaloniaLexiconEdit get { return m_activeUIFramework == UIFramework.Avalonia; } } - // Memoized: the tool's configuration cannot change while the view lives. + // Memoized, a failed read included: a missing filter file is an install fault, and + // re-reading it would log the failure for every record shown. private ISet m_hiddenSliceIds; /// The slice ids this tool's filter list withholds. @@ -348,11 +349,6 @@ private void ScheduleOnUiThread(Action runner) } } - /// - /// Shows the Avalonia detail view for a record: the composed full-entry view when the record is a - /// lexical entry (first-slice fallback if composition fails), or the resource-backed - /// unsupported state otherwise. - /// /// /// The detail this view composes for , under the tool's own /// configuration: its layout, its view overrides and its slice filter list. @@ -378,6 +374,11 @@ internal ComposedDetail ComposeDetail(ICmObject obj, bool showHidden) hiddenSliceIds: HiddenSliceIds); } + /// + /// Shows the Avalonia detail view for a record: the composed full-entry view when the record is a + /// lexical entry (first-slice fallback if composition fails), or the resource-backed + /// unsupported state otherwise. + /// private void ShowAvaloniaEntry(ICmObject obj) { // Auto-save: a session still open from the previous record/edit settles (commit diff --git a/Src/xWorks/xWorksTests/Avalonia/Hosting/SliceFilterListReadingTests.cs b/Src/xWorks/xWorksTests/Avalonia/Hosting/SliceFilterListReadingTests.cs index 0f61bb4a41..617ed27de4 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Hosting/SliceFilterListReadingTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Hosting/SliceFilterListReadingTests.cs @@ -9,9 +9,9 @@ namespace SIL.FieldWorks.XWorks { /// - /// Reading a tool's filter list off its configuration: the step between the file on disk and - /// the ids the composer withholds rows by. Every other test in this area supplies that set by - /// hand, so without these the whole feature could be a silent no-op and still look green. + /// Reading a tool's filter list off its configuration: the filterPath attribute names a file + /// under the code directory, and each of its SliceFilter/node ids becomes a row the composer + /// withholds. A missing path, file or configuration yields no ids. /// [TestFixture] public class SliceFilterListReadingTests