From fed379d4ff56bc7bd056be35c3da53a8a0b4677c Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 17 Jan 2024 22:22:23 +0000 Subject: [PATCH 1/9] Fix and improve acceptance testing attributes https://eaflood.atlassian.net/browse/WATER-4336 In WATER-4070 we logged that the Billing & Data team were struggling because they could not access the larger bill runs. The service would appear to be busy for a while and then eventually error. We were able to figure out it was simply that the service was not able to return all the data needed using the current design, which listed out all the transactions in a bill run. We also knew the data could be collected in a more performant way. So, we ended up rebuilding the whole journey of viewing a bill run, the bills in it and the transaction details. This also gave us the opportunity to add attributes to elements likely to be referenced in our acceptance tests. A [recognised best practice in selecting elements in tests](https://docs.cypress.io/guides/references/best-practices#Selecting-Elements) is to avoid brittle, complex selectors and to use `data-*` attributes instead. > For reference we have opted for `data-test` as our convention The legacy pages had none of this, and often didn't even bother with ID's. This means a common battle with the acceptance tests we inherited is finding suitable selectors for the elements we needed. We now need to [update those tests we broke](https://github.com/DEFRA/water-abstraction-acceptance-tests/pull/78) because of our new pages. Doing this has allowed us to spot a few places where we've made mistakes and others where `data-test` would help. This change is us making those tweaks to the billing templates. From f5d19fdc8153c44cc5fbad1e773081cd57d82853 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 17 Jan 2024 22:34:54 +0000 Subject: [PATCH 2/9] Add attribute to meta-data in view single licence --- .../bills/view-single-licence-presroc.njk | 20 ++++++++++++------- app/views/bills/view-single-licence-sroc.njk | 20 ++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/app/views/bills/view-single-licence-presroc.njk b/app/views/bills/view-single-licence-presroc.njk index 9dc3ba68ae..204c3a3d6d 100644 --- a/app/views/bills/view-single-licence-presroc.njk +++ b/app/views/bills/view-single-licence-presroc.njk @@ -40,6 +40,12 @@

{# Bill meta-data #} + {# + GOV.UK summary lists only allow us to assign attributes at the top level and not to each row. This means we + can't assign our data-test attribute using the component. Our solution is to use the html option for each row + instead of text and wrap each value in a . That way we can manually assign our data-test attribute to the + span. + #} {{ govukSummaryList({ classes: 'govuk-summary-list--no-border', @@ -49,31 +55,31 @@ rows: [ { key: { text: "Date created", classes: "meta-data__label" }, - value: { text: dateCreated, classes: "meta-data__value" } + value: { html: '' + dateCreated + '', classes: "meta-data__value" } }, { key: { text: "Region", classes: "meta-data__label" }, - value: { text: region, classes: "meta-data__value" } + value: { html: '' + region + '', classes: "meta-data__value" } }, { key: { text: "Bill run type", classes: "meta-data__label" }, - value: { text: billRunType, classes: "meta-data__value" } + value: { html: '' + billRunType + '', classes: "meta-data__value" } }, { key: { text: "Charge scheme", classes: "meta-data__label" }, - value: { text: chargeScheme, classes: "meta-data__value" } + value: { html: '' + chargeScheme + '', classes: "meta-data__value" } }, { key: { text: "Financial year", classes: "meta-data__label" }, - value: { text: financialYear, classes: "meta-data__value" } + value: { html: '' + financialYear + '', classes: "meta-data__value" } }, { key: { text: "Transaction file", classes: "meta-data__label" }, - value: { text: transactionFile, classes: "meta-data__value" } + value: { html: '' + transactionFile + '', classes: "meta-data__value" } } if transactionFile, { key: { text: "Bill number", classes: "meta-data__label" }, - value: { text: billNumber, classes: "meta-data__value" } + value: { html: '' + billNumber + '', classes: "meta-data__value" } } if billNumber ] }) diff --git a/app/views/bills/view-single-licence-sroc.njk b/app/views/bills/view-single-licence-sroc.njk index 41ef094117..5b6b73b3e1 100644 --- a/app/views/bills/view-single-licence-sroc.njk +++ b/app/views/bills/view-single-licence-sroc.njk @@ -40,6 +40,12 @@

{# Bill meta-data #} + {# + GOV.UK summary lists only allow us to assign attributes at the top level and not to each row. This means we + can't assign our data-test attribute using the component. Our solution is to use the html option for each row + instead of text and wrap each value in a . That way we can manually assign our data-test attribute to the + span. + #} {{ govukSummaryList({ classes: 'govuk-summary-list--no-border', @@ -49,31 +55,31 @@ rows: [ { key: { text: "Date created", classes: "meta-data__label" }, - value: { text: dateCreated, classes: "meta-data__value" } + value: { html: '' + dateCreated + '', classes: "meta-data__value" } }, { key: { text: "Region", classes: "meta-data__label" }, - value: { text: region, classes: "meta-data__value" } + value: { html: '' + region + '', classes: "meta-data__value" } }, { key: { text: "Bill run type", classes: "meta-data__label" }, - value: { text: billRunType, classes: "meta-data__value" } + value: { html: '' + billRunType + '', classes: "meta-data__value" } }, { key: { text: "Charge scheme", classes: "meta-data__label" }, - value: { text: chargeScheme, classes: "meta-data__value" } + value: { html: '' + chargeScheme + '', classes: "meta-data__value" } }, { key: { text: "Financial year", classes: "meta-data__label" }, - value: { text: financialYear, classes: "meta-data__value" } + value: { html: '' + financialYear + '', classes: "meta-data__value" } }, { key: { text: "Transaction file", classes: "meta-data__label" }, - value: { text: transactionFile, classes: "meta-data__value" } + value: { html: '' + transactionFile + '', classes: "meta-data__value" } } if transactionFile, { key: { text: "Bill number", classes: "meta-data__label" }, - value: { text: billNumber, classes: "meta-data__value" } + value: { html: '' + billNumber + '', classes: "meta-data__value" } } if billNumber ] }) From 24286eeb2aad65828d5ff431ee42366d6c73d442 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 17 Jan 2024 22:37:29 +0000 Subject: [PATCH 3/9] Add attribute to meta data in view multi licence --- app/views/bills/view-multi-licence.njk | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/app/views/bills/view-multi-licence.njk b/app/views/bills/view-multi-licence.njk index e4e2671cc1..620edbfd8c 100644 --- a/app/views/bills/view-multi-licence.njk +++ b/app/views/bills/view-multi-licence.njk @@ -40,6 +40,12 @@

{# Bill meta-data #} + {# + GOV.UK summary lists only allow us to assign attributes at the top level and not to each row. This means we + can't assign our data-test attribute using the component. Our solution is to use the html option for each row + instead of text and wrap each value in a . That way we can manually assign our data-test attribute to the + span. + #} {{ govukSummaryList({ classes: 'govuk-summary-list--no-border', @@ -49,31 +55,31 @@ rows: [ { key: { text: "Date created", classes: "meta-data__label" }, - value: { text: dateCreated, classes: "meta-data__value" } + value: { html: '' + dateCreated + '', classes: "meta-data__value" } }, { key: { text: "Region", classes: "meta-data__label" }, - value: { text: region, classes: "meta-data__value" } + value: { html: '' + region + '', classes: "meta-data__value" } }, { key: { text: "Bill run type", classes: "meta-data__label" }, - value: { text: billRunType, classes: "meta-data__value" } + value: { html: '' + billRunType + '', classes: "meta-data__value" } }, { key: { text: "Charge scheme", classes: "meta-data__label" }, - value: { text: chargeScheme, classes: "meta-data__value" } + value: { html: '' + chargeScheme + '', classes: "meta-data__value" } }, { key: { text: "Financial year", classes: "meta-data__label" }, - value: { text: financialYear, classes: "meta-data__value" } + value: { html: '' + financialYear + '', classes: "meta-data__value" } }, { key: { text: "Transaction file", classes: "meta-data__label" }, - value: { text: transactionFile, classes: "meta-data__value" } + value: { html: '' + transactionFile + '', classes: "meta-data__value" } } if transactionFile, { key: { text: "Bill number", classes: "meta-data__label" }, - value: { text: billNumber, classes: "meta-data__value" } + value: { html: '' + billNumber + '', classes: "meta-data__value" } } if billNumber ] }) From 41ba8be7ec387c54fea6e16523da8dfcd099408b Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 17 Jan 2024 22:39:50 +0000 Subject: [PATCH 4/9] Fix incorrect name for debits total --- app/views/bills/view-multi-licence.njk | 2 +- app/views/bills/view-single-licence-presroc.njk | 2 +- app/views/bills/view-single-licence-sroc.njk | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/bills/view-multi-licence.njk b/app/views/bills/view-multi-licence.njk index 620edbfd8c..575c06695b 100644 --- a/app/views/bills/view-multi-licence.njk +++ b/app/views/bills/view-multi-licence.njk @@ -140,7 +140,7 @@

- {{ debitsTotal }}
+ {{ debitsTotal }}
Debits

diff --git a/app/views/bills/view-single-licence-presroc.njk b/app/views/bills/view-single-licence-presroc.njk index 204c3a3d6d..3a865a9918 100644 --- a/app/views/bills/view-single-licence-presroc.njk +++ b/app/views/bills/view-single-licence-presroc.njk @@ -140,7 +140,7 @@

- {{ debitsTotal }}
+ {{ debitsTotal }}
Debits

diff --git a/app/views/bills/view-single-licence-sroc.njk b/app/views/bills/view-single-licence-sroc.njk index 5b6b73b3e1..809d7fa6f0 100644 --- a/app/views/bills/view-single-licence-sroc.njk +++ b/app/views/bills/view-single-licence-sroc.njk @@ -140,7 +140,7 @@

- {{ debitsTotal }}
+ {{ debitsTotal }}
Debits

From 97c21c4fe98d3b23e7a301d9898cc14e06f72bb4 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 17 Jan 2024 22:42:52 +0000 Subject: [PATCH 5/9] Fix duplicate data attributes in the same page --- app/views/bills/view-single-licence-presroc.njk | 4 ++-- app/views/bills/view-single-licence-sroc.njk | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/bills/view-single-licence-presroc.njk b/app/views/bills/view-single-licence-presroc.njk index 3a865a9918..31f9ce46e6 100644 --- a/app/views/bills/view-single-licence-presroc.njk +++ b/app/views/bills/view-single-licence-presroc.njk @@ -378,7 +378,7 @@ text: creditTotal, format: 'numeric', classes: 'govuk-!-font-weight-bold', - attributes: { 'data-test': 'credits-total' } + attributes: { 'data-test': 'transactions-credits-total' } }, { text: '', @@ -395,7 +395,7 @@ text: debitTotal, format: 'numeric', classes: 'govuk-!-font-weight-bold', - attributes: { 'data-test': 'debits-total' } + attributes: { 'data-test': 'transactions-debits-total' } } ] %} diff --git a/app/views/bills/view-single-licence-sroc.njk b/app/views/bills/view-single-licence-sroc.njk index 809d7fa6f0..fa4fab2b4c 100644 --- a/app/views/bills/view-single-licence-sroc.njk +++ b/app/views/bills/view-single-licence-sroc.njk @@ -373,7 +373,7 @@ text: creditTotal, format: 'numeric', classes: 'govuk-!-font-weight-bold', - attributes: { 'data-test': 'credits-total' } + attributes: { 'data-test': 'transactions-credits-total' } }, { text: '', @@ -390,7 +390,7 @@ text: debitTotal, format: 'numeric', classes: 'govuk-!-font-weight-bold', - attributes: { 'data-test': 'debits-total' } + attributes: { 'data-test': 'transactions-debits-total' } } ] %} From a0246dfc67b92b35ad0e310848334ef2af74d33a Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 18 Jan 2024 00:40:32 +0000 Subject: [PATCH 6/9] Add attribute to meta data in view bill run --- app/views/bill-runs/view.njk | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/app/views/bill-runs/view.njk b/app/views/bill-runs/view.njk index e85fe68c0e..c11c1de2f9 100644 --- a/app/views/bill-runs/view.njk +++ b/app/views/bill-runs/view.njk @@ -38,7 +38,13 @@ {{ badge(billRunStatus, badgeType) }}

