Skip to content

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

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 28 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
08ecfd6
Issue 53306: Some LKS forms don't distinguish between fields differin…
labkey-jeckels Jun 26, 2025
0e626f7
Merge branch 'develop' into fb_53306_propertyName
labkey-jeckels Jun 27, 2025
b9cf772
Update expected field names
labkey-jeckels Jun 27, 2025
6450182
Merge branch 'develop' into fb_53306_propertyName
labkey-jeckels Jun 27, 2025
45027f5
Update input element names
labkey-jeckels Jun 28, 2025
51e01da
Merge branch 'develop' into fb_53306_propertyName
labkey-jeckels Jun 28, 2025
99abd18
Fix build
labkey-jeckels Jun 28, 2025
d16d0ab
Test fixes
labkey-jeckels Jun 28, 2025
b26f444
Test fixes
labkey-jeckels Jun 28, 2025
6a24d8f
Test fixes
labkey-jeckels Jun 29, 2025
b5f34d1
Test fixes
labkey-jeckels Jul 2, 2025
5521cb8
Merge branch 'develop' into fb_53306_propertyName
labkey-jeckels Jul 2, 2025
a7f98ce
Merge branch 'develop' into fb_53306_propertyName
labkey-jeckels Jul 6, 2025
5fbb177
Test fixes, consolidate code, add comments
labkey-jeckels Jul 6, 2025
16b7465
Code cleanup
labkey-jeckels Jul 6, 2025
221f85d
Test fixes
labkey-jeckels Jul 7, 2025
759e7a0
Test fixes
labkey-jeckels Jul 7, 2025
8f29bc3
Merge branch 'develop' into fb_53306_propertyName
labkey-jeckels Jul 7, 2025
09fe0c6
Test fixes
labkey-jeckels Jul 7, 2025
5130303
Test fixes
labkey-jeckels Jul 9, 2025
2d2c3e9
Merge branch 'develop' into fb_53306_propertyName
labkey-jeckels Jul 9, 2025
48eb667
Test fixes
labkey-jeckels Jul 10, 2025
0554b85
Merge branch 'develop' into fb_53306_propertyName
labkey-jeckels Jul 10, 2025
cfd49f3
Make IssuesApiForm non-static
labkey-jeckels Jul 14, 2025
1ca0b86
More constants for field names
labkey-jeckels Jul 14, 2025
fd0be6f
Merge branch 'develop' into fb_53306_propertyName
labkey-jeckels Jul 14, 2025
ebcd5d1
Eliminate ColumnInfo.propNameFromName()
labkey-jeckels Jul 14, 2025
2c15648
Fix many issue tests
labkey-jeckels Jul 14, 2025
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
10 changes: 5 additions & 5 deletions api/src/org/labkey/api/assay/actions/UploadWizardAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ protected ModelAndView getRunPropertiesView(FormType newRunForm, boolean errorRe

if (!newRunForm.getBatchProperties().isEmpty())
{
JspView<AssayRunUploadForm> batchPropsView = new JspView<>("/org/labkey/assay/view/newUploadBatchProperties.jsp", newRunForm);
JspView<AssayRunUploadForm<?>> batchPropsView = new JspView<>("/org/labkey/assay/view/newUploadBatchProperties.jsp", newRunForm);
batchPropsView.setTitle("Batch Properties");
vbox.addView(batchPropsView);
}
Expand Down Expand Up @@ -769,9 +769,9 @@ protected void addHiddenRunProperties(FormType newRunForm, InsertView insertView
public static String getInputName(DomainProperty property, String disambiguationId)
{
if (disambiguationId != null)
return ColumnInfo.propNameFromName(disambiguationId + "_" + property.getName());
return disambiguationId + "_" + property.getName();
else
return ColumnInfo.propNameFromName(property.getName());
return property.getName();
}

public static String getInputName(DomainProperty property)
Expand All @@ -783,7 +783,7 @@ protected void addHiddenProperties(Map<DomainProperty, String> properties, Inser
{
for (Map.Entry<DomainProperty, String> entry : properties.entrySet())
{
String name = ColumnInfo.propNameFromName(entry.getKey().getName());
String name = entry.getKey().getName();
String value = entry.getValue();
insertView.getDataRegion().addHiddenFormField(name, value);
}
Expand Down Expand Up @@ -940,7 +940,7 @@ public void validateStep(FormType form, Errors errors)
{
validatePostedProperties(getViewContext(), form.getBatchProperties(), errors);

if (ThawListResolverType.NAME.equals(form.getRequest().getParameter("participantVisitResolver")))
if (ThawListResolverType.NAME.equals(form.getRequest().getParameter(AbstractAssayProvider.PARTICIPANT_VISIT_RESOLVER_PROPERTY_NAME)))
ThawListResolverType.validationHelper(form, errors);

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.apache.logging.log4j.Logger;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.labkey.api.assay.AbstractAssayProvider;
import org.labkey.api.assay.AssayProvider;
import org.labkey.api.assay.AssayRunUploadContext;
import org.labkey.api.assay.AssayService;
Expand Down Expand Up @@ -206,7 +207,7 @@ public void logProperties(Logger logger)
{
if(entry.getValue() == null)
valueText = "[Blank]";
else if(entry.getKey().getName().equals("TargetStudy"))
else if(entry.getKey().getName().equals(AbstractAssayProvider.TARGET_STUDY_PROPERTY_NAME))
valueText = ContainerManager.getForId(getTargetStudy()).getName();
else
valueText = entry.getValue();
Expand Down
4 changes: 3 additions & 1 deletion api/src/org/labkey/api/data/AbstractTableInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import org.labkey.api.util.logging.LogHelper;
import org.labkey.api.view.ActionURL;
import org.labkey.api.view.ViewContext;
import org.labkey.api.view.template.PageConfig;
import org.labkey.data.xml.AuditType;
import org.labkey.data.xml.ColumnType;
import org.labkey.data.xml.FilterGroupType;
Expand Down Expand Up @@ -693,7 +694,8 @@ protected ColumnInfo resolveColumn(String name)
{
if (null != col) // #19358
{
String propName = col.getPropertyName();
// Keep backwards compatible with historic value
String propName = PageConfig.makeIdFromName(col.getName());
if (null != propName && propName.equalsIgnoreCase(name))
return col;
}
Expand Down
6 changes: 0 additions & 6 deletions api/src/org/labkey/api/data/AbstractWrappedColumnInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -215,12 +215,6 @@ public boolean isShouldLog()
return delegate.isShouldLog();
}

@Override
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah!

public String getPropertyName()
{
return delegate.getPropertyName();
}

@Override
public boolean isVersionColumn()
{
Expand Down
37 changes: 2 additions & 35 deletions api/src/org/labkey/api/data/BaseColumnInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
import org.labkey.data.xml.DbSequenceType;
import org.labkey.data.xml.PropertiesType;

import java.beans.Introspector;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.sql.ResultSet;
Expand Down Expand Up @@ -86,8 +85,6 @@ public class BaseColumnInfo extends ColumnRenderPropertiesImpl implements Mutabl

@NotNull
private FieldKey _fieldKey;
// _propertyName is computed from getName();
private String _propertyName = null;
private DatabaseIdentifier _alias;
private String _sqlTypeName;
private JdbcType _jdbcType = null;
Expand Down Expand Up @@ -253,7 +250,6 @@ public void setName(@NotNull String name)
FieldKey newFieldKey = new FieldKey(null, name);
assert !_lockName || 0 == _fieldKey.compareTo(newFieldKey);
_fieldKey = newFieldKey;
_propertyName = null;
}


Expand All @@ -272,7 +268,6 @@ public void setFieldKey(@NotNull FieldKey key)
{
checkLocked();
_fieldKey = Objects.requireNonNull(key);
_propertyName = null;
}


Expand Down Expand Up @@ -887,16 +882,6 @@ public void setShouldLog(boolean shouldLog)
_shouldLog = shouldLog;
}

@Override
public String getPropertyName()
{
// this is surprisingly expensive, cache it!
if (null == _propertyName)
_propertyName = propNameFromName(getName());
return _propertyName;
}


/**
* Version column can be used for optimistic concurrency.
* for now we assume that this column is never updated
Expand Down Expand Up @@ -1401,6 +1386,7 @@ else if (Character.isUpperCase(c) && Character.isLowerCase(chars[i - 1]))
return buf.toString();
}

/** @return a version of the supplied name that is considered a valid Java identifier */

public static String legalNameFromName(String name)
{
Expand Down Expand Up @@ -1431,19 +1417,6 @@ public static String legalNameFromName(String name)
return buf.toString();
}


public static String propNameFromName(String name)
{
if (name == null)
return null;

if (name.isEmpty())
return null;

return Introspector.decapitalize(legalNameFromName(name));
}


// TODO why is there here? and not something like RequestHelper or PageFlowUtil
public static boolean booleanFromString(String str)
{
Expand Down Expand Up @@ -1657,12 +1630,6 @@ public Set<FieldKey> getSuggestedColumns()
{
return null;
}

@Override
public boolean isShowAsPublicDependency()
{
return false;
}
}

@Override
Expand Down Expand Up @@ -1727,7 +1694,7 @@ public static Collection<BaseColumnInfo> createFromDatabaseMetaData(String schem
col._label = reader.getLabel();
col._description = reader.getDescription();

if (NON_EDITABLE_COL_NAMES.contains(col.getPropertyName()))
if (NON_EDITABLE_COL_NAMES.contains(col.getName()))
col.setUserEditable(false);

colMap.put(col.getName(), col);
Expand Down
10 changes: 2 additions & 8 deletions api/src/org/labkey/api/data/ColumnInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.labkey.api.util.StringExpression;
import org.labkey.data.xml.ColumnType;

import java.beans.Introspector;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
Expand Down Expand Up @@ -216,8 +217,6 @@ static int findColumn(ResultSet rs, String name)

boolean isShouldLog();

String getPropertyName();

/**
* Version column can be used for optimistic concurrency.
* for now we assume that this column is never updated
Expand Down Expand Up @@ -383,12 +382,7 @@ static String labelFromName(String name)
{
return BaseColumnInfo.labelFromName(name);
}

static String propNameFromName(String name)
{
return BaseColumnInfo.propNameFromName(name);
}


static String legalNameFromName(String name)
{
return BaseColumnInfo.legalNameFromName(name);
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/data/DisplayColumn.java
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ public Object getJsonValue(RenderContext ctx)
public String getName()
{
if (null != getColumnInfo())
return getColumnInfo().getPropertyName();
return getColumnInfo().getName();
else
return super.getName();
}
Expand Down
4 changes: 3 additions & 1 deletion api/src/org/labkey/api/data/DisplayColumnGroup.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.labkey.api.util.DOM;
import org.labkey.api.util.InputBuilder;
import org.labkey.api.view.HttpView;
import org.labkey.api.view.template.PageConfig;
import org.labkey.api.writer.HtmlWriter;

import java.io.IOException;
Expand Down Expand Up @@ -89,9 +90,10 @@ public void writeSameCheckboxCell(RenderContext ctx, HtmlWriter out)
).appendTo(out);
}

/** Use propName because DOM ids and function names can't have spaces */
private String getGroupFormFieldName(RenderContext ctx)
{
return ColumnInfo.propNameFromName(getColumns().get(0).getFormFieldName(ctx));
return PageConfig.makeIdFromName(getColumns().get(0).getFormFieldName(ctx));
}

public void writeCopyableJavaScript(RenderContext ctx, Writer out) throws IOException
Expand Down
5 changes: 3 additions & 2 deletions api/src/org/labkey/api/data/ImportAliasable.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import org.labkey.api.collections.CaseInsensitiveHashMap;
import org.labkey.api.exp.MvColumn;
import org.labkey.api.view.template.PageConfig;

import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -64,10 +65,10 @@ public static <T extends ImportAliasable> Map<String, T> createImportMap(List<T>
{
m.put(property.getPropertyURI(), property);
}
// Then propName variant of name
// Then propName variant of name, kept for backwards compatibility
for (T property : reversedProperties)
{
m.put(BaseColumnInfo.propNameFromName(property.getName()), property);
m.put(PageConfig.makeIdFromName(property.getName()), property);
}
// Then aliases
for (T property : reversedProperties)
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/data/Table.java
Original file line number Diff line number Diff line change
Expand Up @@ -1189,7 +1189,7 @@ public static void ensureRequiredColumns(TableInfo table, Map<FieldKey, ColumnIn
{
if (cols.containsKey(column.getFieldKey()))
continue;
if (requiredColumns.contains(column.getFieldKey()) || requiredColumns.contains(new FieldKey(null,column.getAlias().getId())) || requiredColumns.contains(new FieldKey(null,column.getPropertyName())))
if (requiredColumns.contains(column.getFieldKey()) || requiredColumns.contains(new FieldKey(null,column.getAlias().getId())) || requiredColumns.contains(new FieldKey(null,column.getName())))
cols.put(column.getFieldKey(), column);
else if (column.isKeyField())
cols.put(column.getFieldKey(), column);
Expand Down
4 changes: 2 additions & 2 deletions api/src/org/labkey/api/data/TableViewForm.java
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ else if (_validateRequired && null != _tinfo)
Container container = fk.getLookupContainer() != null ? fk.getLookupContainer() : getContainer();
try
{
Object remappedValue = cache.remap(SchemaKey.fromParts(fk.getLookupSchemaName()), fk.getLookupTableName(), getUser(), container, ContainerFilter.Type.CurrentPlusProjectAndShared, str);
Object remappedValue = cache.remap(fk.getLookupSchemaKey(), fk.getLookupTableName(), getUser(), container, ContainerFilter.Type.CurrentPlusProjectAndShared, str);
if (remappedValue != null)
{
values.put(propName, remappedValue);
Expand Down Expand Up @@ -781,7 +781,7 @@ public void forceReselect()

public String getFormFieldName(@NotNull ColumnInfo column)
{
return column.getPropertyName();
return column.getName();
}

public String getMultiPartFormFieldName(@NotNull ColumnInfo column)
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/data/TableWrapperDynaClass.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ private TableWrapperDynaClass(TableInfo tinfo)
List<ColumnInfo> cols = tinfo.getColumns();
Map<String, Class<?>> propMap = new CaseInsensitiveHashMap<>();
for (ColumnInfo col : cols)
propMap.put(col.getPropertyName(), col.getJavaClass());
propMap.put(col.getName(), col.getJavaClass());

init(tinfo.getName(), propMap);
}
Expand Down
4 changes: 2 additions & 2 deletions api/src/org/labkey/api/defaults/SetDefaultValuesAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ public HttpView getView(FormType domainIdForm, boolean reshow, BindException err
if (entry.getValue() != null)
{
String stringValue = entry.getValue().toString();
decodePropertyValues(formDefaults, ColumnInfo.propNameFromName(entry.getKey().getName()), stringValue);
decodePropertyValues(formDefaults, entry.getKey().getName(), stringValue);
}
}
view.setInitialValues(formDefaults);
Expand Down Expand Up @@ -408,7 +408,7 @@ public boolean handlePost(FormType domainIdForm, BindException errors) throws Ex
Map<DomainProperty, Object> values = new HashMap<>();
for (DomainProperty property : domain.getProperties())
{
String propName = ColumnInfo.propNameFromName(property.getName());
String propName = property.getName();
String value = encodePropertyValues(domainIdForm, propName);
PropertyType type = property.getPropertyDescriptor().getPropertyType();
if (value != null && !value.isEmpty())
Expand Down
7 changes: 3 additions & 4 deletions api/src/org/labkey/api/exp/SamplePropertyHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public void addSampleColumns(InsertView view, User user)
}
}

public void addSampleColumns(InsertView view, User user, @Nullable AssayRunUploadForm defaultValueContext, boolean errorReshow) throws ExperimentException
public void addSampleColumns(InsertView view, User user, @Nullable AssayRunUploadForm<?> defaultValueContext, boolean errorReshow) throws ExperimentException
{
DataRegion region = view.getDataRegion();
List<String> sampleNames = getSampleNames();
Expand Down Expand Up @@ -215,10 +215,9 @@ public Map<String, Map<DomainProperty, String>> getPostedPropertyValues(HttpServ
for (DomainProperty sampleProperty : _domainProperties)
{
String name = UploadWizardAction.getInputName(sampleProperty, sampleName);
String inputName = ColumnInfo.propNameFromName(name);
// handle the hidden input field prefixed with FIELD_MARKER for checkboxes
String value = request.getParameter(inputName);
if (value == null && request.getParameter(SpringActionController.FIELD_MARKER + inputName) != null)
String value = request.getParameter(name);
if (value == null && request.getParameter(SpringActionController.FIELD_MARKER + name) != null)
value = "0";
values.put(sampleProperty, value);
}
Expand Down
14 changes: 13 additions & 1 deletion api/src/org/labkey/api/query/UserIdForeignKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,23 @@ public UserIdForeignKey(UserSchema userSchema)
public TableInfo getLookupTableInfo()
{
TableInfo tinfoUsersData = CoreSchema.getInstance().getTableInfoUsersData();
FilteredTable ret = new FilteredTable<>(tinfoUsersData, _userSchema);
FilteredTable<?> ret = new FilteredTable<>(tinfoUsersData, _userSchema);
ret.addWrapColumn(tinfoUsersData.getColumn("UserId"));
ret.addColumn(ret.wrapColumn("DisplayName", tinfoUsersData.getColumn("DisplayName")));
ret.setTitleColumn("DisplayName");
ret.setPublic(false);
return ret;
}

@Override
public SchemaKey getLookupSchemaKey()
{
return SchemaKey.fromParts("core");
}

@Override
public String getLookupTableName()
{
return "Users";
}
}
7 changes: 5 additions & 2 deletions api/src/org/labkey/api/reports/report/ScriptEngineReport.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import org.labkey.api.view.HttpView;
import org.labkey.api.view.VBox;
import org.labkey.api.view.ViewContext;
import org.labkey.api.view.template.PageConfig;
import org.labkey.api.writer.ContainerUser;
import org.labkey.vfs.FileLike;
import org.labkey.vfs.FileSystemLike;
Expand Down Expand Up @@ -408,7 +409,8 @@ protected List<String> outputColumnNames(Results r)
continue;
}

alias = ColumnInfo.propNameFromName(name).toLowerCase();
// Stay consistent with previous script-friendly identifier now used primarily for DOM ids
alias = PageConfig.makeIdFromName(name).toLowerCase();

if (!aliases.add(alias))
{
Expand All @@ -433,7 +435,8 @@ protected List<String> outputColumnNames(Results r)
private String oldLegalName(FieldKey fkey, @NotNull SqlDialect dialect)
{
String r = AliasManager.makeLegalName(StringUtils.join(fkey.getParts(),"_"), dialect, false);
return ColumnInfo.propNameFromName(r).toLowerCase();
// Stay consistent with previous script-friendly identifier
return ColumnInfo.legalNameFromName(r).toLowerCase();
}

/**
Expand Down
Loading