Update confirmation.html - #1072
Conversation
Content tweaks to reflect new service name
WalkthroughThe terminology in multiple view templates and their corresponding tests was updated, changing references from "Submit" and "submitted" to "Check and provide" and "provided." This includes updates to page titles, headings, and configuration values. No other logic, content, or exported/public entities were altered. Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)test/unit/endpointSubmissionForm/confirmationPage.test.js (2)🪛 Biome (1.9.4)test/unit/endpointSubmissionForm/confirmationPage.test.js[error] 24-24: Use a regular expression literal instead of the RegExp constructor. Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically. (lint/complexity/useRegexLiterals) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Coverage Report
File CoverageNo changed files found. |
|
CodeRabbit raised some suggested changes to tests on #1069, which might resolve some of the errors with tests seen here. I’ll take a look shortly. |
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/views/submit/confirmation.html (1)
6-7: Service name string is hard-coded – consider centralising
Embedding'Provide'directly in every template risks divergence if the service name changes again. A single source of truth (e.g. a Nunjucks global or app-level config) would prevent future drift.test/unit/endpointSubmissionForm/confirmationPage.test.js (2)
10-10: Test suite description still refers to “Submit”
Thedescribeblock name hasn’t been updated, which can confuse future greps and reporters.-describe('Submit confirmation View', () => { +describe('Provide confirmation view', () => {
24-26: Prefer regex literal over RegExp constructor
Static-analysis warning:-const regex = new RegExp('<h1 class="govuk-panel__title".*mockDataset provided.*</h1>', 'g') +const regex = /<h1 class="govuk-panel__title".*mockDataset provided.*<\/h1>/gReduces escaping noise and satisfies Biome rule
useRegexLiterals.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/views/submit/confirmation.html(1 hunks)test/unit/endpointSubmissionForm/confirmationPage.test.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
test/unit/endpointSubmissionForm/confirmationPage.test.js (2)
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#609
File: test/unit/noErrorsPage.test.js:68-68
Timestamp: 2024-11-12T10:54:09.485Z
Learning: In `test/unit/noErrorsPage.test.js`, avoid using `prettifyColumnName` for column headers because the table can contain spec fields, as requested by Alex.
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#510
File: test/unit/middleware/datasetOverview.middleware.test.js:1-3
Timestamp: 2024-10-21T08:57:17.338Z
Learning: Tests for `pullOutDatasetSpecification` have been moved to `common.middleware.test`.
🪛 Biome (1.9.4)
test/unit/endpointSubmissionForm/confirmationPage.test.js
[error] 24-24: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: run-tests / test
🔇 Additional comments (1)
test/unit/endpointSubmissionForm/confirmationPage.test.js (1)
20-21: Page-title expectation retains old branding
If the public-facing copy has moved to Provide your planning data, the assertion should follow suit; otherwise we’re encoding legacy wording.
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/unit/endpointSubmissionForm/confirmationPage.test.js (1)
24-24: Terminology update is correct, but consider using regex literal.The change from "submitted" to "provided" aligns with the service rebranding. However, as suggested by the static analysis tool, using a regex literal would improve readability and performance.
Apply this diff to use regex literal:
- const regex = new RegExp('<h1 class="govuk-panel__title".*mockDataset provided.*</h1>', 'g') + const regex = /<h1 class="govuk-panel__title".*mockDataset provided.*<\/h1>/g
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/unit/endpointSubmissionForm/confirmationPage.test.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
test/unit/endpointSubmissionForm/confirmationPage.test.js (2)
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#609
File: test/unit/noErrorsPage.test.js:68-68
Timestamp: 2024-11-12T10:54:09.485Z
Learning: In `test/unit/noErrorsPage.test.js`, avoid using `prettifyColumnName` for column headers because the table can contain spec fields, as requested by Alex.
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#510
File: test/unit/middleware/datasetOverview.middleware.test.js:1-3
Timestamp: 2024-10-21T08:57:17.338Z
Learning: Tests for `pullOutDatasetSpecification` have been moved to `common.middleware.test`.
🪛 Biome (1.9.4)
test/unit/endpointSubmissionForm/confirmationPage.test.js
[error] 24-24: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: run-tests / test
🔇 Additional comments (1)
test/unit/endpointSubmissionForm/confirmationPage.test.js (1)
20-20: LGTM! Terminology update aligns with service rebranding.The page title change from "submitted" to "provided" correctly reflects the service name change from "submit" to "provide" as outlined in the PR objectives.
GitHub Actions
Failure
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/unit/endpointSubmissionForm/confirmationPage.test.js (1)
24-24: Use a regular expression literal for better readability.The static analysis tool correctly identifies that using a regex literal would be more readable and efficient than the RegExp constructor.
Apply this diff to use a regex literal:
- const regex = new RegExp('<h1 class="govuk-panel__title".*mockDataset provided.*</h1>', 'g') + const regex = /<h1 class="govuk-panel__title".*mockDataset provided.*<\/h1>/g
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/views/submit/confirmation.html(1 hunks)test/unit/endpointSubmissionForm/confirmationPage.test.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
test/unit/endpointSubmissionForm/confirmationPage.test.js (2)
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#609
File: test/unit/noErrorsPage.test.js:68-68
Timestamp: 2024-11-12T10:54:09.485Z
Learning: In `test/unit/noErrorsPage.test.js`, avoid using `prettifyColumnName` for column headers because the table can contain spec fields, as requested by Alex.
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#510
File: test/unit/middleware/datasetOverview.middleware.test.js:1-3
Timestamp: 2024-10-21T08:57:17.338Z
Learning: Tests for `pullOutDatasetSpecification` have been moved to `common.middleware.test`.
🪛 Biome (1.9.4)
test/unit/endpointSubmissionForm/confirmationPage.test.js
[error] 24-24: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
🪛 GitHub Actions: Unit test coverage report
test/unit/endpointSubmissionForm/confirmationPage.test.js
[error] 30-30: Test failure: expected document title 'mockDataset provided - Provide' to be 'mockDataset provided - Provide your planning data'. AssertionError due to Object.is equality mismatch.
🪛 GitHub Actions: Code change pipeline to development by @stevenjmesser
test/unit/endpointSubmissionForm/confirmationPage.test.js
[error] 30-30: Test failure: expected document title 'mockDataset provided - Provide' to be 'mockDataset provided - Provide your planning data'. AssertionError due to title mismatch.
🔇 Additional comments (1)
src/views/submit/confirmation.html (1)
6-7: LGTM! Terminology updates are consistent.The service type and page name changes correctly reflect the rebranding from "Submit" to "Provide" terminology. These variables are properly used in the template layout and panel title respectively.
|
|
||
| runGenericPageTests(html, { | ||
| pageTitle: 'mockDataset submitted - Submit your planning data' | ||
| pageTitle: 'mockDataset provided - Provide your planning data' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the page title assertion against pipeline failures.
The pipeline failures indicate a mismatch between expected and actual document titles. The test expects 'mockDataset provided - Provide your planning data' but the actual result appears to be 'mockDataset provided - Provide'. This suggests the serviceType variable might not be generating the full expected title in the layout template.
Let me verify how the page title is constructed in the layout template:
🏁 Script executed:
#!/bin/bash
# Description: Check how the page title is constructed in the layout template
# Expected: Find the layout template and see how serviceType and pageName are used to construct the title
# Search for the main layout template that uses serviceType and pageName
rg -A 10 -B 5 "serviceType.*pageName|pageName.*serviceType" --type html
# Also search for title construction patterns
rg -A 5 -B 5 "<title>" --type htmlLength of output: 110
I couldn’t locate the title construction in the last search―let’s broaden our scope to search all relevant files for pageTitle, document.title, and <title> usage:
🏁 Script executed:
#!/bin/bash
# Search for pageTitle property in JS/TS files
rg -H -n "pageTitle" --glob "*.{js,jsx,ts,tsx}"
# Search for direct document.title assignments in JS/TS files
rg -H -n "document\\.title" --glob "*.{js,jsx,ts,tsx}"
# Find <title> tags in HTML and Nunjucks templates
fd --extension html --extension njk | xargs rg -H -n "<title"Length of output: 2719
Let’s locate the confirmationPage view and inspect how its pageTitle is built, plus verify the value of serviceNames.submit:
🏁 Script executed:
#!/bin/bash
# 1. Find the confirmationPage view file(s)
fd --type f -I confirmationPage src
# 2. Show how pageTitle is set in that file
fd --type f -I confirmationPage src | xargs rg -H -n "pageTitle"
# 3. Locate the config file defining serviceNames.submit
rg -H -n "serviceNames" -n srcLength of output: 4353
Let’s inspect the test and relevant templates to confirm how the title is composed and spot the regex usage:
🏁 Script executed:
#!/bin/bash
# 1. Show the confirmationPage test
sed -n '1,200p' test/unit/endpointSubmissionForm/confirmationPage.test.js
# 2. Inspect the endpoint-submitted view template
sed -n '1,50p' src/views/endpoint-submitted.html
# 3. Inspect the main layout template for the pageTitle block
sed -n '1,50p' src/views/layouts/main.htmlLength of output: 3365
I still can’t see the submit/confirmation template—let’s pull in its pageTitle block to confirm the missing text:
🏁 Script executed:
#!/bin/bash
# Show the pageTitle override in the submit confirmation template
sed -n '1,30p' src/views/submit/confirmation.htmlLength of output: 1002
Fix page title generation & switch to regex literal
-
The confirmation page title is currently rendered as “mockDataset provided – Provide” because there’s no entry for
providein your serviceNames mapping. Please update your configuration (whereverconfig.serviceNamesis defined) to include:provide: 'Provide your planning data',
-
In
test/unit/endpointSubmissionForm/confirmationPage.test.js, replace the RegExp constructor with a literal for better readability and performance:test/unit/endpointSubmissionForm/confirmationPage.test.js - const regex = new RegExp('<h1 class="govuk-panel__title".*mockDataset provided.*</h1>', 'g') + const regex = /<h1 class="govuk-panel__title".*mockDataset provided.*<\/h1>/g
🤖 Prompt for AI Agents
In test/unit/endpointSubmissionForm/confirmationPage.test.js at line 20, update
the config.serviceNames object to include the key 'provide' with the value
'Provide your planning data' to fix the page title rendering issue. Then,
replace the RegExp constructor used in the test with a regex literal to improve
readability and performance.
stevenjmesser
left a comment
stevenjmesser
left a comment
There was a problem hiding this comment.
I had to tweak this PR as the page name and title is provided by the serviceNames key, which meant I also had to update the other tests for page titles. All tests now passing – and I decided against changing the submit key to provide as it would probably bring some refactoring – so approved.
2 participants
Content tweaks to reflect new service name
Description
Content tweaks to reflect new service name from 'submit' to 'provide'
What type of PR is this? (check all applicable)
Related Tickets & Documents
#1013
QA Instructions, Screenshots, Recordings
Before
Before screenshot here
After
After screenshot here
Added/updated tests?
We encourage you to keep the code coverage percentage at 80% and above.
QA sign off
[optional] Are there any post-deployment tasks we need to perform?
[optional] Are there any dependencies on other PRs or Work?
Summary by CodeRabbit
Style
Tests