diff --git a/nbri_ehr/resources/queries/ehr/activeProtocols.sql b/nbri_ehr/resources/queries/ehr/activeProtocols.sql index a33358e..7c1a24a 100644 --- a/nbri_ehr/resources/queries/ehr/activeProtocols.sql +++ b/nbri_ehr/resources/queries/ehr/activeProtocols.sql @@ -3,5 +3,9 @@ * * Licensed under the Apache License, Version 2.0: http://www.apache.org/licenses/LICENSE-2.0 */ -SELECT protocol, title FROM ehr.protocol pr +SELECT + pr.protocol, + pr.description, + CASE WHEN pr.investigatorId.lastName IS NULL THEN pr.displayName ELSE pr.displayName || ' - ' || pr.investigatorId.lastName END AS displayText +FROM ehr.protocol pr WHERE pr.inactiveDate IS NULL OR pr.inactiveDate > now() \ No newline at end of file diff --git a/nbri_ehr/resources/queries/ehr/project.query.xml b/nbri_ehr/resources/queries/ehr/project.query.xml index 8ce9715..97ce065 100644 --- a/nbri_ehr/resources/queries/ehr/project.query.xml +++ b/nbri_ehr/resources/queries/ehr/project.query.xml @@ -11,43 +11,32 @@ Project + false NBRI Project Project Id - true - - - true - - - true - - - true + + + IACUC Protocol - true ehr protocol protocol - - true - - - true - - - true + + + + + false - true - + false ehr investigators @@ -55,26 +44,22 @@ lastName - - true - - - true - + Investigator true - - Requestor Name + true - + true - - true + + Requestor Name + + diff --git a/nbri_ehr/resources/queries/ehr/project/.qview.xml b/nbri_ehr/resources/queries/ehr/project/.qview.xml index 2022e60..1a0f2b6 100644 --- a/nbri_ehr/resources/queries/ehr/project/.qview.xml +++ b/nbri_ehr/resources/queries/ehr/project/.qview.xml @@ -1,8 +1,22 @@ - + + + + + + + + + + + + + + + - \ No newline at end of file + diff --git a/nbri_ehr/resources/queries/ehr/protocol.query.xml b/nbri_ehr/resources/queries/ehr/protocol.query.xml index edf95db..995a4ea 100644 --- a/nbri_ehr/resources/queries/ehr/protocol.query.xml +++ b/nbri_ehr/resources/queries/ehr/protocol.query.xml @@ -15,6 +15,14 @@ lastName + + + false + + + false + + true diff --git a/nbri_ehr/resources/queries/ehr/protocol/.qview.xml b/nbri_ehr/resources/queries/ehr/protocol/.qview.xml index 62dfd8e..9aff599 100644 --- a/nbri_ehr/resources/queries/ehr/protocol/.qview.xml +++ b/nbri_ehr/resources/queries/ehr/protocol/.qview.xml @@ -1,15 +1,25 @@ - + + + + + + + + + - + + + + - diff --git a/nbri_ehr/resources/web/nbri_ehr/data/AssignmentsClientStore.js b/nbri_ehr/resources/web/nbri_ehr/data/AssignmentsClientStore.js index bdd28b5..be4d68e 100644 --- a/nbri_ehr/resources/web/nbri_ehr/data/AssignmentsClientStore.js +++ b/nbri_ehr/resources/web/nbri_ehr/data/AssignmentsClientStore.js @@ -9,6 +9,102 @@ Ext4.define('NBRI_EHR.data.AssignmentsClientStore', { extend: 'EHR.data.DataEntryClientStore', + // Read-only columns echoing a value from the row's lookup target. Maintained here rather than from the + // combo's select event because nothing guarantees a combo: bulk add builds its rows through createModel(), + // and the parent store populates the project itself when an animal has a single active assignment. + lookupEchoes: [ + {keyField: 'protocol', echoField: 'protocol/description', valueColumn: 'description'}, + {keyField: 'project', echoField: 'project/account', valueColumn: 'account'} + ], + + constructor: function(){ + this.callParent(arguments); + + // One class serves both assignment sections, so keep only the pair this query actually carries + this.activeEchoes = Ext4.Array.filter(this.lookupEchoes, function(echo){ + return !!this.getFields().get(echo.keyField) && !!this.getFields().get(echo.echoField); + }, this); + }, + + onUpdate: function(record, operation, modified){ + this.callParent(arguments); + + this.syncLookupEchoes(record); + }, + + insert: function(index, records){ + // After the parent, which may set the project itself + var ret = this.callParent(arguments); + + Ext4.Array.forEach(Ext4.Array.from(records), function(record){ + this.syncLookupEchoes(record); + }, this); + + return ret; + }, + + syncLookupEchoes: function(record){ + // set() below re-enters this through onUpdate, and a field whose convert() rewrites the value would + // never satisfy the equality check that normally stops it, so stop on the way in instead. + if (this.syncingEchoes) + return; + + this.syncingEchoes = true; + try { + Ext4.Array.forEach(this.activeEchoes, function(echo){ + this.syncLookupEcho(record, echo); + }, this); + } + finally { + this.syncingEchoes = false; + } + }, + + syncLookupEcho: function(record, echo){ + var keyValue = record.get(echo.keyField); + if (Ext4.isEmpty(keyValue)){ + this.setEchoValue(record, echo.echoField, null); + return; + } + + var field = this.getFields().get(echo.keyField); + if (!field || !field.lookup || !field.lookup.keyColumn) + return; + + var lookupStore = Ext4.StoreMgr.get(LABKEY.ext4.Util.getLookupStoreId(field)); + if (!lookupStore) + return; + + // The lookup store autoLoads, so a row can arrive while it is still in flight. getCount() reads 0 both + // then and for a store that loaded nothing, and neither can resolve anything, so wait for the load + // rather than echoing the null an empty store would produce. + if (lookupStore.isLoading() || !lookupStore.getCount()){ + lookupStore.on('load', function(){ + this.syncLookupEcho(record, echo); + }, this, {single: true}); + + return; + } + + // findRecord(field, value, start, anyMatch, caseSensitive, exactMatch) defaults to a prefix match, + // which would resolve one protocol id to another that merely starts with it. + var match = lookupStore.findRecord(field.lookup.keyColumn, keyValue, 0, false, false, true); + + this.setEchoValue(record, echo.echoField, match ? match.get(echo.valueColumn) : null); + }, + + setEchoValue: function(record, fieldName, value){ + // A row loaded from the server already carries the right echo, so this keeps the sync from dirtying it. + // Empty-aware because a string field with no useNull converts a set null straight back to ''. + var current = record.get(fieldName); + if (current === value || (Ext4.isEmpty(current) && Ext4.isEmpty(value))) + return; + + record.suspendEvents(); + record.set(fieldName, value); + record.resumeEvents(); + }, + getExtraContext: function(){ var rows = []; var allRecords = this.getRange(); diff --git a/nbri_ehr/resources/web/nbri_ehr/model/sources/Arrival.js b/nbri_ehr/resources/web/nbri_ehr/model/sources/Arrival.js index af420c2..c6e6a94 100644 --- a/nbri_ehr/resources/web/nbri_ehr/model/sources/Arrival.js +++ b/nbri_ehr/resources/web/nbri_ehr/model/sources/Arrival.js @@ -97,15 +97,16 @@ EHR.model.DataModelManager.registerMetadata('Arrival', { nullable: false, columnConfig: { fixed: true, - width: 150 + width: 250 }, - // set displayColumn: ehr.protocol's title column (displayName) is not returned by this query lookup: { schemaName: 'ehr', queryName: 'activeProtocols', keyColumn: 'protocol', - displayColumn: 'protocol', - columns: 'protocol,title' + displayColumn: 'displayText', + // description is unused here, but the lookup store is shared by display column alone, so every + // consumer must request the same columns or whichever loads first starves the others + columns: 'protocol,description,displayText' } }, groupId: { diff --git a/nbri_ehr/resources/web/nbri_ehr/model/sources/Assignment.js b/nbri_ehr/resources/web/nbri_ehr/model/sources/Assignment.js index 31645f0..6ba035b 100644 --- a/nbri_ehr/resources/web/nbri_ehr/model/sources/Assignment.js +++ b/nbri_ehr/resources/web/nbri_ehr/model/sources/Assignment.js @@ -18,11 +18,21 @@ EHR.model.DataModelManager.registerMetadata('Assignment', { schemaName: 'ehr', queryName: 'project', keyColumn: 'project', - columns: 'project,name', + columns: 'project,name,account', filterArray: [ LABKEY.Filter.create('isActive', true, LABKEY.Filter.Types.EQUAL), ] } + }, + // read-only echo of the selected project's account; NBRI_EHR.data.AssignmentsClientStore keeps it current + 'project/account': { + header: 'Project Account', + label: 'Project Account', + editable: false, + columnConfig: { + editable: false, + width: 200 + } } }, 'study.protocolAssignment': { @@ -37,15 +47,24 @@ EHR.model.DataModelManager.registerMetadata('Assignment', { nullable: false, columnConfig: { fixed: true, - width: 150 + width: 250 }, - // set displayColumn: ehr.protocol's title column (displayName) is not returned by this query lookup: { schemaName: 'ehr', queryName: 'activeProtocols', keyColumn: 'protocol', - displayColumn: 'protocol', - columns: 'protocol,title' + displayColumn: 'displayText', + columns: 'protocol,description,displayText' + } + }, + // read-only echo of the selected protocol's description; NBRI_EHR.data.AssignmentsClientStore keeps it current + 'protocol/description': { + header: 'Protocol Description', + label: 'Protocol Description', + editable: false, + columnConfig: { + editable: false, + width: 300 } } } diff --git a/nbri_ehr/resources/web/nbri_ehr/model/sources/Birth.js b/nbri_ehr/resources/web/nbri_ehr/model/sources/Birth.js index a961152..fe89d2e 100644 --- a/nbri_ehr/resources/web/nbri_ehr/model/sources/Birth.js +++ b/nbri_ehr/resources/web/nbri_ehr/model/sources/Birth.js @@ -95,15 +95,16 @@ EHR.model.DataModelManager.registerMetadata('Birth', { allowBlank: false, nullable: false, columnConfig: { - width: 150 + width: 250 }, - // set displayColumn: ehr.protocol's title column (displayName) is not returned by this query lookup: { schemaName: 'ehr', queryName: 'activeProtocols', keyColumn: 'protocol', - displayColumn: 'protocol', - columns: 'protocol,title' + displayColumn: 'displayText', + // description is unused here, but the lookup store is shared by display column alone, so every + // consumer must request the same columns or whichever loads first starves the others + columns: 'protocol,description,displayText' } }, groupId: { diff --git a/nbri_ehr/resources/web/nbri_ehr/model/sources/Rearrival.js b/nbri_ehr/resources/web/nbri_ehr/model/sources/Rearrival.js index 3b3a575..d0dde4d 100644 --- a/nbri_ehr/resources/web/nbri_ehr/model/sources/Rearrival.js +++ b/nbri_ehr/resources/web/nbri_ehr/model/sources/Rearrival.js @@ -79,15 +79,16 @@ EHR.model.DataModelManager.registerMetadata('Rearrival', { nullable: false, columnConfig: { fixed: true, - width: 150 + width: 250 }, - // set displayColumn: ehr.protocol's title column (displayName) is not returned by this query lookup: { schemaName: 'ehr', queryName: 'activeProtocols', keyColumn: 'protocol', - displayColumn: 'protocol', - columns: 'protocol,title' + displayColumn: 'displayText', + // description is unused here, but the lookup store is shared by display column alone, so every + // consumer must request the same columns or whichever loads first starves the others + columns: 'protocol,description,displayText' } }, groupId: { diff --git a/nbri_ehr/src/org/labkey/nbri_ehr/dataentry/section/NBRIProjectAssignmentFormSection.java b/nbri_ehr/src/org/labkey/nbri_ehr/dataentry/section/NBRIProjectAssignmentFormSection.java index bc00399..a883de1 100644 --- a/nbri_ehr/src/org/labkey/nbri_ehr/dataentry/section/NBRIProjectAssignmentFormSection.java +++ b/nbri_ehr/src/org/labkey/nbri_ehr/dataentry/section/NBRIProjectAssignmentFormSection.java @@ -15,8 +15,13 @@ */ package org.labkey.nbri_ehr.dataentry.section; +import org.labkey.api.data.TableInfo; +import org.labkey.api.query.FieldKey; import org.labkey.api.view.template.ClientDependency; +import java.util.ArrayList; +import java.util.List; + public class NBRIProjectAssignmentFormSection extends BaseFormSection { public NBRIProjectAssignmentFormSection(boolean allowAnyId, boolean collapsible, boolean initCollapsed) @@ -34,4 +39,13 @@ public NBRIProjectAssignmentFormSection(boolean allowAnyId, boolean collapsible, setClientStoreClass("NBRI_EHR.data.AssignmentsClientStore"); } } + + @Override + protected List getFieldKeys(TableInfo ti) + { + List keys = new ArrayList<>(super.getFieldKeys(ti)); + keys.add(FieldKey.fromString("project/account")); + + return keys; + } } \ No newline at end of file diff --git a/nbri_ehr/src/org/labkey/nbri_ehr/dataentry/section/NBRIProtocolAssignmentFormSection.java b/nbri_ehr/src/org/labkey/nbri_ehr/dataentry/section/NBRIProtocolAssignmentFormSection.java index 4180bb2..3532cda 100644 --- a/nbri_ehr/src/org/labkey/nbri_ehr/dataentry/section/NBRIProtocolAssignmentFormSection.java +++ b/nbri_ehr/src/org/labkey/nbri_ehr/dataentry/section/NBRIProtocolAssignmentFormSection.java @@ -15,8 +15,13 @@ */ package org.labkey.nbri_ehr.dataentry.section; +import org.labkey.api.data.TableInfo; +import org.labkey.api.query.FieldKey; import org.labkey.api.view.template.ClientDependency; +import java.util.ArrayList; +import java.util.List; + public class NBRIProtocolAssignmentFormSection extends BaseFormSection { public NBRIProtocolAssignmentFormSection(boolean allowAnyId, boolean collapsible, boolean initCollapsed) @@ -34,4 +39,13 @@ public NBRIProtocolAssignmentFormSection(boolean allowAnyId, boolean collapsible setClientStoreClass("NBRI_EHR.data.AssignmentsClientStore"); } } + + @Override + protected List getFieldKeys(TableInfo ti) + { + List keys = new ArrayList<>(super.getFieldKeys(ti)); + keys.add(FieldKey.fromString("protocol/description")); + + return keys; + } } \ No newline at end of file diff --git a/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java b/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java index 9904e60..1f5c610 100644 --- a/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java +++ b/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java @@ -85,6 +85,7 @@ import java.util.UUID; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.labkey.test.components.html.Input.Input; @Category({EHR.class}) @@ -143,6 +144,13 @@ public class NBRI_EHRTest extends AbstractGenericEHRTest implements PostgresOnly // cannot change what the snapshot reports. private static final String[] LOCATION_ANIMALS = {"LOC0001"}; + // protocol.investigatorId looks up ehr.investigators rather than the user table, so a protocol shows an + // investigator only when a row there carries its id. + private static final String INVES_LAST_NAME = "Marsh"; + private static final String DUMMY_INVES_LAST_NAME = "Okafor"; + private static final String PROTOCOL_DESCRIPTION = "Chronic implant study"; + private static final String DUMMY_PROTOCOL_DESCRIPTION = "Placeholder protocol for the arrival and assignment fixtures"; + private final String[] weightFields = {"Id", "date", "enddate", "project", "weight", FIELD_QCSTATELABEL, FIELD_OBJECTID, FIELD_LSID, "_recordid", "performedby"}; private final Object[] weightData1 = {getExpectedAnimalIDCasing("TESTSUBJECT1"), EHRClientAPIHelper.DATE_SUBSTITUTION, null, null, "12", EHRQCState.IN_PROGRESS.label, null, null, "_recordID", 1004}; @@ -181,17 +189,36 @@ protected void populateProtocolRecords() throws Exception _permissionsHelper.addUserToProjGroup(inves1.getEmail(), getProjectName(), INVESTIGATOR.getGroup()); _permissionsHelper.addUserToProjGroup(inves2.getEmail(), getProjectName(), INVESTIGATOR.getGroup()); + InsertRowsCommand invesCmd = new InsertRowsCommand("ehr", "investigators"); + invesCmd.addRow(Map.of("lastName", INVES_LAST_NAME, "firstName", "Robin", "userid", inves1.getUserId())); + invesCmd.addRow(Map.of("lastName", DUMMY_INVES_LAST_NAME, "firstName", "Alex", "userid", inves2.getUserId())); + RowsResponse invesResponse = invesCmd.execute(createDefaultConnection(), getContainerPath()); + + Map investigatorIds = new HashMap<>(); + for (Map row : invesResponse.getRows()) + { + Object rowId = row.get("rowid"); + // A null id here reaches the protocols below as a null investigator, which only surfaces much later + // as a protocol dropdown missing the investigator half of its text. + assertNotNull("Investigator insert did not return a row id for " + row.get("lastName"), rowId); + investigatorIds.put((String) row.get("lastName"), rowId); + } + + assertEquals("Investigator insert did not return a row per investigator", 2, investigatorIds.size()); + InsertRowsCommand insertCmd = new InsertRowsCommand("ehr", "protocol"); Map rowMap = new HashMap<>(); rowMap.put("protocol", PROTOCOL_ID); - rowMap.put("InvestigatorId", inves1.getUserId()); + rowMap.put("InvestigatorId", investigatorIds.get(INVES_LAST_NAME)); rowMap.put("title", PROTOCOL_ID); + rowMap.put("description", PROTOCOL_DESCRIPTION); insertCmd.addRow(rowMap); rowMap = new HashMap<>(); rowMap.put("protocol", DUMMY_PROTOCOL); - rowMap.put("InvestigatorId", inves2.getUserId()); + rowMap.put("InvestigatorId", investigatorIds.get(DUMMY_INVES_LAST_NAME)); rowMap.put("title", DUMMY_PROTOCOL); + rowMap.put("description", DUMMY_PROTOCOL_DESCRIPTION); insertCmd.addRow(rowMap); insertCmd.execute(createDefaultConnection(), getContainerPath()); @@ -709,7 +736,7 @@ public void testArrivalForm() throws IOException, CommandException // the animal's opening project, protocol and group are entered on the arrival row itself; the trigger script // opens the matching assignment record for each one arrivals.setGridCell(1, "project", "640991"); - arrivals.setGridCell(1, "arrivalProtocol", "dummyprotocol"); + arrivals.setGridCell(1, "arrivalProtocol", protocolChoice(DUMMY_PROTOCOL, DUMMY_INVES_LAST_NAME)); arrivals.setGridCell(1, "groupId", animalGroup); log("Verifying the opening project, protocol and group are required"); @@ -719,7 +746,7 @@ public void testArrivalForm() throws IOException, CommandException arrivals.setGridCellJS(1, "arrivalProtocol", null); waitForFormError("The field: Protocol is required"); - arrivals.setGridCell(1, "arrivalProtocol", "dummyprotocol"); + arrivals.setGridCell(1, "arrivalProtocol", protocolChoice(DUMMY_PROTOCOL, DUMMY_INVES_LAST_NAME)); arrivals.setGridCellJS(1, "groupId", null); waitForFormError("The field: Group is required"); @@ -775,6 +802,75 @@ public void testArrivalForm() throws IOException, CommandException arrivalBirthDay, getDatasetDay("demographics", arrivedAnimal, "birth")); } + @Test + public void testAssignmentFormLookupColumns() + { + gotoEnterData(); + waitAndClickAndWait(Locator.linkWithText("Assignment")); + lockForm(); + + Ext4GridRef protocols = _helper.getExt4GridForFormSection("Protocol Assignment"); + _helper.addRecordToGrid(protocols); + protocols.setGridCell(1, "Id", aliveAnimalId); + + log("Verifying the protocol dropdown lists each protocol with its investigator"); + // setGridCell clicks the list item whose text is exactly this, so selecting it is the check on the format + protocols.setGridCell(1, "protocol", protocolChoice(DUMMY_PROTOCOL, DUMMY_INVES_LAST_NAME)); + Assert.assertEquals("Protocol dropdown stored something other than the protocol id", + DUMMY_PROTOCOL, protocols.getFieldValue(1, "protocol")); + + log("Verifying the protocol description follows the selected protocol"); + Assert.assertEquals("Protocol description did not follow the selected protocol", + DUMMY_PROTOCOL_DESCRIPTION, protocols.getFieldValue(1, "protocol/description")); + protocols.setGridCell(1, "protocol", protocolChoice(PROTOCOL_ID, INVES_LAST_NAME)); + Assert.assertEquals("Protocol description did not follow a changed protocol", + PROTOCOL_DESCRIPTION, protocols.getFieldValue(1, "protocol/description")); + + Assert.assertEquals("Protocol Description is not the last column", + "protocol/description", getLastVisibleColumn(protocols)); + Assert.assertEquals("Wrong header over the protocol description", + "Protocol Description", getColumnProperty(protocols, "protocol/description", "text")); + Assert.assertEquals("Protocol Description should not be editable", + false, getColumnProperty(protocols, "protocol/description", "editable")); + + Ext4GridRef projects = _helper.getExt4GridForFormSection("Project Assignment"); + _helper.addRecordToGrid(projects); + projects.setGridCell(1, "Id", aliveAnimalId); + projects.setGridCell(1, "project", PROJECT_ID); + + log("Verifying the project account follows the selected project"); + Assert.assertEquals("Project account did not follow the selected project", + ACCOUNT_ID_2, projects.getFieldValue(1, "project/account")); + Assert.assertEquals("Project Account is not the last column", + "project/account", getLastVisibleColumn(projects)); + Assert.assertEquals("Wrong header over the project account", + "Project Account", getColumnProperty(projects, "project/account", "text")); + Assert.assertEquals("Project Account should not be editable", + false, getColumnProperty(projects, "project/account", "editable")); + + // every check above reads the unsaved row, so discard rather than submit and leave the animal's own + // assignments alone + _helper.discardForm(); + } + + /** The text every protocol dropdown displays for a protocol, built by ehr/activeProtocols.sql. */ + private static String protocolChoice(String protocol, String investigatorLastName) + { + return protocol + " - " + investigatorLastName; + } + + /** A column config property, such as the rendered header or editability, which only the config carries. Null when + * the grid has no such column, so an assertion against it fails rather than passing vacuously. */ + private Object getColumnProperty(Ext4GridRef grid, String dataIndex, String property) + { + return grid.getFnEval("for (var i=0;i