Fix/normalize export data #2530
Conversation
WalkthroughRecent updates across various modules focused on enhancing file handling, environment variable management, and object utilities. Key changes include refining file operations, introducing more flexible environment variable defaults, and adding thorough test coverage for object manipulation utilities. These modifications improve code readability, maintainability, and robustness. Changes
Sequence DiagramsFile Handling EnhancementsequenceDiagram
participant Client
participant BuildFile
participant FileSystem
Client ->> BuildFile: Request to create/update file
BuildFile ->> FileSystem: Open file with write and append options
FileSystem -->> BuildFile: File descriptor
BuildFile ->> FileSystem: Write data in batches
BuildFile -->> Client: Log file export status
BuildFile ->> FileSystem: Close file
Enhanced Environment Variable HandlingsequenceDiagram
participant Application
participant EnvModule
participant Environment
Application ->> EnvModule: Request environment variable
EnvModule ->> Environment: Check for multiple keys
Environment -->> EnvModule: Return found value or fallback
EnvModule -->> Application: Provide environment variable/fallback
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- api/src/lib/file/index.ts (1 hunks)
- api/src/lib/object/get.unit.spec.ts (1 hunks)
- api/src/lib/object/index.ts (1 hunks)
- api/src/lib/process/index.ts (1 hunks)
- api/src/pdc/services/trip/actions/file/BuildFile.ts (7 hunks)
- api/src/pdc/services/trip/helpers/normalizeExportDataHelper.ts (1 hunks)
- api/src/shared/trip/sendExport.contract.ts (2 hunks)
Additional context used
Biome
api/src/shared/trip/sendExport.contract.ts
[error] 33-33: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
api/src/pdc/services/trip/actions/file/BuildFile.ts
[error] 24-24: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
[error] 108-108: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Additional comments not posted (16)
api/src/shared/trip/sendExport.contract.ts (4)
1-1: LGTM!The import statement is correct.
24-31: LGTM!The changes improve type safety by using a constant array for
ExportTypeListand definingExportTypebased on it.
36-37: LGTM!The changes ensure consistent use of double quotes in the
handlerConfigobject.
40-41: LGTM!The changes ensure consistent use of double quotes in the
signatureconstant.api/src/lib/file/index.ts (2)
1-1: LGTM!The import statement for
joinis correct and necessary for thegetTmpDirfunction.
19-47: LGTM!The type aliases
OpenFileOptionsandOpenFileDescriptorimprove code readability. Theopenfunction is correctly implemented and includes helpful examples.api/src/lib/object/index.ts (1)
3-30: LGTM!The changes to the
defaultValueparameter improve thegetfunction by providing a default value ofnull. The function implementation correctly handles various scenarios.api/src/lib/object/get.unit.spec.ts (1)
1-49: LGTM!The test cases for the
getfunction are comprehensive and cover various scenarios, ensuring the function works as expected.api/src/lib/process/index.ts (1)
88-88: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
unhandledRejectionHandlermatch the new signature.api/src/pdc/services/trip/actions/file/BuildFile.ts (6)
49-49: Ensure batch size is appropriate.The batch size has been introduced as a private constant. Ensure that it is appropriate for the use case.
69-70: Consider combining error logs.Consider combining the error message and stack trace into a single log entry for better readability.
- logger.error(e.message); - logger.debug(e.stack); + logger.error(`${e.message}\n${e.stack}`);
33-37: LGTM!The code changes are approved.
97-99: LGTM!The code changes are approved.
22-22: Remove unnecessary constructor.The constructor is unnecessary.
- constructor() {}Likely invalid or redundant comment.
33-37: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
buildCsvFromCursormatch the new signature.Verification successful
Verified function usage in the codebase.
The function call to
buildCsvFromCursorinBuildExportAction.tsmatches the new signature as defined inBuildFile.ts.
The function signature in
BuildFile.ts:public async buildCsvFromCursor( cursor: PgCursorHandler<ExportTripInterface>, params: ParamsInterface, date: Date, isOpendata: boolean, ): Promise<string> {The function call in
BuildExportAction.ts:const filepath: string = await this.buildFile.buildCsvFromCursor( cursor, params, queryParams.date.end, isOpendata, );The parameters match correctly.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `buildCsvFromCursor` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type typescript $'buildCsvFromCursor'Length of output: 76
Script:
#!/bin/bash # Description: Verify all function calls to `buildCsvFromCursor` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg 'buildCsvFromCursor'Length of output: 235
Script:
#!/bin/bash # Description: Extract lines around the usage of `buildCsvFromCursor` in `BuildExportAction.ts` to verify it matches the new signature. # Extract lines around the function call rg -A 5 -B 5 'buildCsvFromCursor' api/src/pdc/services/trip/actions/BuildExportAction.tsLength of output: 382
Script:
#!/bin/bash # Description: Extract the function signature of `buildCsvFromCursor` from `BuildFile.ts` to verify the parameters. # Extract the function signature rg -A 5 -B 5 'buildCsvFromCursor' api/src/pdc/services/trip/actions/file/BuildFile.tsLength of output: 346
api/src/pdc/services/trip/helpers/normalizeExportDataHelper.ts (1)
80-81: LGTM!The code changes are approved.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- api/src/lib/object/get.unit.spec.ts (1 hunks)
- api/src/lib/object/index.ts (1 hunks)
- api/src/pdc/providers/middleware/Cast/CastToArrayMiddleware.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- api/src/lib/object/get.unit.spec.ts
- api/src/lib/object/index.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- api/src/pdc/services/trip/actions/file/BuildFile.ts (6 hunks)
- api/src/shared/trip/sendExport.contract.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- api/src/shared/trip/sendExport.contract.ts
Additional context used
Biome
api/src/pdc/services/trip/actions/file/BuildFile.ts
[error] 24-24: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
[error] 108-108: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Additional comments not posted (11)
api/src/pdc/services/trip/actions/file/BuildFile.ts (11)
1-3: Imports: Ensure consistency with usage.The import modifications align with the changes in the method implementations. Ensure that all necessary imports are included and unused imports are removed.
22-22: Good practice: Use of constants.The introduction of a
batchSizeconstant improves code readability and maintainability.
33-37: Improvement: Default parameter value.Setting a default value for
params.typeimproves the robustness of the method.
39-40: Improvement: File opening options and logging.Updating the file opening options to include writing and appending, along with adding a debug log, enhances functionality and traceability.
49-49: Improvement: Use of batch size constant.Using the
batchSizeconstant for reading from the cursor improves maintainability.
59-59: Ensure proper file closure.The
fd.close()method should be verified to ensure it is synchronous, as previously discussed.
66-67: Ensure proper file closure.The
fd.close()method should be verified to ensure it is synchronous, as previously discussed.
69-70: Improvement: Separate error logging.Separating the error message and stack trace into different log levels improves debugging and log readability.
Line range hint
84-84: Improvement: Default values incastmethod.Providing default values for
tzandfilenameimproves the robustness of the method.
97-99: Improvement: Type adjustments.Changing the type annotations to use primitive types (
booleanandnumber) improves type safety.
108-108: Avoid assignment in expressions.The assignment should not be in an expression. Refactor to improve readability.
- while ((row = stringifier.read()) !== null) { + let row; + while ((row = stringifier.read()) !== null) {Likely invalid or redundant comment.
Tools
Biome
[error] 108-108: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- api/src/pdc/services/trip/actions/file/BuildFile.ts (6 hunks)
Additional context used
Learnings (1)
api/src/pdc/services/trip/actions/file/BuildFile.ts (1)
Learnt from: jonathanfallon PR: betagouv/preuve-covoiturage#2530 File: api/src/pdc/services/trip/actions/file/BuildFile.ts:66-67 Timestamp: 2024-07-02T07:48:51.054Z Learning: The `fd.close()` method in the context of `BuildFile` class is not an async function and does not require the `await` keyword.
Biome
api/src/pdc/services/trip/actions/file/BuildFile.ts
[error] 106-106: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Additional comments not posted (8)
api/src/pdc/services/trip/actions/file/BuildFile.ts (8)
22-22: Good practice: Introduce constants for configuration.Introducing the
batchSizeconstant improves configurability and readability.
31-35: Update: Add default value forparams.type.Adding a default value for
params.typein thecastmethod call ensures that the function handles missing or undefinedparams.typegracefully.
37-38: Update: Improve file opening options and logging.The updated
openmethod call with options for writing and appending, along with the debug log, enhances clarity and debugging capabilities.
47-47: Update: UsebatchSizeconstant for cursor read.Using the
batchSizeconstant instead of a hardcoded value improves maintainability and configurability.
57-57: Clarification:fd.close()does not needawait.As per the user's previous feedback,
fd.close()is not asynchronous and does not require theawaitkeyword.
67-68: Refactor: Separate error logging.Separating the error message and stack trace into different log levels improves readability and debugging.
31-35: Good practice: Handle default values forparams.type.Providing a default value for
params.typeensures that the function operates correctly even with incomplete parameters.
95-97: Update: Improve type annotations.Updating type annotations for
booleanandnumberconversions improves type safety and readability.
| let row: string = ""; | ||
| while ((row = stringifier.read()) !== null) { |
There was a problem hiding this comment.
Refactor: Avoid assignment in expressions.
The assignment should not be in an expression to improve readability and maintainability.
- while ((row = stringifier.read()) !== null) {
+ let row;
+ while ((row = stringifier.read()) !== null) {Committable suggestion was skipped due to low confidence.
Tools
Biome
[error] 106-106: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- api/src/lib/env/index.ts (1 hunks)
- api/src/lib/file/index.ts (1 hunks)
- api/src/lib/object/get.unit.spec.ts (1 hunks)
Additional comments not posted (13)
api/src/lib/file/index.ts (3)
1-17: LGTM!The changes to
getTmpDirimprove flexibility by checking multiple environment variables before falling back to a default value.
19-20: LGTM!The introduction of
OpenFileOptionsandOpenFileDescriptorimproves type safety and clarity for file operations.
22-46: LGTM!The
openfunction is well-implemented, with clear documentation and examples. The use of default options and the spread operator ensures flexibility.api/src/lib/env/index.ts (1)
29-39: LGTM!The
envs_or_defaultfunction is a useful addition for checking multiple environment variables before falling back to a default value.api/src/lib/object/get.unit.spec.ts (9)
5-8: LGTM!The test case "should get value from object" is well-implemented and covers the expected behavior.
10-13: LGTM!The test case "should get object from object" is well-implemented and covers the expected behavior.
15-18: LGTM!The test case "should get value from indexed array" is well-implemented and covers the expected behavior.
20-23: LGTM!The test case "should return default value if not found" is well-implemented and covers the expected behavior.
25-28: LGTM!The test case "should return value from an array of objects" is well-implemented and covers the expected behavior.
30-34: LGTM!The test case "should return value from an array of objects at root level" is well-implemented and covers the expected behavior.
36-39: LGTM!The test case "should return default value if the obj is null or undefined" is well-implemented and covers the expected behavior.
41-45: LGTM!The test case "should return null or undefined if the path ends and the value is null" is well-implemented and covers the expected behavior.
48-53: LGTM!The test case "should return undefined as defaultValue if the defaultValue is not provided" is well-implemented and covers the expected behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- api/src/lib/file/index.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- api/src/lib/file/index.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- api/src/lib/env/index.ts (1 hunks)
- api/src/lib/file/index.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- api/src/lib/env/index.ts
- api/src/lib/file/index.ts
Summary by CodeRabbit
New Features
getobject helper function, covering various scenarios including retrieving values from nested objects and arrays.Improvements
Bug Fixes
Documentation