-
Notifications
You must be signed in to change notification settings - Fork 0
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
PRMT-3439 - Multiple EHR requests for the same EHR from the same GP #23
base: main
Are you sure you want to change the base?
Changes from 25 commits
0ff49b1
b746f75
c72be7f
0264513
83d6646
73221e1
e548cd8
7266c1c
a2972cd
ebaa402
c349de9
e8c1fae
a2f073b
6e37ee4
956dfa7
3f7a68d
f3cc475
8f6eaf9
2b3103d
225cdd4
f33e576
1ecb43b
a34a5c9
699e43a
fbcaa75
161bc2d
ea8bd7c
b0a4c3e
5cd81b1
9491c24
6c7f962
ca0349f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,43 @@ | ||||||||||||||||||||||||||
package uk.nhs.prm.deduction.e2e.models; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
import uk.nhs.prm.deduction.e2e.utility.Resources; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
import java.util.UUID; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
import static uk.nhs.prm.deduction.e2e.utility.TestUtils.getUUIDAsUpperCaseString; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
public class ContinueRequestMessage { | ||||||||||||||||||||||||||
private UUID conversationId; | ||||||||||||||||||||||||||
private UUID messageId; | ||||||||||||||||||||||||||
private String sourceGpOds; | ||||||||||||||||||||||||||
private String destinationGpOds; | ||||||||||||||||||||||||||
private String sourceGpAsid; | ||||||||||||||||||||||||||
private String destinationGpAsid; | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These can be final :D
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, comment addressed |
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
public ContinueRequestMessage(UUID conversationId, UUID messageId, String sourceGpOds, String destinationGpOds, String sourceGpAsid, String destinationGpAsid) { | ||||||||||||||||||||||||||
this.conversationId = conversationId; | ||||||||||||||||||||||||||
this.messageId = messageId; | ||||||||||||||||||||||||||
this.sourceGpOds = sourceGpOds; | ||||||||||||||||||||||||||
this.destinationGpOds = destinationGpOds; | ||||||||||||||||||||||||||
this.sourceGpAsid = sourceGpAsid; | ||||||||||||||||||||||||||
this.destinationGpAsid = destinationGpAsid; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please leverage Lombok here in favour of adding the following annotation to the class level, in favour of this constructor - just removes some boilerplate stuff :)
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! It's so good to know this useful annotation! Boilerplate gone now :D |
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
public String toJsonString() { | ||||||||||||||||||||||||||
martin-nhs marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||
return Resources.readTestResourceFile("COPC_IN000001UK01") | ||||||||||||||||||||||||||
.replaceAll("DBC31D30-F984-11ED-A4C4-956AA80C6B4E", conversationId()) | ||||||||||||||||||||||||||
.replaceAll("DE304CA0-F984-11ED-808B-AC162D1F16F0", messageId()) | ||||||||||||||||||||||||||
.replaceAll("B85002", sourceGpOds) | ||||||||||||||||||||||||||
.replaceAll("200000001613", sourceGpAsid) | ||||||||||||||||||||||||||
.replaceAll("M85019", destinationGpOds) | ||||||||||||||||||||||||||
.replaceAll("200000000149", destinationGpAsid); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
public String conversationId() { | ||||||||||||||||||||||||||
return getUUIDAsUpperCaseString(conversationId); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
public String messageId() { | ||||||||||||||||||||||||||
return getUUIDAsUpperCaseString(messageId); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+29
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these really required? I would say that if all the function does is return a single UUID in uppercase - could we not just use:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, previously we used toString().toUpperCase() here, but we extracted that to a static function as suggested by a PR comment in PRMT-3413, so probably I am keeping it like this for now. |
||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
package uk.nhs.prm.deduction.e2e.models; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
import uk.nhs.prm.deduction.e2e.TestConfiguration; | ||
import uk.nhs.prm.deduction.e2e.utility.TestUtils; | ||
|
||
import java.util.UUID; | ||
|
||
public class ContinueRequestMessageBuilder { | ||
private UUID conversationId; | ||
private UUID messageId; | ||
private String sourceGpOds; | ||
private String destinationGpOds; | ||
private String sourceGpAsid; | ||
private String destinationGpAsid; | ||
|
||
private static final Logger LOGGER = LogManager.getLogger(ContinueRequestMessageBuilder.class); | ||
|
||
public ContinueRequestMessageBuilder() { | ||
withRandomlyGeneratedConversationId(); | ||
withRandomlyGeneratedMessageId(); | ||
withEhrSourceGp(Gp2GpSystem.REPO_DEV); | ||
withEhrDestinationGp(Gp2GpSystem.TPP_PTL_INT); | ||
} | ||
|
||
public ContinueRequestMessageBuilder withConversationId(UUID conversationId) { | ||
this.conversationId = conversationId; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withRandomlyGeneratedConversationId() { | ||
this.conversationId = UUID.randomUUID(); | ||
LOGGER.info("generated conversation id {}", conversationId); | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withEhrSourceAsRepo(TestConfiguration config) { | ||
Gp2GpSystem repoInEnv = Gp2GpSystem.repoInEnv(config); | ||
this.sourceGpOds = repoInEnv.odsCode(); | ||
this.sourceGpAsid = repoInEnv.asidCode(); | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withEhrSourceGp(Gp2GpSystem ehrSourceGp) { | ||
this.sourceGpOds = ehrSourceGp.odsCode(); | ||
this.sourceGpAsid = ehrSourceGp.asidCode(); | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withEhrDestinationGp(Gp2GpSystem ehrDestinationGp) { | ||
this.destinationGpOds = ehrDestinationGp.odsCode(); | ||
this.destinationGpAsid = ehrDestinationGp.asidCode(); | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withMessageId(UUID messageId) { | ||
this.messageId = messageId; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withRandomlyGeneratedMessageId() { | ||
this.messageId = UUID.randomUUID(); | ||
LOGGER.info("generated message id {}", messageId); | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withSourceGpOds(String sourceGpOds) { | ||
this.sourceGpOds = sourceGpOds; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withDestinationGpOds(String destinationGpOds) { | ||
this.destinationGpOds = destinationGpOds; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withSourceGpAsid(String sourceGpAsid) { | ||
this.sourceGpAsid = sourceGpAsid; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withDestinationGpAsid(String destinationGpAsid) { | ||
this.destinationGpAsid = destinationGpAsid; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessage build() { | ||
return new ContinueRequestMessage(conversationId, messageId, sourceGpOds, destinationGpOds, sourceGpAsid, destinationGpAsid); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,46 @@ | ||||||
package uk.nhs.prm.deduction.e2e.models; | ||||||
|
||||||
import uk.nhs.prm.deduction.e2e.utility.Resources; | ||||||
|
||||||
import java.util.UUID; | ||||||
|
||||||
import static uk.nhs.prm.deduction.e2e.utility.TestUtils.getUUIDAsUpperCaseString; | ||||||
|
||||||
public class EhrRequestMessage { | ||||||
private final String nhsNumber; | ||||||
private final String sourceGpOds; | ||||||
private final String destinationGpOds; | ||||||
private final String sourceGpAsid; | ||||||
private final String destinationGpAsid; | ||||||
private final UUID conversationId; | ||||||
private final UUID messageId; | ||||||
|
||||||
public EhrRequestMessage(String nhsNumber, String sourceGpOds, String destinationGpOds, String sourceGpAsid, String destinationGpAsid, UUID conversationId, UUID messageId) { | ||||||
this.nhsNumber = nhsNumber; | ||||||
this.sourceGpOds = sourceGpOds; | ||||||
this.destinationGpOds = destinationGpOds; | ||||||
this.sourceGpAsid = sourceGpAsid; | ||||||
this.destinationGpAsid = destinationGpAsid; | ||||||
this.conversationId = conversationId; | ||||||
this.messageId = messageId; | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as previously mentioned, please use Lombok's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure! Replaced boilerplate with @ AllArgsConstructor here as well |
||||||
|
||||||
public String toJsonString() { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above again! :D
Suggested change
|
||||||
return Resources.readTestResourceFile("RCMR_IN010000UK05") | ||||||
.replaceAll("9692842304", nhsNumber) | ||||||
.replaceAll("17a757f2-f4d2-444e-a246-9cb77bef7f22", conversationId()) | ||||||
.replaceAll("C445C720-B0EB-4E36-AF8A-48CD1CA5DE4F", messageId()) | ||||||
.replaceAll("B86041", sourceGpOds) | ||||||
.replaceAll("200000001161", sourceGpAsid) | ||||||
.replaceAll("A91720", destinationGpOds) | ||||||
.replaceAll("200000000631", destinationGpAsid); | ||||||
} | ||||||
|
||||||
public String conversationId() { | ||||||
return getUUIDAsUpperCaseString(conversationId); | ||||||
} | ||||||
|
||||||
public String messageId() { | ||||||
return getUUIDAsUpperCaseString(messageId); | ||||||
} | ||||||
Comment on lines
+31
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, do we need these? |
||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
package uk.nhs.prm.deduction.e2e.models; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
import uk.nhs.prm.deduction.e2e.TestConfiguration; | ||
import uk.nhs.prm.deduction.e2e.tests.Patient; | ||
import uk.nhs.prm.deduction.e2e.utility.TestUtils; | ||
|
||
import java.util.UUID; | ||
|
||
public class EhrRequestMessageBuilder { | ||
private String nhsNumber; | ||
private String sourceGpOds; | ||
private String destinationGpOds; | ||
private String sourceGpAsid; | ||
private String destinationGpAsid; | ||
private UUID conversationId; | ||
private UUID messageId; | ||
|
||
private static final Logger LOGGER = LogManager.getLogger(EhrRequestMessageBuilder.class); | ||
|
||
public EhrRequestMessageBuilder() { | ||
withRandomlyGeneratedConversationId(); | ||
withRandomlyGeneratedMessageId(); | ||
withNhsNumber("9692842304"); // nhs number in test template file | ||
withEhrSourceGp(Gp2GpSystem.REPO_DEV); | ||
withEhrDestinationGp(Gp2GpSystem.TPP_PTL_INT); | ||
} | ||
|
||
public EhrRequestMessageBuilder withNhsNumber(String nhsNumber) { | ||
this.nhsNumber = nhsNumber; | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withPatient(Patient patient) { | ||
return withNhsNumber(patient.nhsNumber()); | ||
} | ||
|
||
public EhrRequestMessageBuilder withEhrSourceAsRepo(TestConfiguration config) { | ||
Gp2GpSystem repoInEnv = Gp2GpSystem.repoInEnv(config); | ||
this.sourceGpOds = repoInEnv.odsCode(); | ||
this.sourceGpAsid = repoInEnv.asidCode(); | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withEhrSourceGp(Gp2GpSystem ehrSourceGp) { | ||
this.sourceGpOds = ehrSourceGp.odsCode(); | ||
this.sourceGpAsid = ehrSourceGp.asidCode(); | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withEhrDestinationGp(Gp2GpSystem ehrDestinationGp) { | ||
this.destinationGpOds = ehrDestinationGp.odsCode(); | ||
this.destinationGpAsid = ehrDestinationGp.asidCode(); | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withConversationId(UUID conversationId) { | ||
this.conversationId = conversationId; | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withRandomlyGeneratedConversationId() { | ||
conversationId = UUID.randomUUID(); | ||
LOGGER.info("generated conversation id {}", conversationId); | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withMessageId(UUID messageId) { | ||
this.messageId = messageId; | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withRandomlyGeneratedMessageId() { | ||
messageId = UUID.randomUUID(); | ||
LOGGER.info("generated message id {}", messageId); | ||
return this; | ||
} | ||
|
||
public EhrRequestMessage build() { | ||
return new EhrRequestMessage(nhsNumber, sourceGpOds, destinationGpOds, sourceGpAsid, destinationGpAsid, conversationId, messageId); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this not just be a class-level member constant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, reflect throughout the URLs :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
I have addressed this by changing the urls into static constants and updating the references in the new tests that we written.
There are some references to those getXXXUrl methods in other test suites which is not in the scope of this ticket, so I added a TODO comment to address those after we finish moving all packages to main.