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 d748be529a..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; @@ -536,10 +541,9 @@ 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. /// /// 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 @@ -572,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..2bfb81d8f3 100644 --- a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs +++ b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs @@ -85,6 +85,49 @@ private bool ShouldUseAvaloniaLexiconEdit get { return m_activeUIFramework == UIFramework.Avalonia; } } + // 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 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. + /// + /// The tool's configuration parameters; null yields an empty + /// set. + internal static ISet ReadSliceFilterIds(XmlNode configuration) + { + var ids = new HashSet(StringComparer.Ordinal); + try + { + 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")) + { + var id = XmlUtils.GetOptionalAttributeValue(node, "id"); + if (!string.IsNullOrEmpty(id)) + ids.Add(id); + } + } + catch (Exception e) + { + Logger.WriteError("Reading the tool's slice filter failed; no row is withheld " + + "by it.", e); + } + + return ids; + } + /// /// 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 +403,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 +413,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 9883332317..7a88d7066e 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. 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; 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 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"); + } + } +} 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); + } + } +}