From 90bb2bd41b1b1771e7939b2f2188b7fa5f682c78 Mon Sep 17 00:00:00 2001 From: Alen Mujezinovic Date: Fri, 24 Nov 2017 10:59:46 +0000 Subject: [PATCH] Updated summary controller to use annual statement --- .../epayeapi/connectors/ConnectorBase.scala | 1 + .../epayeapi/connectors/EpayeConnector.scala | 7 ++-- .../controllers/GetSummaryController.scala | 28 ++++++------- .../epayeapi/models/EpayeTotalsResponse.scala | 23 +++++++++++ app/uk/gov/hmrc/epayeapi/models/Formats.scala | 4 ++ .../epayeapi/models/SummaryResponse.scala | 8 ++-- conf/logback.xml | 4 +- .../connectors/EpayeConnectorSpec.scala | 39 +++++++++++++++---- .../epayeapi/controllers/GetSummarySpec.scala | 39 ++++++++++++------- 9 files changed, 107 insertions(+), 46 deletions(-) create mode 100644 app/uk/gov/hmrc/epayeapi/models/EpayeTotalsResponse.scala diff --git a/app/uk/gov/hmrc/epayeapi/connectors/ConnectorBase.scala b/app/uk/gov/hmrc/epayeapi/connectors/ConnectorBase.scala index 4ee859e..21c393f 100644 --- a/app/uk/gov/hmrc/epayeapi/connectors/ConnectorBase.scala +++ b/app/uk/gov/hmrc/epayeapi/connectors/ConnectorBase.scala @@ -38,6 +38,7 @@ trait ConnectorBase { result.recover({ case ex: Exception => + Logger.error("HTTP request threw exception", ex) ApiException(ex.getMessage) }) } diff --git a/app/uk/gov/hmrc/epayeapi/connectors/EpayeConnector.scala b/app/uk/gov/hmrc/epayeapi/connectors/EpayeConnector.scala index 2820a91..0e62ffb 100644 --- a/app/uk/gov/hmrc/epayeapi/connectors/EpayeConnector.scala +++ b/app/uk/gov/hmrc/epayeapi/connectors/EpayeConnector.scala @@ -29,6 +29,7 @@ import scala.concurrent.{ExecutionContext, Future} case class EpayeApiConfig(baseUrl: String) + @Singleton case class EpayeConnector @Inject() ( config: EpayeApiConfig, @@ -36,14 +37,14 @@ case class EpayeConnector @Inject() ( implicit val ec: ExecutionContext ) extends ConnectorBase { - def getTotals(empRef: EmpRef, headers: HeaderCarrier): Future[ApiResponse[AggregatedTotals]] = { + def getTotal(empRef: EmpRef, headers: HeaderCarrier): Future[ApiResponse[EpayeTotalsResponse]] = { val url = s"${config.baseUrl}" + s"/epaye" + s"/${empRef.encodedValue}" + - s"/api/v1/totals" + s"/api/v1/annual-statement" - get[AggregatedTotals](url, headers) + get[EpayeTotalsResponse](url, headers) } def getTotalsByType(empRef: EmpRef, headers: HeaderCarrier): Future[ApiResponse[AggregatedTotalsByType]] = { diff --git a/app/uk/gov/hmrc/epayeapi/controllers/GetSummaryController.scala b/app/uk/gov/hmrc/epayeapi/controllers/GetSummaryController.scala index 696f728..338d8b0 100644 --- a/app/uk/gov/hmrc/epayeapi/controllers/GetSummaryController.scala +++ b/app/uk/gov/hmrc/epayeapi/controllers/GetSummaryController.scala @@ -16,7 +16,6 @@ package uk.gov.hmrc.epayeapi.controllers -import java.util.NoSuchElementException import javax.inject.{Inject, Singleton} import akka.stream.Materializer @@ -27,13 +26,13 @@ import uk.gov.hmrc.auth.core.AuthConnector import uk.gov.hmrc.domain.EmpRef import uk.gov.hmrc.epayeapi.connectors.EpayeConnector import uk.gov.hmrc.epayeapi.models.Formats._ -import uk.gov.hmrc.epayeapi.models.api.{ApiJsonError, ApiNotFound, ApiSuccess} +import uk.gov.hmrc.epayeapi.models.api.{ApiJsonError, ApiNotFound, ApiResponse, ApiSuccess} import uk.gov.hmrc.epayeapi.models.{ApiError, SummaryResponse} import scala.concurrent.ExecutionContext @Singleton -case class GetSummaryController @Inject()( +case class GetSummaryController @Inject() ( authConnector: AuthConnector, epayeConnector: EpayeConnector, implicit val ec: ExecutionContext, @@ -43,20 +42,17 @@ case class GetSummaryController @Inject()( def getSummary(empRef: EmpRef): EssentialAction = EmpRefAction(empRef) { Action.async { request => - val totalsResp = epayeConnector.getTotals(empRef, hc(request)) - val totalsByTypeResp = epayeConnector.getTotalsByType(empRef, hc(request)) - - val resp = for { - ApiSuccess(totals) <- totalsResp - ApiSuccess(totalsByType) <- totalsByTypeResp - } yield { - Ok(Json.toJson(SummaryResponse(empRef, totals, totalsByType))) - } - - // TODO: Revisit and add error handling! - resp.recover { - case ex: NoSuchElementException => + epayeConnector.getTotal(empRef, hc(request)).map { + case ApiSuccess(totals) => + Ok(Json.toJson(SummaryResponse(empRef, totals))) + case ApiJsonError(err) => + Logger.error(s"Upstream returned invalid json: $err") + InternalServerError(Json.toJson(ApiError.InternalServerError)) + case ApiNotFound() => NotFound(Json.toJson(ApiError.EmpRefNotFound)) + case error: ApiResponse[_] => + Logger.error(s"Error while fetching totals: $error") + InternalServerError(Json.toJson(ApiError.InternalServerError)) } } } diff --git a/app/uk/gov/hmrc/epayeapi/models/EpayeTotalsResponse.scala b/app/uk/gov/hmrc/epayeapi/models/EpayeTotalsResponse.scala new file mode 100644 index 0000000..6571433 --- /dev/null +++ b/app/uk/gov/hmrc/epayeapi/models/EpayeTotalsResponse.scala @@ -0,0 +1,23 @@ +/* + * Copyright 2017 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 uk.gov.hmrc.epayeapi.models + +case class EpayeTotals(balance: DebitAndCredit) +case class EpayeTotalsItem(totals: EpayeTotals) +case class EpayeTotalsResponse(rti: EpayeTotalsItem, nonRti: EpayeTotalsItem) { + def overall: BigDecimal = rti.totals.balance.debit + nonRti.totals.balance.debit +} diff --git a/app/uk/gov/hmrc/epayeapi/models/Formats.scala b/app/uk/gov/hmrc/epayeapi/models/Formats.scala index a5a08cf..34b5324 100644 --- a/app/uk/gov/hmrc/epayeapi/models/Formats.scala +++ b/app/uk/gov/hmrc/epayeapi/models/Formats.scala @@ -52,6 +52,10 @@ trait Formats { implicit val rtiChargeFormat: Format[RtiCharge] = Json.format[RtiCharge] implicit val nonRtiChargeFormat: Format[NonRtiCharge] = Json.format[NonRtiCharge] implicit val chargesSummaryFormat: Format[ChargesSummary] = Json.format[ChargesSummary] + implicit val epayeTotals: Format[EpayeTotals] = Json.format[EpayeTotals] + implicit val epayeTotalsItems: Format[EpayeTotalsItem] = Json.format[EpayeTotalsItem] + implicit val epayeTotalsResponse: Format[EpayeTotalsResponse] = Json.format[EpayeTotalsResponse] + } object Formats extends Formats diff --git a/app/uk/gov/hmrc/epayeapi/models/SummaryResponse.scala b/app/uk/gov/hmrc/epayeapi/models/SummaryResponse.scala index 2404534..6df7e17 100644 --- a/app/uk/gov/hmrc/epayeapi/models/SummaryResponse.scala +++ b/app/uk/gov/hmrc/epayeapi/models/SummaryResponse.scala @@ -34,13 +34,13 @@ case class SummaryResponse( ) object SummaryResponse { - def apply(empRef: EmpRef, totals: AggregatedTotals, byType: AggregatedTotalsByType): SummaryResponse = + def apply(empRef: EmpRef, total: EpayeTotalsResponse): SummaryResponse = SummaryResponse( OutstandingCharges( - totals.debit, + total.overall, Breakdown( - byType.rti.debit, - byType.nonRti.debit + rti = total.rti.totals.balance.debit, + nonRti = total.nonRti.totals.balance.debit ) ), SummaryLinks(empRef) diff --git a/conf/logback.xml b/conf/logback.xml index d966dce..0f080a6 100644 --- a/conf/logback.xml +++ b/conf/logback.xml @@ -1,6 +1,8 @@ + + logs/epaye-api.log @@ -10,7 +12,7 @@ - %date{ISO8601} level=[%level] logger=[%logger] thread=[%thread] rid=[%X{X-Request-ID}] user=[%X{Authorization}] message=[%message] %replace(exception=[%xException]){'^exception=\[\]$',''}%n + %date{ISO8601} level=[%coloredLevel] logger=[%logger] thread=[%thread] rid=[%X{X-Request-ID}] user=[%X{Authorization}] message=[%message] %replace(exception=[%xException]){'^exception=\[\]$',''}%n diff --git a/test/uk/gov/hmrc/epayeapi/connectors/EpayeConnectorSpec.scala b/test/uk/gov/hmrc/epayeapi/connectors/EpayeConnectorSpec.scala index 52d9d79..debd672 100644 --- a/test/uk/gov/hmrc/epayeapi/connectors/EpayeConnectorSpec.scala +++ b/test/uk/gov/hmrc/epayeapi/connectors/EpayeConnectorSpec.scala @@ -39,7 +39,7 @@ class EpayeConnectorSpec extends UnitSpec with MockitoSugar with ScalaFutures { val config = EpayeApiConfig("https://EPAYE") val connector = EpayeConnector(config, http, global) val empRef = EmpRef("123", "456") - val urlTotals = s"${config.baseUrl}/epaye/${empRef.encodedValue}/api/v1/totals" + val urlTotals = s"${config.baseUrl}/epaye/${empRef.encodedValue}/api/v1/annual-statement" val urlTotalsByType = s"${config.baseUrl}/epaye/${empRef.encodedValue}/api/v1/totals/by-type" val urlAnnualStatement = s"${config.baseUrl}/epaye/${empRef.encodedValue}/api/v1/annual-statement" } @@ -48,12 +48,38 @@ class EpayeConnectorSpec extends UnitSpec with MockitoSugar with ScalaFutures { "retrieve the total credit and debit for a given empRef" in new Setup { when(connector.http.GET(urlTotals)).thenReturn { successful { - HttpResponse(Status.OK, responseString = Some(""" {"credit": 100, "debit": 0} """)) + HttpResponse(Status.OK, responseString = Some( + """ + |{ + | "rti": { + | "totals": { + | "balance": { + | "debit": 100, + | "credit": 0 + | } + | } + | }, + | "nonRti": { + | "totals": { + | "balance": { + | "debit": 23, + | "credit": 0 + | } + | } + | } + |} + """.stripMargin + )) } } - connector.getTotals(empRef, hc).futureValue shouldBe - ApiSuccess(AggregatedTotals(credit = 100, debit = 0)) + connector.getTotal(empRef, hc).futureValue shouldBe + ApiSuccess( + EpayeTotalsResponse( + EpayeTotalsItem(EpayeTotals(DebitAndCredit(100, 0))), + EpayeTotalsItem(EpayeTotals(DebitAndCredit(23, 0))) + ) + ) } "retrieve the total by type for a given empRef" in new Setup { when(connector.http.GET(urlTotalsByType)).thenReturn { @@ -83,13 +109,12 @@ class EpayeConnectorSpec extends UnitSpec with MockitoSugar with ScalaFutures { AnnualTotal(DebitAndCredit(100.2, 0), Cleared(0, 0), DebitAndCredit(100.2, 0)) ), AnnualSummary( - List(LineItem(TaxYear(2017),None,DebitAndCredit(20.0,0),Cleared(0,0),DebitAndCredit(20.0,0), new LocalDate(2018, 2, 22),false, Some("P11D_CLASS_1A_CHARGE"))), - AnnualTotal(DebitAndCredit(20.0,0),Cleared(0,0),DebitAndCredit(20.0,0)) + List(LineItem(TaxYear(2017), None, DebitAndCredit(20.0, 0), Cleared(0, 0), DebitAndCredit(20.0, 0), new LocalDate(2018, 2, 22), false, Some("P11D_CLASS_1A_CHARGE"))), + AnnualTotal(DebitAndCredit(20.0, 0), Cleared(0, 0), DebitAndCredit(20.0, 0)) ) ) ) } } - } diff --git a/test/uk/gov/hmrc/epayeapi/controllers/GetSummarySpec.scala b/test/uk/gov/hmrc/epayeapi/controllers/GetSummarySpec.scala index 75527a0..77bc15c 100644 --- a/test/uk/gov/hmrc/epayeapi/controllers/GetSummarySpec.scala +++ b/test/uk/gov/hmrc/epayeapi/controllers/GetSummarySpec.scala @@ -57,7 +57,7 @@ class GetSummarySpec extends AppSpec with BeforeAndAfterEach { val differentEnrolment = AuthOk(Enrolment("IR-Else", Seq(ton, tor), "activated", ConfidenceLevel.L300)) - def config(implicit a: Application) = + def config(implicit a: Application): EpayeApiConfig = inject[EpayeApiConfig] def request(implicit a: Application): Future[Result] = @@ -73,23 +73,32 @@ class GetSummarySpec extends AppSpec with BeforeAndAfterEach { val firstUrl = s"${config.baseUrl}" + s"/epaye" + s"/${empRef.encodedValue}" + - s"/api/v1/totals" - - val secondUrl = - s"${config.baseUrl}" + - s"/epaye" + - s"/${empRef.encodedValue}" + - s"/api/v1/totals/by-type" + s"/api/v1/annual-statement" when(http.GET[HttpResponse](Matchers.eq(firstUrl))(anyObject(), anyObject())).thenReturn { successful { - HttpResponse(200, responseString = Some(""" {"credit": 0, "debit": 123} """)) - } - } - - when(http.GET[HttpResponse](Matchers.eq(secondUrl))(anyObject(), anyObject())).thenReturn { - successful { - HttpResponse(200, responseString = Some(""" { "rti": {"credit": 0, "debit": 100}, "nonRti": {"credit": 0, "debit": 23} } """)) + HttpResponse(OK, responseString = Some( + """ + |{ + | "rti": { + | "totals": { + | "balance": { + | "debit": 100, + | "credit": 0 + | } + | } + | }, + | "nonRti": { + | "totals": { + | "balance": { + | "debit": 23, + | "credit": 0 + | } + | } + | } + |} + """.stripMargin + )) } }