Skip to content

Issue 53306: Some LKS forms don't distinguish between fields properly #900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public List<FieldKey> getDefaultVisibleColumns()
FieldKey runPropFieldKey = FieldKey.fromParts("Run");
fieldKeys.add(FieldKey.fromParts(runPropFieldKey, FieldKey.fromString("ProtocolName")));
fieldKeys.add(FieldKey.fromParts(runPropFieldKey, FieldKey.fromString("PlateReader")));
fieldKeys.add(FieldKey.fromParts(runPropFieldKey, FieldKey.fromString("Batch"), FieldKey.fromString("TargetStudy")));
fieldKeys.add(FieldKey.fromParts(runPropFieldKey, FieldKey.fromString("Batch"), FieldKey.fromString(AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME)));
return fieldKeys;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
<%@ page import="java.util.LinkedHashMap" %>
<%@ page import="java.util.Map" %>
<%@ page import="java.util.Set" %>
<%@ page import="org.labkey.api.assay.AbstractAssayProvider" %>
<%@ page extends="org.labkey.api.jsp.JspBase" %>
<%
JspView<ImportRunsForm> me = HttpView.currentView();
Expand Down Expand Up @@ -86,8 +87,8 @@
to look up specimen information from the target study's specimen repository.
</p>
<p class="labkey-indented">
<label for="targetStudy">Optionally, choose a target study folder:</label><br>
<select id="targetStudy" name="targetStudy">
<label for="<%= h(AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME)%>">Optionally, choose a target study folder:</label><br>
<select id="<%= h(AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME)%>" name="<%= h(AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME)%>">
<labkey:options value="<%=unsafe(form.getTargetStudy())%>" map="<%=targetStudies%>"/>
</select>
</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
<%@ page import="java.util.List" %>
<%@ page import="java.util.Map" %>
<%@ page import="java.util.Set" %>
<%@ page import="org.labkey.api.assay.AbstractAssayProvider" %>
<%@ page extends="org.labkey.api.jsp.JspBase" %>
<%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %>
<%
Expand Down Expand Up @@ -233,11 +234,8 @@ if (form.getKeywordDir() != null && form.getKeywordDir().length > 0 && StudyPubl
<div style="padding-left: 2em; padding-bottom: 1em;">
<br>
Choose a target study folder:<br>
<%=select().name("targetStudy").className(null).addOptions(targetStudies).selected(unsafe(form.getTargetStudy())).onChange("document.getElementById('studyChanged').value = true;")
<%=select().name(AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME).className(null).addOptions(targetStudies).selected(unsafe(form.getTargetStudy())).onChange("document.getElementById('studyChanged').value = true;")
%>
<%-- <select id="targetStudy" name="targetStudy">--%>
<%-- <labkey:options value="<%=text(form.getTargetStudy())%>" map="<%=targetStudies%>"/>--%>
<%-- </select>--%>
<br><br>
</div>
<%
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
<%@ page import="java.util.Map" %>
<%@ page import="java.util.Set" %>
<%@ page import="static org.labkey.flow.controllers.executescript.AnalysisScriptController.BACK_BUTTON_ACTION" %>
<%@ page import="org.labkey.api.assay.AbstractAssayProvider" %>
<%@ page extends="org.labkey.api.jsp.JspBase" %>
<%
ImportAnalysisForm form = (ImportAnalysisForm)getModelBean();
Expand Down Expand Up @@ -93,7 +94,7 @@
<input type="hidden" name="existingAnalysisId" id="existingAnalysisId" value="<%=form.getExistingAnalysisId()%>">
<% } %>

<input type="hidden" name="targetStudy" id="targetStudy" value="<%=h(form.getTargetStudy())%>">
<input type="hidden" name="<%= h(AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME)%>" id="<%= h(AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME)%>" value="<%=h(form.getTargetStudy())%>">

