check tool: render column names unchanged#1012
Conversation
User will see the same column names as in their CSV etc.
|
""" WalkthroughThe pull request implements multiple changes across production and test files. In production code, it enhances request handling and data mapping by augmenting the RequestWithDetails typedef, adding a new field mapping function, and adjusting table parameter construction. Additionally, obsolete error summary and column functionality are removed from model classes and the table component is made more configurable. Test files are updated accordingly with simplified imports, adjusted expectations, and removal of deprecated tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as ResultsController
participant Response as ResponseDetails
Client->>Controller: Send HTTP request with parsedParams
Controller->>Response: Retrieve response details (cached if available)
Controller->>Controller: Execute fieldToColumnMapping(details)
Controller->>Controller: Construct tableParams (columns, fields, mapping)
Controller->>Client: Return response with tableParams
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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
|
||||||||||||||||||||||||||||||||||||||||||||||||||
Also: simplify and remove code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/controllers/resultsController.js(3 hunks)src/models/requestData.js(0 hunks)src/models/responseDetails.js(3 hunks)src/views/components/table.html(2 hunks)test/integration/test_recieving_results.playwright.test.js(2 hunks)test/unit/requestData.test.js(1 hunks)test/unit/responseDetails.test.js(1 hunks)test/unit/resultsController.test.js(3 hunks)
💤 Files with no reviewable changes (1)
- src/models/requestData.js
🧰 Additional context used
🧬 Code Graph Analysis (3)
test/unit/resultsController.test.js (1)
src/controllers/resultsController.js (14)
setupTableParams(154-210)req(121-121)req(203-203)req(229-229)req(240-240)req(246-246)req(271-271)req(351-351)req(364-364)req(383-383)req(434-434)req(478-478)fieldToColumnMapping(142-146)fieldToColumnMapping(142-146)
test/integration/test_recieving_results.playwright.test.js (3)
test/acceptance/request_check.test.js (1)
response(119-119)src/middleware/entryIssueDetails.middleware.js (1)
details(71-75)src/utils/table.js (2)
leading(19-19)trailing(20-20)
src/controllers/resultsController.js (2)
src/utils/table.js (2)
splitByLeading(18-31)splitByLeading(18-31)src/services/performanceDbApi.js (1)
req(387-387)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: run-tests / test
🔇 Additional comments (22)
test/unit/responseDetails.test.js (1)
121-121: Update to test expectations reflects column naming change.The test now expects raw field names from the
getFieldsmethod rather than the previously transformed versions, which aligns with the PR objective to display column names exactly as they appear in the user's CSV files.test/unit/requestData.test.js (2)
2-2: Consistent file extension usage for imports.This change standardises the import by adding the
.jsextension to the path, which improves consistency across the codebase.
5-5: Consistent file extension usage for imports.Same standardisation for the logger import.
src/views/components/table.html (5)
31-38: Added optional parameters for enhanced column name handling.These new parameters provide flexibility for column name rendering:
columnNameProcessingallows the raw field names to be displayed without transformationmappingenables field-to-column mapping for correct data displayThis implementation directly supports the PR objective to display column names exactly as they appear in source files.
54-62: Added conditional rendering for column names.This change allows column names to be displayed either as they are (with basic escaping) or with the
prettifyColumnNamefilter applied, based on the newcolumnNameProcessingparameter. Good use of the template's conditional logic to maintain backward compatibility.
66-66: Simplified mapping access in templates.Setting
mappingas a local variable improves readability in the template.
70-70: Enhanced column data lookup using mappings.Now supports field-to-column mapping with a fallback to direct access, allowing for dynamic column name handling.
72-77: Consistent error message handling with column name processing.Appropriately applies the same column name processing logic to error messages, ensuring consistent display throughout the table.
test/unit/resultsController.test.js (4)
2-14: Updated imports to include new fieldToColumnMapping function.The import statement now includes the new utility function that handles field-to-column mapping, ensuring all necessary components are available for testing.
135-135: Removed async/await for synchronous function call.The
setupTableParamsfunction is now called withoutawaitsince it operates synchronously, improving code correctness.
137-143: Updated test expectations for table parameters.The test now expects the new parameters required by the enhanced table component:
columnNameProcessingset to 'none' to preserve original field namesmappinginitialized as an empty Map for column mappingThis correctly validates the updated behavior of the
setupTableParamsfunction.
307-329: Added comprehensive tests for the new fieldToColumnMapping function.Good test coverage for the new utility function:
- Verifies correct mapping creation with various field types
- Handles the edge case of empty column data
These tests ensure the mapping function works correctly, which is essential for the proper display of column names in the check tool result table.
src/models/responseDetails.js (3)
34-35: Good addition of a private cache field.Adding a private cache field for storing the result of potentially expensive operations is a good performance improvement.
53-55: Improved documentation with JSDoc return type.Adding the return type to the JSDoc comment improves code clarity and helps with type checking.
66-74: Improved documentation clarifying field usage.The updated JSDoc comments provide better clarity about what the fields represent and how they're used.
test/integration/test_recieving_results.playwright.test.js (2)
12-17: Good practice adding .js extensions to imports.Adding the .js extension to import statements is good practice and helps with module resolution in modern JavaScript environments.
104-120: Simplified table value generation aligns with PR objectives.The function has been simplified to directly use column names from the converted row data without transformation, which aligns with the PR objective to "render column names unchanged".
src/controllers/resultsController.js (5)
106-111: Enhanced type definitions improve code clarity.The expanded TypeScript definitions for
RequestWithDetailsimprove code clarity and documentation.
138-146: Well-structured mapping function.The new
fieldToColumnMappingfunction is well-structured, appropriately documented with JSDoc comments, and follows functional programming principles by using tuple destructuring and mapping.
180-185: Good bidirectional mapping implementation.Creating bidirectional mappings between fields and columns is a good approach for maintaining data relationships.
187-189: Clear explanation of the intention in comments.The comment clearly explains the intention to preserve original CSV column names for users, which aligns well with the PR objectives.
190-198: Simplified table parameters with explicit column name handling.The updated table parameters explicitly specify 'none' for column name processing and include the mapping, which supports the goal of preserving original column names.
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)
test/integration/test_recieving_results.playwright.test.js(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/integration/test_recieving_results.playwright.test.js (1)
src/utils/table.js (2)
leading(19-19)trailing(20-20)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: run-tests / test
🔇 Additional comments (3)
test/integration/test_recieving_results.playwright.test.js (3)
12-12: Good update adding .js extensions to import paths.The addition of explicit .js extensions to import statements improves module compatibility across different environments.
Also applies to: 14-14, 17-17
50-54: Good addition of 'reference' column handling.This code correctly adds a 'reference' column header and empty values to match the updated table structure, ensuring test data aligns with the new column display requirements.
110-127: Good refactoring to preserve original column names.The function has been correctly simplified to derive column headers directly from the data structure without transformation, aligning with the PR objective to display column names unchanged from the CSV files.
2 participants
Preview Link
https://submit-pr-[PR_NUMBER].herokuapp.com/
What type of PR is this? (check all applicable)
Description
Ensures the column names in check tool result table are rendered unchanged. User will see the same column names as in their CSV etc.
Additionally
table.htmltemplate to take optional options (if not passed, no change in current behaviour).Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Before
After
Added/updated tests?
QA sign off
Summary by CodeRabbit
New Features
Refactor
Tests