Merge pull request #946 from hmrc/DL-8882

DL-8882 - aligned errors with specs

app/v1/connectors/PenaltiesConnector.scala

@@ -17,15 +17,13 @@
 package v1.connectors
 
 import config.AppConfig
-import play.api.http.Status
-
 import javax.inject.{Inject, Singleton}
+import play.api.http.Status
 import uk.gov.hmrc.http.{HeaderCarrier, HttpClient}
 import utils.Logging
-import utils.pagerDutyLogging.{Endpoint, PagerDutyLogging, PagerDutyLoggingEndpointName}
+import utils.pagerDutyLogging.{Endpoint, PagerDutyLogging}
 import v1.connectors.httpparsers.FinancialDataHttpParser._
 import v1.connectors.httpparsers.PenaltiesHttpParser._
-import v1.connectors.httpparsers.StandardDesHttpParser.logger
 import v1.controllers.UserRequest
 import v1.models.errors.{ConnectorError, ErrorWrapper, MtdError}
 import v1.models.request.penalties.{FinancialRequest, PenaltiesRequest}

app/v1/connectors/httpparsers/PenaltiesHttpParser.scala

@@ -34,16 +34,8 @@ object PenaltiesHttpParser extends Logging {
       val penaltiesErrors = jsonString.as[PenaltiesErrors]
       val mtdErrorsConvert = penaltiesErrors.failures.map{ error =>
         (error.code, status) match {
-          case ("INVALID_IDTYPE", BAD_REQUEST) => PenaltiesInvalidIdType
           case ("INVALID_IDVALUE", BAD_REQUEST) => PenaltiesInvalidIdValue
-          case ("INVALID_DATELIMIT", BAD_REQUEST) => PenaltiesInvalidDataLimit
-          case ("INVALID_CORRELATIONID", BAD_REQUEST) => PenaltiesInvalidCorrelationId
           case ("NO_DATA_FOUND", NOT_FOUND) => PenaltiesNotDataFound
-          case ("DUPLICATE_SUBMISSION", CONFLICT) => PenaltiesDuplicateSubmission
-          case ("INVALID_IDTYPE", UNPROCESSABLE_ENTITY) => PenaltiesInvalidIdTypeUnprocessEntity
-          case ("INVALID_ID", UNPROCESSABLE_ENTITY) => PenaltiesInvalidIdValueUnprocessEntity
-          case ("REQUEST_NOT_PROCESSED", UNPROCESSABLE_ENTITY) => PenaltiesRequestNotProcessedUnprocessEntity
-          case ("SERVICE_UNAVAILABLE", SERVICE_UNAVAILABLE) => PenaltiesServiceUnavailable
           case _ => MtdError(error.code, error.reason)
         }
       }

app/v1/controllers/PenaltiesController.scala

@@ -91,18 +91,9 @@ class PenaltiesController @Inject()(val authService: EnrolmentsAuthService,
 
   private def errorResult(errorWrapper: ErrorWrapper): Result = {
     (errorWrapper.error: @unchecked) match {
-      case PenaltiesInvalidIdType |
-           PenaltiesInvalidIdValue |
-           PenaltiesInvalidDataLimit |
-           PenaltiesInvalidCorrelationId |
-           VrnFormatError |
-           MtdError("INVALID_REQUEST", _, _)=> BadRequest(Json.toJson(errorWrapper))
+      case PenaltiesInvalidIdValue |
+           MtdError("INVALID_REQUEST", _, _) => BadRequest(Json.toJson(errorWrapper))
       case PenaltiesNotDataFound => NotFound(Json.toJson(errorWrapper))
-      case PenaltiesDuplicateSubmission => Conflict(Json.toJson(errorWrapper))
-      case PenaltiesInvalidIdTypeUnprocessEntity |
-           PenaltiesInvalidIdValueUnprocessEntity |
-           PenaltiesRequestNotProcessedUnprocessEntity => UnprocessableEntity(Json.toJson(errorWrapper))
-      case PenaltiesServiceUnavailable => ServiceUnavailable(Json.toJson(errorWrapper))
       case _ => InternalServerError(Json.toJson(errorWrapper))
     }
   }

app/v1/models/errors/mtdErrors.scala

@@ -59,18 +59,8 @@ object VrnFormatErrorDes extends MtdError("VRN_INVALID", "The provided VRN is in
 object VrnNotFound extends MtdError("VRN_NOT_FOUND", "The provided VRN was not found")
 
 //Penalties Errors
-object PenaltiesInvalidIdType extends MtdError("INVALID_IDTYPE", "Invalid Id Type")
-object PenaltiesInvalidIdValue extends MtdError("INVALID_IDVALUE", "Invalid Id value")
-object PenaltiesInvalidDataLimit extends MtdError("INVALID_DATELIMIT", "Invalid Date Limit")
-object PenaltiesInvalidCorrelationId extends MtdError("INVALID_CORRELATIONID", "Invalid correlation ID")
-object PenaltiesNotDataFound extends MtdError("NO_DATA_FOUND", "No data found")
-object PenaltiesDuplicateSubmission extends MtdError("DUPLICATE_SUBMISSION", "Duplicate Submission")
-object PenaltiesInvalidIdTypeUnprocessEntity extends MtdError("INVALID_IDTYPE", "Invalid Id Type")
-object PenaltiesInvalidIdValueUnprocessEntity extends MtdError("INVALID_ID", "Invalid Id value")
-object PenaltiesRequestNotProcessedUnprocessEntity extends MtdError("REQUEST_NOT_PROCESSED", "Request not processed")
-object PenaltiesServiceUnavailable extends MtdError("SERVICE_UNAVAILABLE", "Service Unavailable")
-
-
+object PenaltiesInvalidIdValue extends MtdError("VRN_INVALID", "The provided VRN is invalid.")
+object PenaltiesNotDataFound extends MtdError("MATCHING_RESOURCE_NOT_FOUND", "No penalties could be found in the last 24 months")
 
 //Financial Data Errors
 object FinancialInvalidCorrelationId extends MtdError("INVALID_CORRELATIONID", "Invalid Correlation Id")

func/v1/endpoints/PenaltiesControllerISpec.scala

@@ -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, PenaltiesInvalidCorrelationId, PenaltiesNotDataFound, VrnFormatError}
+import v1.models.errors.{MtdError, PenaltiesNotDataFound, VrnFormatError}
 import v1.stubs.{AuditStub, AuthStub, PenaltiesStub}
 
 class PenaltiesControllerISpec extends IntegrationBaseSpec {
@@ -115,12 +115,12 @@ class PenaltiesControllerISpec extends IntegrationBaseSpec {
             override def setupStubs(): StubMapping = {
               AuditStub.audit()
               AuthStub.authorised()
-              PenaltiesStub.onError(PenaltiesStub.GET, PenaltiesConstants.penaltiesURl(), BAD_REQUEST, errorBody)
+              PenaltiesStub.onError(PenaltiesStub.GET, PenaltiesConstants.penaltiesURl(), INTERNAL_SERVER_ERROR, errorBody)
             }
 
             val response: WSResponse = await(request.get())
-            response.status shouldBe BAD_REQUEST
-            response.json shouldBe Json.toJson(PenaltiesInvalidCorrelationId)
+            response.status shouldBe INTERNAL_SERVER_ERROR
+            response.json shouldBe Json.toJson(MtdError("INVALID_CORRELATIONID","Some Reason"))
             response.header("Content-Type") shouldBe Some("application/json")
           }
 
@@ -148,11 +148,11 @@ class PenaltiesControllerISpec extends IntegrationBaseSpec {
                 |"message":"Invalid request penalties",
                 |"errors": [{
                 | "code":"INVALID_CORRELATIONID",
-                | "message":"Invalid correlation ID"
+                | "message":"Some Reason"
                 |},
                 |{
                 | "code":"INVALID_DATELIMIT",
-                | "message":"Invalid Date Limit"
+                | "message":"Some Reason"
                 |}
                 |]
                 |}

test/v1/connectors/PenaltiesConnectorSpec.scala

@@ -50,7 +50,7 @@ class PenaltiesConnectorSpec extends GuiceBox with ConnectorSpec with MockHttpCl
           await(result) shouldBe expectedResult
         }
 
-        "return Future(Right(PenaltiesResponse) max" in {
+        "return Future(Right(PenaltiesResponse)) max" in {
 
           MockedHttpClient.get(
             url = PenaltiesConstants.penaltiesURlWithConfig(),
@@ -72,7 +72,7 @@ class PenaltiesConnectorSpec extends GuiceBox with ConnectorSpec with MockHttpCl
 
       "a invalid json response is returned" must {
 
-        "return Future(Left(InvalidJson)" in {
+        "return Future(Left(InvalidJson))" in {
 
           MockedHttpClient.get(
             url = PenaltiesConstants.penaltiesURlWithConfig(),
@@ -91,7 +91,7 @@ class PenaltiesConnectorSpec extends GuiceBox with ConnectorSpec with MockHttpCl
 
       "a InvalidVrn response is returned" must {
 
-        "return Future(Left(InvalidVrn)" in {
+        "return Future(Left(InvalidVrn))" in {
 
           MockedHttpClient.get(
             url = PenaltiesConstants.penaltiesURlWithConfig(),

test/v1/connectors/httpparsers/PenaltiesHttpParserSpec.scala

@@ -115,7 +115,7 @@ class PenaltiesHttpParserSpec extends UnitSpec {
               )
             )
           )
-          result shouldBe Left(PenaltiesConstants.errorWrapper(MtdError("INVALID_IDTYPE", "Invalid Id Type")))
+          result shouldBe Left(PenaltiesConstants.errorWrapper(MtdError("INVALID_IDTYPE", "Submission has not passed validation. Invalid parameter idType.")))
         }
 
         "return Left(MultiErrors)" in {
@@ -144,7 +144,10 @@ class PenaltiesHttpParserSpec extends UnitSpec {
             )
           )
           result shouldBe Left(PenaltiesConstants.errorWrapper(
-            MtdError("INVALID_REQUEST", "Invalid request penalties", customJson = Some(Json.toJson(Seq(PenaltiesInvalidIdType, PenaltiesInvalidIdValue))))))
+            MtdError("INVALID_REQUEST", "Invalid request penalties",
+              customJson = Some(Json.toJson(Seq(
+                MtdError("INVALID_IDTYPE", "Submission has not passed validation. Invalid parameter idType."),
+                PenaltiesInvalidIdValue))))))
         }
       }
 
@@ -242,10 +245,11 @@ class PenaltiesHttpParserSpec extends UnitSpec {
 
         "return multi errors when passed multiple errors" in {
           val expected: MtdError = MtdError("INVALID_REQUEST", "Invalid request penalties",
-            Some(Json.toJson(PenaltiesInvalidIdType,
+            Some(Json.toJson(
+              MtdError("INVALID_IDTYPE", "Some reason"),
             PenaltiesInvalidIdValue,
-            PenaltiesInvalidDataLimit,
-            PenaltiesInvalidCorrelationId
+              MtdError("INVALID_DATELIMIT", "Some reason"),
+                MtdError("INVALID_CORRELATIONID", "Some reason")
           )))
           val result = PenaltiesHttpReads.errorHelper(multiJsonString(), BAD_REQUEST)
           result shouldBe expected
@@ -258,31 +262,31 @@ class PenaltiesHttpParserSpec extends UnitSpec {
           }
 
           "return PenaltiesServiceUnavailable mtd error when passed a service unavailable and SERVICE_UNAVAILABLE json" in {
-            val expected: MtdError = PenaltiesServiceUnavailable
+            val expected: MtdError = MtdError("SERVICE_UNAVAILABLE","Some reason")
             val result = PenaltiesHttpReads.errorHelper(jsonString("SERVICE_UNAVAILABLE"), SERVICE_UNAVAILABLE)
             result shouldBe expected
           }
 
           "return PenaltiesRequestNotProcessedUnprocessEntity mtd error when passed a Unprocess Entity and REQUEST_NOT_PROCESSED json" in {
-            val expected: MtdError = PenaltiesRequestNotProcessedUnprocessEntity
+            val expected: MtdError = MtdError("REQUEST_NOT_PROCESSED","Some reason")
             val result = PenaltiesHttpReads.errorHelper(jsonString("REQUEST_NOT_PROCESSED"), UNPROCESSABLE_ENTITY)
             result shouldBe expected
           }
 
           "return PenaltiesInvalidIdValueUnprocessEntity mtd error when passed a Unprocess Entity and INVALID_ID json" in {
-            val expected: MtdError = PenaltiesInvalidIdValueUnprocessEntity
+            val expected: MtdError = MtdError("INVALID_ID","Some reason")
             val result = PenaltiesHttpReads.errorHelper(jsonString("INVALID_ID"), UNPROCESSABLE_ENTITY)
             result shouldBe expected
           }
 
           "return PenaltiesInvalidIdTypeUnprocessEntity mtd error when passed a Unprocess Entity and INVALID_IDTYPE json" in {
-            val expected: MtdError = PenaltiesInvalidIdTypeUnprocessEntity
+            val expected: MtdError = MtdError("INVALID_IDTYPE","Some reason")
             val result = PenaltiesHttpReads.errorHelper(jsonString("INVALID_IDTYPE"), UNPROCESSABLE_ENTITY)
             result shouldBe expected
           }
 
           "return PenaltiesDuplicateSubmission mtd error when passed a Conflict and DUPLICATE_SUBMISSION json" in {
-            val expected: MtdError = PenaltiesDuplicateSubmission
+            val expected: MtdError = MtdError("DUPLICATE_SUBMISSION","Some reason")
             val result = PenaltiesHttpReads.errorHelper(jsonString("DUPLICATE_SUBMISSION"), CONFLICT)
             result shouldBe expected
           }
@@ -294,7 +298,7 @@ class PenaltiesHttpParserSpec extends UnitSpec {
           }
 
           "return PenaltiesInvalidIdType mtd error when passed a Bad Request and INVALID_IDTYPE json" in {
-            val expected: MtdError = PenaltiesInvalidIdType
+            val expected: MtdError = MtdError("INVALID_IDTYPE","Some reason")
             val result = PenaltiesHttpReads.errorHelper(jsonString("INVALID_IDTYPE"), BAD_REQUEST)
             result shouldBe expected
           }
@@ -306,13 +310,13 @@ class PenaltiesHttpParserSpec extends UnitSpec {
           }
 
           "return PenaltiesInvalidDataLimit mtd error when passed a Bad Request and INVALID_DATELIMIT json" in {
-            val expected: MtdError = PenaltiesInvalidDataLimit
+            val expected: MtdError = MtdError("INVALID_DATELIMIT","Some reason")
             val result = PenaltiesHttpReads.errorHelper(jsonString("INVALID_DATELIMIT"), BAD_REQUEST)
             result shouldBe expected
           }
 
           "return PenaltiesInvalidCorrelationId mtd error when passed a Bad Request and INVALID_CORRELATIONID json" in {
-            val expected: MtdError = PenaltiesInvalidCorrelationId
+            val expected: MtdError = MtdError("INVALID_CORRELATIONID","Some reason")
             val result = PenaltiesHttpReads.errorHelper(jsonString("INVALID_CORRELATIONID"), BAD_REQUEST)
             result shouldBe expected
           }

test/v1/controllers/PenaltiesControllerSpec.scala

@@ -16,9 +16,7 @@
 
 package v1.controllers
 
-import com.typesafe.config.ConfigFactory
 import mocks.MockAppConfig
-import play.api.Configuration
 import play.api.libs.json.Json
 import play.api.mvc.Result
 import uk.gov.hmrc.http.HeaderCarrier
@@ -28,7 +26,7 @@ import v1.mocks.MockIdGenerator
 import v1.mocks.requestParsers.MockPenaltiesRequestParser
 import v1.mocks.services.{MockAuditService, MockEnrolmentsAuthService, MockPenaltiesService}
 import v1.models.audit.{AuditError, AuditResponse}
-import v1.models.errors.{BadRequestError, MtdError, PenaltiesInvalidIdType, PenaltiesNotDataFound, UnexpectedFailure, VrnFormatError, VrnNotFound}
+import v1.models.errors._
 
 import scala.concurrent.ExecutionContext.Implicits.global
 import scala.concurrent.Future
@@ -105,7 +103,7 @@ class PenaltiesControllerSpec extends ControllerBaseSpec with MockEnrolmentsAuth
         "errors are returned from Penalties" when {
 
           val errors: Seq[(MtdError, Int)] = Seq(
-            (PenaltiesInvalidIdType, BAD_REQUEST),
+            (PenaltiesInvalidIdValue, BAD_REQUEST),
             (PenaltiesNotDataFound, NOT_FOUND),
             (UnexpectedFailure.mtdError(INTERNAL_SERVER_ERROR, "error"), INTERNAL_SERVER_ERROR)
           )
@@ -142,21 +140,21 @@ class PenaltiesControllerSpec extends ControllerBaseSpec with MockEnrolmentsAuth
 
       "valid request is not supplied" must {
 
-        "return BadRequest" in new Test {
+        "return Internal Server Error" in new Test {
 
           MockPenaltiesRequestParser.parse(PenaltiesConstants.rawData)(Left(PenaltiesConstants.errorWrapper(VrnFormatError)))
 
           val result: Future[Result] = controller.retrievePenalties(PenaltiesConstants.vrn)(fakeGetRequest)
 
-          status(result) shouldBe BAD_REQUEST
+          status(result) shouldBe INTERNAL_SERVER_ERROR
           contentAsJson(result) shouldBe Json.toJson(VrnFormatError)
           contentType(result) shouldBe Some("application/json")
           header("X-CorrelationId", result) shouldBe Some(PenaltiesConstants.correlationId)
 
           MockedAuditService.verifyAuditEvent(AuditEvents.auditPenalties(
             correlationId = PenaltiesConstants.correlationId,
             userDetails = PenaltiesConstants.userDetails,
-            auditResponse = AuditResponse(BAD_REQUEST, Some(Seq(AuditError(VrnFormatError.code))), None)
+            auditResponse = AuditResponse(INTERNAL_SERVER_ERROR, Some(Seq(AuditError(VrnFormatError.code))), None)
           ))
         }
       }