diff --git a/app/uk/gov/hmrc/vatapi/connectors/ObligationsConnector.scala b/app/uk/gov/hmrc/vatapi/connectors/ObligationsConnector.scala index 6cc3c840..dc771547 100644 --- a/app/uk/gov/hmrc/vatapi/connectors/ObligationsConnector.scala +++ b/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) } } diff --git a/app/uk/gov/hmrc/vatapi/models/Errors.scala b/app/uk/gov/hmrc/vatapi/models/Errors.scala index 07df219d..7fcf4061 100644 --- a/app/uk/gov/hmrc/vatapi/models/Errors.scala +++ b/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") diff --git a/app/uk/gov/hmrc/vatapi/models/ObligationsQueryParams.scala b/app/uk/gov/hmrc/vatapi/models/ObligationsQueryParams.scala index 634e3e96..29fe20ed 100644 --- a/app/uk/gov/hmrc/vatapi/models/ObligationsQueryParams.scala +++ b/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) } diff --git a/app/uk/gov/hmrc/vatapi/resources/wrappers/ObligationsResponse.scala b/app/uk/gov/hmrc/vatapi/resources/wrappers/ObligationsResponse.scala index c69a6d33..461a46a2 100644 --- a/app/uk/gov/hmrc/vatapi/resources/wrappers/ObligationsResponse.scala +++ b/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)) } diff --git a/func/uk/gov/hmrc/vatapi/resources/ObligationsResourceSpec.scala b/func/uk/gov/hmrc/vatapi/resources/ObligationsResourceSpec.scala index e5a033fc..dd0df888 100644 --- a/func/uk/gov/hmrc/vatapi/resources/ObligationsResourceSpec.scala +++ b/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,12 +119,12 @@ 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 @@ -131,7 +132,7 @@ class ObligationsResourceSpec extends BaseFunctionalSpec { .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") diff --git a/func/uk/gov/hmrc/vatapi/resources/SetXContentTypeOptionsFilterSpec.scala b/func/uk/gov/hmrc/vatapi/resources/SetXContentTypeOptionsFilterSpec.scala index 3ddb9aad..efbfdfe7 100644 --- a/func/uk/gov/hmrc/vatapi/resources/SetXContentTypeOptionsFilterSpec.scala +++ b/func/uk/gov/hmrc/vatapi/resources/SetXContentTypeOptionsFilterSpec.scala @@ -25,7 +25,7 @@ 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 @@ -33,16 +33,27 @@ class SetXContentTypeOptionsFilterSpec extends BaseFunctionalSpec { .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) diff --git a/test/uk/gov/hmrc/vatapi/connectors/ObligationsConnectorSpec.scala b/test/uk/gov/hmrc/vatapi/connectors/ObligationsConnectorSpec.scala index a3c8b371..225dbfec 100644 --- a/test/uk/gov/hmrc/vatapi/connectors/ObligationsConnectorSpec.scala +++ b/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) diff --git a/test/uk/gov/hmrc/vatapi/models/ObligationsQueryParamsSpec.scala b/test/uk/gov/hmrc/vatapi/models/ObligationsQueryParamsSpec.scala index d9dc1d84..1f8f250e 100644 --- a/test/uk/gov/hmrc/vatapi/models/ObligationsQueryParamsSpec.scala +++ b/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 diff --git a/test/uk/gov/hmrc/vatapi/resources/ObligationsResourceSpec.scala b/test/uk/gov/hmrc/vatapi/resources/ObligationsResourceSpec.scala index 301f5767..124ec190 100644 --- a/test/uk/gov/hmrc/vatapi/resources/ObligationsResourceSpec.scala +++ b/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)))