Merge '#149' SAR-4578: Updated the service and connectors to return M…

…TDITID

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
+}

app/connectors/PropertyConnector.scala

@@ -17,38 +17,35 @@
 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)
       .copy(authorization = Some(Authorization(appConfig.desToken)))
 
     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
-
+}

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}")
+      }
+  }
+
+}

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)
 
 }

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,19 +15,20 @@ 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
       }
     }
 
     "business subscription errors " should {
       "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)
         }

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)
         }
       }
     }
   }
+}

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)
   }
 

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)

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)
   }
 

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 {