Add Tests

for the part that changed significantly.

TODO: consider whether checking `field` instead of `label'
would make more sense.

Change-Id: I04290497ebc11dc625867ca33ce1b1d33027c7aa

Author: papeh

Src/Common/Controls/XMLViews/XMLViewsTests/XmlBrowseViewBaseVcTests.cs

@@ -1,12 +1,12 @@
-// Copyright (c) 2015 SIL International
+// Copyright (c) 2015-2025 SIL International
 // This software is licensed under the LGPL, version 2.1 or later
 // (http://www.gnu.org/licenses/lgpl-2.1.html)
 
+using NUnit.Framework;
+using SIL.FieldWorks.Common.Controls;
 using System.Collections.Generic;
 using System.Linq;
 using System.Xml;
-using NUnit.Framework;
-using SIL.FieldWorks.Common.Controls;
 
 namespace XMLViewsTests
 {
@@ -168,7 +168,6 @@ public void GetHeaderLabels_ReturnsColumnSpecLabels()
 
 			var columnDoc = new XmlDocument();
 			columnDoc.LoadXml(testColumns);
-			columnDoc.SelectNodes("column");
 			var testVc = new XmlBrowseViewBaseVc
 			{
 				ColumnSpecs = new List<XmlNode>(columnDoc.DocumentElement.GetElementsByTagName("column").OfType<XmlNode>())
@@ -182,17 +181,41 @@ public void GetHeaderLabels_ReturnsColumnSpecLabels()
 		[Test]
 		public void IsValidColumnSpec_ValidReturnsTrue()
 		{
-			// set up PossibleColumnSpecs
-			// Pass a valid node
-			Assert.IsTrue(false);
+			var vc = new XmlBrowseViewBaseVc { PossibleColumnSpecs = new List<XmlNode>(), ListItemsClass = -1 /* can't be 0 */ };
+			var possibleColumns = new XmlDocument();
+			possibleColumns.LoadXml("<columns><column label='Ref'/><column label='Other'/></columns>");
+			foreach (XmlNode node in possibleColumns.DocumentElement.GetElementsByTagName("column"))
+			{
+				vc.PossibleColumnSpecs.Add(node);
+			}
+
+			var savedColumns = new XmlDocument();
+			savedColumns.LoadXml("<root><column label='Ref'/><column label='Other (Best Ana)' originalLabel='Other'/></root>");
+
+			// SUT
+			foreach (XmlNode node in savedColumns.DocumentElement.GetElementsByTagName("column"))
+			{
+				Assert.IsTrue(vc.IsValidColumnSpec(node), $"Should have found this node to be valid: {node.OuterXml}");
+			}
 		}
 
 		[Test]
 		public void IsValidColumnSpec_InvalidReturnsFalse()
 		{
-			// set up PossibleColumnSpecs
-			// Pass an invalid node
-			Assert.IsFalse(true);
+			var vc = new XmlBrowseViewBaseVc { PossibleColumnSpecs = new List<XmlNode>(), ListItemsClass = -1 /* can't be 0 */ };
+			var possibleColumns = new XmlDocument();
+			possibleColumns.LoadXml("<columns><column label='Ref'/><column label='Other'/></columns>");
+			foreach (XmlNode node in possibleColumns.DocumentElement.GetElementsByTagName("column"))
+			{
+				vc.PossibleColumnSpecs.Add(node);
+			}
+
+			var savedColumns = new XmlDocument();
+			savedColumns.LoadXml("<root><column label='DoesNotExist' originalLabel='MayHaveExistedBefore'/></root>");
+			var invalidColumn = savedColumns.DocumentElement.SelectSingleNode("column");
+
+			// SUT
+			Assert.IsFalse(vc.IsValidColumnSpec(invalidColumn), $"Should have found this node to be invalid: {invalidColumn.OuterXml}");
 		}
 	}
-}
+}

Src/Common/Controls/XMLViews/XmlBrowseViewBaseVc.cs

@@ -56,10 +56,7 @@ public class XmlBrowseViewBaseVc : XmlVc
 		/// Specifications of columns to display
 		/// </summary>
 		protected List<XmlNode> m_columns = new List<XmlNode>();
-		/// <summary>
-		/// Specs of columns that COULD be displayed, but which have not been selected.
-		/// </summary>
-		protected List<XmlNode> m_possibleColumns;
+
 		/// <summary>// Top-level fake property for list of objects.</summary>
 		protected int m_fakeFlid = 0;
 		/// <summary>// Controls appearance of column for check boxes.</summary>
@@ -216,15 +213,15 @@ public XmlBrowseViewBaseVc(XmlNode xnSpec, int fakeFlid, XmlBrowseViewBase xbv)
 			if (doc == null) // nothing saved, or saved info won't parse
 			{
 				// default: the columns that have 'width' specified.
-				foreach(XmlNode node in m_possibleColumns)
+				foreach(XmlNode node in PossibleColumnSpecs)
 				{
 					if (XmlUtils.GetOptionalAttributeValue(node, "visibility", "always") == "always")
 						m_columns.Add(node);
 				}
 			}
 			else
 			{
-				var newPossibleColumns = new List<XmlNode>(m_possibleColumns);
+				var newPossibleColumns = new List<XmlNode>(PossibleColumnSpecs);
 				foreach (XmlNode node in doc.DocumentElement.SelectNodes("//column"))
 				{
 
@@ -550,9 +547,7 @@ public List<XmlNode> ComputePossibleColumns()
 			XmlVc vc = null;
 			if (ListItemsClass != 0)
 				vc = this;
-			m_possibleColumns = PartGenerator.GetGeneratedChildren(m_xnSpec.SelectSingleNode("columns"),
-						 m_xbv.Cache, vc, (int)ListItemsClass);
-			return m_possibleColumns;
+			return PossibleColumnSpecs = PartGenerator.GetGeneratedChildren(m_xnSpec.SelectSingleNode("columns"), m_xbv.Cache, vc, (int)ListItemsClass);
 		}
 
 		int m_listItemsClass = 0;
@@ -604,6 +599,7 @@ internal bool IsValidColumnSpec(XmlNode colSpec)
 				return isValid;
 			}
 			// In the simple case, `node`s label should match a label in PossibleColumnSpecs. There may be more complicated cases.
+			// TODO (Hasso) 2025.11: 'layout' (madatory?) and 'field' (optional) would be better attributes to match.
 			var label = XmlUtils.GetLocalizedAttributeValue(colSpec, "label", null) ??
 						XmlUtils.GetMandatoryAttributeValue(colSpec, "label");
 			var originalLabel = XmlUtils.GetLocalizedAttributeValue(colSpec, "originalLabel", null) ??
@@ -763,13 +759,10 @@ internal virtual List<XmlNode> ColumnSpecs
 			}
 		}
 
-		internal List<XmlNode> PossibleColumnSpecs
-		{
-			get
-			{
-				return m_possibleColumns;
-			}
-		}
+		/// <summary>
+		/// Specs of columns that COULD be displayed, but which have not been selected.
+		/// </summary>
+		protected internal List<XmlNode> PossibleColumnSpecs { get; set; }
 
 		internal ISortItemProvider SortItemProvider
 		{