Skip to content

Allow registering multiple InvoicedItemsProcessingService impls #1102

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

Open
labkey-adam wants to merge 4 commits into
base: develop
Choose a base branch
from
+41 −19
Open

Allow registering multiple InvoicedItemsProcessingService impls #1102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9a068a0
Allow registering multiple InvoicedItemsProcessingService impls
labkey-adam Feb 14, 2026
e5e61d6
Code review feedback
labkey-adam Feb 15, 2026
e9dc6c3
Use billing container
labkey-adam Feb 16, 2026
5460be7
Set ehr.Project.Created and Modified to NOT NULL on SQL Server for co…
labkey-adam Feb 17, 2026
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
7 changes: 7 additions & 0 deletions ehr/resources/schemas/dbscripts/sqlserver/ehr-26.001-26.002.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- ehr.Project.Created and ehr.Project.Modified are NULL on SQL Server but NOT NULL on PostgreSQL. Set the NULL values
-- and switch the columns to NOT NULL to match PostgreSQL.
UPDATE ehr.Project SET Created = diCreated WHERE Created IS NULL;
UPDATE ehr.Project SET Modified = diModified WHERE Modified IS NULL;
Copy link
Contributor

@labkey-jeckels labkey-jeckels Feb 17, 2026

Choose a reason for hiding this comment

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

We could fall back on GETDATE() if the di columns are null. Unless we want to hard-fail the upgrade on a server that might not have values as a way of assessing real-world data.


ALTER TABLE ehr.Project ALTER COLUMN Created DATETIME NOT NULL;
ALTER TABLE ehr.Project ALTER COLUMN Modified DATETIME NOT NULL;
2 changes: 1 addition & 1 deletion ehr/src/org/labkey/ehr/EHRModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public String getName()
@Override
public @Nullable Double getSchemaVersion()
{
return 26.001;
return 26.002;
}

@Override
Expand Down
23 changes: 18 additions & 5 deletions ehr_billing/api-src/org/labkey/api/ehr_billing/pipeline/InvoicedItemsProcessingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,45 @@
import org.labkey.api.data.SimpleFilter;
import org.labkey.api.data.TableInfo;
import org.labkey.api.data.TableSelector;
import org.labkey.api.module.Module;
import org.labkey.api.pipeline.PipelineJobException;
import org.labkey.api.query.FieldKey;
import org.labkey.api.query.QueryService;
import org.labkey.api.security.User;
import org.labkey.api.services.ServiceRegistry;
import org.labkey.api.util.Pair;

import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CopyOnWriteArrayList;

/**
* Service to get a list of queries to be processed during a Billing Run. The listing is a collection of
* BillingPipelineJobProcess objects that define what schema.query to execute and the mapping from that query's
* columns to the ehr_billing.invoicedItem table's columns.
* Additionally, get center specific generated invoice number.
*
* Currently registered server wide but should allow multiple co-existing services and resolve per container's active modules
*/
public interface InvoicedItemsProcessingService
{
record Registration(String moduleName, InvoicedItemsProcessingService impl){}

List<Registration> REGISTRATION_LIST = new CopyOnWriteArrayList<>();

static void register(Module module, InvoicedItemsProcessingService impl)
{
REGISTRATION_LIST.add(new Registration(module.getName(), impl));
}

@Nullable
static InvoicedItemsProcessingService get()
static InvoicedItemsProcessingService get(Container billingContainer)
{
return ServiceRegistry.get().getService(InvoicedItemsProcessingService.class);
// Return the service implementation based on the registering module being active in the provided container
return REGISTRATION_LIST.stream()
Copy link
Contributor

@labkey-jeckels labkey-jeckels Feb 17, 2026

Choose a reason for hiding this comment

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

In some other EHR contexts, we use a module-dependency ordering to choose between something provided by the core EHR module and a center-specific module.

Here, I believe that the core EHR module would startup first, and therefore have register things first. Then center-specific modules would register. findFirst() means we'd resolve the core EHR version, if present. Consider switching to reverse the list or otherwise choose the other version.

Copy link
Contributor

@labkey-martyp labkey-martyp Feb 17, 2026

Choose a reason for hiding this comment

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

I assumed we were just registering this in the center specific modules (not in any core EHR modules) so there would be just one service registered in the billing container for each center's project. I believe that would work for the test server with multiple EHR projects.

Copy link
Contributor

@labkey-jeckels labkey-jeckels Feb 17, 2026

Choose a reason for hiding this comment

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

I assumed we were just registering this in the center specific modules (not in any core EHR modules) so there would be just one service registered in the billing container for each center's project. I believe that would work for the test server with multiple EHR projects.

I believe that's the current state in terms of what needs to be registered, but I'm advocating that we consider being consistent so we don't have to remember which one will win in scenarios like this.

Copy link
Contributor

@labkey-martyp labkey-martyp Feb 17, 2026

Choose a reason for hiding this comment

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

Got it. Makes sense.

.filter(reg -> billingContainer.hasActiveModuleByName(reg.moduleName()))
.map(Registration::impl)
.findFirst()
.orElse(null);
}

/** @return the inputs to the billing process that are capable of generating charges */
Expand Down
2 changes: 1 addition & 1 deletion ehr_billing/src/org/labkey/ehr_billing/EHR_BillingController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public ApiResponse execute(BillingPipelineForm form, BindException errors)
throw new PipelineJobException("Cannot create a billing run with the same start and end date");
}

