update endpoint status check to only show error if all endpoints fail - #1063
Conversation
## Walkthrough
The changes update how dataset endpoint error statuses are determined and displayed. Endpoints are now sorted by entry date, and an `entryDate` property is added and shown in the dataset overview. Related schema, middleware, and tests are updated to reflect these changes, including new logic for dataset status assignment and error handling.
## Changes
| Files/Paths | Change Summary |
|-----------------------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------|
| src/middleware/lpa-overview.middleware.js, test/unit/middleware/lpa-overview.middleware.test.js | Refined logic for determining dataset error status and messages based on all/some endpoint errors; expanded tests for new scenarios. |
| src/middleware/datasetOverview.middleware.js, test/unit/middleware/datasetOverview.middleware.test.js | Changed endpoint sorting to use `endpoint_entry_date`; added `entryDate` property to endpoints; updated related tests. |
| src/routes/schemas.js | Added optional, nullable `entryDate` string field to `endpoint` objects in `OrgDatasetOverview` schema. |
| src/views/organisations/dataset-overview.html, test/unit/views/organisations/dataset-overview.test.js | Added display of `entryDate` for each endpoint in the dataset overview; updated tests for new field and formatting. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant Server
participant Middleware
participant View
User->>Server: Request dataset overview
Server->>Middleware: Prepare dataset objects
Middleware->>Middleware: Sort endpoints by entryDate, assign error/status
Middleware->>Server: Return dataset stats with entryDate
Server->>View: Render dataset overview page with entryDate
View-->>User: Display endpoints with entry date and statusPossibly related PRs
Suggested labels
Suggested reviewers
Poem
|
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/middleware/lpa-overview.middleware.js (1)
218-222: LGTM! Logic correctly implements the requirement to show errors only when all endpoints fail.The updated logic properly addresses the PR objective by using
every()to check that all endpoints have non-200 status before showing an error. This prevents showing "Error" status when some endpoints are still functioning correctly.Consider removing the commented-out code for cleanliness:
- // const httpStatus = datasetEndpoints.find(endpoint => endpoint.latest_status !== '200')?.latest_status - // const error = httpStatus !== undefined ? `There was a ${httpStatus} error accessing the endpoint URL` : undefined const allError = datasetEndpoints.every(endpoint => endpoint.latest_status !== '200')Additionally, the error message could be more accurate when multiple endpoints are involved:
- const error = httpStatus !== undefined ? `There was a ${httpStatus} error accessing the endpoint URL` : undefined + const error = httpStatus !== undefined ? `There was a ${httpStatus} error accessing the endpoint URLs` : undefined
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/middleware/lpa-overview.middleware.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#593
File: index.js:20-20
Timestamp: 2024-10-29T14:58:22.843Z
Learning: The function `initDatasetSlugToReadableNameFilter()` in `src/utils/datasetSlugToReadableName.js` already includes appropriate error handling and logging.
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#593
File: test/unit/lpaDetailsController.test.js:20-20
Timestamp: 2024-10-29T15:06:42.432Z
Learning: The function `initDatasetSlugToReadableNameFilter()` includes its own error handling, so additional try-catch blocks around it in test files are unnecessary.
src/middleware/lpa-overview.middleware.js (5)
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#593
File: index.js:20-20
Timestamp: 2024-10-29T14:58:22.843Z
Learning: The function `initDatasetSlugToReadableNameFilter()` in `src/utils/datasetSlugToReadableName.js` already includes appropriate error handling and logging.
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#593
File: test/unit/lpaDetailsController.test.js:20-20
Timestamp: 2024-10-29T15:06:42.432Z
Learning: The function `initDatasetSlugToReadableNameFilter()` includes its own error handling, so additional try-catch blocks around it in test files are unnecessary.
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`.
Learnt from: rosado
PR: digital-land/submit#657
File: test/unit/middleware/issueDetails.middleware.test.js:43-43
Timestamp: 2024-11-14T16:38:49.883Z
Learning: In `test/unit/middleware/issueDetails.middleware.test.js`, template params are verified with a schema, so it's acceptable for the test expectations to use primitive values while the test input uses an object for `issueEntitiesCount`.
Learnt from: rosado
PR: digital-land/submit#657
File: src/services/performanceDbApi.js:290-293
Timestamp: 2024-11-14T17:31:37.924Z
Learning: In the `getEntityCounts` function in `src/services/performanceDbApi.js`, errors are already logged through the `.catch(...)` handlers on each promise in the `requests` array.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
Docstrings generation was requested by @Ben-Hodgkiss. * #1063 (comment) The following files were modified: * `src/middleware/lpa-overview.middleware.js`
…t status logic for dashboard display
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/unit/middleware/lpa-overview.middleware.test.js (1)
161-161: Fix typo in test description.There's a typo in the test description.
- it('should not show an error if atleast one endpoint is 200', () => { + it('should not show an error if at least one endpoint is 200', () => {test/unit/views/organisations/dataset-overview.test.js (1)
34-34: Consider date format for test consistency.The entryDate values use ISO date format which should be fine for date-only strings. However, based on previous learnings about daylight saving issues, ensure these date strings don't cause timezone-related test failures.
Also applies to: 42-42
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/middleware/datasetOverview.middleware.js(1 hunks)src/middleware/lpa-overview.middleware.js(1 hunks)src/routes/schemas.js(1 hunks)src/views/organisations/dataset-overview.html(1 hunks)test/unit/middleware/datasetOverview.middleware.test.js(1 hunks)test/unit/middleware/lpa-overview.middleware.test.js(4 hunks)test/unit/views/organisations/dataset-overview.test.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/unit/middleware/datasetOverview.middleware.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/middleware/lpa-overview.middleware.js
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#593
File: index.js:20-20
Timestamp: 2024-10-29T14:58:22.843Z
Learning: The function `initDatasetSlugToReadableNameFilter()` in `src/utils/datasetSlugToReadableName.js` already includes appropriate error handling and logging.
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`.
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#593
File: test/unit/lpaDetailsController.test.js:20-20
Timestamp: 2024-10-29T15:06:42.432Z
Learning: The function `initDatasetSlugToReadableNameFilter()` includes its own error handling, so additional try-catch blocks around it in test files are unnecessary.
src/middleware/datasetOverview.middleware.js (4)
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`.
Learnt from: rosado
PR: digital-land/submit#657
File: test/unit/middleware/issueDetails.middleware.test.js:43-43
Timestamp: 2024-11-14T16:38:49.883Z
Learning: In `test/unit/middleware/issueDetails.middleware.test.js`, template params are verified with a schema, so it's acceptable for the test expectations to use primitive values while the test input uses an object for `issueEntitiesCount`.
Learnt from: rosado
PR: digital-land/submit#657
File: src/services/performanceDbApi.js:290-293
Timestamp: 2024-11-14T17:31:37.924Z
Learning: In the `getEntityCounts` function in `src/services/performanceDbApi.js`, errors are already logged through the `.catch(...)` handlers on each promise in the `requests` array.
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.
test/unit/views/organisations/dataset-overview.test.js (4)
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#644
File: test/unit/util.test.js:72-74
Timestamp: 2024-11-13T14:25:37.041Z
Learning: In `test/unit/util.test.js`, avoid using ISO 8601 date strings (e.g., `'2024-05-15T00:00:00.000Z'`) in tests, as they cause issues with daylight saving. Instead, use date strings like `'Tue, 15 May 2024 00:00:00'`.
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`.
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: rosado
PR: digital-land/submit#657
File: test/unit/middleware/issueDetails.middleware.test.js:43-43
Timestamp: 2024-11-14T16:38:49.883Z
Learning: In `test/unit/middleware/issueDetails.middleware.test.js`, template params are verified with a schema, so it's acceptable for the test expectations to use primitive values while the test input uses an object for `issueEntitiesCount`.
test/unit/middleware/lpa-overview.middleware.test.js (6)
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`.
Learnt from: rosado
PR: digital-land/submit#657
File: test/unit/middleware/issueDetails.middleware.test.js:43-43
Timestamp: 2024-11-14T16:38:49.883Z
Learning: In `test/unit/middleware/issueDetails.middleware.test.js`, template params are verified with a schema, so it's acceptable for the test expectations to use primitive values while the test input uses an object for `issueEntitiesCount`.
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#593
File: test/unit/lpaDetailsController.test.js:20-20
Timestamp: 2024-10-29T15:06:42.432Z
Learning: The function `initDatasetSlugToReadableNameFilter()` includes its own error handling, so additional try-catch blocks around it in test files are unnecessary.
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#644
File: test/unit/util.test.js:72-74
Timestamp: 2024-11-13T14:25:37.041Z
Learning: In `test/unit/util.test.js`, avoid using ISO 8601 date strings (e.g., `'2024-05-15T00:00:00.000Z'`) in tests, as they cause issues with daylight saving. Instead, use date strings like `'Tue, 15 May 2024 00:00:00'`.
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#593
File: index.js:20-20
Timestamp: 2024-10-29T14:58:22.843Z
Learning: The function `initDatasetSlugToReadableNameFilter()` in `src/utils/datasetSlugToReadableName.js` already includes appropriate error handling and logging.
🧬 Code Graph Analysis (1)
test/unit/middleware/lpa-overview.middleware.test.js (1)
src/middleware/lpa-overview.middleware.js (8)
prepareOverviewTemplateParams(255-308)req(75-75)req(124-124)req(149-149)req(207-207)req(320-320)req(336-336)prepareDatasetObjects(206-237)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: run-tests / test
🔇 Additional comments (8)
test/unit/middleware/lpa-overview.middleware.test.js (4)
85-85: Minor formatting improvement.The spacing adjustment in the anonymous callback functions improves code consistency and readability.
Also applies to: 120-120, 131-131, 155-155
161-175: Well-structured test for mixed endpoint statuses.This test correctly validates the new logic where having at least one successful endpoint (status '200') prevents an error from being displayed whilst still showing 'Needs fixing' status due to the failing endpoints.
176-190: Comprehensive test for all endpoints failing.This test properly validates that when all endpoints have non-200 status codes, an error message containing the status code is displayed and the dataset status is set to 'Error'. The test correctly matches the new middleware logic.
191-209: Thorough test for all endpoints successful.This test appropriately validates the scenario where all endpoints have '200' status, ensuring no error is displayed and the status is set to 'Live'. The test data with multiple successful endpoints provides good coverage.
src/routes/schemas.js (1)
172-172: Schema addition looks correct.The optional nullable
entryDatefield is properly defined and maintains backward compatibility.src/views/organisations/dataset-overview.html (1)
168-177: Template addition follows existing patterns correctly.The new "Endpoint entry date" row is properly implemented with appropriate null handling and date formatting.
test/unit/views/organisations/dataset-overview.test.js (1)
87-87: Test assertions updated correctly.The new assertions properly verify that entryDate values are rendered in the summary list with correct formatting.
Also applies to: 92-92
src/middleware/datasetOverview.middleware.js (1)
228-228: EntryDate addition is correctly implemented.The new
entryDateproperty properly maps fromsource.endpoint_entry_dateand aligns with the schema changes.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/views/organisations/overview.html(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#593
File: index.js:20-20
Timestamp: 2024-10-29T14:58:22.843Z
Learning: The function `initDatasetSlugToReadableNameFilter()` in `src/utils/datasetSlugToReadableName.js` already includes appropriate error handling and logging.
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`.
Learnt from: GeorgeGoodall-GovUk
PR: digital-land/submit#593
File: test/unit/lpaDetailsController.test.js:20-20
Timestamp: 2024-10-29T15:06:42.432Z
Learning: The function `initDatasetSlugToReadableNameFilter()` includes its own error handling, so additional try-catch blocks around it in test files are unnecessary.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: run-tests / test
…ce-endpoint-status
Description
This PR updates the logic in middleware for oragnisation overview page to ensure that the LPA dashboard only shows an "
Error" status for a dataset if all its endpoints are failing (i.e., none have latest_status == '200')What type of PR is this? (check all applicable)
Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Before
After
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