-
-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert duration to decimal before saving to db #8597
base: develop
Are you sure you want to change the base?
Convert duration to decimal before saving to db #8597
Conversation
WalkthroughWalkthroughThe changes primarily introduce functionality for handling and converting duration values across various parts of the codebase. This includes adding utility functions for converting durations to seconds and milliseconds to formatted durations, and updating components and models to utilize these new functions. Additionally, a minor adjustment was made in a test setup file to make the loop condition dynamic. Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant GUI
participant Backend
participant DurationUtils
User->>GUI: Input duration value
GUI->>DurationUtils: Call convertDurationToSeconds(value)
DurationUtils-->>GUI: Return converted duration in seconds
GUI->>Backend: Send converted duration
Backend->>DurationUtils: Call convertDurationToSeconds(value)
DurationUtils-->>Backend: Return converted duration in seconds
Backend-->>GUI: Acknowledge received duration
GUI-->>User: Display confirmation
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/nocodb/src/db/BaseModelSqlv2.ts (1 hunks)
Additional Context Used
Biome (20)
packages/nocodb/src/db/BaseModelSqlv2.ts (20)
87-87: Unexpected any. Specify a different type.
97-97: Unexpected any. Specify a different type.
111-111: Unexpected any. Specify a different type.
164-164: Unexpected any. Specify a different type.
167-167: Unexpected any. Specify a different type.
178-178: Unexpected any. Specify a different type.
188-188: Unexpected any. Specify a different type.
190-190: Unexpected any. Specify a different type.
200-200: Unexpected any. Specify a different type.
219-219: This variable implicitly has the any type.
240-240: Unexpected any. Specify a different type.
240-240: Unexpected any. Specify a different type.
249-249: Template literals are preferred over string concatenation.
249-249: Use !== instead of !=.
!= is only allowed when comparing againstnull
264-264: Unexpected any. Specify a different type.
266-266: Unexpected any. Specify a different type.
297-297: This variable implicitly has the any type.
333-333: Unexpected any. Specify a different type.
344-344: Unexpected any. Specify a different type.
429-429: Change to an optional chain.
@rohittp0 check frontend implementation of converting duration (hh:mm) to seconds
|
Uffizzi Ephemeral Environment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TS compilation error needs a fix
TSError: ⨯ Unable to compile TypeScript:
Error: src/db/BaseModelSqlv2.ts(28,8): error TS2307: Cannot find module 'nc-gui/utils/durationUtils' or its corresponding type declarations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
packages/nocodb/src/db/BaseModelSqlv2.ts (1)
3524-3526
: The logic to handleUITypes.Duration
is correctly implemented. Consider adding a comment explaining the fallback toparseFloat
whenconvertDurationToSeconds
returnsnull
orundefined
.// Convert duration to seconds, fallback to parseFloat if conversion fails val = convertDurationToSeconds(val) ?? parseFloat(val);
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/nocodb/src/db/BaseModelSqlv2.ts (2 hunks)
- packages/nocodb/src/db/util/durationUtils.ts (1 hunks)
Additional Context Used
Biome (35)
packages/nocodb/src/db/BaseModelSqlv2.ts (20)
88-88: Unexpected any. Specify a different type.
98-98: Unexpected any. Specify a different type.
112-112: Unexpected any. Specify a different type.
165-165: Unexpected any. Specify a different type.
168-168: Unexpected any. Specify a different type.
179-179: Unexpected any. Specify a different type.
189-189: Unexpected any. Specify a different type.
191-191: Unexpected any. Specify a different type.
201-201: Unexpected any. Specify a different type.
220-220: This variable implicitly has the any type.
241-241: Unexpected any. Specify a different type.
241-241: Unexpected any. Specify a different type.
250-250: Template literals are preferred over string concatenation.
250-250: Use !== instead of !=.
!= is only allowed when comparing againstnull
265-265: Unexpected any. Specify a different type.
267-267: Unexpected any. Specify a different type.
298-298: This variable implicitly has the any type.
334-334: Unexpected any. Specify a different type.
345-345: Unexpected any. Specify a different type.
430-430: Change to an optional chain.
packages/nocodb/src/db/util/durationUtils.ts (15)
48-48: Declare variables separately
53-53: Use Number.parseInt instead of the equivalent global.
56-56: Use Number.parseFloat instead of the equivalent global.
68-68: Use Number.parseFloat instead of the equivalent global.
69-69: Use Number.parseFloat instead of the equivalent global.
76-76: Use Number.parseFloat instead of the equivalent global.
77-77: Use Number.parseFloat instead of the equivalent global.
79-79: Use Number.parseFloat instead of the equivalent global.
80-80: Use Number.parseFloat instead of the equivalent global.
81-81: Use Number.parseFloat instead of the equivalent global.
93-93: Use Number.parseFloat instead of the equivalent global.
95-95: Use the '**' operator instead of 'Math.pow'.
104-104: Use Number.parseFloat instead of the equivalent global.
106-106: Use the '**' operator instead of 'Math.pow'.
116-116: Use Number.parseFloat instead of the equivalent global.
Additional comments not posted (2)
packages/nocodb/src/db/util/durationUtils.ts (1)
1-31
: ThedurationOptions
array is well-defined with clear examples and regex patterns for each duration type.packages/nocodb/src/db/BaseModelSqlv2.ts (1)
25-25
: The import ofconvertDurationToSeconds
is correctly added and necessary for the new functionality.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/nocodb/src/db/util/durationUtils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/nocodb/src/db/util/durationUtils.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
packages/nocodb/src/db/util/durationUtils.ts (1)
1-31
: Ensure consistency in regex patterns for duration formats.Consider adding more detailed comments explaining each regex pattern for future maintainability.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/nocodb/src/db/BaseModelSqlv2.ts (2 hunks)
- packages/nocodb/src/db/util/durationUtils.ts (1 hunks)
Additional Context Used
Biome (21)
packages/nocodb/src/db/BaseModelSqlv2.ts (20)
88-88: Unexpected any. Specify a different type.
98-98: Unexpected any. Specify a different type.
112-112: Unexpected any. Specify a different type.
165-165: Unexpected any. Specify a different type.
168-168: Unexpected any. Specify a different type.
179-179: Unexpected any. Specify a different type.
189-189: Unexpected any. Specify a different type.
191-191: Unexpected any. Specify a different type.
201-201: Unexpected any. Specify a different type.
220-220: This variable implicitly has the any type.
241-241: Unexpected any. Specify a different type.
241-241: Unexpected any. Specify a different type.
250-250: Template literals are preferred over string concatenation.
250-250: Use !== instead of !=.
!= is only allowed when comparing againstnull
265-265: Unexpected any. Specify a different type.
267-267: Unexpected any. Specify a different type.
298-298: This variable implicitly has the any type.
334-334: Unexpected any. Specify a different type.
345-345: Unexpected any. Specify a different type.
430-430: Change to an optional chain.
packages/nocodb/src/db/util/durationUtils.ts (1)
38-38: Declare variables separately
Additional comments not posted (3)
packages/nocodb/src/db/util/durationUtils.ts (1)
34-117
: Review the logic for converting duration to seconds.Ensure that the function handles edge cases correctly, such as incomplete time strings or unexpected formats. Consider adding unit tests to cover these scenarios.
packages/nocodb/src/db/BaseModelSqlv2.ts (2)
25-25
: The import ofconvertDurationToSeconds
is correctly added and necessary for the new functionality.
3524-3526
: Ensure that similar logic is applied to single insertion scenarios as suggested in previous comments. Consider refactoring this logic into a helper function to avoid duplication.[REFACTOR_SUGGESTion]
- if (col.uidt === UITypes.Duration && typeof val === 'string') - val = convertDurationToSeconds(val) ?? parseFloat(val); + val = handleDurationConversion(col, val);Verification successful
The search results indicate that the specific logic for handling
UITypes.Duration
and converting it usingconvertDurationToSeconds
is unique to the location inBaseModelSqlv2.ts
. No similar patterns were found in other parts of the codebase.
UITypes.Duration
is referenced in various files, but none of them contain similar logic for converting duration values.- The
convertDurationToSeconds
function is defined indurationUtils.ts
and used only inBaseModelSqlv2.ts
.Given this, it seems that the logic is not duplicated elsewhere, and the current implementation is isolated to the specific context in
BaseModelSqlv2.ts
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for similar duration handling logic in single insertion scenarios. ast-grep --lang javascript --pattern $'if ($_ === UITypes.Duration && typeof $_ === "string") { $$$ }'Length of output: 103
Script:
#!/bin/bash # Description: Search for functions or methods that handle duration values. rg 'UITypes.Duration' --glob '*.ts'Length of output: 2522
Script:
#!/bin/bash # Description: Search for functions or methods that handle duration values. rg 'convertDurationToSeconds' --glob '*.ts'Length of output: 488
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/nocodb/src/db/util/durationUtils.ts (1 hunks)
Additional Context Used
Biome (1)
packages/nocodb/src/db/util/durationUtils.ts (1)
36-36: Declare variables separately
Additional comments not posted (2)
packages/nocodb/src/db/util/durationUtils.ts (2)
1-31
: ThedurationOptions
array is well-structured and provides clear examples and regex patterns for different duration formats.
36-36
: Separate the declarations ofh
,mm
,ss
for better readability.- let h: number, mm: number, ss: number; + let h: number; + let mm: number; + let ss: number;Likely invalid or redundant comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- packages/nc-gui/components/cell/Duration.vue (2 hunks)
- packages/nc-gui/components/smartsheet/PlainCell.vue (1 hunks)
- packages/nc-gui/components/smartsheet/column/DurationOptions.vue (3 hunks)
- packages/nocodb-sdk/src/lib/durationUtils.ts (1 hunks)
- packages/nocodb-sdk/src/lib/index.ts (1 hunks)
- packages/nocodb/src/db/BaseModelSqlv2.ts (2 hunks)
- packages/nocodb/src/models/Model.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- packages/nc-gui/components/smartsheet/PlainCell.vue
Additional Context Used
Biome (48)
packages/nocodb-sdk/src/lib/durationUtils.ts (8)
90-90: Unexpected any. Specify a different type.
105-124: This else clause can be omitted because previous branches break early.
108-124: This else clause can be omitted because previous branches break early.
113-124: This else clause can be omitted because previous branches break early.
118-124: This else clause can be omitted because previous branches break early.
98-98: Use Number.parseInt instead of the equivalent global.
99-99: Use Number.parseInt instead of the equivalent global.
100-100: Use Number.parseInt instead of the equivalent global.
packages/nocodb/src/db/BaseModelSqlv2.ts (20)
88-88: Unexpected any. Specify a different type.
98-98: Unexpected any. Specify a different type.
112-112: Unexpected any. Specify a different type.
165-165: Unexpected any. Specify a different type.
168-168: Unexpected any. Specify a different type.
179-179: Unexpected any. Specify a different type.
189-189: Unexpected any. Specify a different type.
191-191: Unexpected any. Specify a different type.
201-201: Unexpected any. Specify a different type.
220-220: This variable implicitly has the any type.
241-241: Unexpected any. Specify a different type.
241-241: Unexpected any. Specify a different type.
250-250: Template literals are preferred over string concatenation.
250-250: Use !== instead of !=.
!= is only allowed when comparing againstnull
265-265: Unexpected any. Specify a different type.
267-267: Unexpected any. Specify a different type.
298-298: This variable implicitly has the any type.
334-334: Unexpected any. Specify a different type.
345-345: Unexpected any. Specify a different type.
430-430: Change to an optional chain.
packages/nocodb/src/models/Model.ts (20)
47-47: Unexpected any. Specify a different type.
62-62: Unexpected any. Specify a different type.
188-188: Using this in a static context can be confusing.
421-421: Using this in a static context can be confusing.
543-548: This default parameter should follow the last required parameter or should be a required parameter.
606-606: Do not use template literals if interpolation and special-character handling are not needed.
623-623: Template literals are preferred over string concatenation.
624-624: Template literals are preferred over string concatenation.
665-665: Using this in a static context can be confusing.
696-696: Using this in a static context can be confusing.
778-778: Using this in a static context can be confusing.
854-854: Unexpected any. Specify a different type.
940-940: Using this in a static context can be confusing.
995-995: Unexpected any. Specify a different type.
1027-1027: Using this in a static context can be confusing.
1036-1036: Using this in a static context can be confusing.
250-250: Use Number.Infinity instead of the equivalent global.
251-251: Use Number.Infinity instead of the equivalent global.
405-405: Avoid the use of spread (
...
) syntax on accumulators.
615-615: Use Number.parseFloat instead of the equivalent global.
Additional comments not posted (13)
packages/nocodb-sdk/src/lib/index.ts (1)
29-29
: ExportingdurationUtils
fromnocodb-sdk
.This change aligns with the PR's objective to handle duration conversions across the system. It ensures that the utility functions are accessible where needed.
packages/nc-gui/components/smartsheet/column/DurationOptions.vue (2)
2-2
: ImportingdurationOptions
fromnocodb-sdk
.This import is necessary for the component to access the duration options for display, ensuring the UI reflects the available duration formats correctly.
Line range hint
35-44
: Updated thev-for
loop to useduration.type
as a key and value.This change ensures that the component uses the correct identifier for duration types, improving maintainability and consistency with the backend data structure.
packages/nc-gui/components/cell/Duration.vue (2)
3-3
: Importing duration utilities fromnocodb-sdk
.This ensures that the component has the necessary functions to convert and display durations correctly, aligning with the PR's objectives.
42-44
: Converting the input value to milliseconds and updating the state.This logic correctly handles the conversion of duration input to milliseconds, which is crucial for accurate data handling and storage.
packages/nocodb-sdk/src/lib/durationUtils.ts (4)
1-31
: DefineddurationOptions
with various formats.This setup is essential for supporting multiple duration formats throughout the application, enhancing flexibility and user experience.
34-77
: ImplementedextractHMS
function to parse duration strings based on type.This function is a critical component of the duration handling logic, ensuring that durations are parsed accurately based on their format.
86-88
: ImplementedpadZero
function to format time strings.This utility function is crucial for ensuring time values are displayed consistently, improving the readability of time data.
128-179
: EnhancedconvertDurationToSeconds
to support detailed duration parsing.This function is key to converting various formatted durations to a consistent unit (seconds), crucial for backend processing and storage.
packages/nocodb/src/models/Model.ts (2)
2-2
: ImportingconvertDurationToSeconds
fromnocodb-sdk
.This import is necessary for the model to handle duration fields correctly, aligning with the PR's objectives to fix duration handling.
614-616
: Converting duration strings to seconds before saving to the database.This change ensures that duration data is stored in a consistent format in the database, which is crucial for accurate data handling and reporting.
packages/nocodb/src/db/BaseModelSqlv2.ts (2)
12-12
: The import ofconvertDurationToSeconds
aligns with the PR's objective to handle duration conversions.
3524-3526
: Ensure thatconvertDurationToSeconds
always returns a number to avoid unnecessary use ofparseFloat
.Verification successful
The
convertDurationToSeconds
function can returnnull
if the input does not match any of the expected duration formats. Therefore, the use of the nullish coalescing operator (??
) withparseFloat
is necessary to handle cases whereconvertDurationToSeconds
returnsnull
.
- : The use of
?? parseFloat(val)
is correct and necessary to handle cases whereconvertDurationToSeconds
returnsnull
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the return type of `convertDurationToSeconds`. # Test: Search for the function definition. Expect: Always returns a number. ast-grep --lang typescript --pattern $'export const convertDurationToSeconds = ($_): number => { $$$ }' packages/nocodb/src/db/util/durationUtils.tsLength of output: 241
Script:
#!/bin/bash # Locate the file containing the `convertDurationToSeconds` function definition. fd 'convertDurationToSeconds' --exec grep -H 'convertDurationToSeconds' {} # Verify the return type of `convertDurationToSeconds`. ast-grep --lang typescript --pattern $'const convertDurationToSeconds = ($_): number => { $$$ }'Length of output: 172
Script:
#!/bin/bash # Search for the `convertDurationToSeconds` function definition across the entire codebase. rg 'function convertDurationToSeconds' --files-with-matchesLength of output: 61
Script:
#!/bin/bash # Search for the `convertDurationToSeconds` function definition using different patterns. rg 'convertDurationToSeconds\s*=\s*\(' --files-with-matchesLength of output: 106
Script:
#!/bin/bash # Verify the return type of the `convertDurationToSeconds` function in the identified file. ast-grep --lang typescript --pattern $'const convertDurationToSeconds = ($_): number => { $$$ }' packages/nocodb-sdk/src/lib/durationUtils.tsLength of output: 142
Script:
#!/bin/bash # Display the contents of the file to inspect the `convertDurationToSeconds` function definition. cat packages/nocodb-sdk/src/lib/durationUtils.tsLength of output: 4945
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/nocodb-sdk/src/lib/durationUtils.ts (1 hunks)
Additional context used
Biome
packages/nocodb-sdk/src/lib/durationUtils.ts
[error] 90-90: Unexpected any. Specify a different type.
[error] 105-124: This else clause can be omitted because previous branches break early.
[error] 108-124: This else clause can be omitted because previous branches break early.
[error] 113-124: This else clause can be omitted because previous branches break early.
[error] 118-124: This else clause can be omitted because previous branches break early.
[error] 99-99: Use Number.parseInt instead of the equivalent global.
[error] 100-100: Use Number.parseInt instead of the equivalent global.
Additional comments not posted (2)
packages/nocodb-sdk/src/lib/durationUtils.ts (2)
1-31
: Constants for duration options are well-defined and cover a variety of common formats.
128-179
: The functionconvertDurationToSeconds
correctly handles conversion from various formatted durations to seconds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the changes requested via Slack.
…to-duration-fails # Conflicts: # packages/nc-gui/components/smartsheet/column/DurationOptions.vue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/nc-gui/components/cell/Duration.vue (2 hunks)
- packages/nc-gui/components/smartsheet/column/DurationOptions.vue (3 hunks)
- packages/nocodb/src/db/BaseModelSqlv2.ts (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/nc-gui/components/cell/Duration.vue
- packages/nc-gui/components/smartsheet/column/DurationOptions.vue
Additional context used
Biome
packages/nocodb/src/db/BaseModelSqlv2.ts
[error] 88-88: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 98-98: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 112-112: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 165-165: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 168-168: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 179-179: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 189-189: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 191-191: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 201-201: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 220-220: This variable implicitly has the any type. (lint/suspicious/noImplicitAnyLet)
Variable declarations without type annotation and initialization implicitly have the any type. Declare a type or initialize the variable with some value.
[error] 241-241: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 241-241: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 250-250: Use !== instead of !=.
!= is only allowed when comparing againstnull
(lint/suspicious/noDoubleEquals)!= is only allowed when comparing against null
Using != may be unsafe if you are relying on type coercion
Unsafe fix: Use !==
[error] 265-265: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 267-267: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 298-298: This variable implicitly has the any type. (lint/suspicious/noImplicitAnyLet)
Variable declarations without type annotation and initialization implicitly have the any type. Declare a type or initialize the variable with some value.
[error] 334-334: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 345-345: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 430-430: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 442-444: Prefer for...of instead of forEach. (lint/complexity/noForEach)
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
Additional comments not posted (2)
packages/nocodb/src/db/BaseModelSqlv2.ts (2)
12-12
: Import ofconvertDurationToSeconds
looks good and is necessary for the changes in this file.
3524-3526
: The logic to handle duration fields looks correct. Ensure that the fallback toparseFloat
is consistent with the expected data types and formats.Verification successful
The fallback mechanism using
parseFloat
afterconvertDurationToSeconds
is consistently applied in the codebase.
packages/nocodb/src/models/Model.ts
packages/nocodb/src/db/BaseModelSqlv2.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of the fallback mechanism in other parts of the codebase. # Test: Search for similar usage of `convertDurationToSeconds` and its fallback. rg --type typescript $'convertDurationToSeconds'Length of output: 82
Script:
#!/bin/bash # Description: Verify the consistency of the fallback mechanism in other parts of the codebase. # Test: Search for similar usage of `convertDurationToSeconds` and its fallback. rg --type ts $'convertDurationToSeconds'Length of output: 493
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range and nitpick comments (4)
tests/playwright/setup/demoTable.ts (1)
Line range hint
4-4
: Specify a more appropriate type instead ofany
for better type safety.- let api: Api<any>; + let api: Api<SpecificType>; // Replace `SpecificType` with the actual expected type.Also applies to: 171-171
packages/nocodb/src/models/Model.ts (3)
Line range hint
188-188
: Replacethis
withModel
in static methods to avoid confusion.- this.get(args.id, ncMeta); + Model.get(args.id, ncMeta);Also applies to: 421-421, 668-668, 699-699, 781-781, 943-943, 1030-1030, 1039-1039
Line range hint
47-47
: Specify a more appropriate type instead ofany
for better type safety.- schema: any; + schema: SpecificSchemaType; // Replace `SpecificSchemaType` with the actual expected type.Also applies to: 62-62, 857-857, 998-998
Line range hint
405-405
: Avoid using spread syntax in accumulators to improve performance.- return (columns || (await this.getColumns())).reduce((sortAgg, c) => ({ ...sortAgg, [c.title]: c }), {}); + const result = {}; + for (const c of columns || (await this.getColumns())) { + result[c.title] = c; + } + return result;Also applies to: 990-990
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- packages/nocodb-sdk/src/lib/durationUtils.ts (1 hunks)
- packages/nocodb/src/db/BaseModelSqlv2.ts (2 hunks)
- packages/nocodb/src/models/Model.ts (2 hunks)
- tests/playwright/setup/demoTable.ts (1 hunks)
Additional context used
Biome
packages/nocodb-sdk/src/lib/durationUtils.ts
[error] 90-90: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
tests/playwright/setup/demoTable.ts
[error] 4-4: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 167-167: Useless rename. (lint/complexity/noUselessRename)
Safe fix: Remove the renaming.
[error] 168-168: Useless rename. (lint/complexity/noUselessRename)
Safe fix: Remove the renaming.
[error] 169-169: Useless rename. (lint/complexity/noUselessRename)
Safe fix: Remove the renaming.
[error] 171-171: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
packages/nocodb/src/models/Model.ts
[error] 47-47: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 62-62: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 188-188: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 421-421: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 668-668: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 699-699: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 781-781: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 857-857: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 943-943: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 998-998: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 1030-1030: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 1039-1039: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 405-405: Avoid the use of spread (
...
) syntax on accumulators. (lint/performance/noAccumulatingSpread)Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.
[error] 990-990: Avoid the use of spread (
...
) syntax on accumulators. (lint/performance/noAccumulatingSpread)Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.packages/nocodb/src/db/BaseModelSqlv2.ts
[error] 88-88: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 98-98: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 112-112: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 165-165: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 168-168: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 179-179: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 189-189: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 191-191: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 201-201: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 220-220: This variable implicitly has the any type. (lint/suspicious/noImplicitAnyLet)
Variable declarations without type annotation and initialization implicitly have the any type. Declare a type or initialize the variable with some value.
[error] 241-241: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 241-241: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 250-250: Use !== instead of !=.
!= is only allowed when comparing againstnull
(lint/suspicious/noDoubleEquals)!= is only allowed when comparing against null
Using != may be unsafe if you are relying on type coercion
Unsafe fix: Use !==
[error] 265-265: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 267-267: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 298-298: This variable implicitly has the any type. (lint/suspicious/noImplicitAnyLet)
Variable declarations without type annotation and initialization implicitly have the any type. Declare a type or initialize the variable with some value.
[error] 334-334: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 345-345: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 430-430: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 442-444: Prefer for...of instead of forEach. (lint/complexity/noForEach)
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
Additional comments not posted (3)
packages/nocodb-sdk/src/lib/durationUtils.ts (2)
1-31
: LGTM! ThedurationOptions
constant is well-defined and matches the PR objectives.
129-185
: LGTM! TheconvertDurationToSeconds
function handles various duration formats effectively.packages/nocodb/src/db/BaseModelSqlv2.ts (1)
12-12
: The import ofconvertDurationToSeconds
is correctly added and aligns with the PR's objective to handle duration values.
Change Summary
During bulk operations duration field need to be parsed if the provide value is in time stamp notation (
hh:mm:ss
)Change type