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

[NIAD-2988] Lowercase Conversation ID for getMigrationRequest() #647

Merged
merged 10 commits into from
Jun 25, 2024

Conversation

martin-nhs
Copy link
Contributor

@martin-nhs martin-nhs commented Jun 24, 2024

What

Previously:

  • Given a POST request made to the /$gpc.ack endpoint.
  • When a request header is passed for conversationId with a lowercase UUIDv4.
  • Then the response would be status 500 (Internal Server Error).

Now:

  • Given a POST request made to the /$gpc.ack endpoint.
  • When a request header is passed for conversationId with a lowercase OR uppercase UUIDv4.
  • Then the response would be status 200 (OK).

Why

This bug could cause confusion and questions from NMEs as to why the endpoint doesn’t work.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated the Changelog with details of my change in the UNRELEASED section if this change will affect end users
  • A corresponding change has been made to the Mapping Documentation repository

@martin-nhs martin-nhs marked this pull request as draft June 24, 2024 15:40
@@ -26,7 +26,7 @@ public class AcknowledgeRecordService {
public Boolean handleAcknowledgeRecord(
@NotNull @NotEmpty String conversationId,
@NotNull @NotEmpty String confirmationResponseString) {
var patientMigrationRequest = patientMigrationRequestDao.getMigrationRequest(conversationId);
var patientMigrationRequest = patientMigrationRequestDao.getMigrationRequest(conversationId.toLowerCase());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, strange as I'm pretty sure we're uppercasing the conversation ID when it's inserted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're correct, my bad!

@martin-nhs martin-nhs requested a review from adrianclay June 24, 2024 23:30
@martin-nhs martin-nhs marked this pull request as ready for review June 25, 2024 08:36
@martin-nhs martin-nhs requested a review from ole4ryb June 25, 2024 09:08
@martin-nhs martin-nhs enabled auto-merge (squash) June 25, 2024 09:09
@@ -138,9 +152,20 @@ public void sendAcknowledgeRequestWithMissingConfirmationResponseHeader() throws
.andExpect(content().json(expectedResponseBody));
}

// Helper Methods
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't need this comments as they a bit clutter it here, the method is simple and it is very easy to see that it is a preparing/helper method for the test.

@martin-nhs martin-nhs merged commit 4b4e549 into main Jun 25, 2024
1 check passed
@martin-nhs martin-nhs deleted the NIAD-2988 branch June 25, 2024 14:46
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