Merge branch 'develop' into fb_pipelineFileLike

Author: labkey-jeckels

src/org/labkey/test/pages/core/admin/CustomizeSitePage.java

@@ -47,7 +47,7 @@ public static CustomizeSitePage beginAt(WebDriverWrapper driver)
 
     public ShowAdminPage save()
     {
-        clickAndWait(elementCache().saveButton);
+        clickAndWait(scrollIntoView(elementCache().saveButton));
 
         return new ShowAdminPage(getDriver());
     }
@@ -217,7 +217,7 @@ protected ElementCache newElementCache()
         return new ElementCache();
     }
 
-    protected class ElementCache extends LabKeyPage.ElementCache
+    protected class ElementCache extends LabKeyPage<?>.ElementCache
     {
         protected final WebElement saveButton = Locator.lkButton("Save").findWhenNeeded(this);
 

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

@@ -182,12 +182,15 @@ public final void testAssayInlineImages() throws Exception
         customizeView.addColumn(FieldKey.fromParts("Run", "RowId"));
         customizeView.addColumn(FieldKey.fromParts("Run", "Protocol", "RowId"));
         customizeView.applyCustomView();
-        String helpJpgFilePath = HELP_JPG_FILE.getName();
+        var protocolId = list.getDataAsText(0, "Run/Protocol/RowId");
+        var runId = list.getDataAsText(0, "Run/RowId");
+        String helpJpgFilePath = String.format("AssayId_%s%sRunId_%s%s%s", protocolId, File.separatorChar,
+                runId, File.separatorChar, HELP_JPG_FILE.getName());
 
         log("Validate that two links to this image file are now present.");
         assertElementPresent("Did not find the expected number of icons for images for " + PNG01_FILE.getName() + " from the runs.", Locator.xpath("//img[contains(@title, '" + PNG01_FILE.getName() + "')]"), 3);
         assertElementPresent("Did not find the expected number of icons for images for " + LRG_PNG_FILE.getName() + " from the runs.", Locator.xpath("//img[contains(@title, '" + LRG_PNG_FILE.getName() + "')]"), 1);
-        assertElementPresent("Did not find the expected number of icons for images for " + helpJpgFilePath + " from the runs.", Locator.xpath("//img[contains(@title, '" + helpJpgFilePath + "')]"), 1);
+        assertElementPresent("Did not find the expected number of icons for images for " + HELP_JPG_FILE.getName() + " from the runs.", Locator.xpath("//img[contains(@title, '" + HELP_JPG_FILE.getName() + "')]"), 1);
 
         log("Export the grid to excel.");
         File exportedFile;
@@ -203,7 +206,7 @@ public final void testAssayInlineImages() throws Exception
 
             log("Validate that the 'File' (last) column is as expected.");
             assertEquals("Values in 'File' column not exported as expected [" + exportedFile.getName() + "]",
-                    Arrays.asList("Batch File Field", XLS_FILE.getName(), XLS_FILE.getName(), XLS_FILE.getName()),
+                    Arrays.asList("Batch File Field", "assaydata" + File.separator + XLS_FILE.getName(), "assaydata" + File.separator + XLS_FILE.getName(), "assaydata" + File.separator + XLS_FILE.getName()),
                     ExcelHelper.getColumnData(workbook.getSheetAt(workbook.getActiveSheetIndex()), 7));
         }
 
@@ -221,7 +224,7 @@ public final void testAssayInlineImages() throws Exception
         log("Verify that the other 'File' fields are not affected.");
         assertElementPresent("Did not find the expected number of icons for images for " + PNG01_FILE.getName() + " from the runs.", Locator.xpath("//img[contains(@title, '" + PNG01_FILE.getName() + "')]"), 3);
         assertElementPresent("Did not find the expected number of icons for images for " + LRG_PNG_FILE.getName() + " from the runs.", Locator.xpath("//img[contains(@title, '" + LRG_PNG_FILE.getName() + "')]"), 1);
