Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion app/config/Module.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import com.google.inject.name.Names
import controllers.actions.*
import services.{MonthlyReturnItemPayloadBuilder, MonthlyReturnItemPayloadBuilderImpl}
import utils.{ReferenceGenerator, ReferenceGeneratorImpl}
import services.guard.{DuplicateMRCreationGuard, DuplicateMRCreationGuardImpl}
import services.guard.{DuplicateMRCreationGuard, DuplicateMRCreationGuardImpl, SubmissionSuccessfulServiceGuard, SubmissionSuccessfulServiceGuardImpl}

import java.time.{Clock, ZoneOffset}

Expand All @@ -45,6 +45,7 @@ class Module extends AbstractModule {
.to(classOf[AgentIdentifierAction])
.asEagerSingleton()
bind(classOf[DuplicateMRCreationGuard]).to(classOf[DuplicateMRCreationGuardImpl])
bind(classOf[SubmissionSuccessfulServiceGuard]).to(classOf[SubmissionSuccessfulServiceGuardImpl])
bind(classOf[Clock]).toInstance(Clock.systemDefaultZone.withZone(ZoneOffset.UTC))
bind(classOf[MonthlyReturnItemPayloadBuilder]).to(classOf[MonthlyReturnItemPayloadBuilderImpl])
}
Expand Down
19 changes: 12 additions & 7 deletions app/controllers/monthlyreturns/SubmissionSuccessController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import models.requests.DataRequest
import play.api.i18n.{I18nSupport, MessagesApi}
import play.api.mvc.{Action, AnyContent, MessagesControllerComponents}
import services.MonthlyReturnService
import services.guard.SubmissionSuccessfulServiceGuard
import uk.gov.hmrc.http.HeaderCarrier
import uk.gov.hmrc.play.http.HeaderCarrierConverter
import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendBaseController
Expand All @@ -49,7 +50,8 @@ class SubmissionSuccessController @Inject() (
val controllerComponents: MessagesControllerComponents,
view: SubmissionSuccessView,
clock: Clock,
monthlyReturnService: MonthlyReturnService
monthlyReturnService: MonthlyReturnService,
submissionSuccessGuard: SubmissionSuccessfulServiceGuard
)(implicit ec: ExecutionContext, appConfig: FrontendAppConfig)
extends FrontendBaseController
with I18nSupport
Expand All @@ -60,12 +62,15 @@ class SubmissionSuccessController @Inject() (
implicit val hc: HeaderCarrier =
HeaderCarrierConverter.fromRequestAndSession(request, request.session)

val ua = request.userAnswers

for {
vm <- buildViewModel(ua)
_ <- monthlyReturnService.completeSubmissionJourney(ua)
} yield Ok(view(vm))
if (!submissionSuccessGuard.check) {
Future.successful(Redirect(controllers.routes.JourneyRecoveryController.onPageLoad()))
} else {
val ua = request.userAnswers
for {
vm <- buildViewModel(ua)
_ <- monthlyReturnService.completeSubmissionJourney(ua)
} yield Ok(view(vm))
}
}