InvoicedItemsProcessingService processingService = InvoicedItemsProcessingService.get();
InvoicedItemsProcessingService processingService = InvoicedItemsProcessingService.get(EHR_BillingManager.get().getBillingContainer(getContainer()));
if (null != processingService)
{
Pair<String,String> previousInvoice = processingService.verifyBillingRunPeriod(getUser(), getContainer(), form.getStartDate(), form.getEndDate());
Expand Down
26 changes: 14 additions & 12 deletions ehr_billing/src/org/labkey/ehr_billing/pipeline/BillingTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,13 @@
public class BillingTask extends PipelineJob.Task<BillingTask.Factory>
{
private final static DbSchema EHR_BILLING_SCHEMA = EHR_BillingSchema.getInstance().getSchema();
private final static InvoicedItemsProcessingService processingService = InvoicedItemsProcessingService.get();

private final InvoicedItemsProcessingService _processingService;

protected BillingTask(Factory factory, PipelineJob job)
{
super(factory, job);
_processingService = InvoicedItemsProcessingService.get(EHR_BillingManager.get().getBillingContainer(job.getContainer()));
}

public static class Factory extends AbstractTaskFactory<AbstractTaskFactorySettings, Factory>
Expand Down Expand Up @@ -148,16 +150,16 @@ public RecordedActionSet run() throws PipelineJobException
{
getOrCreateInvoiceRunRecord();
loadTransactionNumber();
processingService.setBillingStartDate(getSupport().getStartDate());
_processingService.setBillingStartDate(getSupport().getStartDate());

if (null != _previousInvoice)
{
processingService.processBillingRerun(_invoiceId, _invoiceRowId, getSupport().getStartDate(), getSupport().getEndDate(), getNextTransactionNumber(), user, billingContainer, getJob().getLogger());
_processingService.processBillingRerun(_invoiceId, _invoiceRowId, getSupport().getStartDate(), getSupport().getEndDate(), getNextTransactionNumber(), user, billingContainer, getJob().getLogger());
}
else
{

for (BillingPipelineJobProcess process : processingService.getProcessList())
for (BillingPipelineJobProcess process : _processingService.getProcessList())
{
Container billingRunContainer = process.isUseEHRContainer() ? ehrContainer : billingContainer;
runProcessing(process, billingRunContainer);
Expand All @@ -166,7 +168,7 @@ public RecordedActionSet run() throws PipelineJobException

updateInvoiceTable(billingContainer);

processingService.performAdditionalProcessing(_invoiceId, user, container);
_processingService.performAdditionalProcessing(_invoiceId, user, container);

transaction.commit();
}
Expand Down Expand Up @@ -278,7 +280,7 @@ private String getOrCreateInvoiceRunRecord() throws PipelineJobException
@Nullable
private String getOrCreateInvoiceRecord(Map<String, Object> row, Date endDate) throws PipelineJobException
{
String invoiceNumber = processingService.getInvoiceNum(row, endDate);
String invoiceNumber = _processingService.getInvoiceNum(row, endDate);
if (null != invoiceNumber)
{
try
Expand Down Expand Up @@ -499,25 +501,25 @@ private void runProcessing(BillingPipelineJobProcess process, Container billingR

// get cost
Double unitCost = ci.getUnitCost();
procedureRow.put(processingService.getUnitCostColName(), unitCost);
procedureRow.put(_processingService.getUnitCostColName(), unitCost);

// total cost
Double totalCost = unitCost * (Double) procedureRow.get("quantity");
procedureRow.put(processingService.getTotalCostColName(), totalCost);
procedureRow.put(_processingService.getTotalCostColName(), totalCost);

// calculate total cost with additional/other rate (ex. tier rate for WNPRC)
Double otherRate = (Double) procedureRow.get("otherRate");
Double unitCostWithOtherRate;
double totalCostWithOtherRate;
if (null != otherRate &&
null != processingService.getAdditionalUnitCostColName() &&
null != processingService.getAdditionalTotalCostColName())
null != _processingService.getAdditionalUnitCostColName() &&
null != _processingService.getAdditionalTotalCostColName())
{
unitCostWithOtherRate = unitCost + (unitCost * otherRate);
procedureRow.put(processingService.getAdditionalUnitCostColName(), unitCostWithOtherRate);
procedureRow.put(_processingService.getAdditionalUnitCostColName(), unitCostWithOtherRate);

totalCostWithOtherRate = unitCostWithOtherRate * (Double) procedureRow.get("quantity");
procedureRow.put(processingService.getAdditionalTotalCostColName(), totalCostWithOtherRate);
procedureRow.put(_processingService.getAdditionalTotalCostColName(), totalCostWithOtherRate);
}
}
writeToInvoicedItems(process, procedureRows, getSupport());
Expand Down