-        assertElementPresent("Did not find the expected number of icons for images for " + helpJpgFilePath + " from the runs.", Locator.xpath("//img[contains(@title, '" + helpJpgFilePath + "')]"), 1);
+        assertElementPresent("Did not find the expected number of icons for images for " + HELP_JPG_FILE.getName() + " from the runs.", Locator.xpath("//img[contains(@title, '" + HELP_JPG_FILE.getName() + "')]"), 1);
 
 
         log("Export the grid to excel again and make sure that everything is as expected.");
@@ -283,7 +286,7 @@ private void validateExcelExport(File exportedFile, Workbook workbook, String he
 
         exportedColumn = ExcelHelper.getColumnData(sheet, 5);
         assertEquals("Values in 'File' column not exported as expected [" + exportedFile.getName() + "]",
-                Arrays.asList("Run File Field", PNG01_FILE.getName(), PNG01_FILE.getName(), PNG01_FILE.getName()),
+                Arrays.asList("Run File Field", "assaydata" + File.separator + PNG01_FILE.getName(), "assaydata" + File.separator + PNG01_FILE.getName(), "assaydata" + File.separator + PNG01_FILE.getName()),
                 exportedColumn);
 
     }

src/org/labkey/test/tests/flow/FlowSpecimenTest.java

@@ -32,6 +32,7 @@
 import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -227,7 +228,8 @@ private void linkFlowResultsToStudy()
         beginAt(WebTestHelper.buildURL("flow", getContainerPath(), "query", Map.of("schemaName", "flow", "query.queryName", "FCSAnalyses")));
         click(Locator.checkboxByName(".toggle"));
         clickButton("Link to Study");
-        selectOptionByText(AssayConstants.TARGET_STUDY_FIELD_LOCATOR, "/" + getProjectName() + "/" + STUDY_FOLDER + " (" + STUDY_FOLDER + " Study)");
+        // Target study is fixed
+        assertFalse("Target study selector visibility", AssayConstants.TARGET_STUDY_FIELD_LOCATOR.findElement(getDriver()).isDisplayed());
         clickButton("Next");
         assertTitleContains("Link to " + STUDY_FOLDER + " Study: Verify Results");
         // verify specimen information is filled in for '118795.fcs' FCS file

src/org/labkey/test/tests/issues/IssuesTest.java

@@ -86,12 +86,14 @@ public class IssuesTest extends BaseWebDriverTest
     private static final String USER2 = "[email protected]";
     private static final String USER3 = "[email protected]";
     private static final String USER4 = "[email protected]";
+    private static final String CLIENT_USER1 = "[email protected]";
     private static final String user = "[email protected]";
     private static final Map<String, String> ISSUE_0 = new HashMap<>(Maps.of("title", ISSUE_TITLE_0, "Priority", "2", "comment", "a bright flash of light"));
     private static final Map<String, String> ISSUE_1 = new HashMap<>(Maps.of("title", ISSUE_TITLE_1, "Priority", "1", "comment", "alien autopsy"));
     private static final String ISSUE_SUMMARY_WEBPART_NAME = "Issues Summary";
     private static final String ISSUE_LIST_REGION_NAME = "issues-issues";
     private static final String TEST_GROUP = "testers";
+    private static final String CLIENT_GROUP = "clients";
     private static final String TEST_EMAIL_TEMPLATE =
             "You can review this issue here: ^detailsURL^\n" +
                     "Modified by: ^user^\n" +
@@ -105,6 +107,7 @@ public class IssuesTest extends BaseWebDriverTest
     private static String NAME;
     protected IssuesHelper _issuesHelper;
     private final ApiPermissionsHelper _permissionsHelper = new ApiPermissionsHelper(this);
