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.

Sign up for GitHub

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

Jump to bottom

PRMT-3411 #25

Open

MohammadIqbalAD-NHS wants to merge 6 commits into main from PRMT-3411

Contributor

MohammadIqbalAD-NHS commented Jun 23, 2023

No description provided.

joefong-nhs and others added 5 commits

June 19, 2023 09:45


          temp save jdbc connection poc

59542aa


          use JOOQ for querying db

00baaed


          refactor test postgresdb connection

85b65aa


          Merge branch 'PRMT-3459-jooq' into PRMT-3411

8bf727f


          [PRMT-3411] Added E2E tests for expecting acknowledgments to be store…

f59bd5a

…d in ehr-out DB

MohammadIqbalAD-NHS requested review from martin-nhs, AndyFlintNHS and joefong-nhs

June 23, 2023 12:48

martin-nhs reviewed

View reviewed changes

src/test/java/uk/nhs/prm/deduction/e2e/TestConfiguration.java Outdated

Comment on lines 101 to 102

		String port = "5432";
		return "jdbc:postgresql://" + hostname + ":" + port + "/" + dbname + "?user=" + username + "&password=" + password;

Contributor

martin-nhs Jun 23, 2023

I would say the port probably doesn't need a variable of it's own!

Suggested change

      
                    String port = "5432";
          
                    return "jdbc:postgresql://" + hostname + ":" + port + "/" + dbname + "?user=" + username + "&password=" + password;
          
                    return "jdbc:postgresql://" + hostname + ":5432/"+ dbname + "?user=" + username + "&password=" + password;

martin-nhs reviewed

View reviewed changes

src/test/java/uk/nhs/prm/deduction/e2e/tests/EhrOutE2ETests.java Outdated

Comment on lines 99 to 100

		// change transfer db status to ACTION:EHR_REQUEST_SENT before putting on inbound queue
		// Put the patient into inboundQueueFromMhs as a UK05 message

Contributor

martin-nhs Jun 26, 2023

Can we remove these comments please?

martin-nhs reviewed

View reviewed changes

src/test/java/uk/nhs/prm/deduction/e2e/tests/EhrOutE2ETests.java

Comment on lines +131 to +141

+                      String inboundConversationId = UUID.randomUUID().toString();
+                      String outboundConversationId = UUID.randomUUID().toString();
+                      LOGGER.info(" ===============  outboundConversationId: {}", outboundConversationId);
+                      String largeEhrCoreMessageId = UUID.randomUUID().toString();
+                      String fragment1MessageId = UUID.randomUUID().toString();
+                      String fragment2MessageId = UUID.randomUUID().toString();
+                      String nhsNumberForTestPatient = "9727018157";
+                      String previousGpForTestPatient = "N82668";
+                      String newGpForTestPatient = "M85019";

Contributor

martin-nhs Jun 26, 2023

Just popping this in as a note to the discussion we just had - would be beneficial to identify areas that are repeated throughout tests and abstract them out into small POJO's - the builder pattern would be great here!

martin-nhs reviewed

View reviewed changes

src/test/java/uk/nhs/prm/deduction/e2e/tests/EhrOutE2ETests.java Outdated

Comment on lines 66 to 75

+                  @Autowired
+                  public EhrOutE2ETests(
+                          TrackerDb trackerDb,
+                          Gp2gpMessengerQueue gp2gpMessengerQueue,
+                          TestConfiguration config
+                  ) {
+                      this.trackerDb = trackerDb;
+                      this.gp2gpMessengerQueue = gp2gpMessengerQueue;
+                      this.config = config;
+                  }

Contributor

martin-nhs Jun 26, 2023

You can substitute this for the following:

Suggested change

      
                @Autowired
          
                public EhrOutE2ETests(
          
                        TrackerDb trackerDb,
          
                        Gp2gpMessengerQueue gp2gpMessengerQueue,
          
                        TestConfiguration config
          
                ) {
          
                    this.trackerDb = trackerDb;
          
                    this.gp2gpMessengerQueue = gp2gpMessengerQueue;
          
                    this.config = config;
          
                }
          
            @AllArgsConstructor(onConstructor = @__(@Autowired))

at class level!

martin-nhs reviewed

View reviewed changes

src/test/java/uk/nhs/prm/deduction/e2e/tests/EhrOutE2ETests.java

Comment on lines +85 to +90

+                      String inboundConversationId = UUID.randomUUID().toString();
+                      String smallEhrMessageId = UUID.randomUUID().toString();
+                      String outboundConversationId = UUID.randomUUID().toString();
+                      String nhsNumberForTestPatient = "9727018440";
+                      String previousGpForTestPatient = "M85019";
+                      String asidCodeForTestPatient = "200000000149";

Contributor

martin-nhs Jun 26, 2023

Just popping this in as a note to the discussion we just had - would be beneficial to identify areas that are repeated throughout tests and abstract them out into small POJO's - the builder pattern would be great here!

martin-nhs reviewed

View reviewed changes

src/test/java/uk/nhs/prm/deduction/e2e/tests/EhrOutE2ETests.java

+                      String asidCodeForTestPatient = "200000000149";
+                      LOGGER.info(" ===============  outboundConversationId: {}", outboundConversationId);
+                      SimpleAmqpQueue inboundQueueFromMhs = new SimpleAmqpQueue(config);

Contributor

martin-nhs Jun 26, 2023

Can we move this to an instance variable? :)

Suggested change

      
                    SimpleAmqpQueue inboundQueueFromMhs = new SimpleAmqpQueue(config);
          
            private static final SimpleAmqpQueue MHS_INBOUND_QUEUE = new SimpleAmqpQueue(config);

martin-nhs reviewed

View reviewed changes

src/test/java/uk/nhs/prm/deduction/e2e/queue/QueueMessageHelper.java Outdated

Comment on lines 108 to 114

    
                  private SqsMessage findMessageContaining(String substring) {

                      List<SqsMessage>  allMessages = thinlyWrappedSqsClient.readThroughMessages(this.queueUri, 180);

                      for (SqsMessage message : allMessages) {

                          log(String.format("just finding message, checking conversationId: %s", this.queueUri));

                          log(String.format("Finding message with substring %s on queue: %s", substring, this.queueUri));

                          if (message.contains(substring)) {

                              return message;

                          }

Contributor

martin-nhs Jun 26, 2023

    private SqsMessage findMessageContaining(String substring) {
        return thinlyWrappedSqsClient.readThroughMessages(this.queueUri, 180)
                .stream()
                .filter(message -> message.contains(substring))
                .findFirst()
                .orElseThrow(RuntimeException::new);
    }


          [PRMT-3411] Addressed some pull request comments

0d4b532

MohammadIqbalAD-NHS commented

View reviewed changes

build.gradle

Comment on lines +38 to +41

+                  testImplementation 'org.jooq:jooq-codegen:3.13.4'
+                  testImplementation 'org.jooq:jooq-meta:3.13.4'
+                  testImplementation 'org.jooq:jooq:3.13.4'
+                  testImplementation 'org.postgresql:postgresql:42.5.4'

Contributor Author

MohammadIqbalAD-NHS Aug 15, 2023

Reminder: We want to remove these dependencies as we no longer use JOOQ.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet