From 33c5d4937c0f288c9a7de118c424f23008027e5a Mon Sep 17 00:00:00 2001 From: besscerule <63503574+besscerule@users.noreply.github.com> Date: Wed, 16 Oct 2024 10:01:55 +0100 Subject: [PATCH 1/2] TGP-2420: added refactor for patch and put with testing --- .../CyaUpdateRecordController.scala | 71 +++++- .../CyaUpdateRecordControllerSpec.scala | 240 +++++++++++++++++- 2 files changed, 306 insertions(+), 5 deletions(-) diff --git a/app/controllers/CyaUpdateRecordController.scala b/app/controllers/CyaUpdateRecordController.scala index 2742d72a..79586797 100644 --- a/app/controllers/CyaUpdateRecordController.scala +++ b/app/controllers/CyaUpdateRecordController.scala @@ -22,6 +22,8 @@ import com.google.inject.Inject import config.FrontendAppConfig import connectors.{GoodsRecordConnector, OttConnector} import controllers.actions.{DataRequiredAction, DataRetrievalAction, IdentifierAction} +import models.router.requests.PutRecordRequest +import models.router.responses.GetGoodsRecordResponse import models.{CheckMode, Country, NormalMode, UpdateGoodsRecord, UserAnswers, ValidationError} import navigation.Navigator import org.apache.pekko.Done @@ -228,7 +230,6 @@ class CyaUpdateRecordController @Inject() ( newUpdateGoodsRecord: UpdateGoodsRecord )(implicit hc: HeaderCarrier): Future[Done] = if (newValue != oldValue) { - // TODO: remove this flag when EIS has implemented the PATCH method - TGP-2417 and keep the call to patchGoodsRecord as default if (config.useEisPatchMethod) { goodsRecordConnector.patchGoodsRecord( @@ -239,6 +240,30 @@ class CyaUpdateRecordController @Inject() ( newUpdateGoodsRecord ) } + } else { + Future.successful(Done) + } + + private def updateGoodsRecordIfPutValueChanged( + newValue: String, + oldValue: String, + newUpdateGoodsRecord: UpdateGoodsRecord, + oldRecord: GetGoodsRecordResponse, + putRecordRequest: PutRecordRequest + )(implicit hc: HeaderCarrier): Future[Done] = + if (newValue != oldValue) { + + // TODO: remove this flag when EIS has implemented the PATCH method - TGP-2417 and keep the call to putGoodsRecord as default + if (config.useEisPatchMethod) { + goodsRecordConnector.putGoodsRecord( + putRecordRequest, + oldRecord.recordId + ) + } else { + goodsRecordConnector.updateGoodsRecord( + newUpdateGoodsRecord + ) + } } else { Future.successful(Done) @@ -271,8 +296,29 @@ class CyaUpdateRecordController @Inject() ( ) updateGoodsRecord <- Future.successful(UpdateGoodsRecord(request.eori, recordId, countryOfOrigin = Some(countryOfOrigin))) + putGoodsRecord <- Future.successful( + PutRecordRequest( + actorId = oldRecord.eori, + traderRef = oldRecord.traderRef, + comcode = oldRecord.comcode, + goodsDescription = oldRecord.goodsDescription, + countryOfOrigin = countryOfOrigin, + category = None, + assessments = oldRecord.assessments, + supplementaryUnit = oldRecord.supplementaryUnit, + measurementUnit = oldRecord.measurementUnit, + comcodeEffectiveFromDate = oldRecord.comcodeEffectiveFromDate, + comcodeEffectiveToDate = oldRecord.comcodeEffectiveToDate + ) + ) _ = auditService.auditFinishUpdateGoodsRecord(recordId, request.affinityGroup, updateGoodsRecord) - _ <- updateGoodsRecordIfValueChanged(countryOfOrigin, oldRecord.countryOfOrigin, updateGoodsRecord) + _ <- updateGoodsRecordIfPutValueChanged( + countryOfOrigin, + oldRecord.countryOfOrigin, + updateGoodsRecord, + oldRecord, + putGoodsRecord + ) updatedAnswersWithChange <- Future.fromTry(request.userAnswers.remove(HasCountryOfOriginChangePage(recordId))) updatedAnswers <- Future.fromTry(updatedAnswersWithChange.remove(CountryOfOriginUpdatePage(recordId))) _ <- sessionRepository.set(updatedAnswers) @@ -316,11 +362,28 @@ class CyaUpdateRecordController @Inject() ( ) updateGoodsRecord <- Future.successful(UpdateGoodsRecord(request.eori, recordId, commodityCode = Some(commodity))) + putGoodsRecord <- Future.successful( + PutRecordRequest( + actorId = oldRecord.eori, + traderRef = oldRecord.traderRef, + comcode = commodity.commodityCode, + goodsDescription = oldRecord.goodsDescription, + countryOfOrigin = oldRecord.countryOfOrigin, + category = None, + assessments = oldRecord.assessments, + supplementaryUnit = oldRecord.supplementaryUnit, + measurementUnit = oldRecord.measurementUnit, + comcodeEffectiveFromDate = oldRecord.comcodeEffectiveFromDate, + comcodeEffectiveToDate = oldRecord.comcodeEffectiveToDate + ) + ) _ = auditService.auditFinishUpdateGoodsRecord(recordId, request.affinityGroup, updateGoodsRecord) - _ <- updateGoodsRecordIfValueChanged( + _ <- updateGoodsRecordIfPutValueChanged( commodity.commodityCode, oldRecord.comcode, - updateGoodsRecord + updateGoodsRecord, + oldRecord, + putGoodsRecord ) updatedAnswersWithChange <- Future.fromTry(request.userAnswers.remove(HasCommodityCodeChangePage(recordId))) diff --git a/test/controllers/CyaUpdateRecordControllerSpec.scala b/test/controllers/CyaUpdateRecordControllerSpec.scala index 986b039e..b9cbd08a 100644 --- a/test/controllers/CyaUpdateRecordControllerSpec.scala +++ b/test/controllers/CyaUpdateRecordControllerSpec.scala @@ -20,7 +20,7 @@ import base.SpecBase import base.TestConstants.{testEori, testRecordId} import config.FrontendAppConfig import connectors.{GoodsRecordConnector, OttConnector} -import models.{CheckMode, Country, UpdateGoodsRecord} +import models.{CheckMode, Commodity, Country, UpdateGoodsRecord} import org.apache.pekko.Done import org.mockito.ArgumentMatchers.{any, eq => eqTo} import org.mockito.Mockito.{never, verify, when} @@ -316,6 +316,63 @@ class CyaUpdateRecordControllerSpec extends SpecBase with SummaryListFluency wit } } + "must PUT the goods record, cleanse the data and redirect to the Goods record Page" in { + + val userAnswers = emptyUserAnswers + .set(page, answer) + .success + .value + .set(warningPage, true) + .success + .value + + val mockGoodsRecordConnector = mock[GoodsRecordConnector] + val mockAuditService = mock[AuditService] + val mockSessionRepository = mock[SessionRepository] + val mockAppConfig = mock[FrontendAppConfig] + + when(mockAppConfig.useEisPatchMethod).thenReturn(true) + + when(mockGoodsRecordConnector.putGoodsRecord(any(), any())(any())).thenReturn(Future.successful(Done)) + when(mockAuditService.auditFinishUpdateGoodsRecord(any(), any(), any())(any)) + .thenReturn(Future.successful(Done)) + when(mockSessionRepository.set(any())).thenReturn(Future.successful(true)) + + when(mockGoodsRecordConnector.getRecord(any(), any())(any())) thenReturn Future + .successful(record) + + val application = + applicationBuilder(userAnswers = Some(userAnswers)) + .overrides( + bind[GoodsRecordConnector].toInstance(mockGoodsRecordConnector), + bind[SessionRepository].toInstance(mockSessionRepository), + bind[AuditService].toInstance(mockAuditService), + bind[FrontendAppConfig].toInstance(mockAppConfig) + ) + .build() + + running(application) { + val request = FakeRequest(POST, postUrl) + + val result = route(application, request).value + status(result) mustEqual SEE_OTHER + redirectLocation(result).value mustEqual routes.SingleRecordController.onPageLoad(testRecordId).url + verify(mockGoodsRecordConnector).putGoodsRecord(any(), any())(any()) + verify(mockSessionRepository).set(any()) + + withClue("must call the audit connector with the supplied details") { + verify(mockAuditService) + .auditFinishUpdateGoodsRecord( + eqTo(testRecordId), + eqTo(AffinityGroup.Individual), + eqTo(expectedPayload) + )( + any() + ) + } + } + } + } "when user answers cannot create an update goods record" - { @@ -776,6 +833,60 @@ class CyaUpdateRecordControllerSpec extends SpecBase with SummaryListFluency wit } } + "must PATCH the goods record, cleanse the data and redirect to the Goods record Page" in { + + val userAnswers = emptyUserAnswers + .set(page, answer) + .success + .value + + val mockGoodsRecordConnector = mock[GoodsRecordConnector] + val mockAuditService = mock[AuditService] + val mockSessionRepository = mock[SessionRepository] + val mockAppConfig = mock[FrontendAppConfig] + + when(mockAppConfig.useEisPatchMethod).thenReturn(true) + + when(mockGoodsRecordConnector.patchGoodsRecord(any())(any())).thenReturn(Future.successful(Done)) + when(mockAuditService.auditFinishUpdateGoodsRecord(any(), any(), any())(any)) + .thenReturn(Future.successful(Done)) + when(mockSessionRepository.set(any())).thenReturn(Future.successful(true)) + + when(mockGoodsRecordConnector.getRecord(any(), any())(any())) thenReturn Future + .successful(record) + + val application = + applicationBuilder(userAnswers = Some(userAnswers)) + .overrides( + bind[GoodsRecordConnector].toInstance(mockGoodsRecordConnector), + bind[SessionRepository].toInstance(mockSessionRepository), + bind[AuditService].toInstance(mockAuditService), + bind[FrontendAppConfig].toInstance(mockAppConfig) + ) + .build() + + running(application) { + val request = FakeRequest(POST, postUrl) + + val result = route(application, request).value + status(result) mustEqual SEE_OTHER + redirectLocation(result).value mustEqual routes.SingleRecordController.onPageLoad(testRecordId).url + verify(mockGoodsRecordConnector).patchGoodsRecord(any())(any()) + verify(mockSessionRepository).set(any()) + + withClue("must call the audit connector with the supplied details") { + verify(mockAuditService) + .auditFinishUpdateGoodsRecord( + eqTo(testRecordId), + eqTo(AffinityGroup.Individual), + eqTo(expectedPayload) + )( + any() + ) + } + } + } + "when trader reference has not been changed must not update the goods record and redirect to the Home Page" in { val answer = record.traderRef val expectedPayload = UpdateGoodsRecord(testEori, testRecordId, traderReference = Some(answer)) @@ -1153,6 +1264,133 @@ class CyaUpdateRecordControllerSpec extends SpecBase with SummaryListFluency wit } } } + + "must PUT the goods record, cleanse the data and redirect to the Goods record Page" in { + + val userAnswers = emptyUserAnswers + .set(page, testCommodity.commodityCode) + .success + .value + .set(HasCorrectGoodsCommodityCodeUpdatePage(testRecordId), true) + .success + .value + .set(warningPage, true) + .success + .value + .set(HasCommodityCodeChangePage(testRecordId), true) + .success + .value + .set(CommodityUpdateQuery(testRecordId), testCommodity) + .success + .value + + val mockGoodsRecordConnector = mock[GoodsRecordConnector] + val mockAuditService = mock[AuditService] + val mockSessionRepository = mock[SessionRepository] + val mockAppConfig = mock[FrontendAppConfig] + + when(mockAppConfig.useEisPatchMethod).thenReturn(true) + + when(mockGoodsRecordConnector.putGoodsRecord(any(), any())(any())).thenReturn(Future.successful(Done)) + when(mockAuditService.auditFinishUpdateGoodsRecord(any(), any(), any())(any)) + .thenReturn(Future.successful(Done)) + when(mockSessionRepository.set(any())).thenReturn(Future.successful(true)) + + when(mockGoodsRecordConnector.getRecord(any(), any())(any())) thenReturn Future + .successful(record) + + val application = + applicationBuilder(userAnswers = Some(userAnswers)) + .overrides( + bind[GoodsRecordConnector].toInstance(mockGoodsRecordConnector), + bind[SessionRepository].toInstance(mockSessionRepository), + bind[AuditService].toInstance(mockAuditService), + bind[FrontendAppConfig].toInstance(mockAppConfig) + ) + .build() + + running(application) { + val request = FakeRequest(POST, postUrl) + + val result = route(application, request).value + status(result) mustEqual SEE_OTHER + redirectLocation(result).value mustEqual routes.SingleRecordController.onPageLoad(testRecordId).url + verify(mockGoodsRecordConnector).putGoodsRecord(any(), any())(any()) + verify(mockSessionRepository).set(any()) + + withClue("must call the audit connector with the supplied details") { + verify(mockAuditService) + .auditFinishUpdateGoodsRecord( + eqTo(testRecordId), + eqTo(AffinityGroup.Individual), + eqTo(expectedPayload) + )( + any() + ) + } + } + } + + "when commodity code has not been changed must not update the goods record and redirect to the Home Page" in { + + val answer = Commodity(record.comcode, List("test"), validityStartDate, None) + val expectedPayload = UpdateGoodsRecord(testEori, testRecordId, commodityCode = Some(answer)) + + val userAnswers = emptyUserAnswers + .set(page, answer.commodityCode) + .success + .value + .set(HasCorrectGoodsCommodityCodeUpdatePage(testRecordId), true) + .success + .value + .set(warningPage, true) + .success + .value + .set(HasCommodityCodeChangePage(testRecordId), true) + .success + .value + .set(CommodityUpdateQuery(testRecordId), answer) + .success + .value + + val mockConnector = mock[GoodsRecordConnector] + val mockAuditService = mock[AuditService] + + when(mockConnector.getRecord(any(), any())(any())).thenReturn(Future.successful(record)) + when(mockAuditService.auditFinishUpdateGoodsRecord(any(), any(), any())(any)) + .thenReturn(Future.successful(Done)) + + val application = + applicationBuilder(userAnswers = Some(userAnswers)) + .overrides( + bind[GoodsRecordConnector].toInstance(mockConnector), + bind[AuditService].toInstance(mockAuditService) + ) + .build() + + running(application) { + val request = FakeRequest(POST, postUrl) + + val result = route(application, request).value + + status(result) mustEqual SEE_OTHER + redirectLocation(result).value mustEqual routes.SingleRecordController.onPageLoad(testRecordId).url + verify(mockConnector, never()).updateGoodsRecord(any())(any()) + verify(mockConnector).getRecord(eqTo(testEori), eqTo(testRecordId))(any()) + + withClue("must call the audit connector with the supplied details") { + verify(mockAuditService) + .auditFinishUpdateGoodsRecord( + eqTo(testRecordId), + eqTo(AffinityGroup.Individual), + eqTo(expectedPayload) + )( + any() + ) + } + } + } + } "when user answers cannot create an update goods record" - { From 0f0f91cb0baf98fb5b604016194c45b5ba5911fc Mon Sep 17 00:00:00 2001 From: besscerule <63503574+besscerule@users.noreply.github.com> Date: Wed, 16 Oct 2024 10:51:28 +0100 Subject: [PATCH 2/2] TGP-2420: pr changes --- app/controllers/CyaUpdateRecordController.scala | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/controllers/CyaUpdateRecordController.scala b/app/controllers/CyaUpdateRecordController.scala index 79586797..63fb4e21 100644 --- a/app/controllers/CyaUpdateRecordController.scala +++ b/app/controllers/CyaUpdateRecordController.scala @@ -23,7 +23,6 @@ import config.FrontendAppConfig import connectors.{GoodsRecordConnector, OttConnector} import controllers.actions.{DataRequiredAction, DataRetrievalAction, IdentifierAction} import models.router.requests.PutRecordRequest -import models.router.responses.GetGoodsRecordResponse import models.{CheckMode, Country, NormalMode, UpdateGoodsRecord, UserAnswers, ValidationError} import navigation.Navigator import org.apache.pekko.Done @@ -248,7 +247,6 @@ class CyaUpdateRecordController @Inject() ( newValue: String, oldValue: String, newUpdateGoodsRecord: UpdateGoodsRecord, - oldRecord: GetGoodsRecordResponse, putRecordRequest: PutRecordRequest )(implicit hc: HeaderCarrier): Future[Done] = if (newValue != oldValue) { @@ -257,7 +255,7 @@ class CyaUpdateRecordController @Inject() ( if (config.useEisPatchMethod) { goodsRecordConnector.putGoodsRecord( putRecordRequest, - oldRecord.recordId + newUpdateGoodsRecord.recordId ) } else { goodsRecordConnector.updateGoodsRecord( @@ -316,7 +314,6 @@ class CyaUpdateRecordController @Inject() ( countryOfOrigin, oldRecord.countryOfOrigin, updateGoodsRecord, - oldRecord, putGoodsRecord ) updatedAnswersWithChange <- Future.fromTry(request.userAnswers.remove(HasCountryOfOriginChangePage(recordId))) @@ -382,7 +379,6 @@ class CyaUpdateRecordController @Inject() ( commodity.commodityCode, oldRecord.comcode, updateGoodsRecord, - oldRecord, putGoodsRecord ) updatedAnswersWithChange <-