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

[DSM] PEPPER-1408 Fix for Tracking Scan Duplicate Error #2881

Merged
merged 4 commits into from
Jun 3, 2024

Conversation

pegahtah
Copy link
Contributor

@pegahtah pegahtah commented May 30, 2024

Context

The error was being thrown because after checking if the value exists the method still tried to insert the values and would get an exception. Fixed it and added a test

Checklist

  • I have labeled the type of changes involved using the C-* labels.
  • I have assessed potential risks and labeled using the R-* labels.
  • I have considered error handling and alerts, and added L-* labels as needed.
  • I have considered security and privacy, and added I-* labels as needed
  • I have analyzed my changes for stability, fault tolerance, graceful degradation, performance bottlenecks and written a brief summary in this PR.
  • If applicable, I have discussed the analytics needs at both a platform and study level with Product and instrumented code accordingly.
  • If applicable, my UI/UX changes have passed muster with Product/Design via an over-the-shoulder review, screenshots, etc.

If unsure or need help with any of the above items, add the help wanted label. For items that starts with If applicable, if it is not applicable, check it off and add n/a in front.

FUD Score

Overall, how are you feeling about these changes?

  • ☺️ All good, business as usual!
  • 😅 There might be some issues here
  • 😱 I'm sweaty and nervous

How do we demo these changes?

How does one observe these changes in a deployed system? Note that user visible encompasses many personas--not just patients and study staff, but also ops duty, your fellow devs, compliance, etc.

  • They are user-visible in dev as a regular user journey and require no additional instructions.
  • Getting dev into a state where this is user-visible requires some tech fiddling. I have documented these steps in the related ticket.
  • Requires other features before it's human visible. I have documented the blocking issues in jira.
  • I have no idea how to demo this. Please help me!

Testing

  • I have added regression test labels to the jira ticket
  • I have added verification steps to the jira ticket
  • I have written automated positive tests
  • I have written automated negative tests
  • I have written zero automated tests but have poked around locally to verify proper functionality
  • The jira ticket has acceptance criteria and QA has the needed information to test changes

Release

  • These changes require no special release procedures--just code!
  • Releasing these changes requires special handling and I have documented the special procedures in the release plan document

@pegahtah pegahtah changed the title [DSM] Fix for Tracking Scan Duplicate Error [DSM] PEPPER-1408 Fix for Tracking Scan Duplicate Error May 30, 2024
@pegahtah pegahtah requested a review from zyme May 30, 2024 19:05
@@ -450,6 +450,7 @@ private int deleteKitRequest(Connection conn, int kitRequestId) throws SQLExcept
}
}

@VisibleForTesting
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this annotation on a public method on a dao. It's general purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted, will remove it!

@@ -168,16 +168,6 @@ portal {
and deactivated_date is null
"""

insertKitTrackingRequest:"""
Copy link
Contributor

Choose a reason for hiding this comment

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

No longer used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes Sorry I meant to but forgot to put a comment about it, it is never used and also was throwing me off for more than 15 minutes of why is there an insert statement, so I thought better to remove this unused code.

@pegahtah pegahtah merged commit 2a631f7 into develop Jun 3, 2024
8 checks passed
@pegahtah pegahtah deleted the PEPPER-1408-unq-trackingid branch June 3, 2024 17:52
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.

2 participants