Default error hint on LPA overview page#714
Conversation
WalkthroughThe changes involve updates to the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/views/organisations/overview.html (1)
34-38: Fix indentation and use loose equality for string comparison.The error handling logic is good, but there are two minor improvements needed:
- The indentation is inconsistent with the rest of the file
- String comparison in templates typically uses loose equality
Apply this diff to fix the issues:
- {% if not dataset.error or dataset.error === '' %} - <p>There was an error accessing the data URL</p> - {% else %} - <p>{{ dataset.error }}</p> - {% endif %} + {% if not dataset.error or dataset.error == '' %} + <p>There was an error accessing the data URL</p> + {% else %} + <p>{{ dataset.error }}</p> + {% endif %}test/unit/middleware/overview.middleware.test.js (1)
24-29: Enhance the helper function with documentation and error handling.The helper function is well-designed but could benefit from some improvements.
Apply these enhancements:
+/** + * Extracts error card nodes from the rendered HTML template + * @param {Object} templateParams - The template parameters + * @returns {Array<Element>} Array of error card DOM nodes + * @throws {Error} If template rendering fails + */ const getRenderedErrorCards = (templateParams) => { + if (!templateParams) { + throw new Error('Template parameters are required') + } const html = nunjucks.render('organisations/overview.html', templateParams) const doc = new jsdom.JSDOM(html).window.document const errorNodes = doc.querySelectorAll('[data-dataset-status="Error"]') return Array.from(errorNodes) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/views/organisations/overview.html(2 hunks)test/unit/middleware/overview.middleware.test.js(4 hunks)
🔇 Additional comments (3)
src/views/organisations/overview.html (1)
13-13: LGTM! Good addition of the data-dataset-status attribute.
The new attribute enhances testability and provides a hook for styling based on dataset status.
test/unit/middleware/overview.middleware.test.js (2)
58-63: LGTM! Good test coverage for error message scenarios.
The addition of dataset4 with a specific error message ensures proper testing of both default and custom error messages.
98-100: LGTM! Comprehensive error message verification.
The assertions effectively verify both the default error message and custom error message scenarios.
b1fc569 to
c1cfb6e
Compare
|
All LGTM 🙌 |
The dataset.error value can be null
c1cfb6e to
96b8d4b
Compare
What type of PR is this? (check all applicable)
Description
The error hint message on LPA overview page is missing sometimes. This is because error message is not in the database itself. This PR adds a default error message for such scenarios.
Related Tickets & Documents
Closes #659
QA Instructions, Screenshots, Recordings
Currently, on prod, you can see the hint missing here: https://submit.planning.data.gov.uk/organisations/local-authority:BUC
Added/updated tests?
QA sign off
Summary by CodeRabbit
New Features
Bug Fixes
Tests