Add ONPRC parentage validation queries

[bbimber]

@labkey-jeckels @labkey-martyp and @jryurkanin: in EHR you recently added some parentage validation queries: LabKey/ehrModules#534. That's all great, and it's probably not worth changing those at this point. Proper functioning of the EHR depends on having really accurate data, so these queries do matter a lot, and it's a great idea to continually add new ones as you encounter data issues. I made a comment to this effect on that PR, but there's two things here you might want to check out:

Note, in the ONPRC EHR parentage data is stored in a dataset called parentage, while in the default EHR demographics is used. Therefore the queries I'm adding here differ for this reason, but the points I'm making below apply to EHR too.

1. The first suggestion is that when running the query, it's helpful if you can give the user a way to easily rectify whatever problem exists. In the two queries here, you'll see that I set the updateURL to point to the underlying dataset. This way the user can load this SQL query, but LK will give them an edit button update the appropriate record, making things easier for the user.

2. Because ONPRC overrides the main EHR page I'm not really going to push this point all that strongly, but I dont see the point of adding the new basic HTML page (dataValidationReport.html), and especially not making it a top-level feature. There isnt anything wrong with having a report to load per se. The reason I bring this up is b/c there is already a way more functional system within EHR through the notification system (see subclasses of AbstractEHRNotification). Most concrete notifications are implemented in ONPRC_EHR, with ColonyAlertsNotification being one example. The very important difference with these is that: a) the notification runs automatically on a schedule, b) users can subscribe to it, and c) it reports problems when and if they exist automatically. In general, if all the validations pass, no email is sent. It's a very popular and efficient way to manage data validation. It is simply a lot more useful and reliable to make it automatic, as opposed to relying on a busy person to load and review some kind of HTML report regularly. These notifications can be executed through the browser on-demand as well (see example URL here:

   onprcEHRModules/onprc_ehr/src/org/labkey/onprc_ehr/ONPRC_EHRModule.java

   Line 320 in 3f288b6

   EHRService.get().registerReportLink(EHRService.REPORT_LINK_TYPE.moreReports, "View Summary of Clinical Tasks", this, DetailsURL.fromString("/ldk/runNotification.view?key=org.labkey.onprc_ehr.notification.RoutineClinicalTestsNotification"), "Routine Clinical Tasks");

   ), so it's not mutually exclusive with being run in the background and being run through the browser.

Anyway, that's all I'll say on this point.

[labkey-martyp]

@bbimber Yeah I agree with you on all points here. And we do want to add more notifications to core EHR for more broad use. This is a ramp up project for @jryurkanin so we're iterating on it when we have free time.

There are a couple things I'm not understanding about the ONPRC parentage data and queries. Here you're using only study.parentage but in demographicsParents.sql it's coalescing the parents from study.parentage and demographics. Would it be better to use demographicsParents here? And looking at demographicsParents.sql, it's bringing in Foster parents. demographicsParents is then used in pedigree.sql which is the source for pedigree plot, kinship and inbreeding. I would not think we want foster parents in those calculations. Here is what I would expect,

• demographicsParents.sql is used in some places where genetic pedigree is not important so maybe it stays including foster parents
• the queries in this PR use demographicsParents to get all parentage data (possibly filtering out foster parents)
• pedigree.sql needs to filter out foster parents from demographicsParents

I did not compare the data between parentage and demographicsParents, just looking at the queries so it's possible I'm missing something.

[bbimber]

@labkey-martyp , yeah, you're probably correct on demographicsParents vs. study.parentage. I think ONPRC merges birth and parentage and doesnt have dam/sire fields in demographics, but your point is right. I based this on the dataset rather than demographicsParents so we could have access to the LSID and could give the user an edit URL; however, but I bet we could union parentage and study.birth here and parameterize the queryName in the updateURL expression.

Regarding foster dams: i think the constraints of same species and being older than the infant still apply, so i left it. I'll double check what this is doing, but I think we might as well check both kinds.

Thanks for the suggestions

[bbimber]

@kollil or @labkey-martyp: what's the process at this point for getting changes into ONPRC_EHR?

[bbimber]

closing in favor of #688