Skip to content
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

BFD-3683: Evaluate Feature/samhsa2.0 POC #2550

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Conversation

dondevun
Copy link
Contributor

@dondevun dondevun commented Feb 13, 2025

JIRA Ticket:
BFD-3876

What Does This PR Do?

Code for SAMHSA 2.0 Filter. This is being merged in order to start the pipeline SAMHSA tag creation; the feature flag will remain off pending the merge of BFD-3875.

What Should Reviewers Watch For?

If you're reviewing this PR, please check for these things in particular:

What Security Implications Does This PR Have?

Please indicate if this PR does any of the following:

  • Adds any new software dependencies

  • Modifies any security controls

  • Adds new transmission or storage of data

  • Any other changes that could possibly affect security?

  • I have considered the above security implications as it relates to this PR. (If one or more of the above apply, it cannot be merged without the ISSO or team security engineer's (@sb-benohe) approval.)

Validation

Have you fully verified and tested these changes? Is the acceptance criteria met? Please provide reproducible testing instructions, code snippets, or screenshots as applicable.

dondevun and others added 17 commits November 19, 2024 11:41
adding missed dependency
MahiFentaye
MahiFentaye previously approved these changes Feb 13, 2025
@dondevun dondevun requested a review from MahiFentaye February 18, 2025 15:14
@dondevun dondevun changed the title Feature/samhsa2.0 BFD-3683: Evaluate Feature/samhsa2.0 POC Feb 18, 2025
static final Logger LOGGER = LoggerFactory.getLogger(CCWSamhsaBackfill.class);

/** The table to process in this thread. */
private final TableEntry tableEntry;
Copy link
Contributor

Choose a reason for hiding this comment

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

This variable is unused

// original query.
if (!tableEntry.getLineItem()) {
dates = Optional.of(new Object[] {claim[1], claim[2]});
codesOffset = 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't necessarily need to change this right now, but I'm a bit concerned about the robustness of relying on column ordering here. It would be fairly easy to break this by accidentally changing the order of the select statement. Maybe it would be possible to use named parameters instead?

Copy link
Contributor Author

@dondevun dondevun Feb 18, 2025

Choose a reason for hiding this comment

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

I agree, this was top of mind for me, as well, when I was working on it. Changing it now would require another round of testing in the pipeline, so we can talk about whether we want to postpone the deployment for this; but I agree that there may be a better way to do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Your call. I think we can address it as a follow-up since it's not currently problematic.

@Override
public Map<Supplier<Optional<String>>, String> getClaimLineMethods(CarrierClaimLine line) {
return Map.ofEntries(
entry(line::getDiagnosisCode, "line_icd_dgns_cd"), entry(line::getHcpcsCode, "hcpcs_cd"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nice to have a centralized set of constants for these column name strings. A lot of these are repeated several times among a few different files.

@AllArgsConstructor
public class TableEntry {
/** The query for the table. */
String Query;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be camel-cased

Suggested change
String Query;
String query;

String column;

/** The line entry in the table. Will correspond to either rdaPosition, or idrDtlNumber */
Integer clm_line_num;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be camel-cased

Suggested change
Integer clm_line_num;
Integer clmLineNum;

for (Coding code : coding) {
org.hl7.fhir.dstu3.model.Coding securityTag = new org.hl7.fhir.dstu3.model.Coding();
securityTag
.setSystem("http://terminology.hl7.org/CodeSystem/v3-Confidentiality")
Copy link
Contributor

Choose a reason for hiding this comment

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

This should use the constant

Suggested change
.setSystem("http://terminology.hl7.org/CodeSystem/v3-Confidentiality")
.setSystem(TransformerConstants.SAMHSA_CONFIDENTIALITY_CODE_SYSTEM_URL)

@@ -149,6 +149,9 @@
/bfd/${env}/pipeline/nonsensitive/rda/job/batch_size: "20"
/bfd/${env}/pipeline/nonsensitive/rda/job/write_thread_count: "20"
/bfd/${env}/pipeline/nonsensitive/rda/job/process_dlq: "true"
/bfd/${env}/pipeline/nonsensitive/rda/samhsa/backfill/enabled: "true"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to start the backfill immediately on deployment or should we default this to false and enable it as a subsequent step?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see any drawback to starting it immediately, but we can start with it off if you think that would be better.

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.

6 participants