Skip to content

Misc code cleanup; add vet to users and groups in tests #915

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.

Sign up for GitHub

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

Merged
labkey-jeckels merged 1 commit into from
Mar 20, 2025
Merged

Misc code cleanup; add vet to users and groups in tests #915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 9 additions & 14 deletions ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@
import org.labkey.api.query.FieldKey;
import org.labkey.api.query.FilteredTable;
import org.labkey.api.query.QueryForeignKey;
import org.labkey.api.query.QueryService;
import org.labkey.api.query.SimpleUserSchema;
import org.labkey.api.query.UserSchema;
import org.labkey.api.security.Group;
import org.labkey.api.security.MemberType;
import org.labkey.api.security.RoleAssignment;
Expand Down Expand Up @@ -68,15 +66,12 @@
public class EHRLookupsUserSchema extends SimpleUserSchema
{
public static final String TABLE_AREAS = "areas";
public static final String TABLE_BUILDINGS = "buildings";
public static final String TABLE_CAGE = "cage";
public static final String TABLE_CAGE_POSITIONS = "cage_positions";
public static final String TABLE_CAGE_TYPE = "cage_type";
public static final String TABLE_GEOGRAPHIC_ORIGINS = "geographic_origins";
public static final String TABLE_ROOMS = "rooms";
public static final String TABLE_SNOMED = "snomed";
public static final String TABLE_SNOMED_SUBSET_CODES = "snomed_subset_codes";
public static final String TABLE_TREATMENT_CODES = "treatment_codes";
public static final String TABLE_VETERINARIANS = "veterinarians";

public EHRLookupsUserSchema(User user, Container container, DbSchema dbschema)
Expand Down Expand Up @@ -218,7 +213,7 @@ else if (EHRSchema.TABLE_LOOKUP_SETS.equalsIgnoreCase(name))
// By default, any hard tables in the ehr_lookups schema not accounted for above will fall into one of the
// two categories below. Both of these will add a check that makes sure the user has EHRDataAdminPermission
// in order to insert/update/delete on the table. The ContainerScopedTable case is for those tables that
// have a true DB PK or rowid but a User psedoPK that should be accounted for at the container level.
// have a true DB PK or rowid but a User pseudoPK that should be accounted for at the container level.
String pkColName = getPkColName(ti);
if (pkColName != null && !"rowid".equalsIgnoreCase(pkColName) && ti.getColumn("container") != null)
return getContainerScopedTable(name, cf, pkColName, EHRDataAdminPermission.class);
Expand Down Expand Up @@ -252,7 +247,7 @@ private String getPkColName(TableInfo ti)

private TableInfo createVeterinariansTable(String name, ContainerFilter cf)
{
FilteredTable ti = new FilteredTable<>(CoreSchema.getInstance().getTableInfoUsersData(), this, cf);
FilteredTable<EHRLookupsUserSchema> ti = new FilteredTable<>(CoreSchema.getInstance().getTableInfoUsersData(), this, cf);
ti.setPublicSchemaName(EHRSchema.EHR_LOOKUPS);

Set<Integer> userIds = new HashSet<>();
Expand All @@ -271,9 +266,9 @@ private TableInfo createVeterinariansTable(String name, ContainerFilter cf)
else
{
Group assignedGroup = SecurityManager.getGroup(r.getUserId());
if (assignedGroup != null && !assignedGroup.isProjectGroup())
if (assignedGroup != null)
Copy link
Contributor

@labkey-martyp labkey-martyp Mar 20, 2025

Choose a reason for hiding this comment

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

Not really sure what the original use case was here. I don't see anyone using vets that wouldn't be listed in EHR or having different permissions at project level.

Copy link
Collaborator

@bbimber bbimber Mar 20, 2025

Choose a reason for hiding this comment

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

I assume I wrote that, but it's been too long to recall. I cannot think of any reason to differentiate between site groups and project-level groups. If no center is actually using a project-level group in this table then it's a moot point and probably doesnt need to exist.

Given how long ago this was, it's equally likely that some long-ago quirk the in SecurityManager API required this kind of check for some reason.

{
// Add all site groups
// Add all groups, both site and project-scoped
groupsToExpand.add(assignedGroup);
}
}
Expand All @@ -293,7 +288,7 @@ private TableInfo createVeterinariansTable(String name, ContainerFilter cf)

ti.addWrapColumn(ti.getRealTable().getColumn("UserId"));
ti.addWrapColumn(ti.getRealTable().getColumn("DisplayName"));
ti.getMutableColumn("UserId").setFk(new QueryForeignKey(QueryService.get().getUserSchema(getUser(), getContainer(), "core"), null, "Users", "UserId", "DisplayName"));
ti.getMutableColumnOrThrow("UserId").setFk(new QueryForeignKey.Builder(this, getDefaultContainerFilter()).schema(CoreSchema.getInstance().getSchemaName()).table("Users"));
ti.setName(name);
ti.setTitle("Veterinarians");

Expand All @@ -302,7 +297,7 @@ private TableInfo createVeterinariansTable(String name, ContainerFilter cf)

private TableInfo getContainerScopedTable(String name, ContainerFilter cf, String psuedoPk, @Nullable Class<? extends Permission> perm)
{
EHR_LookupsContainerScopedTable ret = new EHR_LookupsContainerScopedTable<>(this, this.createSourceTable(name), cf, psuedoPk);
EHR_LookupsContainerScopedTable<?> ret = new EHR_LookupsContainerScopedTable<>(this, this.createSourceTable(name), cf, psuedoPk);
if (perm != null)
{
ret.addPermissionMapping(InsertPermission.class, perm);
Expand All @@ -314,14 +309,14 @@ private TableInfo getContainerScopedTable(String name, ContainerFilter cf, Strin

private TableInfo getCustomPermissionTable(TableInfo schemaTable, ContainerFilter cf, Class<? extends Permission> perm)
{
EHR_LookupsCustomPermissionsTable ret = new EHR_LookupsCustomPermissionsTable<>(this, schemaTable, cf);
EHR_LookupsCustomPermissionsTable<?> ret = new EHR_LookupsCustomPermissionsTable<>(this, schemaTable, cf);
ret.addPermissionMapping(InsertPermission.class, perm);
ret.addPermissionMapping(UpdatePermission.class, perm);
ret.addPermissionMapping(DeletePermission.class, perm);
return ret.init();
}

private LookupSetTable createForPropertySet(UserSchema us, ContainerFilter cf, String setName, Map<String, Object> map)
private LookupSetTable createForPropertySet(EHRLookupsUserSchema us, ContainerFilter cf, String setName, Map<String, Object> map)
{
SchemaTableInfo table = _dbSchema.getTable(EHRSchema.TABLE_LOOKUPS);
LookupSetTable ret = new LookupSetTable(us, table, cf, setName, map);
Expand All @@ -331,7 +326,7 @@ private LookupSetTable createForPropertySet(UserSchema us, ContainerFilter cf, S
return ret.init();
}

private LabworkTypeTable createForLabwork(UserSchema us, ContainerFilter cf, String tableName, String typeName)
private LabworkTypeTable createForLabwork(EHRLookupsUserSchema us, ContainerFilter cf, String tableName, String typeName)
{
SchemaTableInfo table = _dbSchema.getTable(EHRSchema.TABLE_LAB_TESTS);
return new LabworkTypeTable(us, table, cf, tableName, typeName).init();
Expand Down
26 changes: 10 additions & 16 deletions ehr/src/org/labkey/ehr/query/LabworkTypeTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.labkey.api.query.QueryUpdateServiceException;
import org.labkey.api.query.SimpleTableDomainKind;
import org.labkey.api.query.SimpleUserSchema;
import org.labkey.api.query.UserSchema;
import org.labkey.api.query.ValidationException;
import org.labkey.api.security.User;
import org.labkey.api.security.permissions.DeletePermission;
Expand All @@ -45,21 +44,21 @@

import java.sql.SQLException;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.function.Supplier;

/**
* User: bimber
* Date: 3/27/13
* Time: 6:20 PM
*/
public class LabworkTypeTable extends AbstractDataDefinedTable
public class LabworkTypeTable extends AbstractDataDefinedTable<EHRLookupsUserSchema>
{
public static final String CACHE_KEY = LabworkTypeTable.class.getName() + "||types";

private static final String FILTER_FIELD = "testid";
private static final String TYPE_FIELD = "type";

public LabworkTypeTable(UserSchema schema, SchemaTableInfo table, ContainerFilter cf, String tableName, String filterValue)
public LabworkTypeTable(EHRLookupsUserSchema schema, SchemaTableInfo table, ContainerFilter cf, String tableName, String filterValue)
{
super(schema, table, cf, TYPE_FIELD, FILTER_FIELD, tableName, filterValue);
addPermissionMapping(InsertPermission.class, EHRDataAdminPermission.class);
Expand Down Expand Up @@ -117,7 +116,7 @@ public QueryUpdateService getUpdateService()

private class LabWorkTableUpdateService extends UpdateService
{
public LabWorkTableUpdateService(SimpleUserSchema.SimpleTable ti)
public LabWorkTableUpdateService(SimpleUserSchema.SimpleTable<EHRLookupsUserSchema> ti)
{
super(ti);
}
Expand Down Expand Up @@ -188,19 +187,14 @@ protected void configureTranslator(DataIterator input, final SimpleTranslator it
//append a column that will normalize whitespace in aliases
ColumnInfo aliasColInfo = getRealTable().getColumn(aliasColName);
final int inputIdx = aliasInputIdx;
it.addColumn(aliasColInfo, new Callable()
{
@Override
public Object call()
it.addColumn(aliasColInfo, (Supplier<Object>) () -> {
Object val = it.getInputColumnValue(inputIdx);
if (val instanceof String s)
{
Object val = it.getInputColumnValue(inputIdx);
if (val != null && val instanceof String)
{
val = normalizeAliasString((String)val);
}

return val;
val = normalizeAliasString(s);
}

return val;
});
}
}
Expand Down
13 changes: 6 additions & 7 deletions ehr/src/org/labkey/ehr/query/LookupSetTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.labkey.api.ldk.table.AbstractDataDefinedTable;
import org.labkey.api.query.QueryUpdateService;
import org.labkey.api.query.SimpleUserSchema;
import org.labkey.api.query.UserSchema;

import java.util.Map;

Expand All @@ -32,7 +31,7 @@
* Date: 1/31/13
* Time: 4:33 PM
*/
public class LookupSetTable extends AbstractDataDefinedTable
public class LookupSetTable extends AbstractDataDefinedTable<EHRLookupsUserSchema>
{
private static final String CACHE_KEY = LookupSetTable.class.getName() + "||values";

Expand All @@ -46,7 +45,7 @@ public static String getCacheKey(Container c)
return CACHE_KEY + "||" + c.getId();
}

public LookupSetTable(UserSchema schema, SchemaTableInfo table, ContainerFilter cf, String setName, Map<String, Object> map)
public LookupSetTable(EHRLookupsUserSchema schema, SchemaTableInfo table, ContainerFilter cf, String setName, Map<String, Object> map)
{
super(schema, table, cf, FILTER_COL, VALUE_COL, setName, setName);

Expand Down Expand Up @@ -78,13 +77,13 @@ public LookupSetTable init()
if (keyCol != null)
{
keyCol.setKeyField(true);
getMutableColumn("rowid").setKeyField(false);
getMutableColumnOrThrow("rowid").setKeyField(false);
}
}
else
{
getMutableColumn(VALUE_COL).setKeyField(false);
getMutableColumn("rowid").setKeyField(true);
getMutableColumnOrThrow(VALUE_COL).setKeyField(false);
getMutableColumnOrThrow("rowid").setKeyField(true);
}

if (_titleColumn != null)
Expand All @@ -107,7 +106,7 @@ public QueryUpdateService getUpdateService()

protected class EHRLookupsUpdateService extends UpdateService
{
public EHRLookupsUpdateService(SimpleUserSchema.SimpleTable ti)
public EHRLookupsUpdateService(SimpleUserSchema.SimpleTable<EHRLookupsUserSchema> ti)
{
super(ti);
}
Expand Down
7 changes: 6 additions & 1 deletion ehr/test/src/org/labkey/test/tests/ehr/AbstractEHRTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ abstract public class AbstractEHRTest extends BaseWebDriverTest implements Advan
protected static EHRUser NON_PATHOLOGY_REPORT = new EHRUser("[email protected]","EHR Non Pathology", EHRRole.FULL_UPDATER);
protected static EHRUser INVESTIGATOR = new EHRUser("[email protected]", "EHR Lab", EHRRole.REQUESTER);
protected static EHRUser INVESTIGATOR_PRINCIPAL = new EHRUser("[email protected]", "EHR Lab", EHRRole.DATA_ADMIN);
protected static EHRUser VET = new EHRUser("[email protected]", "EHR Vet", EHRRole.VETERINARIAN);

protected static String REQUESTER_USER = "[email protected]";
protected static String REQUEST_ADMIN_USER ="[email protected]";
Expand Down Expand Up @@ -667,6 +668,7 @@ protected void createUsersandPermissions()
_userHelper.createUser(NON_PATHOLOGY_REPORT.getEmail(), true, true);
_userHelper.createUser(INVESTIGATOR.getEmail(), true, true);
_userHelper.createUser(INVESTIGATOR_PRINCIPAL.getEmail(), true, true);
_userHelper.createUser(VET.getEmail(), true, true);
goToEHRFolder();

_permissionsHelper.createPermissionsGroup(DATA_ADMIN.getGroup(), DATA_ADMIN.getEmail());
Expand All @@ -679,6 +681,7 @@ protected void createUsersandPermissions()
_permissionsHelper.createPermissionsGroup(NON_PATHOLOGY_REPORT.getGroup(), NON_PATHOLOGY_REPORT.getEmail());
_permissionsHelper.createPermissionsGroup(INVESTIGATOR.getGroup(), INVESTIGATOR.getEmail());
_permissionsHelper.createPermissionsGroup(INVESTIGATOR_PRINCIPAL.getGroup(), INVESTIGATOR_PRINCIPAL.getEmail());
_permissionsHelper.createPermissionsGroup(VET.getGroup(), VET.getEmail());

if (!getContainerPath().equals(getProjectName()))
_permissionsHelper.uncheckInheritedPermissions();
Expand All @@ -690,6 +693,7 @@ protected void createUsersandPermissions()
_permissionsHelper.setPermissions(FULL_SUBMITTER.getGroup(), "EHR Data Entry");
_permissionsHelper.setPermissions(FULL_UPDATER.getGroup(), "EHR Data Entry");
_permissionsHelper.setPermissions(REQUEST_ADMIN.getGroup(), "EHR Data Entry");
_permissionsHelper.setPermissions(VET.getGroup(), "EHR Veterinarian");

_permissionsHelper.setPermissions(REQUESTER.getGroup(), "EHR Requestor");
_permissionsHelper.setPermissions(REQUEST_ADMIN.getGroup(), "EHR Request Admin");
Expand Down Expand Up @@ -1015,7 +1019,8 @@ public enum EHRRole
BASIC_SUBMITTER ("EHR Basic Submitter"),
FULL_SUBMITTER ("EHR Full Submitter"),
FULL_UPDATER ("EHR Full Updater"),
REQUEST_ADMIN ("EHR Request Admin");
REQUEST_ADMIN ("EHR Request Admin"),
VETERINARIAN("EHR Veterinarian");

private final String name;

Expand Down