Skip to content

[CARF-645] Implements file confirmation page - #18

Open
ZDevelop94 wants to merge 1 commit into
mainfrom
CARF-645
Open

[CARF-645] Implements file confirmation page#18
ZDevelop94 wants to merge 1 commit into
mainfrom
CARF-645

Conversation

@ZDevelop94

Copy link
Copy Markdown

How to test locally:
1. Run with test-only routes: sbt run -Dapplication.router=testOnlyDoNotUseInAppConf.Routes
2. Sign in via auth wizard with a carfId: CARF0051
4. Populate ExtractedFileDetailsPage in session by hitting: http://localhost:17004/send-a-cryptoasset-report/test-only/save-file-details?sendingEntityIn=ZMCAR0123456788
5. Visit the page http://localhost:17004/send-a-cryptoasset-report/file-passed-checks
6. Click go to confirmation page

def onPageLoad(): Action[AnyContent] = (identify() andThen getData() andThen requireData) { implicit request =>
val userAnswers = request.userAnswers

userAnswers.get(RcaspDetailsPage).fold(recovery) { rcaspDetails =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need to get both SubscriptionDetailsPage and RcaspDetailsPage as we send emails to both the user and (if IsRCASPUser = false) the RCASP in the file.
So the email list would be subscriptionDetails.getEmails ++ rcaspDetails.getEmails and we just have to format that into a string correctly

def onPageLoad(): Action[AnyContent] = (identify() andThen getData() andThen requireData) { implicit request =>
val userAnswers = request.userAnswers

userAnswers.get(RcaspDetailsPage).fold(recovery) { rcaspDetails =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the cases that go to journey recovery due to missing data, we can add a logWarn with the reason?

.fold(recovery) { summaryListRows =>
val summary = SummaryListViewModel(rows = summaryListRows).withCssClass("govuk-!-margin-bottom-5")

val formattedDate = DateTimeFormats.dateTimeToString(LocalDateTime.now(clock))

@matthew-goh matthew-goh Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The time displayed is currently one hour off! I think we have to use a ZonedDateTime and apply ZoneId.of("Europe/London")

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think users can also be outside the UK so that might not be correct. I'll have to grab the Locale

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, we might need to check with Jill whether we always display UK time or it can be the user's local time. (On the RCASP removed page where we also show the date/time, the dev notes say it must be UK time.)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

don't think we need to ask hear the User should see a time relevant to them the actual time is always saved in UTC

Comment on lines +27 to +33
def dateTimeToString(dateTime: LocalDateTime): String =
dateTime
.format(dateFormatter)
.replace("AM", "am")
.replace("PM", "pm")
.replace("00:00", "midnight")
.replace("12:00", "midday")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps call dateFormatter dateTimeFormatter and add a few tests for the formatting as it may not be easy to test everything mentioned in the dev notes manually?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I was meant to do this forgot to add

Comment thread app/utils/FileConfirmationHelper.scala
printLink: components.PrintLink
)

@(summaryList: SummaryList, dateFormated: String, isRcaspUser: Boolean, manageRCASPUrl: String, emailString: String)(implicit request: Request[_], messages: Messages)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't use isRcaspUser in the view; typo in dateFormatted

Comment on lines +73 to +76
@link(
manageRCASPUrl,
"fileConfirmation.link.text.manage",
id = Some("site.backToManagement")

@matthew-goh matthew-goh Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The message key can be "site.backToManagement", then the "fileConfirmation.link.text.manage" message can be removed

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No it's consistent as it is with the others

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's not true. Look at InvalidXmlView, DataErrorsView, StillCheckingYourFileView and VirusFoundView - all use key = "site.backToManagement"

Comment on lines +57 to +64
<div class="govuk-!-padding-top-3">
<ul class="govuk-list">
<li>
@printLink(
key = "fileConfirmation.link.text.print",
id = Some("print-link")
)
</li>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

According to dev notes, in print view, these links at the bottom shouldn't appear - we could add govuk-!-display-none-print to the div

Comment on lines +49 to +54
@link(
routes.PlaceholderController.onPageLoad("Page will be implemented in CARF-369").url,
"fileConfirmation.feedback.link",
id = Some("exit-survey-link")
)
@messages("fileConfirmation.feedback.link.text")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In dev notes: "The survey link should be included so that the sentence makes sense but it should have the URL in brackets after it"
We can add classes = "govuk-link print-url" to the link.

@matthew-goh

Copy link
Copy Markdown
Contributor

According to the ticket, we also need to implement a lock so that a user can't press back after landing on this page.
(The lock would have to be cleared - or just a fresh user answers created - when the user lands on /upload-file)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants