diff --git a/app/connectors/BusinessConnector.scala b/app/connectors/BusinessConnector.scala index c66244f5..7a346856 100644 --- a/app/connectors/BusinessConnector.scala +++ b/app/connectors/BusinessConnector.scala @@ -19,21 +19,20 @@ package connectors import config.MicroserviceAppConfig import javax.inject.{Inject, Singleton} import models.subscription.incomesource.BusinessIncomeModel +import parsers.MtditIdParser.MtditIdHttpReads import play.api.libs.json.Writes -import uk.gov.hmrc.http.HeaderCarrier -import uk.gov.hmrc.http.HttpReads.readRaw import uk.gov.hmrc.http.logging.Authorization +import uk.gov.hmrc.http.{HeaderCarrier, HttpReads} import uk.gov.hmrc.play.bootstrap.http.HttpClient -import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.{ExecutionContext, Future} @Singleton class BusinessConnector @Inject()(val http: HttpClient, - val appConfig: MicroserviceAppConfig) { + val appConfig: MicroserviceAppConfig)(implicit ec: ExecutionContext) { def businessSubscribe(nino: String, businessIncomeModel: BusinessIncomeModel) - (implicit hc: HeaderCarrier): Future[BusinessIncomeSubscriptionSuccess.type] = { + (implicit hc: HeaderCarrier): Future[String] = { val headerCarrier = hc .withExtraHeaders("Environment" -> appConfig.desEnvironment) @@ -44,11 +43,9 @@ class BusinessConnector @Inject()(val http: HttpClient, body = businessIncomeModel )( implicitly[Writes[BusinessIncomeModel]], - readRaw, + implicitly[HttpReads[String]], headerCarrier, implicitly[ExecutionContext] - ) map (_ => BusinessIncomeSubscriptionSuccess) + ) } -} - -case object BusinessIncomeSubscriptionSuccess +} \ No newline at end of file diff --git a/app/connectors/PropertyConnector.scala b/app/connectors/PropertyConnector.scala index 36ee2669..20531488 100644 --- a/app/connectors/PropertyConnector.scala +++ b/app/connectors/PropertyConnector.scala @@ -17,22 +17,22 @@ package connectors import config.MicroserviceAppConfig +import parsers.MtditIdParser.MtditIdHttpReads import javax.inject.{Inject, Singleton} -import play.api.libs.json.{JsObject, Json, Writes} -import uk.gov.hmrc.http.HeaderCarrier -import uk.gov.hmrc.http.HttpReads.readRaw +import models.subscription.incomesource.PropertyIncomeModel +import play.api.libs.json.Writes import uk.gov.hmrc.http.logging.Authorization +import uk.gov.hmrc.http.{HeaderCarrier, HttpReads} import uk.gov.hmrc.play.bootstrap.http.HttpClient -import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.{ExecutionContext, Future} @Singleton class PropertyConnector @Inject()(val http: HttpClient, - val appConfig: MicroserviceAppConfig) { + val appConfig: MicroserviceAppConfig)(implicit ec: ExecutionContext) { - def propertySubscribe(nino: String) - (implicit hc: HeaderCarrier): Future[PropertyIncomeSubscriptionSuccess.type] = { + def propertySubscribe(nino: String, propertyIncomeModel: PropertyIncomeModel) + (implicit hc: HeaderCarrier): Future[String] = { val headerCarrier = hc .withExtraHeaders("Environment" -> appConfig.desEnvironment) @@ -40,15 +40,12 @@ class PropertyConnector @Inject()(val http: HttpClient, http.POST( url = appConfig.propertySubscribeUrl(nino), - body = Json.obj() + body = propertyIncomeModel )( - implicitly[Writes[JsObject]], - readRaw, + implicitly[Writes[PropertyIncomeModel]], + implicitly[HttpReads[String]], headerCarrier, implicitly[ExecutionContext] - ) map (_ => PropertyIncomeSubscriptionSuccess) + ) } -} - -case object PropertyIncomeSubscriptionSuccess - +} \ No newline at end of file diff --git a/app/parsers/MtditIdParser.scala b/app/parsers/MtditIdParser.scala new file mode 100644 index 00000000..6de8a591 --- /dev/null +++ b/app/parsers/MtditIdParser.scala @@ -0,0 +1,39 @@ +/* + * Copyright 2019 HM Revenue & Customs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package parsers + +import play.api.http.Status.OK +import play.api.libs.json.JsSuccess +import uk.gov.hmrc.http.{HttpReads, HttpResponse, InternalServerException} + +object MtditIdParser { + + implicit object MtditIdHttpReads extends HttpReads[String] { + override def read(method: String, url: String, response: HttpResponse): String = + response.status match { + case OK => + (response.json \ "mtditId").validate[String] match { + case JsSuccess(mtditId, _) => + mtditId + case _ => throw new InternalServerException("MTDITID missing from DES response") + } + case status => + throw new InternalServerException(s"DES returned $status with response ${response.body}") + } + } + +} diff --git a/app/services/SubmissionOrchestrationService.scala b/app/services/SubmissionOrchestrationService.scala index 6975c77e..1a0f26f0 100644 --- a/app/services/SubmissionOrchestrationService.scala +++ b/app/services/SubmissionOrchestrationService.scala @@ -19,8 +19,8 @@ package services import connectors.{BusinessConnector, PropertyConnector, RegistrationConnector} import javax.inject.{Inject, Singleton} import models.subscription.incomesource.SignUpRequest -import services.SubmissionOrchestrationService._ -import uk.gov.hmrc.http.HeaderCarrier +import services.SubmissionOrchestrationService.{NoSubmissionNeeded, _} +import uk.gov.hmrc.http.{HeaderCarrier, InternalServerException} import scala.concurrent.{ExecutionContext, Future} @@ -30,25 +30,33 @@ class SubmissionOrchestrationService @Inject()(registrationConnector: Registrati propertyConnector: PropertyConnector )(implicit ec: ExecutionContext) { - def submit(signUpRequest: SignUpRequest)(implicit hc: HeaderCarrier): Future[SuccessfulSubmission.type] = + + def submit(signUpRequest: SignUpRequest)(implicit hc: HeaderCarrier): Future[SuccessfulSubmission] = for { _ <- registrationConnector.register(signUpRequest.nino, signUpRequest.isAgent) - _ <- signUpRequest.businessIncome match { + businessResponse <- signUpRequest.businessIncome match { case Some(business) => businessConnector.businessSubscribe(signUpRequest.nino, business) case None => Future.successful(NoSubmissionNeeded) } - _ <- signUpRequest.propertyIncome match { - case Some(_) => propertyConnector.propertySubscribe(signUpRequest.nino) + propertyResponse <- signUpRequest.propertyIncome match { + case Some(property) => propertyConnector.propertySubscribe(signUpRequest.nino, property) case None => Future.successful(NoSubmissionNeeded) } - } yield SuccessfulSubmission - + } yield + (businessResponse, propertyResponse) match { + case (businessSuccess: String, _) => + SuccessfulSubmission(businessSuccess) + case (_, propertySuccess: String) => + SuccessfulSubmission(propertySuccess) + case _ => + throw new InternalServerException("Unexpected error - income type missing") + } } object SubmissionOrchestrationService { case object NoSubmissionNeeded - case object SuccessfulSubmission + case class SuccessfulSubmission(mtditId: String) } \ No newline at end of file diff --git a/it/connectors/BusinessConnectorISpec.scala b/it/connectors/BusinessConnectorISpec.scala index 1ab0cf95..80f78698 100644 --- a/it/connectors/BusinessConnectorISpec.scala +++ b/it/connectors/BusinessConnectorISpec.scala @@ -2,11 +2,11 @@ package connectors import helpers.ComponentSpecBase -import helpers.IntegrationTestConstants.{testBusinessIncomeModel, testNino} +import helpers.IntegrationTestConstants._ import helpers.servicemocks.BusinessSubscriptionStub.stubBusinessIncomeSubscription import play.api.http.Status._ import play.api.libs.json.Json -import uk.gov.hmrc.http.Upstream5xxResponse +import uk.gov.hmrc.http.InternalServerException class BusinessConnectorISpec extends ComponentSpecBase { @@ -15,11 +15,12 @@ class BusinessConnectorISpec extends ComponentSpecBase { "business connector" when { "business subscription returns a successful response" should { "return businessSubscriptionPayload" in { - stubBusinessIncomeSubscription(testNino, testBusinessIncomeModel)(OK, Json.obj()) + stubBusinessIncomeSubscription(testNino, testBusinessIncomeModel)(OK, Json.obj( + "mtditId" -> testMtditId)) val res = BusinessConnector.businessSubscribe(testNino, testBusinessIncomeModel) - await(res) shouldBe BusinessIncomeSubscriptionSuccess + await(res) shouldBe testMtditId } } @@ -27,7 +28,7 @@ class BusinessConnectorISpec extends ComponentSpecBase { "throw an Exception where the nino number doesn't exist" in { stubBusinessIncomeSubscription(testNino, testBusinessIncomeModel)(INTERNAL_SERVER_ERROR, Json.obj()) - intercept[Upstream5xxResponse] { + intercept[InternalServerException] { val res = BusinessConnector.businessSubscribe(testNino, testBusinessIncomeModel) await(res) } diff --git a/it/connectors/PropertyConnectorISpec.scala b/it/connectors/PropertyConnectorISpec.scala index 61c11295..c5705a93 100644 --- a/it/connectors/PropertyConnectorISpec.scala +++ b/it/connectors/PropertyConnectorISpec.scala @@ -2,36 +2,37 @@ package connectors import helpers.ComponentSpecBase -import helpers.IntegrationTestConstants.testNino +import helpers.IntegrationTestConstants._ import helpers.servicemocks.PropertySubscriptionStub.stubPropertyIncomeSubscription import play.api.http.Status.{INTERNAL_SERVER_ERROR, OK} import play.api.libs.json.Json -import uk.gov.hmrc.http.Upstream5xxResponse +import uk.gov.hmrc.http.InternalServerException class PropertyConnectorISpec extends ComponentSpecBase { - private lazy val PropertyConnector: PropertyConnector = app.injector.instanceOf[PropertyConnector] + private lazy val PropertyConnector: PropertyConnector = app.injector.instanceOf[PropertyConnector] - "property connector" when { - "property subscription returns a successful response" should { - "return propertySubscriptionPayload" in { - stubPropertyIncomeSubscription(testNino)(OK, Json.obj()) + "property connector" when { + "property subscription returns a successful response" should { + "return propertySubscriptionPayload" in { + stubPropertyIncomeSubscription(testNino, testPropertyIncomeModel)(OK, Json.obj( + "mtditId" -> testMtditId)) - val res = PropertyConnector.propertySubscribe(testNino) + val res = PropertyConnector.propertySubscribe(testNino, testPropertyIncomeModel) - await(res) shouldBe PropertyIncomeSubscriptionSuccess - } + await(res) shouldBe testMtditId } + } - "property subscription errors " should { - "throw an exception where the nino number doesn't exist" in { - stubPropertyIncomeSubscription(testNino)(INTERNAL_SERVER_ERROR, Json.obj()) + "property subscription errors " should { + "throw an exception where the nino number doesn't exist" in { + stubPropertyIncomeSubscription(testNino, testPropertyIncomeModel)(INTERNAL_SERVER_ERROR, Json.obj()) - intercept[Upstream5xxResponse] { - val res = PropertyConnector.propertySubscribe(testNino) - await(res) - } + intercept[InternalServerException] { + val res = PropertyConnector.propertySubscribe(testNino, testPropertyIncomeModel) + await(res) } } } } +} diff --git a/it/helpers/servicemocks/PropertySubscriptionStub.scala b/it/helpers/servicemocks/PropertySubscriptionStub.scala index 0400dbdc..887a405f 100644 --- a/it/helpers/servicemocks/PropertySubscriptionStub.scala +++ b/it/helpers/servicemocks/PropertySubscriptionStub.scala @@ -5,12 +5,12 @@ import com.github.tomakehurst.wiremock.stubbing.StubMapping import models.subscription.incomesource.PropertyIncomeModel import play.api.libs.json.JsObject -object PropertySubscriptionStub extends WireMockMethods{ +object PropertySubscriptionStub extends WireMockMethods { private def propertySubscriptionUri(nino: String): String = s"/income-tax-self-assessment/nino/$nino/property" - def stubPropertyIncomeSubscription(nino: String)(status: Int, body: JsObject): StubMapping = { - when(method = POST, uri = propertySubscriptionUri(nino)) + def stubPropertyIncomeSubscription(nino: String, propertyIncomeModel: PropertyIncomeModel)(status: Int, body: JsObject): StubMapping = { + when(method = POST, uri = propertySubscriptionUri(nino), body = propertyIncomeModel) .thenReturn(status, body) } diff --git a/test/connectors/mocks/subscription/MockBusinessConnector.scala b/test/connectors/mocks/subscription/MockBusinessConnector.scala index d7daa89b..a4775922 100644 --- a/test/connectors/mocks/subscription/MockBusinessConnector.scala +++ b/test/connectors/mocks/subscription/MockBusinessConnector.scala @@ -16,7 +16,7 @@ package connectors.mocks.subscription -import connectors.{BusinessConnector, BusinessIncomeSubscriptionSuccess} +import connectors.BusinessConnector import models.subscription.incomesource.BusinessIncomeModel import org.mockito.ArgumentMatchers import org.mockito.Mockito.when @@ -30,7 +30,7 @@ trait MockBusinessConnector extends MockitoSugar { val mockBusinessConnector: BusinessConnector = mock[BusinessConnector] def mockBusinessSubscribe(nino: String, businessIncomeModel: BusinessIncomeModel) - (response: Future[BusinessIncomeSubscriptionSuccess.type]) + (response: Future[String]) (implicit hc: HeaderCarrier): Unit = { when(mockBusinessConnector.businessSubscribe(ArgumentMatchers.eq(nino), ArgumentMatchers.eq(businessIncomeModel))(ArgumentMatchers.any[HeaderCarrier])) .thenReturn(response) diff --git a/test/connectors/mocks/subscription/MockPropertyConnector.scala b/test/connectors/mocks/subscription/MockPropertyConnector.scala index 356469ff..9f91bab4 100644 --- a/test/connectors/mocks/subscription/MockPropertyConnector.scala +++ b/test/connectors/mocks/subscription/MockPropertyConnector.scala @@ -16,7 +16,8 @@ package connectors.mocks.subscription -import connectors.{PropertyConnector, PropertyIncomeSubscriptionSuccess} +import connectors.PropertyConnector +import models.subscription.incomesource.PropertyIncomeModel import org.mockito.ArgumentMatchers import org.mockito.Mockito.when import org.scalatest.mockito.MockitoSugar @@ -28,10 +29,10 @@ trait MockPropertyConnector extends MockitoSugar { val mockPropertyConnector: PropertyConnector = mock[PropertyConnector] - def mockPropertySubscribe(nino: String) - (response: Future[PropertyIncomeSubscriptionSuccess.type]) + def mockPropertySubscribe(nino: String, propertyIncomeModel: PropertyIncomeModel) + (response: Future[String]) (implicit hc: HeaderCarrier): Unit = { - when(mockPropertyConnector.propertySubscribe(ArgumentMatchers.eq(nino))(ArgumentMatchers.any[HeaderCarrier])) + when(mockPropertyConnector.propertySubscribe(ArgumentMatchers.eq(nino), ArgumentMatchers.eq(propertyIncomeModel))(ArgumentMatchers.any[HeaderCarrier])) .thenReturn(response) } diff --git a/test/models/incomesource/SignUpRequestSpec.scala b/test/models/incomesource/SignUpRequestSpec.scala index 70d99157..dd99b7f6 100644 --- a/test/models/incomesource/SignUpRequestSpec.scala +++ b/test/models/incomesource/SignUpRequestSpec.scala @@ -41,7 +41,11 @@ class SignUpRequestSpec extends UnitSpec { accountingMethod = Cash ) - def incomeSource(testArn: Option[String] = None) = SignUpRequest(testNino, testArn, Some(businessIncome), Some(PropertyIncomeModel(None))) + val propertyIncome = PropertyIncomeModel( + accountingMethod = Some(Cash) + ) + + def incomeSource(testArn: Option[String] = None) = SignUpRequest(testNino, testArn, Some(businessIncome), Some(propertyIncome)) def testJson(testArn: Option[String] = None) = Json.obj( "nino" -> testNino, @@ -61,7 +65,9 @@ class SignUpRequestSpec extends UnitSpec { ), "accountingMethod" -> "cash" ), - "propertyIncome" -> Json.obj() + "propertyIncome" -> Json.obj( + "accountingMethod" -> "cash" + ) ) ++ testArn.fold(Json.obj())(arn => Json.obj("arn" -> arn)) "convert to the correct Json" in { diff --git a/test/services/SubmissionOrchestrationServiceSpec.scala b/test/services/SubmissionOrchestrationServiceSpec.scala index bbc22577..d261eb83 100644 --- a/test/services/SubmissionOrchestrationServiceSpec.scala +++ b/test/services/SubmissionOrchestrationServiceSpec.scala @@ -16,13 +16,13 @@ package services -import connectors.{BusinessIncomeSubscriptionSuccess, PropertyIncomeSubscriptionSuccess, RegistrationSuccess} +import connectors.RegistrationSuccess import connectors.mocks.subscription.{MockBusinessConnector, MockPropertyConnector, MockRegistrationConnector} import org.scalatestplus.play.guice.GuiceOneAppPerSuite import services.SubmissionOrchestrationService.SuccessfulSubmission import uk.gov.hmrc.http.HeaderCarrier import uk.gov.hmrc.play.test.UnitSpec -import utils.TestConstants.{testBothIncomeSourceModel, testBusinessIncomeSourceModel, testNino, testPropertyIncomeSourceModel} +import utils.TestConstants._ import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.Future @@ -37,72 +37,74 @@ class SubmissionOrchestrationServiceSpec extends UnitSpec object Service extends SubmissionOrchestrationService(mockRegistrationConnector, mockBusinessConnector, mockPropertyConnector) + val successfulSubmission = SuccessfulSubmission(mtditId = testMtditId) + "submit" should { "register and subscribe a business for an individual" when { "only business income is provided" in { mockRegister(testNino, isAnAgent = false)(Future.successful(RegistrationSuccess)) - mockBusinessSubscribe(testNino, testBusinessIncomeSourceModel.businessIncome.get)(Future.successful(BusinessIncomeSubscriptionSuccess)) + mockBusinessSubscribe(testNino, testBusinessIncomeSourceModel.businessIncome.get)(Future.successful(testMtditId)) val result = Service.submit(testBusinessIncomeSourceModel) - await(result) shouldBe SuccessfulSubmission + await(result) shouldBe successfulSubmission } } "register and subscribe a property for an individual" when { "only property income is provided" in { mockRegister(testNino, isAnAgent = false)(Future.successful(RegistrationSuccess)) - mockPropertySubscribe(testNino)(Future.successful(PropertyIncomeSubscriptionSuccess)) + mockPropertySubscribe(testNino, testPropertyIncomeSourceModel.propertyIncome.get)(Future.successful(testMtditId)) val result = Service.submit(testPropertyIncomeSourceModel) - await(result) shouldBe SuccessfulSubmission + await(result) shouldBe successfulSubmission } } "register and subscribe a business and a property for an individual" when { "both income sources are provided" in { mockRegister(testNino, isAnAgent = false)(Future.successful(RegistrationSuccess)) - mockBusinessSubscribe(testNino, testBusinessIncomeSourceModel.businessIncome.get)(Future.successful(BusinessIncomeSubscriptionSuccess)) - mockPropertySubscribe(testNino)(Future.successful(PropertyIncomeSubscriptionSuccess)) + mockBusinessSubscribe(testNino, testBusinessIncomeSourceModel.businessIncome.get)(Future.successful(testMtditId)) + mockPropertySubscribe(testNino, testPropertyIncomeSourceModel.propertyIncome.get)(Future.successful(testMtditId)) val result = Service.submit(testBothIncomeSourceModel) - await(result) shouldBe SuccessfulSubmission + await(result) shouldBe successfulSubmission } } "register and subscribe a business for an agent" when { "only business income is provided" in { mockRegister(testNino, isAnAgent = true)(Future.successful(RegistrationSuccess)) - mockBusinessSubscribe(testNino, testBusinessIncomeSourceModel.businessIncome.get)(Future.successful(BusinessIncomeSubscriptionSuccess)) + mockBusinessSubscribe(testNino, testBusinessIncomeSourceModel.businessIncome.get)(Future.successful(testMtditId)) val result = Service.submit(testBusinessIncomeSourceModel) - await(result) shouldBe SuccessfulSubmission + await(result) shouldBe successfulSubmission } } "register and subscribe a property for an agent" when { "only property income is provided" in { mockRegister(testNino, isAnAgent = true)(Future.successful(RegistrationSuccess)) - mockPropertySubscribe(testNino)(Future.successful(PropertyIncomeSubscriptionSuccess)) + mockPropertySubscribe(testNino, testPropertyIncomeSourceModel.propertyIncome.get)(Future.successful(testMtditId)) val result = Service.submit(testPropertyIncomeSourceModel) - await(result) shouldBe SuccessfulSubmission + await(result) shouldBe successfulSubmission } } "register and subscribe a business and a property for an agent" when { "both income sources are provided" in { mockRegister(testNino, isAnAgent = true)(Future.successful(RegistrationSuccess)) - mockBusinessSubscribe(testNino, testBusinessIncomeSourceModel.businessIncome.get)(Future.successful(BusinessIncomeSubscriptionSuccess)) - mockPropertySubscribe(testNino)(Future.successful(PropertyIncomeSubscriptionSuccess)) + mockBusinessSubscribe(testNino, testBusinessIncomeSourceModel.businessIncome.get)(Future.successful(testMtditId)) + mockPropertySubscribe(testNino, testPropertyIncomeSourceModel.propertyIncome.get)(Future.successful(testMtditId)) val result = Service.submit(testBothIncomeSourceModel) - await(result) shouldBe SuccessfulSubmission + await(result) shouldBe successfulSubmission } } }