<p>You are about to import the analysis from the workspace with the following settings:</p>
<%
Expand Down
3 changes: 2 additions & 1 deletion flow/src/org/labkey/flow/data/FlowProperty.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.labkey.flow.data;

import org.labkey.api.assay.AbstractAssayProvider;
import org.labkey.api.exp.PropertyType;
import org.labkey.api.exp.property.SystemProperty;

Expand All @@ -35,7 +36,7 @@ abstract public class FlowProperty
static public final SystemProperty AnalysisEngine = new SystemProperty(PROPERTY_BASE + "AnalysisEngine", PropertyType.STRING);

// Property on FlowRun ExpRun object: container id of target study
static public final SystemProperty TargetStudy = new SystemProperty(PROPERTY_BASE + "TargetStudy", PropertyType.STRING);
static public final SystemProperty TargetStudy = new SystemProperty(PROPERTY_BASE + AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME, PropertyType.STRING);

// Property on FlowFCSFile ExpData object: true when the FlowFCSFile was created from an external analysis (extracted from FlowJo workspace or analysis archive)
//static public final SystemProperty ExtraKeywordsFCSFile = new SystemProperty(PROPERTY_BASE + "ExtraKeywordsFCSFile", PropertyType.BOOLEAN);
Expand Down
3 changes: 2 additions & 1 deletion flow/src/org/labkey/flow/query/FCSFileCoalescingColumn.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.labkey.flow.query;

import org.jetbrains.annotations.Nullable;
import org.labkey.api.assay.AbstractAssayProvider;
import org.labkey.api.data.ColumnInfo;
import org.labkey.api.data.JdbcType;
import org.labkey.api.data.SQLFragment;
Expand Down Expand Up @@ -74,7 +75,7 @@ public FCSFileCoalescingColumn(TableInfo parent, FieldKey key, JdbcType type, @N
_dateFieldKeys = Pair.of(dateFieldKey, FlowSchema.rewriteAsOriginalFCSFile(dateFieldKey));
}

FieldKey targetStudyFieldKey = relativeFromFCSFile ? FieldKey.fromParts("Run", "TargetStudy") : FieldKey.fromParts("FCSFile", "Run", "TargetStudy");
FieldKey targetStudyFieldKey = relativeFromFCSFile ? FieldKey.fromParts("Run", AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME) : FieldKey.fromParts("FCSFile", "Run", "TargetStudy");
_targetStudyFieldKeys = Pair.of(targetStudyFieldKey, FlowSchema.rewriteAsOriginalFCSFile(targetStudyFieldKey));
}

Expand Down
2 changes: 1 addition & 1 deletion flow/src/org/labkey/flow/reports/FilterFlowReport.java
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ void addScriptProlog(ViewContext context, StringBuffer sb)
private String oldLegalName(FieldKey fkey)
{
String r = AliasManager.makeLegalName(StringUtils.join(fkey.getParts(), "_"), FlowManager.get().getSchema().getSqlDialect(), false);
return ColumnInfo.propNameFromName(r).toLowerCase();
return ColumnInfo.legalNameFromName(r).toLowerCase();
}

