Skip to content

Commit

Permalink
Decoupled NRS from VAT Submission (#204)
Browse files Browse the repository at this point in the history
* Decoupled NRS from VAT Submission

* Updated as per review comments

* Fixed issues related to DateTime

* Modified code for handling empty NRS data
  • Loading branch information
narendravijayarao authored and westwater committed Jul 24, 2018
1 parent 571b496 commit 9c36894
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 13 deletions.
10 changes: 7 additions & 3 deletions app/uk/gov/hmrc/vatapi/httpparsers/NrsSubmissionHttpParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,23 @@ object NrsSubmissionHttpParser {
override def read(method: String, url: String, response: HttpResponse): NrsSubmissionOutcome = {
logger.debug(s"[NrsSubmissionHttpParser][#reads] - Reading NRS Response")
response.status match {
case BAD_REQUEST =>
logger.warn(s"[NrsSubmissionHttpParser][#reads] - BAD_REQUEST status from NRS Response")
Left(NrsError)
case ACCEPTED =>
response.json.validate[NRSData].fold(
invalid => {
logger.warn(s"[NrsSubmissionHttpParser][#reads] - Error reading NRS Response: $invalid")
Left(NrsError)
},
valid =>{
logger.debug(s"[NrsSubmissionHttpParser][#reads] - Successfully retrieved NRS Data: $valid")
logger.debug(s"[NrsSubmissionHttpParser][#reads] - Retrieved NRS Data: $valid")
Right(valid)
}
)
case e =>
logger.warn(s"[NrsSubmissionHttpParser][#reads] - Non-OK NRS Response: STATUS $e")
Left(NrsError)
logger.debug(s"[NrsSubmissionHttpParser][#reads] - Retrieved NRS status : $e")
Right(EmptyNrsData)
}
}
}
Expand All @@ -56,6 +59,7 @@ case class NRSData(nrSubmissionId: String,
cadesTSignature: String,
timestamp: String
)
object EmptyNrsData extends NRSData("","","")

object NRSData {
implicit val format: OFormat[NRSData] = Json.format[NRSData]
Expand Down
16 changes: 11 additions & 5 deletions app/uk/gov/hmrc/vatapi/orchestrators/VatReturnsOrchestrator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import uk.gov.hmrc.domain.Vrn
import uk.gov.hmrc.http.HeaderCarrier
import uk.gov.hmrc.vatapi.audit.{AuditEvents, AuditService}
import uk.gov.hmrc.vatapi.auth.{Agent, AuthContext}
import uk.gov.hmrc.vatapi.httpparsers.NRSData
import uk.gov.hmrc.vatapi.httpparsers.{EmptyNrsData, NRSData}
import uk.gov.hmrc.vatapi.models.{ErrorResult, Errors, InternalServerErrorResult, VatReturnDeclaration}
import uk.gov.hmrc.vatapi.resources.AuthRequest
import uk.gov.hmrc.vatapi.resources.wrappers.VatReturnResponse
Expand Down Expand Up @@ -56,9 +56,16 @@ trait VatReturnsOrchestrator {
case Agent(_,_,_,enrolments) => enrolments.getEnrolment("HMRC-AS-AGENT").flatMap(_.getIdentifier("AgentReferenceNumber")).map(_.value)
case c: AuthContext => c.agentReference
}
auditService.audit(AuditEvents.nrsAudit(vrn, nrsData,
request.headers.get("Authorization").getOrElse(""), request.headers.get("x-correlationid").getOrElse("")))
vatReturnsService.submit(vrn, vatReturn.toDes(DateTime.parse(nrsData.timestamp), arn)) map { response => Right(response withNrsData nrsData)}
nrsData match {
case EmptyNrsData =>
vatReturnsService.submit(vrn,
vatReturn.toDes(DateTime.now(), arn)) map { response => Right(response withNrsData nrsData)}
case _ =>
auditService.audit(AuditEvents.nrsAudit(vrn, nrsData,
request.headers.get("Authorization").getOrElse(""), request.headers.get("x-correlationid").getOrElse("")))
vatReturnsService.submit(vrn,
vatReturn.toDes(DateTime.parse(nrsData.timestamp), arn)) map { response => Right(response withNrsData nrsData)}
}
case Left(e) =>
logger.error(s"[VatReturnsOrchestrator][submitVatReturn] - Error retrieving data from NRS: $e")
Future.successful(Left(InternalServerErrorResult(Errors.InternalServerError.message)))
Expand All @@ -67,7 +74,6 @@ trait VatReturnsOrchestrator {

case class VatReturnOrchestratorResponse(nrs: NRSData, vatReturnResponse: VatReturnResponse)


}


Expand Down
11 changes: 11 additions & 0 deletions func/uk/gov/hmrc/support/Givens.scala
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,17 @@ class Givens(httpVerbs: HttpVerbs) {
)
givens
}

def nrsFailureforNonBadRequest(vrn: Vrn): Givens = {
stubFor(any(urlMatching(s".*/submission.*"))
.willReturn(
aResponse()
.withStatus(FORBIDDEN)
.withBody("{}")
)
)
givens
}
}

