Skip to content

[DL-18051] Remove HIP/IF logic so that only HIP API responses are accepted #1061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jake-Raffe wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[DL-18051] Remove HIP/IF logic so that only HIP API responses are accepted #1061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
481c3bd
[DL-18051] Remove HIP/IF logic so that only HIP API responses are acc…
Jake-Raffe Dec 10, 2025
65ddbe3
[DL-18051] Remove HIP/IF logic so that only HIP API responses are acc…
Jake-Raffe Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 26 additions & 28 deletions app/v1/connectors/PenaltiesConnector.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,33 +17,30 @@
package v1.connectors

import config.AppConfig

import javax.inject.{Inject, Singleton}
import play.api.http.Status
import uk.gov.hmrc.http.{HeaderCarrier, HttpClient}
import uk.gov.hmrc.http.{ HeaderCarrier, HttpClient }
import utils.Logging
import utils.pagerDutyLogging.{Endpoint, PagerDutyLogging}
import utils.pagerDutyLogging.{ Endpoint, PagerDutyLogging }
import v1.connectors.httpparsers.FinancialDataHttpParser._
import v1.connectors.httpparsers.PenaltiesHttpParser._
import v1.controllers.UserRequest
import v1.models.errors.{ConnectorError, ErrorWrapper, MtdError}
import v1.models.request.penalties.{FinancialRequest, PenaltiesRequest}
import v1.models.errors.{ ConnectorError, ErrorWrapper, MtdError }
import v1.models.request.penalties.{ FinancialRequest, PenaltiesRequest }
import v1.models.response.financialData.FinancialDataResponse
import v1.models.response.penalties.PenaltiesResponse

import scala.concurrent.{ExecutionContext, Future}
import javax.inject.{ Inject, Singleton }
import scala.concurrent.{ ExecutionContext, Future }

