From c03c95a3b35e7527fd71428b3e0eedd29122dbc8 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Thu, 17 Sep 2026 07:49:35 -0400 Subject: [PATCH 1/3] 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/3] 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/3] 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); + } + } +}