def des() = new Des(this)
Expand Down
20 changes: 20 additions & 0 deletions func/uk/gov/hmrc/vatapi/resources/ValueAddedTaxReturnsSpec.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package uk.gov.hmrc.vatapi.resources

import org.joda.time.DateTime
import play.api.libs.json.Json
import uk.gov.hmrc.support.BaseFunctionalSpec

Expand Down Expand Up @@ -54,6 +55,25 @@ class ValueAddedTaxReturnsSpec extends BaseFunctionalSpec {
.responseContainsHeader("Receipt-TimeStamp", "2018-02-14T09:32:15Z")
}

"allow users to submit VAT returns for non bad_request NRS response" in {
given()
.stubAudit
.userIsFullyAuthorisedForTheNrsDependantResource
.nrs().nrsFailureforNonBadRequest(vrn)
.des().vatReturns.expectVatReturnSubmissionFor(vrn)
.when()
.post(s"/$vrn/returns", Some(Json.parse(body())))
.withHeaders("Authorization", "Bearer testtoken")
.thenAssertThat()
.statusIs(201)
.bodyHasPath("\\paymentIndicator", "BANK")
.bodyHasPath("\\processingDate", "2018-03-01T11:43:43.195Z")
.bodyHasPath("\\formBundleNumber", "891713832155")
.responseContainsHeader("Receipt-Id", "")
.responseContainsHeader("Receipt-Signature", "NOT CURRENTLY IMPLEMENTED")
.responseContainsHeader("Receipt-TimeStamp", "")
}

"allow users to submit VAT returns even with negative amounts" in {
given()
.stubAudit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@
package uk.gov.hmrc.vatapi.httpparsers

import org.scalatest.EitherValues
import play.api.http.Status._
import play.api.libs.json.Json
import uk.gov.hmrc.http.HttpResponse
import uk.gov.hmrc.vatapi.UnitSpec
import play.api.http.Status._
import uk.gov.hmrc.vatapi.assets.TestConstants.NRSResponse._
import uk.gov.hmrc.vatapi.httpparsers.NrsSubmissionHttpParser.NrsSubmissionOutcomeReads.read


class NrsSubmissionHttpParserSpec extends UnitSpec with EitherValues {

val successResponse = HttpResponse(ACCEPTED, responseJson = Some(Json.toJson(nrsData)))
val successBadJsonResponse = HttpResponse(ACCEPTED, responseJson = Some(Json.toJson("{}")))
val successBadJsonResponse = HttpResponse(NOT_FOUND, responseJson = Some(Json.toJson("{}")))
val failureResponse = HttpResponse(BAD_REQUEST, responseJson = Some(Json.toJson("{}")))

"NrsSubmissionOutcome#read" when {
Expand All @@ -41,14 +41,14 @@ class NrsSubmissionHttpParserSpec extends UnitSpec with EitherValues {
}
"return NrsError" when {
"the Json returned is not valid" in {
read("", "", successBadJsonResponse).left.value shouldBe NrsError
read("", "", successBadJsonResponse).right.value shouldBe EmptyNrsData
}
}
}

"the response status is not OK" should {
"return NrsError" in {
read("", "", successBadJsonResponse).left.value shouldBe NrsError
read("", "", failureResponse).left.value shouldBe NrsError
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package uk.gov.hmrc.vatapi.resources

import play.api.libs.json.{JsValue, Json}
import play.api.test.FakeRequest
import play.mvc.Http.MimeTypes
import uk.gov.hmrc.http.HttpResponse
Expand Down

0 comments on commit 9c36894

Please sign in to comment.