From 7f6521e84717b7bb76b2175a286a58e5a74efcb0 Mon Sep 17 00:00:00 2001 From: Richard Dowsett <180321016+ridow-hmrc@users.noreply.github.com> Date: Mon, 23 Dec 2024 10:03:54 +0000 Subject: [PATCH] [PIL-1363-nil-return] Fix nil return response (#45) * [PIL-1363-nil-return] Fix nil return response * [PIL-1363-nil-return] PR refinement --- .../gov/hmrc/pillar2stubs/config/Module.scala | 6 +- .../controllers/UKTRSubmitController.scala | 96 +++++++++++-------- .../pillar2stubs/models/UKTRSubmission.scala | 8 +- .../liabilities/LiabilitySuccessResponse.json | 7 -- .../liabilities/NilReturnSuccessResponse.json | 6 -- .../UKTRSubmitControllerSpec.scala | 38 ++++---- 6 files changed, 85 insertions(+), 76 deletions(-) delete mode 100644 conf/resources/liabilities/LiabilitySuccessResponse.json delete mode 100644 conf/resources/liabilities/NilReturnSuccessResponse.json diff --git a/app/uk/gov/hmrc/pillar2stubs/config/Module.scala b/app/uk/gov/hmrc/pillar2stubs/config/Module.scala index 0260b7c..4586b68 100755 --- a/app/uk/gov/hmrc/pillar2stubs/config/Module.scala +++ b/app/uk/gov/hmrc/pillar2stubs/config/Module.scala @@ -18,8 +18,12 @@ package uk.gov.hmrc.pillar2stubs.config import com.google.inject.AbstractModule +import java.time.{Clock, ZoneId} + class Module extends AbstractModule { - override def configure(): Unit = + override def configure(): Unit = { bind(classOf[AppConfig]).asEagerSingleton() + bind(classOf[Clock]).toInstance(Clock.system(ZoneId.of("Z"))) + } } diff --git a/app/uk/gov/hmrc/pillar2stubs/controllers/UKTRSubmitController.scala b/app/uk/gov/hmrc/pillar2stubs/controllers/UKTRSubmitController.scala index 9b78b23..43d413c 100644 --- a/app/uk/gov/hmrc/pillar2stubs/controllers/UKTRSubmitController.scala +++ b/app/uk/gov/hmrc/pillar2stubs/controllers/UKTRSubmitController.scala @@ -17,23 +17,24 @@ package uk.gov.hmrc.pillar2stubs.controllers import play.api.Logging -import play.api.libs.json.{JsError, JsSuccess, Json} +import play.api.libs.json.{JsError, JsObject, JsSuccess, Json} import play.api.mvc.{Action, ControllerComponents} import uk.gov.hmrc.pillar2stubs.controllers.actions.AuthActionFilter import uk.gov.hmrc.pillar2stubs.models.{UKTRSubmission, UKTRSubmissionData, UKTRSubmissionNilReturn} -import uk.gov.hmrc.pillar2stubs.utils.ResourceHelper.resourceAsString import uk.gov.hmrc.play.bootstrap.backend.controller.BackendController -import java.time.{LocalDate, ZoneId, ZonedDateTime} +import java.time.format.DateTimeFormatter +import java.time.{Clock, ZoneId, ZonedDateTime} import javax.inject.{Inject, Singleton} import scala.concurrent.Future import scala.util.{Failure, Success, Try} @Singleton class UKTRSubmitController @Inject() ( - cc: ControllerComponents, - authFilter: AuthActionFilter -) extends BackendController(cc) + cc: ControllerComponents, + authFilter: AuthActionFilter +)(implicit clock: Clock) + extends BackendController(cc) with Logging { def submitUKTR: Action[String] = (Action(parse.tolerantText) andThen authFilter).async { implicit request => @@ -45,41 +46,51 @@ class UKTRSubmitController @Inject() ( case Success(json) => json.validate[UKTRSubmission] match { case JsSuccess(submission, _) => - submission match { - case d @ UKTRSubmissionData(_, _, _, _, _) => - if (!d.isValid) { - logger.error("Invalid date range: accountingPeriodTo is before accountingPeriodFrom") - Future.successful( - UnprocessableEntity( - Json.obj( - "errors" -> Json.obj( - "processingDate" -> ZonedDateTime.now(ZoneId.of("UTC")), - "code" -> "001", - "text" -> "Invalid date range: accountingPeriodTo must be after accountingPeriodFrom" + if (!submission.accountingPeriodValid) { + logger.error("Invalid date range: accountingPeriodTo is before accountingPeriodFrom") + Future.successful( + UnprocessableEntity( + Json.obj( + "errors" -> Json.obj( + "processingDate" -> ZonedDateTime.now(clock).format(DateTimeFormatter.ISO_INSTANT), + "code" -> "001", + "text" -> "Invalid date range: accountingPeriodTo must be after accountingPeriodFrom" + ) + ) + ) + ) + } else { + submission match { + case d @ UKTRSubmissionData(_, _, _, _, _) => + if (d.liabilities.liableEntities.isEmpty) { + logger.error("Liable entities array is empty in liability submission") + Future.successful( + UnprocessableEntity( + Json.obj( + "errors" -> Json.obj( + "processingDate" -> ZonedDateTime.now(ZoneId.of("UTC")).format(DateTimeFormatter.ISO_INSTANT), + "code" -> "002", + "text" -> "Liable entities must not be empty" + ) ) ) ) + } else { + logger.info("Returning success response for liability request") + Future.successful( + Created( + successfulResponse + ) + ) + } + case UKTRSubmissionNilReturn(_, _, _, _, _) => + logger.info("Returning success response for Nil return request") + Future.successful( + Created( + successfulResponse + ) ) - } else if (d.liabilities.liableEntities.isEmpty) { - logger.error("Liable entities array is empty in liability submission") - Future.successful(BadRequest(Json.obj("error" -> "liableEntities must not be empty")).as("application/json")) - } else { - val liabilitySuccessResponse = resourceAsString("/resources/liabilities/LiabilitySuccessResponse.json") - .map(replaceDate(_, LocalDate.now().toString + "T09:26:17Z")) - .map(Json.parse) - .getOrElse(Json.obj("error" -> "Success response not found")) - - logger.info("Returning success response for liability request") - Future.successful(Created(liabilitySuccessResponse).as("application/json")) - } - case UKTRSubmissionNilReturn(_, _, _, _, _) => - val nilReturnResponse = resourceAsString("/resources/liabilities/NilReturnSuccessResponse.json") - .map(replaceDate(_, LocalDate.now().toString + "T09:26:17Z")) - .map(Json.parse) - .getOrElse(Json.obj("error" -> "Nil return response not found")) - - logger.info("Returning success response for Nil return request") - Future.successful(Created(nilReturnResponse).as("application/json")) + } } case JsError(errors) => @@ -97,7 +108,12 @@ class UKTRSubmitController @Inject() ( Future.successful(BadRequest(Json.obj("error" -> "Invalid JSON data")).as("application/json")) } } - - private def replaceDate(response: String, registrationDate: String): String = - response.replace("2022-01-31T09:26:17Z", registrationDate) + private def successfulResponse(implicit clock: Clock): JsObject = Json.obj( + "success" -> Json + .obj( + "processingDate" -> ZonedDateTime.now(clock).format(DateTimeFormatter.ISO_INSTANT), + "formBundleNumber" -> "119000004320", + "chargeReference" -> "XTC01234123412" + ) + ) } diff --git a/app/uk/gov/hmrc/pillar2stubs/models/UKTRSubmission.scala b/app/uk/gov/hmrc/pillar2stubs/models/UKTRSubmission.scala index 8845abb..6770e51 100644 --- a/app/uk/gov/hmrc/pillar2stubs/models/UKTRSubmission.scala +++ b/app/uk/gov/hmrc/pillar2stubs/models/UKTRSubmission.scala @@ -26,6 +26,9 @@ sealed trait UKTRSubmission { val obligationMTT: Boolean val electionUKGAAP: Boolean val liabilities: Liability + + def accountingPeriodValid: Boolean = + accountingPeriodFrom.isBefore(accountingPeriodTo) } case class UKTRSubmissionData( @@ -34,10 +37,7 @@ case class UKTRSubmissionData( obligationMTT: Boolean, electionUKGAAP: Boolean, liabilities: LiabilityData -) extends UKTRSubmission { - def isValid: Boolean = - accountingPeriodFrom.isBefore(accountingPeriodTo) -} +) extends UKTRSubmission {} object UKTRSubmissionData { implicit val uktrSubmissionDataFormat: OFormat[UKTRSubmissionData] = Json.format[UKTRSubmissionData] diff --git a/conf/resources/liabilities/LiabilitySuccessResponse.json b/conf/resources/liabilities/LiabilitySuccessResponse.json deleted file mode 100644 index 8c68c56..0000000 --- a/conf/resources/liabilities/LiabilitySuccessResponse.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "success": { - "processingDate": "2022-01-31T09:26:17Z", - "formBundleNumber": "119000004320", - "chargeReference": "XTC01234123412" - } -} \ No newline at end of file diff --git a/conf/resources/liabilities/NilReturnSuccessResponse.json b/conf/resources/liabilities/NilReturnSuccessResponse.json deleted file mode 100644 index ce48fad..0000000 --- a/conf/resources/liabilities/NilReturnSuccessResponse.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "success": { - "processingDate": "2022-01-31T09:26:17Z", - "message": "Nil return received and processed successfully" - } -} \ No newline at end of file diff --git a/test/uk/gov/hmrc/pillar2stubs/controllers/UKTRSubmitControllerSpec.scala b/test/uk/gov/hmrc/pillar2stubs/controllers/UKTRSubmitControllerSpec.scala index 63dddf1..bacf588 100644 --- a/test/uk/gov/hmrc/pillar2stubs/controllers/UKTRSubmitControllerSpec.scala +++ b/test/uk/gov/hmrc/pillar2stubs/controllers/UKTRSubmitControllerSpec.scala @@ -30,7 +30,7 @@ import play.api.test.Helpers._ import uk.gov.hmrc.http.HeaderNames import uk.gov.hmrc.pillar2stubs.models.{LiabilityData, LiableEntity, UKTRSubmissionData} -import java.time.LocalDate +import java.time._ class UKTRSubmitControllerSpec extends AnyFreeSpec with Matchers with GuiceOneAppPerSuite with OptionValues { @@ -42,9 +42,12 @@ class UKTRSubmitControllerSpec extends AnyFreeSpec with Matchers with GuiceOneAp case _ => None } + val cogsworth = Clock.fixed(Instant.ofEpochMilli(1734699180110L), ZoneId.of("Z")) + override def fakeApplication(): Application = new GuiceApplicationBuilder() .overrides( - bind[String => Option[String]].qualifiedWith("resourceLoader").toInstance(stubResourceLoader) + bind[String => Option[String]].qualifiedWith("resourceLoader").toInstance(stubResourceLoader), + bind[Clock].toInstance(cogsworth) ) .build() @@ -76,12 +79,12 @@ class UKTRSubmitControllerSpec extends AnyFreeSpec with Matchers with GuiceOneAp .withHeaders("X-Pillar2-Id" -> "XTC01234123412") .withBody(Json.toJson(validLiabilityRequestBody)) - val result = route(app, request).value - val currentDate = LocalDate.now().toString + val result = route(app, request).value status(result) mustBe CREATED - contentAsJson(result) mustBe Json.parse( - s"""{"success":{"processingDate":"${currentDate}T09:26:17Z","formBundleNumber":"119000004320","chargeReference":"XTC01234123412"}}""" - ) + val jsonResult = contentAsJson(result) + (jsonResult \ "success" \ "formBundleNumber").as[String] mustEqual "119000004320" + (jsonResult \ "success" \ "chargeReference").as[String] mustEqual "XTC01234123412" + (jsonResult \ "success" \ "processingDate").as[ZonedDateTime] mustEqual ZonedDateTime.now(cogsworth) } "return CREATED with success response for a valid NIL_RETURN submission" in { @@ -102,12 +105,12 @@ class UKTRSubmitControllerSpec extends AnyFreeSpec with Matchers with GuiceOneAp .withHeaders("X-Pillar2-Id" -> "XTC01234123412") .withBody(validNilReturnRequestBody) - val result = route(app, request).value - val currentDate = LocalDate.now().toString + val result = route(app, request).value status(result) mustBe CREATED - contentAsJson(result) mustBe Json.parse( - s"""{"success":{"processingDate":"${currentDate}T09:26:17Z","message":"Nil return received and processed successfully"}}""" - ) + val jsonResult = contentAsJson(result) + (jsonResult \ "success" \ "formBundleNumber").as[String] mustEqual "119000004320" + (jsonResult \ "success" \ "chargeReference").as[String] mustEqual "XTC01234123412" + (jsonResult \ "success" \ "processingDate").as[ZonedDateTime] mustEqual ZonedDateTime.now(cogsworth) } "return BAD_REQUEST for invalid JSON structure" in { @@ -269,12 +272,11 @@ class UKTRSubmitControllerSpec extends AnyFreeSpec with Matchers with GuiceOneAp .withHeaders("X-Pillar2-Id" -> "XTC01234123412") .withBody(extraFieldsRequestBody) - val result = route(app, request).value - val currentDate = LocalDate.now().toString - status(result) mustBe CREATED - contentAsJson(result) mustBe Json.parse( - s"""{"success":{"processingDate":"${currentDate}T09:26:17Z","formBundleNumber":"119000004320","chargeReference":"XTC01234123412"}}""" - ) + val result = route(app, request).value + val jsonResult = contentAsJson(result) + (jsonResult \ "success" \ "formBundleNumber").as[String] mustEqual "119000004320" + (jsonResult \ "success" \ "chargeReference").as[String] mustEqual "XTC01234123412" + (jsonResult \ "success" \ "processingDate").as[ZonedDateTime] mustEqual ZonedDateTime.now(cogsworth) } "return UNPROCESSABLE_ENTITY if accountingPeriodTo is before accountingPeriodFrom" in {