+    private static String CLIENT_PORTAL = "Client Issues";
 
     public IssuesTest()
     {
@@ -158,8 +161,9 @@ protected String getProjectName()
     @Override
     protected void doCleanup(boolean afterTest) throws TestTimeoutException
     {
-        _userHelper.deleteUsers(false, USER1, USER2, USER3, USER4);
+        _userHelper.deleteUsers(false, USER1, USER2, USER3, CLIENT_USER1);
         _containerHelper.deleteProject(getProjectName(), afterTest);
+        _containerHelper.deleteProject(CLIENT_PORTAL, afterTest);
     }
 
     public void doInit()
@@ -193,6 +197,15 @@ public void doInit()
         waitAndClickAndWait(Locator.linkContainingText(ISSUE_SUMMARY_WEBPART_NAME));
         _issuesHelper.addIssue(ISSUE_0);
         _issuesHelper.addIssue(ISSUE_1);
+
+        // Create a second project with different permissions
+        _containerHelper.createProject(CLIENT_PORTAL);
+        goToProjectHome(CLIENT_PORTAL);
+        _userHelper.createUser(CLIENT_USER1);
+        _permissionsHelper.createPermissionsGroup(CLIENT_GROUP);
+        _permissionsHelper.setPermissions(CLIENT_GROUP, "Editor");
+        _permissionsHelper.addUserToProjGroup(CLIENT_USER1, CLIENT_PORTAL, CLIENT_GROUP);
+        _issuesHelper.createNewIssuesList("tickets", _containerHelper);
     }
 
     @Before
@@ -834,7 +847,32 @@ public void relatedIssueTest()
                 .clickSubMenu(false, "Hide related comments");
         assertElementNotVisible(related);
 
-        // NOTE: still need to test for case where user doesn't have permission to related issue...
+        // related issue permission tests
+        Locator commentLocator = Locator.name("related");
+        goToProjectHome(CLIENT_PORTAL);
+        waitAndClickAndWait(Locator.linkContainingText(ISSUE_SUMMARY_WEBPART_NAME));
+        String clientIssueId = _issuesHelper.addIssue(Maps.of("assignedTo", NAME, "title", "Client ticket", "priority", "3", "related", issueIdA)).getIssueId();
+
+        // impersonate a user without permissions to the related issues
+        impersonate(CLIENT_USER1);
+        clickAndWait(Locator.linkWithText("Issues List"));
+        clickAndWait(Locator.linkWithText(clientIssueId));
+        updateIssue();
+        setFormElement(Locator.name("comment"), "This should work");
+        clickButton("Save");
+
+        // try to add related issue they don't have permission to see
+        updateIssue();
+        setFormElement(relatedLocator, String.format("%s,%s", issueIdA, issueIdB));
+        clickButton("Save");
+        assertTextPresent("User does not have Read Permission for related issue");
+
+        // Issue 53820 try to remove a related issue they don't have permission to see
+        setFormElement(relatedLocator, "");
+        clickButton("Save");
+        assertTextPresent(String.format("User does not have Read Permission for related issue '%s'", issueIdA));
+        clickButton("Cancel");
+        stopImpersonating();
     }
 
     @Test

src/org/labkey/test/tests/list/ListTest.java

@@ -208,6 +208,13 @@ protected String getProjectName()
         return PROJECT_VERIFY;
     }
 
+    @Override
+    protected void doCleanup(boolean afterTest)
+    {
+        _containerHelper.deleteProject(PROJECT_VERIFY, afterTest);
+        _containerHelper.deleteProject(PROJECT_OTHER, afterTest);
+    }
+
     @BeforeClass
     public static void setupProject()
     {
@@ -1629,7 +1636,7 @@ public void testAutoIncrementKeyEncoded()
         // setup a list with an auto-increment key that we need to make sure is encoded in the form input
         String encodedListName = "autoIncrementEncodeList";
         String keyName = "'><script>alert(\":(\")</script>'";
-        String encodedKeyFieldName = EscapeUtil.getFormFieldName(keyName).replaceAll("\"", "&quot;");
+        String encodedKeyFieldName = EscapeUtil.getFormFieldName(keyName);
         _listHelper.createList(PROJECT_VERIFY, encodedListName, keyName, col("Name", ColumnType.String));
         _listHelper.goToList(encodedListName);
 
@@ -1639,10 +1646,9 @@ public void testAutoIncrementKeyEncoded()
         customizeView.addColumn(EscapeUtil.fieldKeyEncodePart(keyName));
         customizeView.applyCustomView();
 
-        // insert a new row and verify the key is encoded in the form input
+        // insert a new row and verify the key field is not present
         table.clickInsertNewRow();
-        String html = getHtmlSource();
-        checker().verifyFalse("List key hidden input not present.", html.contains(encodedKeyFieldName));
+        checker().withScreenshot().verifyEquals("List fields on insert form.", List.of("quf_Name"), getQueryFormFieldNames());
         String nameValue = "test";
         setFormElement(Locator.name(EscapeUtil.getFormFieldName("Name")), nameValue);
         clickButton("Submit");
@@ -1654,8 +1660,7 @@ public void testAutoIncrementKeyEncoded()
 
         // verify name value can be updated
         table.clickEditRow(0);
-        html = getHtmlSource();
-        checker().verifyTrue("List key hidden input not present.", html.contains(encodedKeyFieldName));
+        checker().withScreenshot().verifyEquals("List fields on update form.", List.of("quf_Name", encodedKeyFieldName), getQueryFormFieldNames());
         nameValue = "test updated";
         setFormElement(Locator.name(EscapeUtil.getFormFieldName("Name")), nameValue);
         clickButton("Submit");
@@ -1668,6 +1673,14 @@ public void testAutoIncrementKeyEncoded()
         _listHelper.deleteList();
     }
 
