MTDSA-1677 VAT Obligations - Making STATUS parameter optional, correc…

…t validation and remove reference to STATUS = A (#152)

app/uk/gov/hmrc/vatapi/connectors/ObligationsConnector.scala

@@ -41,8 +41,6 @@ trait ObligationsConnector extends BaseConnector {
   def get(vrn: Vrn, queryParams: ObligationsQueryParams)(implicit hc: HeaderCarrier): Future[ObligationsResponse] = {
     logger.debug(s"[ObligationsConnector][get] Retrieve obligations for VRN $vrn with the given query parameters.")
 
-    val queryString = s"from=${queryParams.from}&to=${queryParams.to}&status=${queryParams.status}"
-
-    httpGet[ObligationsResponse](baseUrl + s"/enterprise/obligation-data/vrn/$vrn/VATC?$queryString", ObligationsResponse)
+    httpGet[ObligationsResponse](baseUrl + s"/enterprise/obligation-data/vrn/$vrn/VATC?${queryParams.queryString}", ObligationsResponse)
   }
 }

app/uk/gov/hmrc/vatapi/models/Errors.scala

@@ -71,6 +71,7 @@ object Errors {
   object DuplicateVatSubmission extends Error("DUPLICATE_SUBMISSION", "The VAT return was already submitted for the given period.", None)
   object ClientOrAgentNotAuthorized extends Error("CLIENT_OR_AGENT_NOT_AUTHORISED", "The client and/or agent is not authorised.", None)
   object InvalidData extends Error("INVALID_DATA", "The provided data is failed validation, contains invalid data", None)
+  object InvalidStatus extends Error("INVALID_STATUS", "The provided data is failed validation, invalid status", None)
 
   def badRequest(validationErrors: JsonValidationErrors) = BadRequest(flattenValidationErrors(validationErrors), "Invalid request")
   def badRequest(error: Error) = BadRequest(Seq(error), "Invalid request")

app/uk/gov/hmrc/vatapi/models/ObligationsQueryParams.scala

@@ -21,13 +21,21 @@ import org.joda.time.LocalDate
 
 import scala.util.Try
 
-case class ObligationsQueryParams(from: LocalDate, to: LocalDate, status: String) {
+case class ObligationsQueryParams(from: LocalDate, to: LocalDate, status: Option[String] = None) {
   val map = Map("from" -> from, "to" -> to, "status" -> status)
+
+  def queryString: String ={
+    status match {
+      case None | Some("") => s"from=$from&to=$to"
+      case Some(status) => s"from=$from&to=$to&status=$status"
+
+    }
+  }
 }
 
 object ObligationsQueryParams {
   val dateRegex = """^\d{4}-\d{2}-\d{2}$"""
-  val statusRegex = "^[OFA]$"
+  val statusRegex = "^[OF]$"
 
   def from(fromOpt: OptEither[String], toOpt: OptEither[String], statusOpt: OptEither[String]): Either[String, ObligationsQueryParams] = {
     val from = dateQueryParam(fromOpt, "INVALID_DATE_FROM")
@@ -41,7 +49,7 @@ object ObligationsQueryParams {
     } yield param.left.get
 
     if (errors.isEmpty) {
-      Right(ObligationsQueryParams(from.map(_.right.get).get, to.map(_.right.get).get, status.map(_.right.get).get))
+      Right(ObligationsQueryParams(from.map(_.right.get).get, to.map(_.right.get).get, status.map(_.right.get)))
     } else {
       Left(errors.head)
     }
@@ -70,7 +78,7 @@ object ObligationsQueryParams {
           Right(status)
         else
           Left(errorCode)
-      case None => Left(errorCode)
+      case None => Right("")
     }
     Some(paramValue)
   }

app/uk/gov/hmrc/vatapi/resources/wrappers/ObligationsResponse.scala

@@ -71,7 +71,8 @@ case class ObligationsResponse(underlying: HttpResponse) extends Response {
     case 400 if errorCodeIsOneOf(INVALID_DATE_TO) => BadRequest(toJson(Errors.InvalidDateTo))
     case 400 if errorCodeIsOneOf(INVALID_DATE_FROM) => BadRequest(toJson(Errors.InvalidDateFrom))
     case 400 if errorCodeIsOneOf(INVALID_DATE_RANGE) => BadRequest(toJson(Errors.DateRangeTooLarge))
-    case 400 if errorCodeIsOneOf(INVALID_IDTYPE, INVALID_STATUS, INVALID_REGIME, NOT_FOUND_BPKEY) =>
+    case 400 if errorCodeIsOneOf(INVALID_STATUS) => BadRequest(toJson(Errors.InvalidStatus))
+    case 400 if errorCodeIsOneOf(INVALID_IDTYPE, INVALID_REGIME, NOT_FOUND_BPKEY) =>
       InternalServerError(toJson(Errors.InternalServerError))
     case 404 if errorCodeIsOneOf(NOT_FOUND) => NotFound(toJson(Errors.NotFound))
   }

func/uk/gov/hmrc/vatapi/resources/ObligationsResourceSpec.scala

@@ -12,7 +12,17 @@ class ObligationsResourceSpec extends BaseFunctionalSpec {
       given()
         .stubAudit
       when()
-        .get(s"/abc/obligations?from=2017-01-01&to=2017-03-31&status=A")
+        .get(s"/abc/obligations?from=2017-01-01&to=2017-03-31")
+        .thenAssertThat()
+        .statusIs(BAD_REQUEST)
+        .isBadRequest("VRN_INVALID")
+    }
+
+    "return code 400 when status is A" in {
+      given()
+        .stubAudit
+      when()
+        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-03-31&status=A")
         .thenAssertThat()
         .statusIs(BAD_REQUEST)
         .isBadRequest("VRN_INVALID")
@@ -54,15 +64,6 @@ class ObligationsResourceSpec extends BaseFunctionalSpec {
         .statusIs(BAD_REQUEST)
     }
 
-    "return code 400 when status is missing" in {
-      given()
-        .stubAudit
-      when()
-        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-03-31")
-        .thenAssertThat()
-        .statusIs(BAD_REQUEST)
-    }
-
     "return code 400 when status is invalid" in {
       given()
         .stubAudit
@@ -107,7 +108,7 @@ class ObligationsResourceSpec extends BaseFunctionalSpec {
         .userIsFullyAuthorisedForTheResource
         .des().obligations.obligationNotFoundFor(vrn)
         .when()
-        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31&status=A")
+        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31")
         .thenAssertThat()
         .statusIs(NOT_FOUND)
     }
@@ -118,20 +119,20 @@ class ObligationsResourceSpec extends BaseFunctionalSpec {
         .userIsFullyAuthorisedForTheResource
         .des().obligations.obligationParamsFor(vrn, Errors.invalidRegime)
         .when()
-        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31&status=A")
+        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31")
         .thenAssertThat()
         .statusIs(INTERNAL_SERVER_ERROR)
     }
 
-    "return code 500 when status parameter is invalid" in {
+    "return code 400 when status parameter is invalid" in {
       given()
         .stubAudit
         .userIsFullyAuthorisedForTheResource
         .des().obligations.obligationParamsFor(vrn, Errors.invalidStatus)
         .when()
         .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31&status=A")
         .thenAssertThat()
-        .statusIs(INTERNAL_SERVER_ERROR)
+        .statusIs(BAD_REQUEST)
     }
 
     "return code 500 when idType parameter is invalid" in {
@@ -140,7 +141,7 @@ class ObligationsResourceSpec extends BaseFunctionalSpec {
         .userIsFullyAuthorisedForTheResource
         .des().obligations.obligationParamsFor(vrn, Errors.invalidIdType)
         .when()
-        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31&status=A")
+        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31")
         .thenAssertThat()
         .statusIs(INTERNAL_SERVER_ERROR)
     }
@@ -151,7 +152,7 @@ class ObligationsResourceSpec extends BaseFunctionalSpec {
         .userIsFullyAuthorisedForTheResource
         .des().obligations.returnObligationsFor(vrn)
         .when()
-        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31&status=A")
+        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31")
         .thenAssertThat()
         .statusIs(OK)
         .bodyIsLike(Jsons.Obligations(firstMet = "F").toString)
@@ -163,7 +164,7 @@ class ObligationsResourceSpec extends BaseFunctionalSpec {
         .userIsFullyAuthorisedForTheResource
         .des().obligations.returnObligationsWithoutIdentificationFor(vrn)
         .when()
-        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31&status=A")
+        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31")
         .thenAssertThat()
         .statusIs(OK)
         .bodyIsLike(Jsons.Obligations(firstMet = "F").toString)
@@ -175,7 +176,7 @@ class ObligationsResourceSpec extends BaseFunctionalSpec {
         .userIsFullyAuthorisedForTheResource
         .des().obligations.returnObligationsWithIdentificationButNoIncomeSourceTypeFor(vrn)
         .when()
-        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31&status=A")
+        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31")
         .thenAssertThat()
         .statusIs(OK)
         .bodyIsLike(Jsons.Obligations(firstMet = "F").toString)
@@ -186,7 +187,7 @@ class ObligationsResourceSpec extends BaseFunctionalSpec {
         .stubAudit
         .userIsNotAuthorisedForTheResource
         .when()
-        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31&status=A")
+        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31")
         .thenAssertThat()
         .statusIs(FORBIDDEN)
         .bodyHasPath("\\code", "CLIENT_OR_AGENT_NOT_AUTHORISED")

func/uk/gov/hmrc/vatapi/resources/SetXContentTypeOptionsFilterSpec.scala

@@ -25,24 +25,35 @@ class SetXContentTypeOptionsFilterSpec extends BaseFunctionalSpec {
         .responseContainsHeader(SetXContentTypeOptionsFilter.xContentTypeOptionsHeader, "nosniff".r)
     }
 
-    "be applied for obligations " in {
+    "be applied for obligations with status A" in {
       given()
         .stubAudit
         .userIsFullyAuthorisedForTheResource
         .des().obligations.returnObligationsFor(vrn)
         .when()
         .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31&status=A")
         .thenAssertThat()
-        .statusIs(200)
+        .statusIs(400)
         .responseContainsHeader(SetXContentTypeOptionsFilter.xContentTypeOptionsHeader, "nosniff".r)
     }
 
+    "be applied for obligations " in {
+      given()
+        .stubAudit
+        .userIsFullyAuthorisedForTheResource
+        .des().obligations.returnObligationsFor(vrn)
+        .when()
+        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31")
+        .thenAssertThat()
+        .statusIs(200)
+        .responseContainsHeader(SetXContentTypeOptionsFilter.xContentTypeOptionsHeader, "nosniff".r)
+    }
 
     "be applied when vrn is invalid" in {
       given()
         .stubAudit
         .when()
-        .get(s"/abc/obligations?from=2017-01-01&to=2017-03-31&status=A")
+        .get(s"/abc/obligations?from=2017-01-01&to=2017-03-31&status=O")
         .thenAssertThat()
         .statusIs(400)
         .responseContainsHeader(SetXContentTypeOptionsFilter.xContentTypeOptionsHeader, "nosniff".r)
@@ -55,7 +66,7 @@ class SetXContentTypeOptionsFilterSpec extends BaseFunctionalSpec {
         .userIsFullyAuthorisedForTheResource
         .des().obligations.obligationNotFoundFor(vrn)
         .when()
-        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31&status=A")
+        .get(s"/$vrn/obligations?from=2017-01-01&to=2017-08-31&status=O")
         .thenAssertThat()
         .statusIs(404)
         .responseContainsHeader(SetXContentTypeOptionsFilter.xContentTypeOptionsHeader, "nosniff".r)

test/uk/gov/hmrc/vatapi/connectors/ObligationsConnectorSpec.scala

@@ -50,8 +50,8 @@ class ObligationsConnectorSpec extends UnitSpec with OneAppPerSuite
   lazy val desBaseUrl = "des-base-url"
 
   val vrn: Vrn = generateVrn
-  val queryParams = ObligationsQueryParams(now.minusDays(7), now, "O")
-  val queryString = s"from=${queryParams.from}&to=${queryParams.to}&status=${queryParams.status}"
+  val queryParams = ObligationsQueryParams(now.minusDays(7), now, Some("O"))
+  val queryString = queryParams.queryString
   val desUrl = s"$desBaseUrl/enterprise/obligation-data/vrn/$vrn/VATC?$queryString"
   val desObligationsJson: JsValue = Jsons.Obligations.desResponse(vrn)
 

test/uk/gov/hmrc/vatapi/models/ObligationsQueryParamsSpec.scala

@@ -48,8 +48,8 @@ class ObligationsQueryParamsSpec extends UnitSpec {
 
     "return error when the status query parameter is missing" in {
       val response = ObligationsQueryParams.from(Some(Right("2017-01-01")), Some(Right("2017-03-31")), None)
-      response.isLeft shouldBe true
-      response.left.get shouldBe "INVALID_STATUS"
+      response.isRight shouldBe true
+      response.right.get.from shouldEqual LocalDate.parse("2017-01-01")
     }
 
     "return error when the status query parameter is not a valid status" in {
@@ -58,6 +58,12 @@ class ObligationsQueryParamsSpec extends UnitSpec {
       response.left.get shouldBe "INVALID_STATUS"
     }
 
+    "return error when the status query parameter is A" in {
+      val response = ObligationsQueryParams.from(Some(Right("2017-01-01")), Some(Right("2017-03-31")), Some(Right("A")))
+      response.isLeft shouldBe true
+      response.left.get shouldBe "INVALID_STATUS"
+    }
+
     "return error when from date is after to date " in {
       val response = ObligationsQueryParams.from(Some(Right("2017-01-02")), Some(Right("2017-01-01")), Some(Right("F")))
       response.isLeft shouldBe true

test/uk/gov/hmrc/vatapi/resources/ObligationsResourceSpec.scala

@@ -47,7 +47,8 @@ class ObligationsResourceSpec extends ResourceSpec
       .returns(BusinessResult.success(Future.successful(())))
   }
 
-  val queryParams = ObligationsQueryParams(now.minusDays(7), now, "O")
+  val queryParams = ObligationsQueryParams(now.minusDays(7), now, Some("O"))
+  val queryParamsWithNoStatus = ObligationsQueryParams(now.minusDays(7), now)
   val desObligationsJson: JsValue = Jsons.Obligations.desResponse(vrn)
   val desObligationsNoDetailsJson: JsValue = Jsons.Obligations.desResponseWithoutObligationDetails(vrn)
   val clientObligationsJson: JsValue = Jsons.Obligations()
@@ -67,6 +68,20 @@ class ObligationsResourceSpec extends ResourceSpec
       }
     }
 
+    "return a 200 and the correct obligations json for no status" when {
+      "DES returns a 200 response with the correct obligations body" in new Setup {
+        val desResponse = ObligationsResponse(HttpResponse(200, Some(desObligationsJson)))
+
+        MockObligationsConnector.get(vrn, queryParamsWithNoStatus)
+          .returns(Future.successful(desResponse))
+
+        val result = testObligationResource.retrieveObligations(vrn, queryParamsWithNoStatus)(FakeRequest())
+        status(result) shouldBe 200
+        contentType(result) shouldBe Some(MimeTypes.JSON)
+        contentAsJson(result) shouldBe clientObligationsJson
+      }
+    }
+
     "return a 404 with no body" when {
       "DES returns a 200 response with the correct obligations body but the obligationDetails are empty" in new Setup {
         val desResponse = ObligationsResponse(HttpResponse(200, Some(desObligationsNoDetailsJson)))