Fix data region column name references (#2526)

Remove unnecessary logging parameters from CustomizeView
Update CustomizeView to use FieldKey
Fix data region column name references

Author: labkey-tchad

src/org/labkey/test/components/CustomizeView.java

@@ -15,28 +15,30 @@ public final class FieldKey implements CharSequence, WrapsFieldKey
     private static final String[] ILLEGAL = {"$", "/", "&", "}", "~", ",", "."};
     private static final String[] REPLACEMENT = {"$D", "$S", "$A", "$B", "$T", "$C", "$P"};
 
-    public static final FieldKey EMPTY = new FieldKey(""); // Useful as a sort of FieldKey builder starting point
-    public static final FieldKey SOURCES_FK = new FieldKey("DataInputs");
-    public static final FieldKey PARENTS_FK = new FieldKey("MaterialInputs");
+    public static final FieldKey EMPTY = new FieldKey(null, ""); // Useful as a sort of FieldKey builder starting point
+    public static final FieldKey SOURCES_FK = FieldKey.fromParts("DataInputs");
+    public static final FieldKey PARENTS_FK = FieldKey.fromParts("MaterialInputs");
 
     private static final String SEPARATOR = "/";
 
     private final FieldKey _parent;
     private final String _name;
     private final String _fieldKey;
 
-    private FieldKey(String name)
-    {
-        _parent = null;
-        _name = name;
-        _fieldKey = encodePart(name);
-    }
-
     private FieldKey(FieldKey parent, String child)
     {
-        _parent = parent;
-        _name = parent.getName() + SEPARATOR + child;
-        _fieldKey = parent + SEPARATOR + encodePart(child);
+        if (parent != null && !parent.isEmpty())
+        {
+            _parent = parent;
+            _name = parent.getName() + SEPARATOR + child;
+            _fieldKey = parent + SEPARATOR + encodePart(child);
+        }
+        else
+        {
+            _parent = null;
+            _name = child;
+            _fieldKey = encodePart(child);
+        }
     }
 
     public static List<String> getIllegalChars()
@@ -46,14 +48,7 @@ public static List<String> getIllegalChars()
 
     public static FieldKey fromParts(List<String> parts)
     {
-        FieldKey fieldKey = EMPTY;
-
-        for (String part : parts)
-        {
-            fieldKey = fieldKey.child(part);
-        }
-
-        return fieldKey;
+        return EMPTY.child(parts);
     }
 
     public static FieldKey fromParts(String... parts)
@@ -113,19 +108,23 @@ public FieldKey getParent()
         return _parent;
     }
 
-    public FieldKey child(String part)
+    public FieldKey child(String... parts)
     {
-        if (StringUtils.isBlank(part))
-            throw new IllegalArgumentException("FieldKey can't have blank part(s): " + this);
+        return child(Arrays.asList(parts));
+    }
 
-        if (StringUtils.isBlank(getName()))
-        {
-            return new FieldKey(part);
-        }
-        else
+    public FieldKey child(List<String> parts)
+    {
+        FieldKey child = this;
+
+        for (String part : parts)
         {
-            return new FieldKey(this, part);
+            if (StringUtils.isBlank(part))
+                throw new IllegalArgumentException("FieldKey can't have blank part(s): " + parts);
+
+            child = new FieldKey(child, part);
         }
+        return child;
     }
 
     public Iterator<FieldKey> getIterator()

src/org/labkey/test/params/FieldKey.java

@@ -188,7 +188,7 @@ public void testSteps()
                 Locator.lkButton(METADATA_LINK_BUTTON).waitForElement(getDriver(), WAIT_FOR_JAVASCRIPT)
                         .getAttribute("class").contains("labkey-disabled-button"));
 
-        buttonRegion.checkCheckbox(buttonRegion.getRowIndex("Portland", "Name"));
+        buttonRegion.checkCheckbox(buttonRegion.getRowIndex("Name", "Portland"));
         // wait for the button to enable:
         waitForElement(Locator.lkButton(METADATA_LINK_BUTTON), 10000);
 

src/org/labkey/test/tests/ButtonCustomizationTest.java

@@ -35,6 +35,7 @@
 import org.labkey.test.categories.Data;
 import org.labkey.test.pages.reports.ScriptReportPage;
 import org.labkey.test.params.FieldDefinition;
+import org.labkey.test.params.FieldKey;
 import org.labkey.test.params.list.IntListDefinition;
 import org.labkey.test.util.DataRegionTable;
 import org.labkey.test.util.Ext4Helper;
