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

fix automated confirmation messages not sending #1635

Merged
merged 4 commits into from
Mar 28, 2025

Conversation

OmarZubaidi
Copy link
Contributor

@OmarZubaidi OmarZubaidi commented Mar 24, 2025

#1239

  • lints
  • builds
  • e2e tests pass

  1. the task that triggers the subscription was not being created
  2. some appointments don't have a location but have a healthcareservice or practitioner instead, so i updated the subscription zambda to consider these

@OmarZubaidi OmarZubaidi requested a review from VladMstv March 24, 2025 19:25
@VladMstv VladMstv requested a review from imbenham March 26, 2025 00:01
@@ -251,6 +254,23 @@ export async function createAppointment(
}

console.log('success, here is the id: ', appointment.id);

// this is done after the performTransactionalFhirRequests because it needs the appointment id
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that you can reference an intra-batch-created resource via the fullUrl property. Maybe you wanted to include this post with the rest of the transactional writes?

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 had no idea this was a thing. thank you! 1216023

Copy link
Contributor

@imbenham imbenham Mar 28, 2025

Choose a reason for hiding this comment

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

Yep, just note there is some possibility of a bug where subscriptions can be improperly invoked mid-transaction, causing some expected supporting resources to be missing when the subscription handler is run. I believe we've resolved to fix that bug before it reaches production, but is something I think I should flag here all the same.

@OmarZubaidi OmarZubaidi requested a review from imbenham March 28, 2025 16:09
@OmarZubaidi OmarZubaidi merged commit 4796de6 into develop Mar 28, 2025
4 checks passed
@OmarZubaidi OmarZubaidi deleted the omar/1239-confirmation-sms branch March 28, 2025 17:27
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