From e65d3f76d374d0c98d9cc4e9b3c936d8089004b6 Mon Sep 17 00:00:00 2001 From: Alex Rimmer <31283208+AlexRimmerHMRC@users.noreply.github.com> Date: Thu, 2 May 2024 16:49:21 +0100 Subject: [PATCH] ITSASU-3081 - Update sign up API to handle and return an alternate response for 422 status --- app/controllers/SignUpController.scala | 6 +- app/models/SignUpResponse.scala | 10 ++- app/parsers/SignUpParser.scala | 18 ++++- .../SignUpTaxYearConnectorISpec.scala | 79 +++++++++++++++++-- .../controllers/SignUpControllerISpec.scala | 4 +- .../helpers/IntegrationTestConstants.scala | 8 +- test/controllers/SignUpControllerSpec.scala | 20 ++++- test/parsers/SignUpParserSpec.scala | 43 ++++++++-- 8 files changed, 160 insertions(+), 28 deletions(-) diff --git a/app/controllers/SignUpController.scala b/app/controllers/SignUpController.scala index d7c993fc..54c52dd9 100644 --- a/app/controllers/SignUpController.scala +++ b/app/controllers/SignUpController.scala @@ -20,6 +20,7 @@ package controllers import common.Extractors import config.AppConfig import connectors.SignUpTaxYearConnector +import models.SignUpResponse.{AlreadySignedUp, SignUpSuccess} import models.monitoring.{RegistrationFailureAudit, RegistrationSuccessAudit} import play.api.libs.json.Json import play.api.mvc._ @@ -53,12 +54,15 @@ class SignUpController @Inject()(authService: AuthService, private def signUp(nino: String, taxYear: String, agentReferenceNumber: Option[String])(implicit request: Request[AnyContent]) = signUpTaxYearConnector.signUp(nino, taxYear).map { - case Right(response) => + case Right(response: SignUpSuccess) => val path: Option[String] = request.headers.get(ITSASessionKeys.RequestURI) auditService.audit(RegistrationSuccessAudit( agentReferenceNumber, nino, response.mtdbsa, appConfig.signUpServiceAuthorisationToken, path )) Ok(Json.toJson(response)) + case Right(AlreadySignedUp) => + logger.warn(s"[SignUpController][signUp] - sign up returned customer already signed up") + UnprocessableEntity("Customer already signed up") case Left(error) => logger.error(s"Error processing Sign up request with status ${error.status} and message ${error.reason}") auditService.audit(RegistrationFailureAudit(nino, error.status, error.reason)) diff --git a/app/models/SignUpResponse.scala b/app/models/SignUpResponse.scala index ef6fa7da..6a1022e3 100644 --- a/app/models/SignUpResponse.scala +++ b/app/models/SignUpResponse.scala @@ -18,8 +18,14 @@ package models import play.api.libs.json.{Json, OFormat} -case class SignUpResponse(mtdbsa: String) +sealed trait SignUpResponse object SignUpResponse { - implicit val format: OFormat[SignUpResponse] = Json.format[SignUpResponse] + + case class SignUpSuccess(mtdbsa: String) extends SignUpResponse + + case object AlreadySignedUp extends SignUpResponse + + implicit val format: OFormat[SignUpSuccess] = Json.format[SignUpSuccess] + } diff --git a/app/parsers/SignUpParser.scala b/app/parsers/SignUpParser.scala index e3f7ebd3..87b388e2 100644 --- a/app/parsers/SignUpParser.scala +++ b/app/parsers/SignUpParser.scala @@ -16,23 +16,33 @@ package parsers +import models.SignUpResponse.SignUpSuccess import models.{ErrorModel, SignUpResponse} -import play.api.http.Status.OK +import play.api.http.Status.{OK, UNPROCESSABLE_ENTITY} +import play.api.libs.json.{JsError, JsSuccess} import uk.gov.hmrc.http.{HttpReads, HttpResponse} object SignUpParser { + type PostSignUpResponse = Either[ErrorModel, SignUpResponse] implicit val signUpResponseHttpReads: HttpReads[PostSignUpResponse] = { (_: String, _: String, response: HttpResponse) => response.status match { - case OK => response.json.asOpt[SignUpResponse] match { - case Some(successResponse) => Right(successResponse) - case None => Left(ErrorModel(OK, "Failed to read Json for MTD Sign Up Response")) + case OK => response.json.validate[SignUpSuccess] match { + case JsSuccess(value, _) => Right(value) + case JsError(_) => Left(ErrorModel(OK, s"Failed to read Json for MTD Sign Up Response")) + } + case UNPROCESSABLE_ENTITY => (response.json \ "failures" \\ "code").map(_.validate[String]).headOption match { + case Some(JsSuccess(CustomerAlreadySignedUp, _)) => Right(SignUpResponse.AlreadySignedUp) + case Some(JsSuccess(code, _)) => Left(ErrorModel(UNPROCESSABLE_ENTITY, code)) + case _ => Left(ErrorModel(UNPROCESSABLE_ENTITY, s"Failed to read Json for MTD Sign Up Response")) } case status => Left(ErrorModel(status, response.body)) } } + private val CustomerAlreadySignedUp: String = "CUSTOMER_ALREADY_SIGNED_UP" + } diff --git a/it/test/connectors/SignUpTaxYearConnectorISpec.scala b/it/test/connectors/SignUpTaxYearConnectorISpec.scala index 6fc2e0f8..6266ccc5 100644 --- a/it/test/connectors/SignUpTaxYearConnectorISpec.scala +++ b/it/test/connectors/SignUpTaxYearConnectorISpec.scala @@ -20,8 +20,10 @@ import config.MicroserviceAppConfig import helpers.ComponentSpecBase import helpers.IntegrationTestConstants._ import helpers.servicemocks.SignUpTaxYearStub -import models.{ErrorModel, SignUpResponse} +import models.ErrorModel +import models.SignUpResponse.{AlreadySignedUp, SignUpSuccess} import play.api.http.Status._ +import play.api.libs.json.Json import play.api.mvc.Request import play.api.test.FakeRequest @@ -35,35 +37,96 @@ class SignUpTaxYearConnectorISpec extends ComponentSpecBase { "The sign up tax year connector" when { "receiving a 200 response" should { "return a valid MTDBSA number when valid json is found" in { - SignUpTaxYearStub.stubSignUp(testTaxYearSignUpSubmission(testNino, testTaxYear), appConfig.signUpServiceAuthorisationToken, appConfig.signUpServiceEnvironment)( + SignUpTaxYearStub.stubSignUp( + testTaxYearSignUpSubmission(testNino, testTaxYear), + appConfig.signUpServiceAuthorisationToken, + appConfig.signUpServiceEnvironment + )( OK, testSignUpSuccessBody ) val result = signUpConnector.signUp(testNino, testTaxYear) - result.futureValue shouldBe Right(SignUpResponse("XQIT00000000001")) + result.futureValue shouldBe Right(SignUpSuccess("XQIT00000000001")) } "return a Json parse failure when invalid json is found" in { - SignUpTaxYearStub.stubSignUp(testTaxYearSignUpSubmission(testNino, testTaxYear), appConfig.signUpServiceAuthorisationToken, appConfig.signUpServiceEnvironment)( + SignUpTaxYearStub.stubSignUp( + testTaxYearSignUpSubmission(testNino, testTaxYear), + appConfig.signUpServiceAuthorisationToken, + appConfig.signUpServiceEnvironment + )( OK, testSignUpInvalidBody ) val result = signUpConnector.signUp(testNino, testTaxYear) - result.futureValue shouldBe Left(ErrorModel(status = 200, "Failed to read Json for MTD Sign Up Response")) + result.futureValue shouldBe Left(ErrorModel(status = OK, "Failed to read Json for MTD Sign Up Response")) } } - "receiving a non-200 response" should { + "receiving a 422 response with a customer already signed up code" should { + "return a already signed up result" in { + SignUpTaxYearStub.stubSignUp( + testTaxYearSignUpSubmission(testNino, testTaxYear), + appConfig.signUpServiceAuthorisationToken, + appConfig.signUpServiceEnvironment + )( + status = UNPROCESSABLE_ENTITY, + body = Json.obj("failures" -> Json.arr( + Json.obj("code" -> "CUSTOMER_ALREADY_SIGNED_UP", "reason" -> "The customer is already signed up") + )) + ) + + val result = signUpConnector.signUp(testNino, testTaxYear) + + result.futureValue shouldBe Right(AlreadySignedUp) + } + + "return a Json parse failure when invalid json is found" in { + SignUpTaxYearStub.stubSignUp( + testTaxYearSignUpSubmission(testNino, testTaxYear), + appConfig.signUpServiceAuthorisationToken, + appConfig.signUpServiceEnvironment + )( + UNPROCESSABLE_ENTITY, testSignUpInvalidBody + ) + + val result = signUpConnector.signUp(testNino, testTaxYear) + + result.futureValue shouldBe Left(ErrorModel(status = UNPROCESSABLE_ENTITY, "Failed to read Json for MTD Sign Up Response")) + } + } + + "receiving a 422 response without customer already signed up code" should { + "return the status and error received" in { + SignUpTaxYearStub.stubSignUp( + testTaxYearSignUpSubmission(testNino, testTaxYear), + appConfig.signUpServiceAuthorisationToken, + appConfig.signUpServiceEnvironment + )( + UNPROCESSABLE_ENTITY, failureResponse("code", "reason") + ) + + val result = signUpConnector.signUp(testNino, testTaxYear) + + result.futureValue shouldBe Left(ErrorModel(UNPROCESSABLE_ENTITY, "code")) + } + } + + "receiving a 500 response" should { "return the status and error received" in { - SignUpTaxYearStub.stubSignUp(testTaxYearSignUpSubmission(testNino, testTaxYear), appConfig.signUpServiceAuthorisationToken, appConfig.signUpServiceEnvironment)( + SignUpTaxYearStub.stubSignUp( + testTaxYearSignUpSubmission(testNino, testTaxYear), + appConfig.signUpServiceAuthorisationToken, + appConfig.signUpServiceEnvironment + )( INTERNAL_SERVER_ERROR, failureResponse("code", "reason") ) val result = signUpConnector.signUp(testNino, testTaxYear) - result.futureValue shouldBe Left(ErrorModel(INTERNAL_SERVER_ERROR, """{"code":"code","reason":"reason"}""")) + result.futureValue shouldBe Left(ErrorModel(INTERNAL_SERVER_ERROR, """{"failures":[{"code":"code","reason":"reason"}]}""")) } } } diff --git a/it/test/controllers/SignUpControllerISpec.scala b/it/test/controllers/SignUpControllerISpec.scala index 771cc39e..8c11ffe8 100644 --- a/it/test/controllers/SignUpControllerISpec.scala +++ b/it/test/controllers/SignUpControllerISpec.scala @@ -21,7 +21,7 @@ import config.featureswitch.FeatureSwitching import helpers.ComponentSpecBase import helpers.IntegrationTestConstants._ import helpers.servicemocks.{AuthStub, SignUpTaxYearStub} -import models.SignUpResponse +import models.SignUpResponse.SignUpSuccess import play.api.Configuration import play.api.http.Status._ @@ -47,7 +47,7 @@ class SignUpControllerISpec extends ComponentSpecBase with FeatureSwitching { httpStatus(OK) ) res should have( - jsonBodyAs[SignUpResponse](SignUpResponse("XQIT00000000001")) + jsonBodyAs[SignUpSuccess](SignUpSuccess("XQIT00000000001")) ) } diff --git a/it/test/helpers/IntegrationTestConstants.scala b/it/test/helpers/IntegrationTestConstants.scala index e4ebe4e7..ea5ad58c 100644 --- a/it/test/helpers/IntegrationTestConstants.scala +++ b/it/test/helpers/IntegrationTestConstants.scala @@ -138,8 +138,12 @@ object IntegrationTestConstants extends JsonUtils { def failureResponse(code: String, reason: String): JsValue = s""" |{ - | "code":"$code", - | "reason":"$reason" + | "failures":[ + | { + | "code":"$code", + | "reason":"$reason" + | } + | ] |} """.stripMargin diff --git a/test/controllers/SignUpControllerSpec.scala b/test/controllers/SignUpControllerSpec.scala index 0ea7ad5b..6582b77b 100644 --- a/test/controllers/SignUpControllerSpec.scala +++ b/test/controllers/SignUpControllerSpec.scala @@ -20,10 +20,11 @@ package controllers import common.CommonSpec import config.MicroserviceAppConfig import config.featureswitch.FeatureSwitching +import models.SignUpResponse.SignUpSuccess import models.monitoring.{RegistrationFailureAudit, RegistrationSuccessAudit} import models.{ErrorModel, SignUpResponse} import play.api.Configuration -import play.api.http.Status.{INTERNAL_SERVER_ERROR, OK} +import play.api.http.Status.{INTERNAL_SERVER_ERROR, OK, UNPROCESSABLE_ENTITY} import play.api.libs.json.Json import play.api.mvc.{ControllerComponents, Result} import play.api.test.FakeRequest @@ -64,7 +65,7 @@ class SignUpControllerSpec extends CommonSpec val fakeRequest = FakeRequest().withJsonBody(Json.toJson(testTaxYearSignUpSubmission(testNino, testTaxYear))) mockRetrievalSuccess(Enrolments(Set())) - signUpTaxYear(testNino, testTaxYear)(Future.successful(Right(SignUpResponse("XAIT000000")))) + signUpTaxYear(testNino, testTaxYear)(Future.successful(Right(SignUpSuccess("XAIT000000")))) val result: Future[Result] = TestController.signUp(testNino, testTaxYear)(fakeRequest) @@ -78,7 +79,7 @@ class SignUpControllerSpec extends CommonSpec val fakeRequest = FakeRequest().withJsonBody(Json.toJson(testTaxYearSignUpSubmission(testNino, testTaxYear))) mockRetrievalSuccess(Enrolments(Set(Enrolment(hmrcAsAgent, Seq(EnrolmentIdentifier("AgentReferenceNumber", "123456789")), "Activated")))) - signUpTaxYear(testNino, testTaxYear)(Future.successful(Right(SignUpResponse("XAIT000000")))) + signUpTaxYear(testNino, testTaxYear)(Future.successful(Right(SignUpSuccess("XAIT000000")))) val result: Future[Result] = TestController.signUp(testNino, testTaxYear)(fakeRequest) @@ -90,6 +91,19 @@ class SignUpControllerSpec extends CommonSpec } } } + "return UnprocessableEntity" when { + "the customer is already signed up" in { + val fakeRequest = FakeRequest() + + mockRetrievalSuccess(Enrolments(Set())) + signUpTaxYear(testNino, testTaxYear)(Future.successful(Right(SignUpResponse.AlreadySignedUp))) + + val result: Future[Result] = TestController.signUp(testNino, testTaxYear)(fakeRequest) + + status(result) shouldBe UNPROCESSABLE_ENTITY + contentAsString(result) shouldBe "Customer already signed up" + } + } "return InternalServerError" when { "sign up was unsuccessful and an error was returned" in { val fakeRequest = FakeRequest().withJsonBody(Json.toJson(testTaxYearSignUpSubmission(testNino, testTaxYear))) diff --git a/test/parsers/SignUpParserSpec.scala b/test/parsers/SignUpParserSpec.scala index 81163849..764f2ada 100644 --- a/test/parsers/SignUpParserSpec.scala +++ b/test/parsers/SignUpParserSpec.scala @@ -17,7 +17,8 @@ package parsers import common.CommonSpec -import models.{ErrorModel, SignUpResponse} +import models.ErrorModel +import models.SignUpResponse.{AlreadySignedUp, SignUpSuccess} import play.api.http.Status._ import play.api.libs.json.Json import uk.gov.hmrc.http.HttpResponse @@ -28,10 +29,10 @@ class SignUpParserSpec extends CommonSpec { "supplied with an OK response" should { - "return a SignUpResponse when the response has valid json" in { - val response = HttpResponse(OK, body = Json.toJson(SignUpResponse("XAIT000000")).toString()) + "return a SignUpSuccess when the response has valid json" in { + val response = HttpResponse(OK, body = Json.toJson(SignUpSuccess("XAIT000000")).toString()) - SignUpParser.signUpResponseHttpReads.read("", "", response) shouldBe Right(SignUpResponse("XAIT000000")) + SignUpParser.signUpResponseHttpReads.read("", "", response) shouldBe Right(SignUpSuccess("XAIT000000")) } "return a SignUpFailure when the response has invalid json" in { @@ -46,12 +47,42 @@ class SignUpParserSpec extends CommonSpec { } } - "supplied with a non-OK response" should { + "supplied with a Unprocessable entity response" should { + "return a already signed up when the response has a customer already signed up code" in { + val response = HttpResponse( + status = UNPROCESSABLE_ENTITY, + body = Json.obj("failures" -> Json.arr( + Json.obj("code" -> "CUSTOMER_ALREADY_SIGNED_UP") + )).toString() + ) + + SignUpParser.signUpResponseHttpReads.read("", "", response) shouldBe Right(AlreadySignedUp) + } + + "return a sign up failure when the response hs not got a customer already signed up code" in { + val response = HttpResponse( + status = UNPROCESSABLE_ENTITY, + body = Json.obj("failures" -> Json.arr(Json.obj("code" -> "OTHER_CODE"))).toString() + ) + + SignUpParser.signUpResponseHttpReads.read("", "", response) shouldBe + Left(ErrorModel(UNPROCESSABLE_ENTITY, "OTHER_CODE")) + } + + "return a sign up failure when the response json has no code" in { + val response = HttpResponse(UNPROCESSABLE_ENTITY, body = Json.obj().toString()) + + SignUpParser.signUpResponseHttpReads.read("", "", response) shouldBe + Left(ErrorModel(UNPROCESSABLE_ENTITY, s"Failed to read Json for MTD Sign Up Response")) + } + } + + "supplied with a non OK or Unprocessable entity response" should { "return a SignUpFailure with the response message" in { val response = HttpResponse(INTERNAL_SERVER_ERROR, body = "Error body") - SignUpParser.signUpResponseHttpReads.read("", "", response) shouldBe Left(ErrorModel(INTERNAL_SERVER_ERROR, "Error body")) + SignUpParser.signUpResponseHttpReads.read("", "", response) shouldBe Left(ErrorModel(INTERNAL_SERVER_ERROR, "Error body")) } } }