Conversation
|
|
||
| def check(implicit request: DataRequest[_]): Future[SubmissionSuccessfulCheck] = { | ||
| val result = request.userAnswers.get(SubmissionDetailsPage).exists { details => | ||
| val statusOrAmendment = details.status == "SUBMITTED" || details.amendment.contains("Y") |
There was a problem hiding this comment.
should we call this submisttedOrAmendment?
| newStatus = result.status | ||
| timedOut = Instant.now().isAfter(timeoutDateTime) && (newStatus == "ACCEPTED" || newStatus == "PENDING") | ||
| finalStatus = if (timedOut) "TIMED_OUT" else newStatus | ||
| irMarkValidated = newStatus == "SUBMITTED" || newStatus == "SUBMITTED_NO_RECEIPT" |
There was a problem hiding this comment.
how does this check ensure the irMark is validated?
There was a problem hiding this comment.
So hmrcMarkGgis starts as None and only gets set when the poll status becomes SUBMITTED.. meaning CHRIS accepted and validated the submission. The guard then checks that hmrcMarkGgis exists and matches irMark. So you can't reach the confirmation page unless CHRIS has confirmed the submission.
| newDetails = submissionDetails.copy( | ||
| status = newStatus, | ||
| hmrcMarkGgis = | ||
| if (irMarkValidated) Some(submissionDetails.irMark) else submissionDetails.hmrcMarkGgis |
There was a problem hiding this comment.
This doesn't seem right... shouldn't hmrcMarkGgis come from the final chris response? why are we using the one we sent as if it came from the response?
There was a problem hiding this comment.
The CHRIS poll response doesn't return an IRMark. It only gives us status, pollUrl, intervalSeconds and lastMessageDate. So we don't have a separate "received" mark to compare against. Instead, when CHRIS returns SUBMITTED, we know it validated the XML (including the IRMark), so we treat the sent mark as validated at that point.
There was a problem hiding this comment.
Yesterday afternoon we merged 3664, we now extract the irMark from the CHRIS submitted response. Try rebasing and see if it works.
If the response is SUBMITTED we should have it. If it is SUBMITTED_NO_RECEIPT this guard probably shouldn't apply...
| } | ||
|
|
||
| trait SubmissionSuccessfulServiceGuard { | ||
| def check(implicit request: DataRequest[_]): Future[SubmissionSuccessfulCheck] |
There was a problem hiding this comment.
Does this need to return a Future? There is no IO/async operations
| sealed trait SubmissionSuccessfulCheck | ||
| object SubmissionSuccessfulCheck { | ||
| case object GuardPassed extends SubmissionSuccessfulCheck | ||
| case object GuardFailed extends SubmissionSuccessfulCheck | ||
| } |
There was a problem hiding this comment.
Can we use a boolean here in it's place?
| val ym = selectedYearMonth(ua) | ||
| cisConnector | ||
| .retrieveMonthlyReturnForEditDetails(instanceId, ym.getMonthValue, ym.getYear) | ||
| .map(_.monthlyReturn.headOption.flatMap(_.amendment)) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
This doesn't seem to be tested. This goes beyond a private helper, this should probably be pulled out and have unit tests.
There was a problem hiding this comment.
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.
4 participants
No description provided.