Fix 1st pass issues with bill & bill licence pages (#511)

https://eaflood.atlassian.net/browse/WATER-4155 https://eaflood.atlassian.net/browse/WATER-4156 After a first pass of the new view bill and bill-licence pages we've built some issues have been spotted. This change resolves *** Broken screen reader text in 'View transactions link'** We overlooked updating the context property being referenced in a refactor. *** Fix undefined in view bill licence We were only setting, for example, the `additionalChargesDetail` variable in the template if there were additional charges. But we were then referencing `additionalChargesDetail` when setting `detailChargeInfo`. ```nunjucks {% set detailChargeInfo = detailChargeInfo + additionalChargesDetail + adjustmentsDetail + '</p>' + elementDetail %} ``` This is why the template was showing `undefinedundefined` when a transaction had no additional charges or adjustments. By flipping the logic, i.e. always instantiating a variable but only populating it when something exists we avoid Nunjucks thinking `additionalChargesDetail` or `adjustmentsDetail` are ever undefined. *** Use set address for view bill backlink The initial requirement for the story was that however a user came to the view bill page the backlink should take them to where they came from. This could be 1 of 3 places - bill run summary - billing account summary - billing account all bills When you first come to the view bill page we use [referrer](#486) to generate the backlink. At this point we can get you back to where you came from. But that list is incomplete with the addition of the view bill-licence page (see WATER-4156 ). So, you come to the view bill page, then click through to the view bill-licence page and then go back, now `referrer` is the view bill-licence. This results in the user being stuck in a loop. ``` [source] --> [view bill ] --> [view bill-licence] --> [view bill ] [back = source] [back = view bill ] [back = view bill-licence] ``` Having discussed this with our designer, it was agreed having the backlink attempt to recreate what the back button does is a type of anti-pattern. Backlinks we build into the pages should help a user navigate through a _planned_ journey. Essentially, if you come from, for example, billing account summary, you have jumped from the licence summary journey to the bill runs journey. As such when you go back, you should go back through the bill runs journey. *** Better backlink text Following on from our decision to always have backlinks take you to a specific point in a _planned_ journey we also agreed we'd make it clearer where the backlink will take you in the UI. We're using the [advice in the design system for back links](https://design-system.service.gov.uk/components/back-link/) to use different link text. > For more complex user journeys, consider using different link text, like ‘Go back to [page]’. For example, in an admin system with many different areas. In this case, if you used ‘Back’, it might not be clear to users what they are going back to. We've applied this advice to both the view bill and view bill-licence pages.
DEFRA · Nov 7, 2023 · aa02929 · aa02929
1 parent 1f2ed76
commit aa02929
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 20 deletions.
diff --git a/app/presenters/bills/multi-licence-bill.presenter.js b/app/presenters/bills/multi-licence-bill.presenter.js
@@ -25,6 +25,7 @@ function go (bill, licenceSummaries, billingAccount) {
     billLicences,
     billNumber: bill.invoiceNumber,
     billRunId: billRun.billingBatchId,
+    billRunNumber: billRun.billRunNumber,
     billRunStatus: billRun.status,
     billRunType: _billRunType(billRun),
     chargeScheme: _scheme(billRun),

diff --git a/app/views/bill-licences/view-presroc.njk b/app/views/bill-licences/view-presroc.njk
@@ -7,7 +7,7 @@
   {# Back link #}
   {{
     govukBackLink({
-      text: "Back",
+      text: 'Go back to bill for ' + accountNumber,
       href: "/system/bills/" + billingInvoiceId
     })
   }}

diff --git a/app/views/bill-licences/view-sroc.njk b/app/views/bill-licences/view-sroc.njk
@@ -7,7 +7,7 @@
   {# Back link #}
   {{
     govukBackLink({
-      text: "Back",
+      text: 'Go back to bill for ' + accountNumber,
       href: "/system/bills/" + billingInvoiceId
     })
   }}
@@ -86,20 +86,20 @@
           {% endset %}
 
           {# Only show additional charges if we have any #}
-          {% if transaction.additionalCharges %}
-            {% set additionalChargesDetail %}
-                <br><br>
-                <span data-test="additional-charges-{{ rowIndex }}"><strong>Additional charges:</strong> {{ transaction.additionalCharges }}</span>
-            {% endset %}
-          {% endif %}
+          {% set additionalChargesDetail %}
+            {% if transaction.additionalCharges %}
+              <br><br>
+              <span data-test="additional-charges-{{ rowIndex }}"><strong>Additional charges:</strong> {{ transaction.additionalCharges }}</span>
+            {% endif %}
+          {% endset %}
 
           {# Only show adjustments if we have any #}
-          {% if transaction.adjustments %}
-            {% set adjustmentsDetail %}
+          {% set adjustmentsDetail %}
+            {% if transaction.adjustments %}
               <br><br>
               <span data-test="adjustments-{{ rowIndex }}"><strong>Adjustments:</strong> {{ transaction.adjustments }}</span>
-            {% endset %}
-          {% endif %}
+            {% endif %}
+          {% endset %}
 
           {# Charge element details section #}
           {% set elementDetail %}

diff --git a/app/views/bills/view.njk b/app/views/bills/view.njk
@@ -10,15 +10,10 @@
 
 {% block breadcrumbs %}
   {# Back link #}
-  {% if referrer %}
-    {% set backlink = referrer %}
-  {% else %}
-    {% set backlink = 'javascript:history.back()' %}
-  {% endif %}
   {{
     govukBackLink({
-      text: "Back",
-      href: backlink
+      text: 'Go back to bill run ' + billRunNumber,
+      href: '/billing/batch/' + billRunId + '/summary'
     })
   }}
 {% endblock %}
@@ -170,7 +165,7 @@
       {% for billLicence in billLicences %}
         {# Generate the link to view the transactions in a bill licence #}
         {% set actionLink %}
-          <a class="govuk-link" href="/system/bill-licences/{{ billLicence.id }}">View transactions<span class="govuk-visually-hidden"> for licence {{ licenceRef }}</span></a>
+          <a class="govuk-link" href="/system/bill-licences/{{ billLicence.id }}">View transactions<span class="govuk-visually-hidden"> for licence {{ billLicence.reference }}</span></a>
         {% endset %}
 
         {# Create the licence row and add it to our array of table rows #}

diff --git a/test/presenters/bills/multi-licence-bill.presenter.test.js b/test/presenters/bills/multi-licence-bill.presenter.test.js
@@ -53,6 +53,7 @@ describe('Multi Licence Bill presenter', () => {
         ],
         billNumber: 'EAI0000007T',
         billRunId: '2c80bd22-a005-4cf4-a2a2-73812a9861de',
+        billRunNumber: 10003,
         billRunStatus: 'sent',
         billRunType: 'Annual',
         chargeScheme: 'Current',

diff --git a/test/services/bills/view-bill.service.test.js b/test/services/bills/view-bill.service.test.js
@@ -64,6 +64,7 @@ describe('View Bill service', () => {
         ],
         billNumber: 'EAI0000007T',
         billRunId: '2c80bd22-a005-4cf4-a2a2-73812a9861de',
+        billRunNumber: 10003,
         billRunStatus: 'sent',
         billRunType: 'Annual',
         chargeScheme: 'Current',