protected void convertDateColumn(CachedResultSet rs, String fromCol, String toCol) throws SQLException
Expand Down
2 changes: 1 addition & 1 deletion luminex/src/org/labkey/luminex/LuminexDataHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public class LuminexDataHandler extends AbstractExperimentDataHandler implements
public static final String QC_FLAG_SINGLE_POINT_CONTROL_ID = "SinglePointControlId"; // Column name to use in createQCFlagEnabledSQLFragment
public static final String POSITIVITY_THRESHOLD_COLUMN_NAME = "PositivityThreshold";
public static final String POSITIVITY_THRESHOLD_DISPLAY_NAME = "Positivity Threshold";
public static final String CALCULATE_POSITIVITY_COLUMN_NAME = "calculatePositivity";
public static final String CALCULATE_POSITIVITY_COLUMN_NAME = "CalculatePositivity";
public static final String NEGATIVE_CONTROL_COLUMN_NAME = "NegativeControl";
public static final String NEGATIVE_BEAD_COLUMN_NAME = "NegativeBead";
public static final String NEGATIVE_BEAD_DISPLAY_NAME = "Subtract Negative Bead";
Expand Down
4 changes: 2 additions & 2 deletions luminex/src/org/labkey/luminex/LuminexRunUploadForm.java
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ public List<Titration> getTitrations() throws ExperimentException
titration.setName(titrationName);
for (Titration.Type type : Titration.Type.values())
{
String propertyName = LuminexUploadWizardAction.getTitrationTypeCheckboxName(type, titration);
String propertyName = LuminexUploadWizardAction.getTitrationTypeCheckboxNameAndId(type, titration);
if (getViewContext().getRequest().getParameter(propertyName) != null)
{
String hiddenValue = getViewContext().getRequest().getParameter(propertyName);
Expand All @@ -293,7 +293,7 @@ public List<SinglePointControl> getSinglePointControls() throws ExperimentExcept
{
SinglePointControl singlePointControl = new SinglePointControl();
singlePointControl.setName(singlePointControlName);
String propertyName = LuminexUploadWizardAction.getSinglePointControlCheckboxName(singlePointControlName);
String propertyName = LuminexUploadWizardAction.getSinglePointControlCheckboxNameAndId(singlePointControlName);
if (StringUtils.isNotBlank(getViewContext().getRequest().getParameter(propertyName)))
{
result.add(singlePointControl);
Expand Down
48 changes: 27 additions & 21 deletions luminex/src/org/labkey/luminex/LuminexUploadWizardAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.labkey.api.view.JspView;
import org.labkey.api.view.VBox;
import org.labkey.api.view.ViewServlet;
import org.labkey.api.view.template.PageConfig;
import org.labkey.api.writer.HtmlWriter;
import org.labkey.luminex.model.Analyte;
import org.labkey.luminex.model.SinglePointControl;
Expand Down Expand Up @@ -324,7 +325,7 @@ else if (analyteDefaultValue != null)
}

Titration existingTitration = existingTitrations.get(titrationEntry.getKey());
String propertyName = getTitrationTypeCheckboxName(Titration.Type.standard, titrationEntry.getValue());
String propertyName = getTitrationTypeCheckboxNameAndId(Titration.Type.standard, titrationEntry.getValue());
// If we have an existing titration as a baseline from the run we're replacing, use its value
String defVal = existingTitration == null ? defaultWellRoleValues.get(propertyName) : Boolean.toString(existingTitration.isStandard());

Expand Down Expand Up @@ -359,7 +360,7 @@ else if (titrationEntry.getValue().isStandard())

if (!titrationEntry.getValue().isUnknown())
{
propertyName = getTitrationTypeCheckboxName(Titration.Type.standard, titrationEntry.getValue());
propertyName = getTitrationTypeCheckboxNameAndId(Titration.Type.standard, titrationEntry.getValue());
// If we have an existing titration as a baseline from the run we're replacing, use its value
defVal = existingTitration == null ? defaultWellRoleValues.get(propertyName) : Boolean.toString(existingTitration.isStandard());
value = setInitialTitrationInput(errorReshow, propertyName, defVal, titrationEntry.getValue().isStandard()) ? "true" : "";
Expand All @@ -369,21 +370,21 @@ else if (titrationEntry.getValue().isStandard())
value = toShowStandardCheckboxColumn(errorReshow, standardTitrations, titrationEntry.getValue()) ? "true" : "";
view.getDataRegion().addHiddenFormField(getShowStandardCheckboxColumnName(titrationEntry.getValue()), value);

propertyName = getTitrationTypeCheckboxName(Titration.Type.qccontrol, titrationEntry.getValue());
propertyName = getTitrationTypeCheckboxNameAndId(Titration.Type.qccontrol, titrationEntry.getValue());
// If we have an existing titration as a baseline from the run we're replacing, use its value
defVal = existingTitration == null ? defaultWellRoleValues.get(propertyName) : Boolean.toString(existingTitration.isQcControl());
value = setInitialTitrationInput(errorReshow, propertyName, defVal, titrationEntry.getValue().isQcControl()) ? "true" : "";
view.getDataRegion().addHiddenFormField(propertyName, value);

propertyName = getTitrationTypeCheckboxName(Titration.Type.othercontrol, titrationEntry.getValue());
propertyName = getTitrationTypeCheckboxNameAndId(Titration.Type.othercontrol, titrationEntry.getValue());
// If we have an existing titration as a baseline from the run we're replacing, use its value
defVal = existingTitration == null ? defaultWellRoleValues.get(propertyName) : Boolean.toString(existingTitration.isOtherControl());
value = setInitialTitrationInput(errorReshow, propertyName, defVal, titrationEntry.getValue().isOtherControl()) ? "true" : "";
view.getDataRegion().addHiddenFormField(propertyName, value);
}
else
{
propertyName = getTitrationTypeCheckboxName(Titration.Type.unknown, titrationEntry.getValue());
propertyName = getTitrationTypeCheckboxNameAndId(Titration.Type.unknown, titrationEntry.getValue());
// If we have an existing titration as a baseline from the run we're replacing, use its value
defVal = existingTitration == null ? defaultWellRoleValues.get(propertyName) : Boolean.toString(existingTitration.isUnknown());
value = setInitialTitrationInput(errorReshow, propertyName, defVal, titrationEntry.getValue().isUnknown()) ? "true" : "";
Expand All @@ -400,7 +401,7 @@ else if (titrationEntry.getValue().isStandard())

boolean existingSinglePointControl = existingSinglePointControls.contains(singlePointControl);

propertyName = getSinglePointControlCheckboxName(singlePointControl);
propertyName = getSinglePointControlCheckboxNameAndId(singlePointControl);
// If we have an existing singlePointControl as a baseline from the run we're replacing, use its value
defVal = existingSinglePointControl ? "true" : defaultWellRoleValues.get(propertyName);
value = setInitialSinglePointControlInput(errorReshow, propertyName, defVal) ? "true" : "";
Expand Down Expand Up @@ -431,8 +432,9 @@ else if (titrationEntry.getValue().isStandard())
@Override
public void writeSameCheckboxCell(RenderContext ctx, HtmlWriter out)
{
String titrationCellName = PageFlowUtil.filter(getTitrationColumnCellName(titrationEntry.getValue().getName()));
String groupName = ColumnInfo.propNameFromName(getColumns().get(0).getFormFieldName(ctx));
String titrationCellName = PageFlowUtil.filter(getTitrationColumnCellNameAndId(titrationEntry.getValue().getName()));
// DOM ids and JS function names can't have spaces
String groupName = PageConfig.makeIdFromName(getColumns().get(0).getFormFieldName(ctx));
String id = groupName + "CheckBox";

TD(
Expand All @@ -453,7 +455,8 @@ public void writeSameCheckboxCell(RenderContext ctx, HtmlWriter out)
@Override
public void writeCopyableJavaScript(RenderContext ctx, Writer out) throws IOException
{
String groupName = ColumnInfo.propNameFromName(getColumns().get(0).getFormFieldName(ctx));
// DOM ids and JS function names can't have spaces
String groupName = PageConfig.makeIdFromName(getColumns().get(0).getFormFieldName(ctx));
out.write("function " + groupName + "Updated() {\n");
out.write(" if (document.getElementById('" + groupName + "CheckBox') != null && document.getElementById('" + groupName + "CheckBox').checked) {\n");
out.write(" var v = document.getElementsByName('" + getColumns().get(0).getFormFieldName(ctx) + "')[0].checked;\n");
Expand Down Expand Up @@ -550,7 +553,7 @@ private JspView<LuminexRunUploadForm> addExclusionWarning(LuminexRunUploadForm f

private String getShowStandardCheckboxColumnName(Titration standard)
{
String titrationCheckboxName = getTitrationTypeCheckboxName(Titration.Type.standard, standard);
String titrationCheckboxName = getTitrationTypeCheckboxNameAndId(Titration.Type.standard, standard);
return titrationCheckboxName + "_showcol";
}

Expand Down Expand Up @@ -691,19 +694,22 @@ protected RunStepHandler getRunStepHandler()
return new LuminexRunStepHandler();
}

public static String getTitrationTypeCheckboxName(Titration.Type type, Titration titration)
public static String getTitrationTypeCheckboxNameAndId(Titration.Type type, Titration titration)
{
return ColumnInfo.propNameFromName("_titrationRole_" + type + "_" + titration.getName());
// DOM ids and JS function names can't have spaces
return PageConfig.makeIdFromName("_titrationRole_" + type + "_" + titration.getName());
}

public static String getSinglePointControlCheckboxName(String singlePointControl)
public static String getSinglePointControlCheckboxNameAndId(String singlePointControl)
{
return ColumnInfo.propNameFromName("_singlePointControl_" + singlePointControl);
// DOM ids and JS function names can't have spaces
return PageConfig.makeIdFromName("_singlePointControl_" + singlePointControl);
}

public static String getTitrationColumnCellName(String titrationName)
public static String getTitrationColumnCellNameAndId(String titrationName)
{
return ColumnInfo.propNameFromName("_titrationcell_" + titrationName);
// DOM ids and JS function names can't have spaces
return PageConfig.makeIdFromName("_titrationcell_" + titrationName);
}

protected class LuminexRunStepHandler extends RunStepHandler
Expand Down Expand Up @@ -806,21 +812,21 @@ public boolean executeStep(LuminexRunUploadForm form, BindException errors) thro
// add the name/value pairs for the titration well role definition section
if (!titrationEntry.getValue().isUnknown())
{
propertyName = getTitrationTypeCheckboxName(Titration.Type.standard, titrationEntry.getValue());
propertyName = getTitrationTypeCheckboxNameAndId(Titration.Type.standard, titrationEntry.getValue());
value = getViewContext().getRequest().getParameter(propertyName).equals("true");
defaultWellRoleValues.put(propertyName, Boolean.toString(value));

propertyName = getTitrationTypeCheckboxName(Titration.Type.qccontrol, titrationEntry.getValue());
propertyName = getTitrationTypeCheckboxNameAndId(Titration.Type.qccontrol, titrationEntry.getValue());
value = getViewContext().getRequest().getParameter(propertyName).equals("true");
defaultWellRoleValues.put(propertyName, Boolean.toString(value));

propertyName = getTitrationTypeCheckboxName(Titration.Type.othercontrol, titrationEntry.getValue());
propertyName = getTitrationTypeCheckboxNameAndId(Titration.Type.othercontrol, titrationEntry.getValue());
value = getViewContext().getRequest().getParameter(propertyName).equals("true");
defaultWellRoleValues.put(propertyName, Boolean.toString(value));
}
else
{
propertyName = getTitrationTypeCheckboxName(Titration.Type.unknown, titrationEntry.getValue());
propertyName = getTitrationTypeCheckboxNameAndId(Titration.Type.unknown, titrationEntry.getValue());
value = getViewContext().getRequest().getParameter(propertyName).equals("true");
defaultWellRoleValues.put(propertyName, Boolean.toString(value));
}
Expand All @@ -845,7 +851,7 @@ public boolean executeStep(LuminexRunUploadForm form, BindException errors) thro
for (String singlePointControl : form.getParser().getSinglePointControls())
{
// add the name/value pairs for the singlePointControl well role definition section
String propertyName = getSinglePointControlCheckboxName(singlePointControl);
String propertyName = getSinglePointControlCheckboxNameAndId(singlePointControl);
boolean value = getViewContext().getRequest().getParameter(propertyName).equals("true");
defaultWellRoleValues.put(propertyName, Boolean.toString(value));
}
Expand Down
Loading