From d70693c71163961d6db29df5e8899f7539a7efbf Mon Sep 17 00:00:00 2001 From: Tom Watkins Date: Fri, 7 Mar 2025 16:27:05 +0000 Subject: [PATCH 1/2] Added validation to data class --- .../controllers/v1/TransactionsController.kt | 2 +- .../models/hmpps/TransactionTransferRequest.kt | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/TransactionsController.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/TransactionsController.kt index 901b1eb99..cdc349d6b 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/TransactionsController.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/TransactionsController.kt @@ -302,7 +302,7 @@ class TransactionsController( @Parameter(description = "The ID of the prison that holds the account") @PathVariable prisonId: String, @Parameter(description = "The HMPPS ID of the person") @PathVariable hmppsId: String, @RequestAttribute filters: ConsumerFilters?, - @RequestBody transactionTransferRequest: TransactionTransferRequest, + @Valid @RequestBody transactionTransferRequest: TransactionTransferRequest, ): DataResponse { val response = postTransactionTransferForPersonService.execute(prisonId, hmppsId, transactionTransferRequest, filters) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/models/hmpps/TransactionTransferRequest.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/models/hmpps/TransactionTransferRequest.kt index 5cda79d76..210a9639c 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/models/hmpps/TransactionTransferRequest.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/models/hmpps/TransactionTransferRequest.kt @@ -1,8 +1,10 @@ package uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps import io.swagger.v3.oas.annotations.media.Schema +import jakarta.validation.constraints.NotBlank data class TransactionTransferRequest( + @field:NotBlank(message = "Description must not be blank") @Schema(description = "Description of the transaction.") val description: String, @Schema( @@ -10,12 +12,16 @@ data class TransactionTransferRequest( example = "1234", ) val amount: Int, + @field:NotBlank(message = "Client transaction ID must not be blank") @Schema(description = "Client Transaction Id.") val clientTransactionId: String, + @field:NotBlank(message = "Client unique ref must not be blank") @Schema(description = "A reference unique to the client making the post. Maximum size 64 characters, only alphabetic, numeric, '-' and '_' are allowed.") val clientUniqueRef: String, + @field:NotBlank(message = "From account must not be blank") @Schema(description = "The account to move money from. Must be 'spends'.", example = "spends") val fromAccount: String, + @field:NotBlank(message = "To account must not be blank") @Schema(description = "The account to move money to. Must be 'savings'.", example = "savings") val toAccount: String, ) { From bd315e90abdd27811e889f243d06736020b24cb8 Mon Sep 17 00:00:00 2001 From: Tom Watkins Date: Fri, 7 Mar 2025 16:42:17 +0000 Subject: [PATCH 2/2] Add test for validation --- .../v1/TransactionsControllerTest.kt | 198 ++++++++++-------- 1 file changed, 106 insertions(+), 92 deletions(-) diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/TransactionsControllerTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/TransactionsControllerTest.kt index f2e523e2f..e53f4c28f 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/TransactionsControllerTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/TransactionsControllerTest.kt @@ -53,8 +53,6 @@ class TransactionsControllerTest( val basePath = "/v1/prison/$prisonId/prisoners/$hmppsId" val transactionsPath = "$basePath/accounts/$accountCode/transactions" val transactionPath = "$basePath/transactions/$clientUniqueRef" - val postTransactionPath = "$basePath/transactions" - val postTransactionTransferPath = "$postTransactionPath/transfer" val mockMvc = IntegrationAPIMockMvc(springMockMvc) val type = "CANT" @@ -64,7 +62,6 @@ class TransactionsControllerTest( val postClientUniqueRef = "CLIENT121131-0_11" val fromAccount = "spends" val toAccount = "savings" - val exampleTransactionTransfer = TransactionTransferRequest(description, amount, clientTransactionId, postClientUniqueRef, fromAccount, toAccount) val transactions = Transactions( @@ -225,6 +222,7 @@ class TransactionsControllerTest( } describe("POST transaction") { + val postTransactionPath = "$basePath/transactions" val exampleTransaction = TransactionRequest(type, description, amount, clientTransactionId, postClientUniqueRef) it("returns a response with a transaction ID") { @@ -327,117 +325,133 @@ class TransactionsControllerTest( } } - // Post transaction transfer + describe("POST transaction/transfer") { + val postTransactionTransferPath = "$basePath/transactions/transfer" + val exampleTransactionTransfer = TransactionTransferRequest(description, amount, clientTransactionId, postClientUniqueRef, fromAccount, toAccount) - it("returns a response with a transaction ID, debit and credit transaction IDs") { - whenever( - postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, null), - ).thenReturn( - Response(transactionTransferCreateResponse), - ) + it("returns a response with a transaction ID, debit and credit transaction IDs") { + whenever( + postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, null), + ).thenReturn( + Response(transactionTransferCreateResponse), + ) - val result = mockMvc.performAuthorisedPost(postTransactionTransferPath, exampleTransactionTransfer) + val result = mockMvc.performAuthorisedPost(postTransactionTransferPath, exampleTransactionTransfer) - result.response.contentAsString.shouldContain( - """ - { - "data": { - "debitTransactionId": "6179604-1", - "creditTransactionId": "6179604-1", - "transactionId": "6179604" + result.response.contentAsString.shouldContain( + """ + { + "data": { + "debitTransactionId": "6179604-1", + "creditTransactionId": "6179604-1", + "transactionId": "6179604" + } } - } - """.removeWhitespaceAndNewlines(), - ) - } + """.removeWhitespaceAndNewlines(), + ) + } - it("POST transfer - returns a 200 status code when successful") { - whenever( - postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, null), - ).thenReturn( - Response(transactionTransferCreateResponse), - ) + it("returns a 200 status code when successful") { + whenever( + postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, null), + ).thenReturn( + Response(transactionTransferCreateResponse), + ) - val result = mockMvc.performAuthorisedPost(postTransactionTransferPath, exampleTransactionTransfer) + val result = mockMvc.performAuthorisedPost(postTransactionTransferPath, exampleTransactionTransfer) - result.response.status.shouldBe(HttpStatus.OK.value()) - } + result.response.status.shouldBe(HttpStatus.OK.value()) + } - // Prison filter rejection test - it("POST transfer - returns 400 if prison filter is not matched") { - whenever( - postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, ConsumerFilters(prisons = listOf("XYZ"))), - ).thenReturn( - Response( - data = null, - errors = - listOf( - UpstreamApiError( - causedBy = UpstreamApi.NOMIS, - type = UpstreamApiError.Type.BAD_REQUEST, + it("returns 400 if prison filter is not matched") { + whenever( + postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, ConsumerFilters(prisons = listOf("XYZ"))), + ).thenReturn( + Response( + data = null, + errors = + listOf( + UpstreamApiError( + causedBy = UpstreamApi.NOMIS, + type = UpstreamApiError.Type.BAD_REQUEST, + ), ), - ), - ), - ) + ), + ) - val result = mockMvc.performAuthorisedPostWithCN(postTransactionTransferPath, "limited-prisons", exampleTransactionTransfer) + val result = mockMvc.performAuthorisedPostWithCN(postTransactionTransferPath, "limited-prisons", exampleTransactionTransfer) - result.response.status.shouldBe(HttpStatus.BAD_REQUEST.value()) - } + result.response.status.shouldBe(HttpStatus.BAD_REQUEST.value()) + } - it("POST transfers - calls the API with the correct filters") { - whenever( - postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, ConsumerFilters(prisons = listOf("XYZ"))), - ).thenReturn( - Response(transactionTransferCreateResponse), - ) + it("returns 400 if request body validation does not succeed") { + val invalidTransactionTransfer = TransactionTransferRequest(description = "", amount, clientTransactionId = "", clientUniqueRef = "", fromAccount = "", toAccount = "") + whenever( + postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, null), + ).thenReturn( + Response(transactionTransferCreateResponse), + ) + + val result = mockMvc.performAuthorisedPost(postTransactionTransferPath, invalidTransactionTransfer) + result.response.run { + status.shouldBe(HttpStatus.BAD_REQUEST.value()) + contentAsJson().validationErrors.shouldContainAll("Description must not be blank", "Client transaction ID must not be blank", "Client unique ref must not be blank", "From account must not be blank", "To account must not be blank") + } + } - val result = mockMvc.performAuthorisedPostWithCN(postTransactionTransferPath, "limited-prisons", exampleTransactionTransfer) + it("calls the API with the correct filters") { + whenever( + postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, ConsumerFilters(prisons = listOf("XYZ"))), + ).thenReturn( + Response(transactionTransferCreateResponse), + ) - result.response.status.shouldBe(HttpStatus.OK.value()) - } + val result = mockMvc.performAuthorisedPostWithCN(postTransactionTransferPath, "limited-prisons", exampleTransactionTransfer) - // bad request - it("POST transfer - returns a 400 BAD REQUEST status code when there is an invalid HMPPS ID or incorrect prison, or an invalid request body") { - whenever( - postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, null), - ).thenReturn( - Response( - data = null, - errors = - listOf( - UpstreamApiError( - causedBy = UpstreamApi.NOMIS, - type = UpstreamApiError.Type.BAD_REQUEST, + result.response.status.shouldBe(HttpStatus.OK.value()) + } + + it("returns a 400 BAD REQUEST status code when there is an invalid HMPPS ID or incorrect prison, or an invalid request body") { + whenever( + postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, null), + ).thenReturn( + Response( + data = null, + errors = + listOf( + UpstreamApiError( + causedBy = UpstreamApi.NOMIS, + type = UpstreamApiError.Type.BAD_REQUEST, + ), ), - ), - ), - ) + ), + ) - val result = mockMvc.performAuthorisedPost(postTransactionTransferPath, exampleTransactionTransfer) + val result = mockMvc.performAuthorisedPost(postTransactionTransferPath, exampleTransactionTransfer) - result.response.status.shouldBe(HttpStatus.BAD_REQUEST.value()) - } + result.response.status.shouldBe(HttpStatus.BAD_REQUEST.value()) + } - it("POST transfer - returns a 409 CONFLICT status code when there is a duplicate transaction requested") { - whenever( - postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, null), - ).thenReturn( - Response( - data = null, - errors = - listOf( - UpstreamApiError( - causedBy = UpstreamApi.NOMIS, - type = UpstreamApiError.Type.CONFLICT, + it("returns a 409 CONFLICT status code when there is a duplicate transaction requested") { + whenever( + postTransactionTransferForPersonService.execute(prisonId, hmppsId, exampleTransactionTransfer, null), + ).thenReturn( + Response( + data = null, + errors = + listOf( + UpstreamApiError( + causedBy = UpstreamApi.NOMIS, + type = UpstreamApiError.Type.CONFLICT, + ), ), - ), - ), - ) + ), + ) - val result = mockMvc.performAuthorisedPost(postTransactionTransferPath, exampleTransactionTransfer) + val result = mockMvc.performAuthorisedPost(postTransactionTransferPath, exampleTransactionTransfer) - result.response.status.shouldBe(HttpStatus.CONFLICT.value()) + result.response.status.shouldBe(HttpStatus.CONFLICT.value()) + } } }, )