-
Notifications
You must be signed in to change notification settings - Fork 3
Merge from onprc19.1 & onprc19.1Prod r.63271 to 65949 #74
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
Merge from onprc19.1 & onprc19.1Prod r.63271 to 65949 #74
Conversation
labkey-jeckels
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall code is looking pretty good.
EHR_ComplianceDB/resources/schemas/dbscripts/sqlserver/EHR_ComplianceDB-12.39-12.40.sql
Show resolved
Hide resolved
ehr/resources/schemas/dbscripts/sqlserver/EHR_Lookups-17.20-17.21.sql
Outdated
Show resolved
Hide resolved
ehr/resources/schemas/dbscripts/sqlserver/a_ehr_lookups-17.20-17.21.sql
Outdated
Show resolved
Hide resolved
…r updates to metadata.
- Add matching postgres script for EHR_ComplianceDB-12.39-12.40.sql - Add Treatment_Frequencies_Active.sql - Other minor cleanup
labkey-jeckels
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One Postgres syntax issue, otherwise all set
EHR_ComplianceDB/resources/schemas/dbscripts/postgresql/EHR_ComplianceDB-12.39-12.40.sql
Outdated
Show resolved
Hide resolved
| // The Animal group search is working fine now after this change. | ||
| if (groupName) | ||
| config.removeableFilters = [LABKEY.Filter.create('groupId/name', groupName, LABKEY.Filter.Types.EQUAL)]; | ||
| //config.removeableFilters = [LABKEY.Filter.create('groupId/name', groupName, LABKEY.Filter.Types.EQUAL)]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isnt userFilters the new name for removeableFilters? this change prevents the user from removing the group name filter, doesnt it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that is the intention given that user enters filter criteria in a form, which then results in a filtered grid in a different panel, having removable filters on this resultant grid would be confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's clear that if there was a bug preventing the filter from being applied, that's a problem.
I can see the argument on there being a filter panel above, but by that same logic then shouldnt start/end date also be immutable? It is possible for the top filter fields and DR to get out of sync.
This is a small enough change that it's probably not a big deal, but in general I would tend toward preserving capabilities for the user. For example, what is they want to do another filter type? More than one group? Contains? etc.
-Remove broken query study.currentBloodAvailable. -Add module property DCM_NHP_Resources_Container. -Update automated tests.
…fb_merge_from_onprc19.1
…filter that will get applied during insert and update.
…ey is not managing the calculations & validations, it is now done by a third party tool.
…idation() to the parent class.
Use existing & functional utility code to get ehr_lookups.labwork_services store. Replace 'onprc-snomedtreatmentcombo' with 'ehr-snomedtreatmentcombo' (since the onprc one is not defined). Some cleanup.
…tore is not loading when servicerequested is selected)
… and not been run successfully by the client.
…fb_merge_from_onprc19.1
labkey-jeckels
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question but looks good.
| @Override | ||
| protected boolean skipStudyImportQueryValidation() | ||
| { | ||
| return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this for test length / performance reasons? Is at least one of the ONPRC EHR tests doing a query validation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added temporarily for query validation to pass so i could fix actual test failures but had missed to removed it after. Now removed, and query failures are fixed as well.
…Query updates (note: these queries still fail the validation, they are just partially updated/fixed).
Change DATE_TIME_FORMAT in AttributeData and Event to use 0-23 for hours format
Rationale
Merge changes from onprc19.1 r.63271 to 65757
Merge changes from onprc19.1Prod r. 65762 to 65949
Related Pull Requests
LabKey/onprcEHRModules#32
LabKey/platform#1626
LabKey/DiscvrLabKeyModules#64
LabKey/LabDevKitModules#55
https://github.com/LabKey/sampleManagement/pull/381
https://github.com/LabKey/biologics/pull/712
https://github.com/LabKey/labbook/pull/53
LabKey/wnprc-modules#38