@Singleton
class PenaltiesConnector @Inject()(val http: HttpClient,
val appConfig: AppConfig) extends BaseDownstreamConnector with Logging {
class PenaltiesConnector @Inject()(val http: HttpClient, val appConfig: AppConfig) extends BaseDownstreamConnector with Logging {

private def headerCarrier(
additionalHeaders: Seq[String] = Seq.empty
)(implicit
hc: HeaderCarrier,
userRequest: UserRequest[_],
correlationId: String
): HeaderCarrier = {
additionalHeaders: Seq[String] = Seq.empty
)(implicit
hc: HeaderCarrier,
userRequest: UserRequest[_],
correlationId: String): HeaderCarrier = {

val maybeAuthHeader: String = userRequest.request.headers
.get("Authorization")
Expand Down Expand Up @@ -79,11 +76,12 @@ class PenaltiesConnector @Inject()(val http: HttpClient,
case e =>
val logDetails = s"request failed. ${e.getMessage}"

errorLog(ConnectorError.log(
logContext = "[PenaltiesConnector][retrievePenaltiesData]",
vrn = vrn,
details = logDetails,
))
errorLog(
ConnectorError.log(
logContext = "[PenaltiesConnector][retrievePenaltiesData]",
vrn = vrn,
details = logDetails,
))

PagerDutyLogging.log(
pagerDutyLoggingEndpointName = Endpoint.RetrievePenalties.requestFailedMessage,
Expand All @@ -96,14 +94,13 @@ class PenaltiesConnector @Inject()(val http: HttpClient,
}
}


def retrieveFinancialData(request: FinancialRequest)(implicit hc: HeaderCarrier,
ec: ExecutionContext,
userRequest: UserRequest[_],
correlationId: String): Future[Outcome[FinancialDataResponse]] = {
val vrn = request.vrn.vrn
val vrn = request.vrn.vrn
val searchItem = request.searchItem
val url = appConfig.penaltiesBaseUrl + s"/penalties/VATC/penalty/financial-data/VRN/$vrn?searchType=CHGREF&searchItem=${searchItem}"
val url = appConfig.penaltiesBaseUrl + s"/penalties/VATC/penalty/financial-data/VRN/$vrn?searchType=CHGREF&searchItem=$searchItem"

def doGet(implicit hc: HeaderCarrier): Future[Outcome[FinancialDataResponse]] = {
http.GET[Outcome[FinancialDataResponse]](url)
Expand All @@ -113,11 +110,12 @@ class PenaltiesConnector @Inject()(val http: HttpClient,
case e =>
val logDetails = s"request failed. ${e.getMessage}"

errorLog(ConnectorError.log(
logContext = "[PenaltiesConnector][retrieveFinancialData]",
vrn = vrn,
details = logDetails,
))
errorLog(
ConnectorError.log(
logContext = "[PenaltiesConnector][retrieveFinancialData]",
vrn = vrn,
details = logDetails,
))

PagerDutyLogging.log(
pagerDutyLoggingEndpointName = Endpoint.RetrieveFinancialData.requestFailedMessage,
Expand Down
14 changes: 7 additions & 7 deletions app/v1/connectors/httpparsers/FinancialDataHttpParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2024 HM Revenue & Customs
* Copyright 2025 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.
Expand All @@ -23,7 +23,7 @@ import utils.Logging
import v1.connectors.Outcome
import v1.models.errors._
import v1.models.outcomes.ResponseWrapper
import v1.models.response.financialData.{ FinancialDataErrorsHIP, FinancialDataResponse }
import v1.models.response.financialData.{ FinancialDataErrors, FinancialDataResponse }

object FinancialDataHttpParser extends Logging {

Expand All @@ -32,7 +32,7 @@ object FinancialDataHttpParser extends Logging {
def read(method: String, url: String, response: HttpResponse): Outcome[FinancialDataResponse] = {
val responseCorrelationId = retrieveCorrelationId(response)
response.status match {
case OK | CREATED =>
case OK =>
Copy link
Copy Markdown
Contributor Author

@Jake-Raffe Jake-Raffe Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Responses from penalties backend to vat-api are only returned in an OK now, not a CREATED.
https://github.com/hmrc/penalties/blob/main/app/controllers/APIController.scala#L196-L198

response.json.validate[FinancialDataResponse] match {
case JsSuccess(model, _) => Right(ResponseWrapper(responseCorrelationId, model))
case JsError(errors) =>
Expand All @@ -50,15 +50,15 @@ object FinancialDataHttpParser extends Logging {
}

def errorHelper(jsonString: JsValue): MtdError = {
jsonString.validate[FinancialDataErrorsHIP] match {
case JsSuccess(errorsHIP, _) => convertToMtdErrorsHIP(errorsHIP)
jsonString.validate[FinancialDataErrors] match {
case JsSuccess(errors, _) => convertToMtdErrors(errors)
case JsError(errors) =>
MtdError("SERVER_ERROR", s"Unable to validate json error response with errors: $errors", Some(jsonString))
}
}

private def convertToMtdErrorsHIP(errorsHIP: FinancialDataErrorsHIP): MtdError = {
val error = errorsHIP.errors
private def convertToMtdErrors(errors: FinancialDataErrors): MtdError = {
val error = errors.errors
error.code match {
case "002" => DownstreamError // Invalid Tax Regime
case "003" => DownstreamError // Request could not be processed (ETMP issue)
Expand Down
10 changes: 5 additions & 5 deletions app/v1/connectors/httpparsers/PenaltiesHttpParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import utils.Logging
import v1.connectors.Outcome
import v1.models.errors._
import v1.models.outcomes.ResponseWrapper
import v1.models.response.penalties.{ PenaltiesErrorsHIP, PenaltiesResponse }
import v1.models.response.penalties.{ PenaltiesErrors, PenaltiesResponse }

object PenaltiesHttpParser extends Logging {

Expand All @@ -50,16 +50,16 @@ object PenaltiesHttpParser extends Logging {
}

def errorHelper(jsonString: JsValue): MtdError = {
jsonString.validate[PenaltiesErrorsHIP] match {
case JsSuccess(errorsHIP, _) => convertToMtdErrorsHIP(errorsHIP)
jsonString.validate[PenaltiesErrors] match {
case JsSuccess(errors, _) => convertToMtdErrors(errors)
case JsError(errors) =>
MtdError("SERVER_ERROR", s"Unable to validate json error response with errors: $errors", Some(jsonString))
}
}
}

private def convertToMtdErrorsHIP(errorsHIP: PenaltiesErrorsHIP): MtdError = {
val error = errorsHIP.errors
private def convertToMtdErrors(errors: PenaltiesErrors): MtdError = {
val error = errors.errors
error.code match {
case "002" => DownstreamError // Invalid Tax Regime
case "003" => DownstreamError // Request could not be processed (ETMP issue)
Expand Down
6 changes: 2 additions & 4 deletions app/v1/controllers/FinancialDataController.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2024 HM Revenue & Customs
* Copyright 2025 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.
Expand All @@ -18,7 +18,6 @@ package v1.controllers

import cats.data.EitherT
import cats.implicits._
import config.AppConfig
import play.api.libs.json.Json
import play.api.mvc.{ Action, AnyContent, ControllerComponents, Result }
import utils.{ EndpointLogContext, IdGenerator, Logging }
Expand All @@ -38,8 +37,7 @@ class FinancialDataController @Inject()(val authService: EnrolmentsAuthService,
service: PenaltiesService,
auditService: AuditService,
cc: ControllerComponents,
val idGenerator: IdGenerator,
appConfig: AppConfig)(implicit ec: ExecutionContext)
val idGenerator: IdGenerator)(implicit ec: ExecutionContext)
extends AuthorisedController(cc)
with BaseController
with Logging {
Expand Down
6 changes: 2 additions & 4 deletions app/v1/controllers/PenaltiesController.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2024 HM Revenue & Customs
* Copyright 2025 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.
Expand All @@ -18,7 +18,6 @@ package v1.controllers

import cats.data.EitherT
import cats.implicits._
import config.AppConfig
import play.api.libs.json.Json
import play.api.mvc.{ Action, AnyContent, ControllerComponents, Result }
import utils.{ EndpointLogContext, IdGenerator, Logging }
Expand All @@ -38,8 +37,7 @@ class PenaltiesController @Inject()(val authService: EnrolmentsAuthService,
service: PenaltiesService,
auditService: AuditService,
cc: ControllerComponents,
val idGenerator: IdGenerator,
appConfig: AppConfig)(implicit ec: ExecutionContext)
val idGenerator: IdGenerator)(implicit ec: ExecutionContext)
extends AuthorisedController(cc)
with BaseController
with Logging {
Expand Down
35 changes: 13 additions & 22 deletions app/v1/models/response/financialData/FinancialDataResponse.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2024 HM Revenue & Customs
* Copyright 2025 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.
Expand Down Expand Up @@ -97,37 +97,28 @@ object Totalisations {
case class FinancialDataResponse(totalisations: Option[Totalisations], documentDetails: Option[Seq[DocumentDetail]])

object FinancialDataResponse {
private val readIfResponse: Reads[FinancialDataResponse] = (
(JsPath \ "getFinancialData" \ "financialDetails" \ "totalisation").readNullable[Totalisations] and
(JsPath \ "getFinancialData" \ "financialDetails" \ "documentDetails").readNullable[Seq[DocumentDetail]]
)(FinancialDataResponse.apply _)
private val readHipResponse: Reads[FinancialDataResponse] = (
(JsPath \ "success" \ "financialData" \ "totalisation").readNullable[Totalisations] and
(JsPath \ "success" \ "financialData" \ "documentDetails").readNullable[Seq[DocumentDetail]]
)(FinancialDataResponse.apply _)

implicit val reads: Reads[FinancialDataResponse] = { json =>
if ((json \ "getFinancialData").isDefined) {
readIfResponse.reads(json)
} else if ((json \ "success").isDefined) {
readHipResponse.reads(json)
} else {
JsError("Unrecognised FinancialDataResponse format")
(json \ "success" \ "financialData").toOption match {
case None => JsError("Unrecognised FinancialDataResponse format")
case Some(_) =>
((JsPath \ "success" \ "financialData" \ "totalisation").readNullable[Totalisations] and
(JsPath \ "success" \ "financialData" \ "documentDetails").readNullable[Seq[DocumentDetail]])(FinancialDataResponse.apply _).reads(json)
}
}

implicit val writes: OWrites[FinancialDataResponse] = Json.writes[FinancialDataResponse]
}

// Despite the name HIP 'errors' are always singular even if request has multiple issues
case class FinancialDataErrorsHIP(errors: FinancialDataErrorHIP)
// Despite the name, 'errors' field is always singular even if request has multiple issues
case class FinancialDataErrors(errors: FinancialDataError)

object FinancialDataErrorsHIP {
implicit val format: OFormat[FinancialDataErrorsHIP] = Json.format[FinancialDataErrorsHIP]
object FinancialDataErrors {
implicit val format: OFormat[FinancialDataErrors] = Json.format[FinancialDataErrors]
}

case class FinancialDataErrorHIP(processingDate: String, code: String, text: String)
case class FinancialDataError(processingDate: String, code: String, text: String)

object FinancialDataErrorHIP {
implicit val format: Format[FinancialDataErrorHIP] = Json.format[FinancialDataErrorHIP]
object FinancialDataError {
implicit val format: Format[FinancialDataError] = Json.format[FinancialDataError]
}
Loading