@@ -177,7 +178,7 @@ public void testListLookupURL() throws Exception
 
         log("** Adding in lookup list columns to grid");
         _customizeViewsHelper.openCustomizeViewPanel();
-        _customizeViewsHelper.addColumn(new String[] { "ListLookup", "LookupAge" });
+        _customizeViewsHelper.addColumn(FieldKey.fromParts("ListLookup", "LookupAge"));
         _customizeViewsHelper.saveCustomView();
 
         log("** Checking URLs go to correct container...");

src/org/labkey/test/tests/ContainerContextTest.java

@@ -300,7 +300,7 @@ private void saveAfterApplyingView(String name, String newColumnLabel, String ne
     @Test
     public void saveFilterTest()
     {
-        String fieldKey = FieldKey.fromParts(LAST_NAME_COLUMN).toString();
+        FieldKey fieldKey = FieldKey.fromParts(LAST_NAME_COLUMN);
         String op = "Starts With";
         String value = "J";
         String[] viewNames = {TRICKY_CHARACTERS + "view", "AAC", "aaa", "aad", "zzz"};
@@ -309,7 +309,7 @@ public void saveFilterTest()
         for(String name : viewNames)
         {
             _customizeViewsHelper.openCustomizeViewPanel();
-            _customizeViewsHelper.addFilter(new String[]{fieldKey}, fieldKey, op, value);
+            _customizeViewsHelper.addFilter(fieldKey, op, value);
             _customizeViewsHelper.saveCustomView(name);
         }
 
@@ -350,7 +350,7 @@ private void setColumns(String... columnNames)
     private void addFilter(String columnName, String op, String value)
     {
         _customizeViewsHelper.openCustomizeViewPanel();
-        _customizeViewsHelper.addFilter(FieldKey.fromParts(columnName).toString(), columnName, op, value);
+        _customizeViewsHelper.addFilter(FieldKey.fromParts(columnName).toString(), op, value);
         _customizeViewsHelper.applyCustomView();
     }
 

src/org/labkey/test/tests/CustomizeViewTest.java

@@ -191,7 +191,7 @@ public void doQueryReportTests()
         clickAndWait(Locator.linkWithText("AE-1:(VTN) AE Log"));
         DataRegionTable dataSetTable = new DataRegionTable("Dataset", getDriver());
         dataSetTable.openCustomizeGrid();
-        _customizeViewsHelper.addFilter("MouseId", "Mouse Id", "Equals One Of", String.join(";", PTIDS_FOR_CUSTOM_VIEW));
+        _customizeViewsHelper.addFilter("MouseId", "Equals One Of", String.join(";", PTIDS_FOR_CUSTOM_VIEW));
         _customizeViewsHelper.saveCustomView(QUERY_REPORT_VIEW_NAME_2);
 
         goToManageViews().clickAddReport("Query Report");
@@ -337,8 +337,8 @@ public void doRReportsTest()
         DataRegionTable dataSetTable = new DataRegionTable("Dataset", getDriver());
         dataSetTable.openCustomizeGrid();
         _customizeViewsHelper.removeColumn(R_REMCOL);
-        _customizeViewsHelper.addFilter("DEMhisp", "3.Latino\\a or Hispanic?", "Does Not Equal", "Yes");
-        _customizeViewsHelper.addSort(R_SORT, "2.What is your sex?", SortDirection.DESC);
+        _customizeViewsHelper.addFilter("DEMhisp", "Does Not Equal", "Yes");
+        _customizeViewsHelper.addSort(R_SORT, SortDirection.DESC);
         _customizeViewsHelper.saveCustomView("Custom Query View");
 
         log("Check that customize view worked");

src/org/labkey/test/tests/DataReportsTest.java

@@ -12,6 +12,7 @@
 import org.labkey.test.categories.Assays;
 import org.labkey.test.categories.Daily;
 import org.labkey.test.params.FieldDefinition;
+import org.labkey.test.params.FieldKey;
 import org.labkey.test.util.DataRegionTable;
 import org.labkey.test.util.RelativeUrl;
 
@@ -116,9 +117,9 @@ private void doInit()
         clickAndWait(Locator.linkWithText("view results"));
         var customizeView = _customizeViewsHelper.openCustomizeViewPanel();
         customizeView.showHiddenItems();
-        customizeView.addColumn(new String[] { "Run" });
-        customizeView.addColumn(new String[] { "Run", RUN_FLAG});
-        customizeView.addColumn(new String[] { "Run", "AnotherRunFlag" });
+        customizeView.addColumn(FieldKey.fromParts("Run"));
+        customizeView.addColumn(FieldKey.fromParts("Run", RUN_FLAG));
+        customizeView.addColumn(FieldKey.fromParts("Run", "AnotherRunFlag"));
         customizeView.addSort("RowId", SortDirection.ASC);
         customizeView.saveCustomView();
 

src/org/labkey/test/tests/FlagColumnTest.java

@@ -30,6 +30,7 @@
 import org.labkey.test.components.CustomizeView;
 import org.labkey.test.pages.ReactAssayDesignerPage;
 import org.labkey.test.params.FieldDefinition;
+import org.labkey.test.params.FieldKey;
 import org.labkey.test.util.DataRegionExportHelper;
 import org.labkey.test.util.DataRegionTable;
 import org.labkey.test.util.ExcelHelper;
@@ -177,8 +178,8 @@ public final void testAssayInlineImages() throws Exception
         DataRegionTable list = new DataRegionTable("Data", getDriver());
         CustomizeView customizeView = list.openCustomizeGrid();
         customizeView.showHiddenItems();
-        customizeView.addColumn(new String[]{"Run", "RowId"});
-        customizeView.addColumn(new String[]{"Run", "Protocol", "RowId"});
+        customizeView.addColumn(FieldKey.fromParts("Run", "RowId"));
+        customizeView.addColumn(FieldKey.fromParts("Run", "Protocol", "RowId"));
         customizeView.applyCustomView();
         var protocolId = list.getDataAsText(0, "Run/Protocol/RowId");
         var runId = list.getDataAsText(0, "Run/RowId");

src/org/labkey/test/tests/InlineImagesAssayTest.java

@@ -17,6 +17,7 @@
 import org.labkey.test.categories.Daily;
 import org.labkey.test.components.ext4.Window;
 import org.labkey.test.params.FieldDefinition;
+import org.labkey.test.params.FieldKey;
 import org.labkey.test.params.experiment.DataClassDefinition;
 import org.labkey.test.params.experiment.SampleTypeDefinition;
 import org.labkey.test.util.DataRegionTable;
@@ -383,7 +384,7 @@ public void testUpdateLineageUsingFileImport()
 
         log("Check that the imported data is as expected.");
         DataRegionTable dataRegionTable = new DataRegionTable("Material", this);
-        int row = dataRegionTable.getRowIndex(testSample, "Name");
+        int row = dataRegionTable.getRowIndex("Name", testSample);
         String data = dataRegionTable.getDataAsText(row, columnName);
         checker().verifyEquals("Something doesn't look right. Value for column not as expected.",
                 testData, data);
@@ -406,7 +407,7 @@ public void testUpdateLineageUsingFileImport()
 
         log("Check that the updated data is shown.");
         dataRegionTable = new DataRegionTable("Material", this);
-        row = dataRegionTable.getRowIndex(testSample, "Name");
+        row = dataRegionTable.getRowIndex("Name", testSample);
         data = dataRegionTable.getDataAsText(row, columnName);
         checker().verifyEquals("Value for column not updated as expected.",
                 updatedTestData, data);
@@ -459,7 +460,7 @@ public void testLineageWithThreeGenerations()
         DataRegionTable table = sampleHelper.getSamplesDataRegionTable();
         table.openCustomizeGrid();
         _customizeViewsHelper.showHiddenItems();
-        _customizeViewsHelper.addColumn(new String[]{"Inputs", "Materials", parentSampleType});
+        _customizeViewsHelper.addColumn(FieldKey.fromParts("Inputs", "Materials", parentSampleType));
         _customizeViewsHelper.applyCustomView();
         waitAndClickAndWait(Locator.linkWithText("SampleSetBVT4"));
 
@@ -652,7 +653,7 @@ public void testDeriveSampleByUI() throws CommandException, IOException
 
         for(String parent : parents)
         {
-            int index = drt.getRowIndex(parent, "Name");
+            int index = drt.getRowIndex("Name", parent);
             drt.checkCheckbox(index);
         }
 
@@ -1129,7 +1130,7 @@ public void testDeleteSamplesSomeWithDerivedSamples()
         sampleHelper.createSampleType(new SampleTypeDefinition(SAMPLE_TYPE_NAME), sampleData);
         DataRegionTable drtSamples = sampleHelper.getSamplesDataRegionTable();
         log("Derive one sample from another");
-        drtSamples.checkCheckbox(drtSamples.getRowIndex(parentSampleNames.get(0), "Name"));
+        drtSamples.checkCheckbox(drtSamples.getRowIndex("Name", parentSampleNames.get(0)));
         clickButton("Derive Samples");
         waitAndClickAndWait(Locator.lkButton("Next"));
         String childName = parentSampleNames.get(0) + ".1";
@@ -1145,8 +1146,8 @@ public void testDeleteSamplesSomeWithDerivedSamples()
 
         log("Derive a sample with two parents");
         clickAndWait(Locator.linkContainingText(SAMPLE_TYPE_NAME));
-        drtSamples.checkCheckbox(drtSamples.getRowIndex(parentSampleNames.get(1), "Name"));
-        drtSamples.checkCheckbox(drtSamples.getRowIndex(childName, "Name"));
+        drtSamples.checkCheckbox(drtSamples.getRowIndex("Name", parentSampleNames.get(1)));
+        drtSamples.checkCheckbox(drtSamples.getRowIndex("Name", childName));
         clickButton("Derive Samples");
         waitAndClickAndWait(Locator.lkButton("Next"));
         String twoParentChildName = parentSampleNames.get(1) + "+" + childName + ".1";
@@ -1156,14 +1157,14 @@ public void testDeleteSamplesSomeWithDerivedSamples()
         clickAndWait(Locator.linkContainingText(SAMPLE_TYPE_NAME));
 
         log("Try to delete parent sample");
-        drtSamples.checkCheckbox(drtSamples.getRowIndex(parentSampleNames.get(0), "Name"));
+        drtSamples.checkCheckbox(drtSamples.getRowIndex("Name", parentSampleNames.get(0)));
         drtSamples.clickHeaderButton("Delete");
         Window.Window(getDriver()).withTitle("No samples can be deleted").waitFor()
                 .clickButton("Dismiss", true);
 
         log("Try to delete multiple parent samples");
-        drtSamples.checkCheckbox(drtSamples.getRowIndex(parentSampleNames.get(1), "Name"));
-        drtSamples.checkCheckbox(drtSamples.getRowIndex(childName, "Name"));
+        drtSamples.checkCheckbox(drtSamples.getRowIndex("Name", parentSampleNames.get(1)));
+        drtSamples.checkCheckbox(drtSamples.getRowIndex("Name", childName));
         drtSamples.clickHeaderButton("Delete");
         Window.Window(getDriver()).withTitle("No samples can be deleted").waitFor()
                 .clickButton("Dismiss", true);
@@ -1172,21 +1173,21 @@ public void testDeleteSamplesSomeWithDerivedSamples()
         assertEquals("No selection should remain", 0, drtSamples.getSelectedCount());
 
         log("Try to delete parent and child");
-        drtSamples.checkCheckbox(drtSamples.getRowIndex(parentSampleNames.get(1), "Name"));
-        drtSamples.checkCheckbox(drtSamples.getRowIndex(twoParentChildName, "Name"));
+        drtSamples.checkCheckbox(drtSamples.getRowIndex("Name", parentSampleNames.get(1)));
+        drtSamples.checkCheckbox(drtSamples.getRowIndex("Name", twoParentChildName));
         assertEquals("Parent and child should be checked", 2, drtSamples.getCheckedCount());
         assertEquals("Parent and child should be checked", 2, drtSamples.getSelectedCount());
 
         sampleHelper.deleteSamples(drtSamples, "Permanently delete 1 sample");
-        assertEquals("Deleted sample " + twoParentChildName + " still appears in grid", -1, drtSamples.getRowIndex(twoParentChildName, "Name"));
-        assertTrue("Parent sample " + parentSampleNames.get(1) + " does not appears in grid", drtSamples.getRowIndex(parentSampleNames.get(1), "Name") > -1);
+        assertEquals("Deleted sample " + twoParentChildName + " still appears in grid", -1, drtSamples.getRowIndex("Name", twoParentChildName));
+        assertTrue("Parent sample " + parentSampleNames.get(1) + " does not appears in grid", drtSamples.getRowIndex("Name", parentSampleNames.get(1)) > -1);
         assertEquals("Only parent sample should be checked", 1, drtSamples.getCheckedCount());
         assertEquals("Only parent sample should be checked", 1, drtSamples.getSelectedCount());
 
         log("Now that the child is gone, try to delete the parent");
         sampleHelper.deleteSamples(drtSamples, "Permanently delete 1 sample");
 
-        assertEquals("Deleted sample " + parentSampleNames.get(1) + " still appears in grid", -1, drtSamples.getRowIndex(parentSampleNames.get(1), "Name"));
+        assertEquals("Deleted sample " + parentSampleNames.get(1) + " still appears in grid", -1, drtSamples.getRowIndex("Name", parentSampleNames.get(1)));
         assertEquals("No selection should remain", 0, drtSamples.getCheckedCount());
 
         log("Now try to delete what's left, in several hitches");