check tool: use only the transformed_row to get geometry data#963
Conversation
WalkthroughThe changes add a new field, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WireMock
Client->>WireMock: Send GET/POST request to /requests endpoint
WireMock-->>Client: Return JSON response (status, headers, body)
sequenceDiagram
participant App
participant ResponseDetails
participant DataItem
App->>ResponseDetails: Invoke #makeGeometryGetter(item)
ResponseDetails->>DataItem: Access item.transformed_row
alt Field 'geometry' or 'point' found
DataItem-->>ResponseDetails: Return corresponding geometry value
else Field not found
DataItem-->>ResponseDetails: Return undefined
end
ResponseDetails-->>App: Return extracted geometry value or undefined
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ 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:
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 Coverage
|
||||||||||||||||||||||||||||||||||||||
![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 (1)
src/models/responseDetails.js (1)
275-286: Implementation properly leverages transformed_row for geometry dataThe refactoring improves on the previous implementation by directly using
transformed_rowto extract geometry data, which aligns with the PR objective. The code now specifically checks for fields named 'geometry' or 'point', and uses optional chaining to safely handle undefined values.A few observations:
The implementation only checks for 'geometry' and 'point' fields, which may be more restrictive than the previous implementation that also handled 'wkt' and 'geox'. Ensure this won't miss valid geometry data in your application.
The method now returns
undefinedearlier without logging, which might make debugging harder. Consider adding debug logging before returningundefinedto maintain visibility.Consider adding a comment explaining the expected structure of
transformed_row(objects with 'field' and 'value' properties) to help future developers understand the data structure this code relies on.#makeGeometryGetter (item) { /* The api seems to sometimes respond with weird casing, it can be camal case, all lower or all upper I'll implement a fix here, but hopefully infa will be addressing it on the backend to */ const trow = item.transformed_row if (trow) { const key = trow.find(obj => obj.field === 'geometry' || obj.field === 'point')?.field const getter = (row) => { const geometry = row.transformed_row?.find(obj => obj.field === key) return geometry?.value } return getter } + // No valid geometry field found in transformed_row + logger.debug('No valid geometry field found in transformed_row', { + type: types.App, + requestId: this.id + }) return undefined }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docker/request-api-stub/wiremock/__files/check_file/article-4/request-complete-details.json(3 hunks)docker/request-api-stub/wiremock/__files/check_file/article-4/request-complete.json(3 hunks)docker/request-api-stub/wiremock/mappings/requests-fe547850-a713-43a8-8474-446631ab544b.json(1 hunks)docker/request-api-stub/wiremock/mappings/requests_bbw7ebivop3leg7ue9vka4-259eb311-bc53-46c4-80cc-17ff203d6fed.json(1 hunks)docker/request-api-stub/wiremock/mappings/requests_bbw7ebivop3leg7ue9vka4-31b3b282-b265-4f7c-ac66-db61e3f81be2.json(1 hunks)docker/request-api-stub/wiremock/mappings/requests_bbw7ebivop3leg7ue9vka4-6257849e-1550-4530-9e2e-5d7f45e1abdc.json(1 hunks)docker/request-api-stub/wiremock/mappings/requests_bbw7ebivop3leg7ue9vka4-7ffceaf2-278c-46d3-9b8e-f403a36bd93f.json(1 hunks)docker/request-api-stub/wiremock/mappings/requests_bbw7ebivop3leg7ue9vka4-b6a3aed5-d09a-4d22-b04f-8372db46df90.json(1 hunks)src/models/responseDetails.js(1 hunks)test/unit/responseDetails.test.js(3 hunks)
🔇 Additional comments (16)
docker/request-api-stub/wiremock/mappings/requests_bbw7ebivop3leg7ue9vka4-6257849e-1550-4530-9e2e-5d7f45e1abdc.json (1)
1-23: Mapping File Structure is Clear and Consistent
The JSON file is well structured and clearly defines the request and response for the check URL endpoint. The usage of scenario management fields (e.g. requiredScenarioState and newScenarioState) and the inclusion of a detailed response body aligns with standard WireMock mapping practices. Please ensure that the naming and timestamp differences across similar mappings are intentional.docker/request-api-stub/wiremock/mappings/requests_bbw7ebivop3leg7ue9vka4-259eb311-bc53-46c4-80cc-17ff203d6fed.json (1)
1-23: Consistent Mapping with Appropriate Scenario Fields
This mapping mirrors the structure of other similar mappings. The request details, response body and scenario management fields are defined consistently. Double-check that the slight differences (for example, the Date header timestamp) are in line with the intended test scenarios.docker/request-api-stub/wiremock/mappings/requests_bbw7ebivop3leg7ue9vka4-7ffceaf2-278c-46d3-9b8e-f403a36bd93f.json (1)
1-23: Mapping Reflecting Completed State
The mapping correctly reflects a “COMPLETE” status in the response body as opposed to “PROCESSING” in other mappings. This variation is likely to facilitate different test conditions. Kindly verify that this completed status is the intended outcome for this particular endpoint mapping.docker/request-api-stub/wiremock/mappings/requests_bbw7ebivop3leg7ue9vka4-31b3b282-b265-4f7c-ac66-db61e3f81be2.json (1)
1-22: Ordering and Scenario State are Appropriately Managed
This mapping is consistent with the other GET mappings for the same endpoint and utilises a distinct insertion index to ensure the correct order within scenarios. Please verify that the ordering (insertionIndex) correctly reflects the desired workflow.docker/request-api-stub/wiremock/__files/check_file/article-4/request-complete.json (3)
59-67: Addition of the transformed_row Field in Entry 1
The introduction of thetransformed_rowfield containing geometry data in entry 1 is in line with the PR objective. The inclusion of a polygon in Well-Known Text (WKT) format allows the application to retrieve geometry data solely from this field. Please ensure that the provided WKT string is valid and correctly represents the intended geometry.
80-88: Inclusion of transformed_row for Entry 2 is Consistent
The addition of thetransformed_rowfield for entry 2 similarly captures the geometry data as intended. The WKT string provided appears consistent with the format used in entry 1. It is advisable to verify that both the transformed and converted row fields remain in sync or that the application uses only the transformed data as per the new logic.
101-104: Handling of Empty transformed_row in Entry 3 and Data Anomalies
For entry 3, the deliberate assignment of an empty array totransformed_rowis appropriate if no geometry data is available. However, please review the test data for consistency; for example, thestart-datein the converted_row is set to"40/04/2024", which does not conform to recognised date formats. Kindly verify whether this is intentional as part of the test fixture or requires correction.docker/request-api-stub/wiremock/mappings/requests_bbw7ebivop3leg7ue9vka4-b6a3aed5-d09a-4d22-b04f-8372db46df90.json (1)
1-23: New WireMock mapping for GET request simulation looks goodThis is a well-structured WireMock mapping that simulates a GET request to
/requests/bBw7eBivoP3LEg7UE9VKA4endpoint. The mapping is properly configured with scenario states to simulate sequential API interactions during testing.The mapping returns a request in "PROCESSING" status with appropriate metadata for checking a conservation area GeoJSON URL, which aligns with the PR objective of fixing the geometry data retrieval process.
docker/request-api-stub/wiremock/mappings/requests-fe547850-a713-43a8-8474-446631ab544b.json (1)
1-26: New WireMock mapping for POST request simulation looks goodThis WireMock mapping correctly handles the creation of new requests via POST to the
/requestsendpoint. The mapping is properly configured to match requests with the specified payload structure while ignoring array order and extra elements for flexibility.The stub returns a 202 Accepted response with appropriate headers and response body structure, which aligns with the expected API behaviour for initiating a URL check request in the workflow.
docker/request-api-stub/wiremock/__files/check_file/article-4/request-complete-details.json (3)
5-10: Well-structured implementation of transformed_row for geometry dataThis addition of the
transformed_rowfield for entry 1 properly encapsulates the geometry data in a standardised format, with a clear field identifier and WKT value. This structure makes geometry data retrieval more explicit and reliable, addressing the bug described in issue #961.
26-31: Consistent implementation of transformed_row for entry 2The implementation of
transformed_rowfor entry 2 follows the same pattern as entry 1, maintaining consistency in how geometry data is structured. This is good practice for ensuring uniform data access patterns.
47-47: Empty transformed_row for entry without geometryAppropriately initializing
transformed_rowas an empty array for entry 3 that doesn't have geometry data. This consistency ensures that code accessing this property won't encounter undefined errors, even when no geometry is available.test/unit/responseDetails.test.js (4)
1-1: File extension added to import statementAdding the
.jsextension to the import statement ensures compatibility with ES modules and prevents potential import resolution issues, especially in environments that enforce strict module resolution.
28-30: Added transformed_row to test mock data for entry 1The mock data has been properly updated to include the
transformed_rowfield for the first test entry, ensuring the tests accurately reflect the new data structure used in the application.
46-48: Added transformed_row to test mock data for entry 2The mock data has been properly updated to include the
transformed_rowfield for the second test entry, ensuring consistency across all test data.
54-60: Updated mockResponsWithGeoXGeoY mapping functionThe function that generates mock data with GeoX and GeoY coordinates has been properly updated to include the
transformed_rowfield in its output, ensuring that all test data consistently includes this field for geometry retrieval tests.
github-project-automation
Bot
moved this from Refined
to Ready for release
in Providers team (BACKLOG)
4 participants
Preview Link
https://submit-pr-963.herokuapp.com/
What type of PR is this? (check all applicable)
Description
As in the title.
Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Submitting the above URLs to the check tool should work (note: the preview instance only accepts URLs, upload doesn't work).
Added/updated tests?
QA sign off
Summary by CodeRabbit