Skip to content

Conversation

@kollil
Copy link
Collaborator

@kollil kollil commented Apr 22, 2021

Rationale

Retrieved the groups info from eIACUC tables. The SLA authorized projects and IACUC name shows the groups info.
Updated PMIC wiki pages with correct path.

@kollil kollil requested a review from labkey-jeckels April 22, 2021 17:17
LEFT JOIN Site.{substitutePath moduleProperty('ONPRC_Billing','BillingContainer_Public')}.onprc_billing_public.aliases y ON y.alias = a.account
LEFT JOIN onprc_ehr.investigators i ON i.rowId = a.investigatorId
LEFT JOIN Site.{substitutePath moduleProperty('EHR','EHRStudyContainer')}.sla.allowableAnimals_BreedingGroups aa ON a.protocol = aa.protocol
LEFT JOIN "/onprc/admin/finance/public".onprc_billing_public.aliases y ON y.alias = a.account
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you intentionally switch this to be hard-coded? This will trip up our automated tests on TeamCity. It would be better to keep using the moduleProperty if possible

LEFT JOIN onprc_billing.fiscalAuthorities f ON f.rowid = i.financialanalyst
LEFT JOIN Site.{substitutePath moduleProperty('EHR','EHRStudyContainer')}.sla.allowableAnimals_BreedingGroups aa ON a.protocol = aa.protocol
LEFT JOIN (select * from onprc_billing.projectAccountHistory z where (z.StartDate IS NOT NULL AND z.EndDate IS NOT NULL AND now() between z.StartDate AND z.EndDate)) x ON a.project = x.project
LEFT JOIN "/onprc/admin/finance/public".onprc_billing_public.aliases y ON y.alias = x.account
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question here - can this keep using the moduleProperty reference?

LEFT JOIN onprc_billing.fiscalAuthorities f ON f.rowid = i.financialanalyst
LEFT JOIN Site.{substitutePath moduleProperty('EHR','EHRStudyContainer')}.sla.allowableAnimals_BreedingGroups aa ON a.protocol = aa.protocol
LEFT JOIN (select * from onprc_billing.projectAccountHistory z where (z.StartDate IS NOT NULL AND z.EndDate IS NOT NULL AND now() between z.StartDate AND z.EndDate)) x ON a.project = x.project
LEFT JOIN "/onprc/admin/finance/public".onprc_billing_public.aliases y ON y.alias = x.account
Copy link
Contributor

Choose a reason for hiding this comment

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

One more

@kollil
Copy link
Collaborator Author

kollil commented Apr 27, 2021

Yes, Josh. I see the hard coded path for the aliases table("/onprc/admin/finance/public".onprc_billing_public.aliases ). I didn't make this change recently. That change was done few years ago. I can make the change so it uses the moduleProperty reference.

Site.{substitutePath moduleProperty('EHR','EHRStudyContainer')}.sla.allowableAnimals_BreedingGroups is a newly created user defined query directly in the schema browser. I have tested it on my dev machine and it works good. I had issues with the containers creating this query ( sla Vs ehr Vs sla_public). I know the correct way is putting the sql code in intellij. If that is going to interrupt the automated testing, I will change it.

@kollil
Copy link
Collaborator Author

kollil commented Apr 27, 2021

Thank you for noticing the bad xml in housing_transfers.query.xml. I swear I didn't mean to make any changes to this file as this project is already installed in production. It's probably my ghost :-)
Recommitted the fixed version.

@labkey-jeckels
Copy link
Contributor

@kollil I renamed the feature branch in ehrModules as we discussed. TeamCity is running a new installer build now:

https://teamcity.labkey.org/buildConfiguration/LabKey_2011Release_External_Onprc_OnprcInstaller/1393774

@kollil
Copy link
Collaborator Author

kollil commented Apr 29, 2021

Josh, All the fixes are done and tested on prime20test. Please approve to merge.

Thanks
Lakshmi

Copy link
Contributor

@labkey-jeckels labkey-jeckels left a comment

Choose a reason for hiding this comment

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

I was hoping to merge in changes that were added to the release20.11-SNAPSHOT branch to ensure that the automated tests were fully fixed by your updates but it sounds like the timing is too tight.

I merged the change in the ehrModules repo to allow dates in the distant future. Please go ahead and merge this.

@kollil kollil merged commit 2345c6f into release20.11-SNAPSHOT Apr 29, 2021
@kollil kollil deleted the 20.11_fb_SLA_and_pmic branch April 29, 2021 23:56
@kollil
Copy link
Collaborator Author

kollil commented Apr 29, 2021

Oh. Didn't know that Josh. I could have waited till next week but, I already told users to expect the changes tomorrow in production.

@kollil
Copy link
Collaborator Author

kollil commented Apr 30, 2021

Josh, You said, "I merged the change in the ehrModules repo to allow dates in the distant future. Please go ahead and merge this."
I found the changes you made in ehrModules repo, but didn't understand how to merge these,
LabKey/ehrModules@be6ee91

@labkey-jeckels
Copy link
Contributor

@kollil Sorry, my note was confusing.

I merged the ehrModules change a little earlier this afternoon. You don't need to do anything there.

I meant you should go ahead and merge this pull request for onprcEhrModules, which you already did. So you're all set and should be able to get a new installer build from the default branch in TeamCity soon.

@kollil
Copy link
Collaborator Author

kollil commented Apr 30, 2021

Oh ok, Thanks Josh!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants