Skip to content

Commit

Permalink
Merge pull request #974 from hmrc/DL-9054a
Browse files Browse the repository at this point in the history
DL-9054a - charge reference validaton and multiple errors fix
  • Loading branch information
rajanimohan22 authored Dec 20, 2022
2 parents a6a00d3 + 1a2a7ec commit bb3718b
Show file tree
Hide file tree
Showing 12 changed files with 365 additions and 375 deletions.
59 changes: 47 additions & 12 deletions app/v1/connectors/httpparsers/FinancialDataHttpParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package v1.connectors.httpparsers

import play.api.http.Status._
import play.api.libs.json.{JsError, JsSuccess, JsValue}
import play.api.libs.json.{JsError, JsSuccess, JsValue, Json}
import uk.gov.hmrc.http.{HttpReads, HttpResponse}
import utils.Logging
import v1.connectors.Outcome
Expand All @@ -30,20 +30,55 @@ object FinancialDataHttpParser extends Logging {
implicit object FinancialDataHttpReads extends HttpReads[Outcome[FinancialDataResponse]] with HttpParser {

//TODO change content after user research
def errorHelper(jsonString: JsValue, status: Int): (MtdError, Option[List[MtdError]]) = {
def errorHelper(jsonString: JsValue): MtdError = {
val financialDataErrors = jsonString.as[FinancialDataErrors]
val mtdErrorsConvert = financialDataErrors.failures.map{ error =>
(error.code, status) match {
case ("INVALID_IDNUMBER", BAD_REQUEST) => FinancialInvalidIdNumber
case ("INVALID_SEARCH_ITEM", BAD_REQUEST) => FinancialInvalidSearchItem
case ("NO_DATA_FOUND", NOT_FOUND) => FinancialNotDataFound
val mtdErrorsConvert = financialDataErrors.failures.map { error =>
(error.code) match {
case ("INVALID_IDNUMBER") => FinancialInvalidIdNumber
case ("INVALID_SEARCH_ITEM") => FinancialInvalidSearchItem
case ("NO_DATA_FOUND") => FinancialNotDataFound
case ("INVALID_CORRELATIONID") => DownstreamError
case ("INVALID_IDTYPE") => DownstreamError
case ("INVALID_REGIME_TYPE") => DownstreamError
case ("INVALID_SEARCH_TYPE") => DownstreamError
case ("INVALID_SEARCH_ITEM") => DownstreamError
case ("INVALID_DATE_FROM") => DownstreamError
case ("INVALID_DATE_TO") => DownstreamError
case ("INVALID_DATE_TYPE") => DownstreamError
case ("INVALID_DATE_RANGE") => DownstreamError
case ("INVALID_INCLUDE_CLEARED_ITEMS") => DownstreamError
case ("INVALID_INCLUDE_STATISTICAL_ITEMS") => DownstreamError
case ("INVALID_INCLUDE_PAYMENT_ON_ACCOUNT") => DownstreamError
case ("INVALID_ADD_REGIME_TOTALISATION") => DownstreamError
case ("INVALID_ADD_LOCK_INFORMATION") => DownstreamError
case ("INVALID_ADD_PENALTY_DETAILS") => DownstreamError
case ("INVALID_ADD_POSTED_INTEREST_DETAILS") => DownstreamError
case ("INVALID_ADD_ACCRUING_INTEREST_DETAILS") => DownstreamError
case ("INVALID_REQUEST") => DownstreamError
case ("INVALID_TARGETED_SEARCH") => DownstreamError
case ("INVALID_SELECTION_CRITERIA") => DownstreamError
case ("INVALID_DATA_ENRICHMENT") => DownstreamError
case ("DUPLICATE_SUBMISSION") => DownstreamError
case ("INVALID_ID") => DownstreamError
case ("INVALID_DOC_NUMBER_OR_CHARGE_REFERENCE_NUMBER") => FinancialInvalidSearchItem
case ("REQUEST_NOT_PROCESSED") => DownstreamError
case ("INVALID_DATA_TYPE") => DownstreamError
case ("INVALID_DATE_RANGE") => DownstreamError
case ("SERVER_ERROR") => DownstreamError
case ("SERVICE_UNAVAILABLE") => DownstreamError
case _ => MtdError(error.code, error.reason)
}
}

val head = mtdErrorsConvert.head
val tail = if(mtdErrorsConvert.tail.isEmpty) None else Some(mtdErrorsConvert.tail)
(head, tail)
val error = if (mtdErrorsConvert.tail.isEmpty) {
head
} else if (mtdErrorsConvert.contains(DownstreamError)) {
DownstreamError
} else {
MtdError("INVALID_REQUEST", "Invalid request financial details", Some(Json.toJson(mtdErrorsConvert)))
}
error
}

//TODO more error handling can be added once scenarios confirmed by Penalties team
Expand All @@ -57,9 +92,9 @@ object FinancialDataHttpParser extends Logging {
Left(ErrorWrapper(responseCorrelationId, InvalidJson))
}
case status =>
val mtdErrors = errorHelper(response.json, status)
logger.error(s"[FinancialDataHttpParser][read] status: ${status} with Error ${mtdErrors._1} ${mtdErrors._2}")
Left(ErrorWrapper(responseCorrelationId, mtdErrors._1, mtdErrors._2))
val mtdErrors = errorHelper(response.json)
logger.error(s"[FinancialDataHttpParser][read] status: ${status} with Error ${mtdErrors}")
Left(ErrorWrapper(responseCorrelationId, mtdErrors))
}
}
}
Expand Down
22 changes: 17 additions & 5 deletions app/v1/connectors/httpparsers/PenaltiesHttpParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,31 @@ object PenaltiesHttpParser extends Logging {
implicit object PenaltiesHttpReads extends HttpReads[Outcome[PenaltiesResponse]] with HttpParser {

//TODO change content after user research
def errorHelper(jsonString: JsValue, status: Int): MtdError = {
def errorHelper(jsonString: JsValue): MtdError = {
val penaltiesErrors = jsonString.as[PenaltiesErrors]
val mtdErrorsConvert = penaltiesErrors.failures.map{ error =>
(error.code, status) match {
case ("INVALID_IDVALUE", BAD_REQUEST) => PenaltiesInvalidIdValue
case ("NO_DATA_FOUND", NOT_FOUND) => PenaltiesNotDataFound
(error.code) match {
case ("INVALID_IDVALUE") => PenaltiesInvalidIdValue
case ("NO_DATA_FOUND") => PenaltiesNotDataFound
case ("INVALID_REGIME") => DownstreamError
case ("INVALID_IDTYPE") => DownstreamError
case ("INVALID_DATELIMIT") => DownstreamError
case ("INVALID_CORRELATIONID") => DownstreamError
case ("DUPLICATE_SUBMISSION") => DownstreamError
case ("INVALID_IDTYPE") => DownstreamError
case ("INVALID_ID") => DownstreamError
case ("REQUEST_NOT_PROCESSED") => DownstreamError
case ("SERVER_ERROR") => DownstreamError
case ("SERVICE_UNAVAILABLE") => DownstreamError
case _ => MtdError(error.code, error.reason)
}
}

val head = mtdErrorsConvert.head
val error = if(mtdErrorsConvert.tail.isEmpty) {
head
} else if(mtdErrorsConvert.contains(DownstreamError)) {
DownstreamError
} else {
MtdError("INVALID_REQUEST", "Invalid request penalties", Some(Json.toJson(mtdErrorsConvert)))
}
Expand All @@ -59,7 +71,7 @@ object PenaltiesHttpParser extends Logging {
Left(ErrorWrapper(responseCorrelationId, InvalidJson))
}
case status =>
val mtdErrors = errorHelper(response.json, status)
val mtdErrors = errorHelper(response.json)
logger.error(s"[PenaltiesHttpParser][read] status: ${status} with Error ${mtdErrors}")
Left(ErrorWrapper(responseCorrelationId, mtdErrors))
}
Expand Down
3 changes: 2 additions & 1 deletion app/v1/controllers/FinancialDataController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ class FinancialDataController @Inject()(val authService: EnrolmentsAuthService,
(errorWrapper.error: @unchecked) match {
case VrnFormatError |
FinancialInvalidIdNumber |
FinancialInvalidSearchItem => BadRequest(Json.toJson(errorWrapper))
FinancialInvalidSearchItem |
MtdError("INVALID_REQUEST", _, _) => BadRequest(Json.toJson(errorWrapper))
case FinancialNotDataFound => NotFound(Json.toJson(errorWrapper))
case _ => InternalServerError(Json.toJson(errorWrapper))
}
Expand Down
3 changes: 1 addition & 2 deletions app/v1/controllers/PenaltiesController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ class PenaltiesController @Inject()(val authService: EnrolmentsAuthService,

private def errorResult(errorWrapper: ErrorWrapper): Result = {
(errorWrapper.error: @unchecked) match {
case PenaltiesInvalidIdValue |
MtdError("INVALID_REQUEST", _, _) => BadRequest(Json.toJson(errorWrapper))
case PenaltiesInvalidIdValue => BadRequest(Json.toJson(errorWrapper))
case PenaltiesNotDataFound => NotFound(Json.toJson(errorWrapper))
case _ => InternalServerError(Json.toJson(errorWrapper))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package v1.controllers.requestParsers.validators

import v1.controllers.requestParsers.validators.validations.VrnValidation
import v1.controllers.requestParsers.validators.validations.{ChargeReferenceValidation, VrnValidation}
import v1.models.errors.MtdError
import v1.models.request.penalties.{FinancialRawData, PenaltiesRawData}

Expand All @@ -29,7 +29,8 @@ class FinancialDataValidator extends Validator[FinancialRawData] {
private def vrnFormatValidation: FinancialRawData => List[List[MtdError]] = {
(data: FinancialRawData) => {
List(
VrnValidation.validate(data.vrn)
VrnValidation.validate(data.vrn),
ChargeReferenceValidation.validate(data.searchItem)
)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright 2022 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 v1.controllers.requestParsers.validators.validations

import v1.models.errors.{FinancialInvalidSearchItem, MtdError}

object ChargeReferenceValidation {
private val vrnRegex = """^X[a-zA-Z]\d{12}+$"""

def validate(chargeReference: String): List[MtdError] = {
if (chargeReference.matches(vrnRegex)) NoValidationErrors else List(FinancialInvalidSearchItem)
}
}
39 changes: 17 additions & 22 deletions func/v1/endpoints/FinancialDataControllerISpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import play.api.libs.ws.{WSRequest, WSResponse}
import play.api.test.Helpers.AUTHORIZATION
import support.IntegrationBaseSpec
import v1.constants.FinancialDataConstants
import v1.models.errors.{FinancialInvalidIdNumber, FinancialNotDataFound, MtdError, VrnFormatError}
import v1.models.errors.{DownstreamError, FinancialInvalidIdNumber, FinancialInvalidSearchItem, FinancialNotDataFound, MtdError, VrnFormatError}
import v1.stubs.{AuditStub, AuthStub, PenaltiesStub}

class FinancialDataControllerISpec extends IntegrationBaseSpec {
Expand Down Expand Up @@ -101,30 +101,25 @@ class FinancialDataControllerISpec extends IntegrationBaseSpec {
val errorBody: JsValue = Json.parse(
"""
|{
|"failures": [{
| "code":"INVALID_IDNUMBER",
| "reason":"Some Reason"
|},
|{
| "code":"INVALID_DATE_TO",
| "reason":"Some Reason"
|}]
| "failures": [
| {
| "code":"INVALID_IDNUMBER",
| "reason":"Some reason"
| },
| {
| "code":"INVALID_DOC_NUMBER_OR_CHARGE_REFERENCE_NUMBER",
| "reason":"Some reason"
| }
| ]
|}
|""".stripMargin)

val expectedJson: JsValue = Json.parse(
"""
|{
| "code":"VRN_INVALID",
| "message":"The provided Vrn is invalid",
| "errors": [{
| "code":"INVALID_DATE_TO",
| "message": "Some Reason"
| }]
|
|}
|""".stripMargin
)
val expectedJson: JsValue = Json.toJson(MtdError("INVALID_REQUEST", "Invalid request financial details",
Some(Json.toJson(Seq(
FinancialInvalidIdNumber,
FinancialInvalidSearchItem
)))))

override def setupStubs(): StubMapping = {
AuditStub.audit()
AuthStub.authorised()
Expand Down
86 changes: 37 additions & 49 deletions func/v1/endpoints/PenaltiesControllerISpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import play.api.libs.ws.{WSRequest, WSResponse}
import play.api.test.Helpers.AUTHORIZATION
import support.IntegrationBaseSpec
import v1.constants.PenaltiesConstants
import v1.models.errors.{MtdError, PenaltiesNotDataFound, VrnFormatError}
import v1.models.errors.{DownstreamError, MtdError, PenaltiesInvalidIdValue, PenaltiesNotDataFound, VrnFormatError}
import v1.stubs.{AuditStub, AuthStub, PenaltiesStub}

class PenaltiesControllerISpec extends IntegrationBaseSpec {
Expand Down Expand Up @@ -99,65 +99,20 @@ class PenaltiesControllerISpec extends IntegrationBaseSpec {
}

"VRN is invalid" must {

"return 400" in new Test {
"return 400 error" in new Test {

val errorBody: JsValue = Json.parse(
"""
|{
|"failures": [{
| "code":"INVALID_CORRELATIONID",
| "reason":"Some Reason"
|}]
|}
|""".stripMargin)

override def setupStubs(): StubMapping = {
AuditStub.audit()
AuthStub.authorised()
PenaltiesStub.onError(PenaltiesStub.GET, PenaltiesConstants.penaltiesURl(), INTERNAL_SERVER_ERROR, errorBody)
}

val response: WSResponse = await(request.get())
response.status shouldBe INTERNAL_SERVER_ERROR
response.json shouldBe Json.toJson(MtdError("INVALID_CORRELATIONID","Some Reason"))
response.header("Content-Type") shouldBe Some("application/json")
}

"return multiple 400 errors" in new Test {

val errorBody: JsValue = Json.parse(
"""
|{
|"failures": [{
| "code":"INVALID_CORRELATIONID",
| "reason":"Some Reason"
|},
|{
| "code":"INVALID_DATELIMIT",
| "code":"INVALID_IDVALUE",
| "reason":"Some Reason"
|}
|]
|}
|""".stripMargin)

val expectedJson: JsValue = Json.parse(
"""
|{
|"code":"INVALID_REQUEST",
|"message":"Invalid request penalties",
|"errors": [{
| "code":"INVALID_CORRELATIONID",
| "message":"Some Reason"
|},
|{
| "code":"INVALID_DATELIMIT",
| "message":"Some Reason"
|}
|]
|}
|""".stripMargin
)
val expectedJson: JsValue = Json.toJson(PenaltiesInvalidIdValue)

override def setupStubs(): StubMapping = {
AuditStub.audit()
Expand Down Expand Up @@ -224,6 +179,39 @@ class PenaltiesControllerISpec extends IntegrationBaseSpec {
response.json shouldBe Json.toJson(MtdError("REASON", "Some Reason"))
response.header("Content-Type") shouldBe Some("application/json")
}

"multiple errors return 500" in new Test {

val errorBody: JsValue = Json.parse(
"""
|{
|"failures": [{
| "code":"REASON",
| "reason":"Some Reason"
|},
|{
|"code":"INVALID_IDVALUE",
| "reason":"Some Reason"
|}
|]
|}
|""".stripMargin)

override def setupStubs(): StubMapping = {
AuditStub.audit()
AuthStub.authorised()
PenaltiesStub.onError(PenaltiesStub.GET, PenaltiesConstants.penaltiesURl(), INTERNAL_SERVER_ERROR, errorBody)
}

val response: WSResponse = await(request.get())
response.status shouldBe INTERNAL_SERVER_ERROR
response.json shouldBe Json.toJson(MtdError("INVALID_REQUEST", "Invalid request penalties",
Some(Json.toJson(Seq(
MtdError("REASON", "Some Reason"),
MtdError("VRN_INVALID", "The provided VRN is invalid.")
)))))
response.header("Content-Type") shouldBe Some("application/json")
}
}
}
}
Expand Down
Loading

0 comments on commit bb3718b

Please sign in to comment.