private def buildViewModel(ua: UserAnswers)(implicit
Expand Down
4 changes: 3 additions & 1 deletion app/models/submission/SubmissionDetails.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ case class SubmissionDetails(
id: String,
status: String,
irMark: String,
submittedAt: LocalDateTime
submittedAt: LocalDateTime,
amendment: Option[String] = None,
hmrcMarkGgis: Option[String] = None
)

object SubmissionDetails {
Expand Down
47 changes: 47 additions & 0 deletions app/services/guard/SubmissionSuccessfulServiceGuard.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* 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.
* 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 services.guard

import models.requests.DataRequest
import pages.submission.SubmissionDetailsPage
import play.api.Logging

import javax.inject.Singleton

trait SubmissionSuccessfulServiceGuard {
def check(implicit request: DataRequest[_]): Boolean
}

@Singleton
class SubmissionSuccessfulServiceGuardImpl extends SubmissionSuccessfulServiceGuard with Logging {

def check(implicit request: DataRequest[_]): Boolean =
request.userAnswers.get(SubmissionDetailsPage).exists { details =>
val submittedOrAmendment = details.status == "SUBMITTED" || details.amendment.contains("Y")
val irMarksValid = details.irMark.nonEmpty &&
details.hmrcMarkGgis.exists(g => g.nonEmpty && g == details.irMark)

if (!submittedOrAmendment)
logger.warn(
s"[SubmissionSuccessfulServiceGuard] Guard failed: status=${details.status}, amendment=${details.amendment}"
)
if (!irMarksValid)
logger.warn(s"[SubmissionSuccessfulServiceGuard] Guard failed: irMark empty or hmrcMarkGgis mismatch")

submittedOrAmendment && irMarksValid
}
}
100 changes: 60 additions & 40 deletions app/services/submission/SubmissionService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,11 @@ class SubmissionService @Inject() (
cisConnector.getCisTaxpayer()

for {
taxpayer <- taxpayerFut
csr <- chrisRequestBuilder.build(ua, taxpayer, isAgent)(hc)
response <- cisConnector.submitToChris(submissionId, csr)
_ <- writeToFeMongo(ua, submissionId, response)
taxpayer <- taxpayerFut
csr <- chrisRequestBuilder.build(ua, taxpayer, isAgent)(hc)
response <- cisConnector.submitToChris(submissionId, csr)
amendment <- fetchAmendmentFlag(ua)
_ <- writeToFeMongo(ua, submissionId, response, amendment)
} yield response

def updateSubmissionFromChrisResponse(
Expand Down Expand Up @@ -204,34 +205,38 @@ class SubmissionService @Inject() (
} yield "TIMED_OUT"
} else {
for {
cisId <- userAnswers.get(CisIdPage).toFuture
pollUrl <- userAnswers.get(PollUrlPage).toFuture
submissionDetails <- userAnswers.get(SubmissionDetailsPage).toFuture
submissionId <- userAnswers.get(SubmissionDetailsPage).map(_.id).toFuture
result <- cisConnector.getSubmissionStatus(pollUrl, submissionId)
_ <- updateSubmission(
submissionDetails.id,
userAnswers,
submissionDetails.irMark,
result.status,
Some(dateFormatter.format(submissionDetails.submittedAt)),
result.irMarkReceived,
result.error
)
newStatus = result.status
timedOut =
pollUrl <- userAnswers.get(PollUrlPage).toFuture
submissionId <- userAnswers.get(SubmissionDetailsPage).map(_.id).toFuture
result <- cisConnector.getSubmissionStatus(pollUrl, submissionId)
_ <- updateSubmission(
submissionDetails.id,
userAnswers,
submissionDetails.irMark,
result.status,
Some(dateFormatter.format(submissionDetails.submittedAt)),
result.irMarkReceived,
result.error
)
newStatus = result.status
timedOut =
LocalDateTime.now().isAfter(timeoutDateTime) && (newStatus == "ACCEPTED" || newStatus == "PENDING")
finalStatus = if (timedOut) "TIMED_OUT" else newStatus
newDetails = submissionDetails.copy(status = newStatus)
ua1 <- Future.fromTry(userAnswers.set(SubmissionDetailsPage, newDetails))
ua2 <- Future.fromTry(ua1.set(SubmissionStatusTimedOutPage(submissionDetails.id), timedOut))
ua3 <- result.pollUrl.map(url => ua2.set(PollUrlPage, url)).getOrElse(Try(ua2)).toFuture
ua4 <- result.intervalSeconds.map(i => ua3.set(PollIntervalPage, i)).getOrElse(Try(ua3)).toFuture
ua5 <- result.lastMessageDate match {
case Some(ts) => Future.fromTry(ua4.set(LastMessageDatePage, Instant.parse(ts)))
case None => Future.successful(ua4)
}
_ <- sessionRepository.set(ua5)
finalStatus = if (timedOut) "TIMED_OUT" else newStatus
irMarkValidated = newStatus == "SUBMITTED"
newDetails = submissionDetails.copy(
status = newStatus,
hmrcMarkGgis = result.irMarkReceived.orElse(
if (irMarkValidated) Some(submissionDetails.irMark) else submissionDetails.hmrcMarkGgis
)
)
ua1 <- Future.fromTry(userAnswers.set(SubmissionDetailsPage, newDetails))
ua2 <- Future.fromTry(ua1.set(SubmissionStatusTimedOutPage(submissionDetails.id), timedOut))
ua3 <- result.pollUrl.map(url => ua2.set(PollUrlPage, url)).getOrElse(Try(ua2)).toFuture
ua4 <- result.intervalSeconds.map(i => ua3.set(PollIntervalPage, i)).getOrElse(Try(ua3)).toFuture
ua5 <- result.lastMessageDate match {
case Some(ts) => Future.fromTry(ua4.set(LastMessageDatePage, Instant.parse(ts)))
case None => Future.successful(ua4)
}
_ <- sessionRepository.set(ua5)
} yield finalStatus
}
}
Expand Down Expand Up @@ -268,10 +273,11 @@ class SubmissionService @Inject() (

emailOpt match {
case None =>
val updated = userAnswers.set(SuccessEmailSentPage(submissionId), true)
Future
.fromTry(updated)
.flatMap(updatedUa => sessionRepository.set(updatedUa).map(_ => updatedUa))
for {
latestUa <- sessionRepository.get(userAnswers.id).map(_.getOrElse(userAnswers))
updatedUa <- Future.fromTry(latestUa.set(SuccessEmailSentPage(submissionId), true))
_ <- sessionRepository.set(updatedUa)
} yield updatedUa

case Some(email) =>
val locale: Locale = Lang.get(langCode).map(_.locale).getOrElse(Locale.UK)
Expand All @@ -281,11 +287,10 @@ class SubmissionService @Inject() (
year = yearMonth.getYear.toString
)

val updatedUaFuture = Future.fromTry(userAnswers.set(SuccessEmailSentPage(submissionId), true))

for {
_ <- cisConnector.sendSuccessfulEmail(submissionId, request)
updatedUa <- updatedUaFuture
latestUa <- sessionRepository.get(userAnswers.id).map(_.getOrElse(userAnswers))
updatedUa <- Future.fromTry(latestUa.set(SuccessEmailSentPage(submissionId), true))
_ <- sessionRepository.set(updatedUa)
} yield updatedUa
}
Expand Down Expand Up @@ -324,10 +329,23 @@ class SubmissionService @Inject() (
throw new RuntimeException("Date of return missing for monthly return")
)

private def fetchAmendmentFlag(ua: UserAnswers)(implicit hc: HeaderCarrier): Future[Option[String]] = {
val instanceId = ua.get(CisIdPage).getOrElse(throw new RuntimeException("CIS ID missing"))
val ym = selectedYearMonth(ua)
cisConnector
.retrieveMonthlyReturnForEditDetails(instanceId, ym.getMonthValue, ym.getYear)
.map(_.monthlyReturn.headOption.flatMap(_.amendment))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This can be a naive question because I'm not familiar with the domain, but why is the first monthly return the flag that's used? Are we guaranteed that this list is ordered or should we instead be ensuring that it's ordered by date?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This doesn't seem to be tested. This goes beyond a private helper, this should probably be pulled out and have unit tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This can be a naive question because I'm not familiar with the domain, but why is the first monthly return the flag that's used? Are we guaranteed that this list is ordered or should we instead be ensuring that it's ordered by date?

The call ends up at an Oracle stored procedure (Get_Monthly_Return_For_Edit) with instanceId, taxYear and taxMonth as parameters. None of the Scala layers apply any ordering or filtering on the returned list. Other parts of the codebase handle it the same way, e.g. getMonthlyReturnId also takes the first match for the given year/month. So headOption is consistent with the existing pattern but we are relying on the stored procedure returning the active record first.

.recover { case ex =>
logger.warn("[fetchAmendmentFlag] Failed to retrieve amendment flag, defaulting to None", ex)
None
}
}

private def writeToFeMongo(
ua: UserAnswers,
submissionId: String,
response: ChrisSubmissionResponse
response: ChrisSubmissionResponse,
amendment: Option[String]
): Future[Boolean] = {
val updatedUa: Try[UserAnswers] = for {
ua1 <- ua.set(
Expand All @@ -338,7 +356,9 @@ class SubmissionService @Inject() (
irMark = response.hmrcMarkGenerated,
submittedAt = response.gatewayTimestamp
.flatMap(t => Try(LocalDateTime.parse(t)).toOption)
.getOrElse(LocalDateTime.now)
.getOrElse(LocalDateTime.now),
amendment = amendment,
hmrcMarkGgis = None
)
)
ua2 <- response.responseEndPoint match {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
/*
* Copyright 2026 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 controllers.amend

import base.SpecBase
Expand Down
Loading