Skip to content

Commit bea3801

Browse files
authored
DTR-3563 Fix issue where company details is deleted when non-main purchaser removed (#198)
1 parent 69dfa10 commit bea3801

File tree

2 files changed

+84
-25
lines changed

2 files changed

+84
-25
lines changed

app/services/purchaser/PurchaserRemoveService.scala

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,14 @@ class PurchaserRemoveService @Inject()(
140140
)(f: Long => Future[A])(implicit hc: HeaderCarrier,
141141
ec: ExecutionContext,
142142
request: DataRequest[AnyContent]): Future[A] =
143+
logger.info(s"[PurchaserRemoveService][withNewVersion] version passed in: $version")
143144
for {
144145
updateReq <- ReturnVersionUpdateRequest.from(userAnswers, version)
145146
versionResponse <- backendConnector.updateReturnVersion(updateReq)
146147
newVersion <- versionResponse.newVersion match {
147-
case Some(v) => Future.successful(v)
148+
case Some(v) =>
149+
logger.info(s"[PurchaserRemoveService][withNewVersion] update return version response version: $v")
150+
Future.successful(v)
148151
case None => Future.failed(new IllegalStateException("Return version was not updated (newVersion missing)"))
149152
}
150153
result <- f(newVersion)
@@ -161,15 +164,16 @@ class PurchaserRemoveService @Inject()(
161164

162165
def deleteCompanyDetailsIfPresent(
163166
userAnswers: UserAnswers,
167+
isMainPurchaser: Boolean,
164168
companyDetailsIdInSession: Option[String]
165169
)(implicit request: DataRequest[AnyContent], hc: HeaderCarrier, ec: ExecutionContext): Future[Unit] =
166170
companyDetailsIdInSession match {
167-
case Some(companyId) =>
171+
case Some(companyId) if isMainPurchaser =>
168172
for {
169173
req <- DeleteCompanyDetailsRequest.from(userAnswers, companyId)
170174
_ <- backendConnector.deleteCompanyDetails(req)
171175
} yield ()
172-
case None =>
176+
case _ =>
173177
Future.successful(())
174178
}
175179

@@ -281,7 +285,7 @@ class PurchaserRemoveService @Inject()(
281285
.withNewVersion(userAnswers) { returnVersion =>
282286
for {
283287
_ <- PurchaserOps.deletePurchaser(userAnswers, purchaserIdSession)
284-
_ <- PurchaserOps.deleteCompanyDetailsIfPresent(userAnswers, companyDetailsIdInSession)
288+
_ <- PurchaserOps.deleteCompanyDetailsIfPresent(userAnswers, isMainPurchaser, companyDetailsIdInSession)
285289
_ <- changeNextIdOfPreviousPurchaser()
286290
maybeResult <- updateChosenPurchaserIfNeeded(Some(returnVersion))
287291
} yield maybeResult.getOrElse(PurchaserOps.purchaserOverviewRedirect(removedPurchaser))
@@ -294,12 +298,11 @@ class PurchaserRemoveService @Inject()(
294298
}
295299

296300
private def handleMultiplePurchasersWithNewMain(
297-
chosenPurchaserId: String,
298-
purchaserIdSession: String,
299-
companyDetailsIdInSession: Option[String] = None,
300-
userAnswers: UserAnswers,
301-
version: Option[Long] = None
302-
)(implicit request: DataRequest[AnyContent], hc: HeaderCarrier, ec: ExecutionContext): Future[Result] = {
301+
chosenPurchaserId: String,
302+
purchaserIdSession: String,
303+
companyDetailsIdInSession: Option[String] = None,
304+
userAnswers: UserAnswers
305+
)(implicit request: DataRequest[AnyContent], hc: HeaderCarrier, ec: ExecutionContext): Future[Result] = {
303306

304307
val purchasers = PurchaserOps.allPurchasers(userAnswers)
305308
val chosenPurchaser = PurchaserOps.findById(purchasers, chosenPurchaserId)
@@ -316,10 +319,10 @@ class PurchaserRemoveService @Inject()(
316319
)
317320

318321
PurchaserOps
319-
.withNewVersion(userAnswers, version) { newReturnVersion =>
322+
.withNewVersion(userAnswers) { newReturnVersion =>
320323
for {
321324
_ <- PurchaserOps.deletePurchaser(userAnswers, purchaserIdSession)
322-
_ <- PurchaserOps.deleteCompanyDetailsIfPresent(userAnswers, companyDetailsIdInSession)
325+
_ <- PurchaserOps.deleteCompanyDetailsIfPresent(userAnswers, true, companyDetailsIdInSession)
323326
r <- chosenPurchaser match {
324327
case Some(_) => doUpdate(newReturnVersion)
325328
case None => Future.successful(PurchaserOps.purchaserOverviewRedirect())
@@ -349,7 +352,7 @@ class PurchaserRemoveService @Inject()(
349352
_ <- PurchaserOps.updateReturnInfo(
350353
userAnswers,
351354
returnInfo.copy(mainPurchaserID = Some(chosenPurchaserId),
352-
version = Some(newReturnVersion.toString)))
355+
version = Some(newReturnVersion.toString)))
353356
} yield PurchaserOps.purchaserOverviewRedirect(removedPurchaser)
354357
}
355358
}

test/services/purchaser/PurchaserRemoveServiceSpec.scala

Lines changed: 68 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -631,9 +631,9 @@ class PurchaserRemoveServiceSpec extends SpecBase with MockitoSugar with BeforeA
631631
when(mockBackendConnector.updateReturnVersion(any())(any(), any()))
632632
.thenReturn(Future.successful(ReturnVersionUpdateReturn(Some(2))))
633633
when(mockBackendConnector.deletePurchaser(any())(any(), any()))
634-
.thenReturn(Future.successful((DeletePurchaserReturn(deleted = true))))
634+
.thenReturn(Future.successful(DeletePurchaserReturn(deleted = true)))
635635
when(mockBackendConnector.updatePurchaser(any())(any(), any()))
636-
.thenReturn(Future.successful((UpdatePurchaserReturn(updated = true))))
636+
.thenReturn(Future.successful(UpdatePurchaserReturn(updated = true)))
637637

638638
val result = service.handleRemoval(
639639
PurchaserRemove.Remove(nonmainPurchaserID),
@@ -644,6 +644,62 @@ class PurchaserRemoveServiceSpec extends SpecBase with MockitoSugar with BeforeA
644644
verify(mockBackendConnector).updateReturnVersion(any())(any(), any())
645645
verify(mockBackendConnector).deletePurchaser(any())(any(), any())
646646
verify(mockBackendConnector).updatePurchaser(any())(any(), any())
647+
verify(mockBackendConnector, never()).deleteCompanyDetails(any())(any(), any())
648+
}
649+
650+
"must handle Remove for non-main purchaser with multiple purchasers and main purchaser is company" in {
651+
val mainPurchaserID = "PURCH001"
652+
val nonmainPurchaserID = "PURCH002"
653+
654+
val purchaser1 = createPurchaser(
655+
mainPurchaserID,
656+
companyName = Some("TestCo"),
657+
isCompany = Some("YES"),
658+
nextPurchaserID = Some(nonmainPurchaserID),
659+
purchaserRef = Some("1")
660+
)
661+
val purchaser2 = createPurchaser(
662+
nonmainPurchaserID,
663+
forename1 = Some("Jane"),
664+
surname = Some("Doe"),
665+
nextPurchaserID = None,
666+
purchaserRef = Some("2")
667+
)
668+
669+
val returnInfo = ReturnInfo(mainPurchaserID = Some(mainPurchaserID), version = Some("0"))
670+
val fullReturn = emptyFullReturn.copy(
671+
purchaser = Some(Seq(purchaser1, purchaser2)),
672+
returnInfo = Some(returnInfo),
673+
companyDetails = Some(CompanyDetails(companyDetailsID = Some("COMP001")))
674+
)
675+
val purchaserRefs = PurchaserAndCompanyId(nonmainPurchaserID, Some("COMP001"))
676+
val userAnswers = emptyUserAnswers
677+
.copy(fullReturn = Some(fullReturn))
678+
.set(PurchaserOverviewRemovePage, purchaserRefs).success.value
679+
680+
implicit val request: DataRequest[AnyContent] = DataRequest(
681+
FakeRequest(),
682+
"id",
683+
userAnswers
684+
)
685+
686+
when(mockBackendConnector.updateReturnVersion(any())(any(), any()))
687+
.thenReturn(Future.successful(ReturnVersionUpdateReturn(Some(2))))
688+
when(mockBackendConnector.deletePurchaser(any())(any(), any()))
689+
.thenReturn(Future.successful(DeletePurchaserReturn(deleted = true)))
690+
when(mockBackendConnector.updatePurchaser(any())(any(), any()))
691+
.thenReturn(Future.successful(UpdatePurchaserReturn(updated = true)))
692+
693+
val result = service.handleRemoval(
694+
PurchaserRemove.Remove(nonmainPurchaserID),
695+
userAnswers
696+
)
697+
698+
status(result) mustBe SEE_OTHER
699+
verify(mockBackendConnector).updateReturnVersion(any())(any(), any())
700+
verify(mockBackendConnector).deletePurchaser(any())(any(), any())
701+
verify(mockBackendConnector).updatePurchaser(any())(any(), any())
702+
verify(mockBackendConnector, never()).deleteCompanyDetails(any())(any(), any())
647703
}
648704

649705
"must handle Remove for main purchaser with exactly two purchasers" in {
@@ -681,11 +737,11 @@ class PurchaserRemoveServiceSpec extends SpecBase with MockitoSugar with BeforeA
681737
when(mockBackendConnector.updateReturnVersion(any())(any(), any()))
682738
.thenReturn(Future.successful(ReturnVersionUpdateReturn(Some(2))))
683739
when(mockBackendConnector.deletePurchaser(any())(any(), any()))
684-
.thenReturn(Future.successful((DeletePurchaserReturn(deleted = true))))
740+
.thenReturn(Future.successful(DeletePurchaserReturn(deleted = true)))
685741
when(mockBackendConnector.updatePurchaser(any())(any(), any()))
686-
.thenReturn(Future.successful((UpdatePurchaserReturn(updated = true))))
742+
.thenReturn(Future.successful(UpdatePurchaserReturn(updated = true)))
687743
when(mockBackendConnector.updateReturnInfo(any())(any(), any()))
688-
.thenReturn(Future.successful((ReturnInfoReturn(updated = true))))
744+
.thenReturn(Future.successful(ReturnInfoReturn(updated = true)))
689745

690746
val result = service.handleRemoval(
691747
PurchaserRemove.Remove(mainPurchaserID),
@@ -727,9 +783,9 @@ class PurchaserRemoveServiceSpec extends SpecBase with MockitoSugar with BeforeA
727783
when(mockBackendConnector.updateReturnVersion(any())(any(), any()))
728784
.thenReturn(Future.successful(ReturnVersionUpdateReturn(Some(2))))
729785
when(mockBackendConnector.deletePurchaser(any())(any(), any()))
730-
.thenReturn(Future.successful((DeletePurchaserReturn(deleted = true))))
786+
.thenReturn(Future.successful(DeletePurchaserReturn(deleted = true)))
731787
when(mockBackendConnector.deleteCompanyDetails(any())(any(), any()))
732-
.thenReturn(Future.successful((DeleteCompanyDetailsReturn(deleted = true))))
788+
.thenReturn(Future.successful(DeleteCompanyDetailsReturn(deleted = true)))
733789

734790
val result = service.handleRemoval(
735791
PurchaserRemove.Remove(purchaserId),
@@ -782,9 +838,9 @@ class PurchaserRemoveServiceSpec extends SpecBase with MockitoSugar with BeforeA
782838
when(mockBackendConnector.updateReturnVersion(any())(any(), any()))
783839
.thenReturn(Future.successful(ReturnVersionUpdateReturn(Some(2))))
784840
when(mockBackendConnector.deletePurchaser(any())(any(), any()))
785-
.thenReturn(Future.successful((DeletePurchaserReturn(deleted = true))))
841+
.thenReturn(Future.successful(DeletePurchaserReturn(deleted = true)))
786842
when(mockBackendConnector.updateReturnInfo(any())(any(), any()))
787-
.thenReturn(Future.successful((ReturnInfoReturn(updated = true))))
843+
.thenReturn(Future.successful(ReturnInfoReturn(updated = true)))
788844

789845
val result = service.handleRemoval(
790846
PurchaserRemove.SelectNewMain(newmainPurchaserID),
@@ -839,11 +895,11 @@ class PurchaserRemoveServiceSpec extends SpecBase with MockitoSugar with BeforeA
839895
when(mockBackendConnector.updateReturnVersion(any())(any(), any()))
840896
.thenReturn(Future.successful(ReturnVersionUpdateReturn(Some(2))))
841897
when(mockBackendConnector.deletePurchaser(any())(any(), any()))
842-
.thenReturn(Future.successful((DeletePurchaserReturn(deleted = true))))
898+
.thenReturn(Future.successful(DeletePurchaserReturn(deleted = true)))
843899
when(mockBackendConnector.updatePurchaser(any())(any(), any()))
844-
.thenReturn(Future.successful((UpdatePurchaserReturn(updated = true))))
900+
.thenReturn(Future.successful(UpdatePurchaserReturn(updated = true)))
845901
when(mockBackendConnector.updateReturnInfo(any())(any(), any()))
846-
.thenReturn(Future.successful((ReturnInfoReturn(updated = true))))
902+
.thenReturn(Future.successful(ReturnInfoReturn(updated = true)))
847903

848904
val result = service.handleRemoval(
849905
PurchaserRemove.SelectNewMain(newmainPurchaserID),

0 commit comments

Comments
 (0)