+    private List<String> getQueryFormFieldNames()
+    {
+        return Locator.tag("input").attributeStartsWith("name", "quf_")
+            .findElements(getDriver()).stream()
+            .map(el -> el.getDomAttribute("name"))
+            .toList();
+    }
+
     private void viewRawTableMetadata(String listName)
     {
         goToSchemaBrowser();

src/org/labkey/test/tests/upgrade/BaseUpgradeTest.java

@@ -24,9 +24,9 @@
 import static org.apache.commons.lang3.StringUtils.trimToNull;
 
 /**
- * Base test class for tests that setup data and configure a server then verify the persistence or modification of those
- * data and configurations after upgrading to a newer version of LabKey.<br>
- * The {@code EariestVersion} and {@code LatestVersion} annotations can be used to skip particular tests when they are
+ * Base test class for tests that set up data and configure a server, then verify the persistence or modification of
+ * those data and configurations after upgrading to a newer version of LabKey.<br>
+ * The {@link EarliestVersion} and {@link LatestVersion} annotations can be used to skip particular tests when they are
  * not relevant to the version of LabKey being upgraded from (specified in the {@code webtest.upgradePreviousVersion}
  * system property).<br>
  * The setup steps will be skipped if the {@code webtest.upgradeSetup} system property is set to {@code false}.
@@ -77,7 +77,8 @@ public List<String> getAssociatedModules()
      */
     @Retention(RetentionPolicy.RUNTIME)
     @Target({ElementType.METHOD})
-    protected @interface EariestVersion {
+    protected @interface EarliestVersion
+    {
         String value();
     }
 
@@ -98,12 +99,12 @@ private static class UpgradeVersionCheck implements TestRule
         @Override
         public @NotNull Statement apply(Statement base, Description description)
         {
-            String eariestVersion = Optional.ofNullable(description.getAnnotation(EariestVersion.class))
-                .map(EariestVersion::value).orElse(null);
+            String earliestVersion = Optional.ofNullable(description.getAnnotation(EarliestVersion.class))
+                .map(EarliestVersion::value).orElse(null);
             String latestVersion = Optional.ofNullable(description.getAnnotation(LatestVersion.class))
                 .map(LatestVersion::value).orElse(null);
 
-            if (isUpgradeSetupPhase || previousVersion == null || (eariestVersion == null && latestVersion == null))
+            if (isUpgradeSetupPhase || previousVersion == null || (earliestVersion == null && latestVersion == null))
             {
                 return base; // Run the test normally
             }
@@ -114,7 +115,7 @@ private static class UpgradeVersionCheck implements TestRule
                 public void evaluate() throws Throwable
                 {
                     Assume.assumeTrue("Test doesn't support upgrading from version: " + previousVersion,
-                        VersionRange.versionRange(eariestVersion, latestVersion).contains(previousVersion)
+                        VersionRange.versionRange(earliestVersion, latestVersion).contains(previousVersion)
                     );
                     base.evaluate();
                 }