- {# Bill run meta-data #} + {# Bill meta-data #} + {# + GOV.UK summary lists only allow us to assign attributes at the top level and not to each row. This means we + can't assign our data-test attribute using the component. Our solution is to use the html option for each row + instead of text and wrap each value in a . That way we can manually assign our data-test attribute to the + span. + #} {{ govukSummaryList({ classes: 'govuk-summary-list--no-border', @@ -48,31 +54,31 @@ rows: [ { key: { text: "Date created", classes: "meta-data__label" }, - value: { text: dateCreated, classes: "meta-data__value" } + value: { html: '' + dateCreated + '', classes: "meta-data__value" } }, { key: { text: "Region", classes: "meta-data__label" }, - value: { text: region, classes: "meta-data__value" } + value: { html: '' + region + '', classes: "meta-data__value" } }, { key: { text: "Bill run type", classes: "meta-data__label" }, - value: { text: billRunType, classes: "meta-data__value" } + value: { html: '' + billRunType + '', classes: "meta-data__value" } }, { key: { text: "Charge scheme", classes: "meta-data__label" }, - value: { text: chargeScheme, classes: "meta-data__value" } + value: { html: '' + chargeScheme + '', classes: "meta-data__value" } }, { key: { text: "Financial year", classes: "meta-data__label" }, - value: { text: financialYear, classes: "meta-data__value" } - } if financialYear, + value: { html: '' + financialYear + '', classes: "meta-data__value" } + }, { key: { text: "Transaction file", classes: "meta-data__label" }, - value: { text: transactionFile, classes: "meta-data__value" } + value: { html: '' + transactionFile + '', classes: "meta-data__value" } } if transactionFile, { key: { text: "Bill number", classes: "meta-data__label" }, - value: { text: billNumber, classes: "meta-data__value" } + value: { html: '' + billNumber + '', classes: "meta-data__value" } } if billNumber ] }) From 6f741c78898535c89e825d9546dc3c283b3ad858 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 18 Jan 2024 00:41:59 +0000 Subject: [PATCH 7/9] Add missing hyphen to attribute name --- app/views/bill-runs/view.njk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/bill-runs/view.njk b/app/views/bill-runs/view.njk index c11c1de2f9..ba3e3f61ba 100644 --- a/app/views/bill-runs/view.njk +++ b/app/views/bill-runs/view.njk @@ -251,7 +251,7 @@ { text: bill.financialYear, format: 'numeric', - attributes: { 'data-test': 'financial-year' + rowIndex } + attributes: { 'data-test': 'financial-year-' + rowIndex } } ), tableRow) %} {% endif %} From c561e7b3e0450793f83c8049d03ff06e990de965 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 18 Jan 2024 00:44:25 +0000 Subject: [PATCH 8/9] Correct comment --- app/views/bill-runs/view.njk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/bill-runs/view.njk b/app/views/bill-runs/view.njk index ba3e3f61ba..512f12bc45 100644 --- a/app/views/bill-runs/view.njk +++ b/app/views/bill-runs/view.njk @@ -38,7 +38,7 @@ {{ badge(billRunStatus, badgeType) }}

- {# Bill meta-data #} + {# Bill run meta-data #} {# GOV.UK summary lists only allow us to assign attributes at the top level and not to each row. This means we can't assign our data-test attribute using the component. Our solution is to use the html option for each row From d561bbd75c9bf25f4363bfc6c02f47a00097baf5 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Thu, 18 Jan 2024 08:38:26 +0000 Subject: [PATCH 9/9] Add another missing hyphen to attribute name --- app/views/bill-runs/view.njk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/bill-runs/view.njk b/app/views/bill-runs/view.njk index 512f12bc45..f972bf8b25 100644 --- a/app/views/bill-runs/view.njk +++ b/app/views/bill-runs/view.njk @@ -233,7 +233,7 @@ {% set tableRow = [ { text: bill.accountNumber, - attributes: { 'data-test': 'account-number' + rowIndex } + attributes: { 'data-test': 'account-number-' + rowIndex } }, { text: bill.billingContact,