Skip to content

Commit 062796e

Browse files
committed
Clean up
Change-Id: Ia5fb27d9058f83e3ab1fcfa35ba7158e47bb8a88
1 parent cff6b7e commit 062796e

File tree

2 files changed

+19
-26
lines changed

2 files changed

+19
-26
lines changed

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

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public void GetHeaderLabels_ReturnsColumnSpecLabels()
178178
CollectionAssert.AreEqual(new List<string> { "Ref", "Occurrence" }, columnLabels);
179179
}
180180

181-
[Test]
181+
/// <remarks>TODO (Hasso) 2025.11: This test needs further setup to find the layout XmlNode in the LayoutCache</remarks>
182182
public void IsValidColumnSpec_HasLayout_TODO()
183183
{
184184
var vc = new XmlBrowseViewBaseVc { PossibleColumnSpecs = new List<XmlNode>(), ListItemsClass = -1 /* can't be 0 */ };
@@ -207,8 +207,13 @@ public void IsValidColumnSpec_HasLayout_TODO()
207207
}
208208
}
209209

210+
/// <summary>
211+
/// Tests that IsValidColumnSpec can identify valid columns based on their labels or originalLabels.
212+
/// Artificially skips the layout check because that requires a more complex setup.
213+
/// LT-22265: Invalid columns should not be displayed.
214+
/// </summary>
210215
[Test]
211-
public void IsValidColumnSpec_ValidReturnsTrue()
216+
public void IsValidColumnSpec_MatchesLabels()
212217
{
213218
var vc = new XmlBrowseViewBaseVc { PossibleColumnSpecs = new List<XmlNode>(), ListItemsClass = -1 /* can't be 0 */ };
214219
var possibleColumns = new XmlDocument();
@@ -218,30 +223,18 @@ public void IsValidColumnSpec_ValidReturnsTrue()
218223
vc.PossibleColumnSpecs.Add(node);
219224
}
220225

221-
var savedColumns = new XmlDocument();
222-
savedColumns.LoadXml("<root><column label='Ref'/><column label='Other (Best Ana)' originalLabel='Other'/></root>");
226+
var validColumns = new XmlDocument();
227+
validColumns.LoadXml("<root><column label='Ref'/><column label='Other (Best Ana)' originalLabel='Other'/></root>");
223228

224229
// SUT
225-
foreach (XmlNode node in savedColumns.DocumentElement.GetElementsByTagName("column"))
230+
foreach (XmlNode node in validColumns.DocumentElement.GetElementsByTagName("column"))
226231
{
227232
Assert.IsTrue(vc.IsValidColumnSpec(node), $"Should have found this node to be valid: {node.OuterXml}");
228233
}
229-
}
230-
231-
[Test]
232-
public void IsValidColumnSpec_InvalidReturnsFalse()
233-
{
234-
var vc = new XmlBrowseViewBaseVc { PossibleColumnSpecs = new List<XmlNode>(), ListItemsClass = -1 /* can't be 0 */ };
235-
var possibleColumns = new XmlDocument();
236-
possibleColumns.LoadXml("<columns><column label='Ref'/><column label='Other'/></columns>");
237-
foreach (XmlNode node in possibleColumns.DocumentElement.GetElementsByTagName("column"))
238-
{
239-
vc.PossibleColumnSpecs.Add(node);
240-
}
241234

242-
var savedColumns = new XmlDocument();
243-
savedColumns.LoadXml("<root><column label='DoesNotExist' originalLabel='MayHaveExistedBefore'/></root>");
244-
var invalidColumn = savedColumns.DocumentElement.SelectSingleNode("column");
235+
var invalidColumns = new XmlDocument();
236+
invalidColumns.LoadXml("<root><column label='DoesNotExist' originalLabel='MayHaveExistedBefore'/></root>");
237+
var invalidColumn = invalidColumns.DocumentElement.SelectSingleNode("column");
245238

246239
// SUT
247240
Assert.IsFalse(vc.IsValidColumnSpec(invalidColumn), $"Should have found this node to be invalid: {invalidColumn.OuterXml}");

Src/Common/Controls/XMLViews/XmlBrowseViewBaseVc.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,6 @@ public XmlBrowseViewBaseVc(XmlNode xnSpec, int fakeFlid, XmlBrowseViewBase xbv)
224224
var newPossibleColumns = new List<XmlNode>(PossibleColumnSpecs);
225225
foreach (XmlNode node in doc.DocumentElement.SelectNodes("//column"))
226226
{
227-
228227
// if there is a corresponding possible column, remove it from the newPossibleColumns list.
229228
var possible = newPossibleColumns.Find(n =>
230229
XmlUtils.GetOptionalAttributeValue(n, "label", "") ==
@@ -586,20 +585,21 @@ internal int ListItemsClass
586585
}
587586

588587
/// <summary>
589-
/// check to see if column spec is still valid.
588+
/// Check to see if column spec is still valid. If it is a custom field, update the label if necessary.
590589
/// </summary>
591-
/// <returns>True if we can't prove the column is invalid</returns>
592590
internal bool IsValidColumnSpec(XmlNode colSpec)
593591
{
594592
if (GetPartFromParentNode(colSpec, ListItemsClass) == null)
595-
return false; // invalid node, don't add.
596-
// If it is a Custom Field, check that it is valid and has the correct label
593+
{
594+
return false;
595+
}
596+
// If it is a Custom Field, check that it is valid and has the correct label.
597597
if (IsCustomField(colSpec, out var isValid))
598598
{
599599
return isValid;
600600
}
601601
// In the simple case, `node`s label should match a label in PossibleColumnSpecs. There may be more complicated cases.
602-
// TODO (Hasso) 2025.11: 'layout' (madatory?) and 'field' (optional) would be better attributes to match.
602+
// ENHANCE (Hasso) 2025.11: 'layout' (mandatory?) and 'field' (optional) would be better attributes to match, but that would require more test setup.
603603
var label = XmlUtils.GetLocalizedAttributeValue(colSpec, "label", null) ??
604604
XmlUtils.GetMandatoryAttributeValue(colSpec, "label");
605605
var originalLabel = XmlUtils.GetLocalizedAttributeValue(colSpec, "originalLabel", null) ??

0 commit comments

Comments
 (0)