Skip to content
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

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

rohittp0
Copy link
Contributor

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

  • feat: (new feature for the user, not a new feature for build script)
  • fix: (bug fix for the user, not a fix to a build script)
  • docs: (changes to the documentation)
  • style: (formatting, missing semi colons, etc; no production code change)
  • refactor: (refactoring production code, eg. renaming a variable)
  • test: (adding missing tests, refactoring tests; no production code change)
  • chore: (updating grunt tasks etc; no production code change)

@rohittp0 rohittp0 linked an issue May 28, 2024 that may be closed by this pull request
1 task
@rohittp0 rohittp0 requested a review from pranavxc May 28, 2024 10:23
Copy link
Contributor

coderabbitai bot commented May 28, 2024

Walkthrough

Walkthrough

The 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

Files/Paths Change Summaries
packages/nocodb/src/db/BaseModelSqlv2.ts Added a conditional block for handling UITypes.Duration and manipulating its value based on type and format.
packages/nocodb-sdk/src/lib/durationUtils.ts Introduced constants and functions for handling and converting durations: durationOptions, convertMS2Duration, convertDurationToSeconds.
packages/nc-gui/components/cell/Duration.vue Imported duration utility functions and modified the set method of localState to use convertDurationToSeconds without the durationType argument.
packages/nc-gui/components/smartsheet/PlainCell.vue Added import for convertMS2Duration to handle time duration conversion.
packages/nc-gui/components/smartsheet/column/DurationOptions.vue Imported durationOptions and updated comparison logic in a-select-option and component conditions.
packages/nocodb/src/models/Model.ts Added import for convertDurationToSeconds and logic to convert duration values to seconds when the column type is Duration and the value is a string.
tests/playwright/setup/demoTable.ts Changed loop condition in createDemoTable function to iterate based on the recordCnt variable instead of a fixed number.

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@rohittp0 rohittp0 self-assigned this May 28, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits Files that changed from the base of the PR and between 55beb0f and 1308e7c.
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 against null


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.

packages/nocodb/src/db/BaseModelSqlv2.ts Outdated Show resolved Hide resolved
@rameshmane7218
Copy link
Collaborator

@rohittp0 check frontend implementation of converting duration (hh:mm) to seconds

packages/nc-gui/utils/durationUtils.ts

Copy link
Contributor

github-actions bot commented May 28, 2024

Uffizzi Ephemeral Environment deployment-52338

☁️ https://app.uffizzi.com/github.com/nocodb/nocodb/pull/8597

📄 View Application Logs etc.

What is Uffizzi? Learn more!

Copy link
Member

@dstala dstala left a 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 handle UITypes.Duration is correctly implemented. Consider adding a comment explaining the fallback to parseFloat when convertDurationToSeconds returns null or undefined.

// Convert duration to seconds, fallback to parseFloat if conversion fails
val = convertDurationToSeconds(val) ?? parseFloat(val);
Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 1308e7c and e135b27.
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 against null


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: The durationOptions 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 of convertDurationToSeconds is correctly added and necessary for the new functionality.

packages/nocodb/src/db/util/durationUtils.ts Outdated Show resolved Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits Files that changed from the base of the PR and between e135b27 and 6f5f8fe.
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

packages/nocodb/src/db/BaseModelSqlv2.ts Outdated Show resolved Hide resolved
packages/nocodb/src/db/BaseModelSqlv2.ts Outdated Show resolved Hide resolved
packages/nocodb/src/db/util/durationUtils.ts Outdated Show resolved Hide resolved
packages/nocodb/src/db/util/durationUtils.ts Outdated Show resolved Hide resolved
packages/nocodb/src/db/util/durationUtils.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits Files that changed from the base of the PR and between 6f5f8fe and 975a470.
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 against null


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 of convertDurationToSeconds 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 using convertDurationToSeconds is unique to the location in BaseModelSqlv2.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 in durationUtils.ts and used only in BaseModelSqlv2.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

packages/nocodb/src/db/util/durationUtils.ts Outdated Show resolved Hide resolved
rohittp0 and others added 2 commits May 28, 2024 22:26
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits Files that changed from the base of the PR and between 975a470 and f0a2304.
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: The durationOptions array is well-structured and provides clear examples and regex patterns for different duration formats.


36-36: Separate the declarations of h, 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.

packages/nocodb/src/db/util/durationUtils.ts Outdated Show resolved Hide resolved
packages/nocodb/src/db/util/durationUtils.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits Files that changed from the base of the PR and between f0a2304 and c936fca.
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 against null


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: Exporting durationUtils from nocodb-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: Importing durationOptions from nocodb-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 the v-for loop to use duration.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 from nocodb-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: Defined durationOptions with various formats.

This setup is essential for supporting multiple duration formats throughout the application, enhancing flexibility and user experience.


34-77: Implemented extractHMS 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: Implemented padZero 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: Enhanced convertDurationToSeconds 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: Importing convertDurationToSeconds from nocodb-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 of convertDurationToSeconds aligns with the PR's objective to handle duration conversions.


3524-3526: Ensure that convertDurationToSeconds always returns a number to avoid unnecessary use of parseFloat.

Verification successful

The convertDurationToSeconds function can return null if the input does not match any of the expected duration formats. Therefore, the use of the nullish coalescing operator (??) with parseFloat is necessary to handle cases where convertDurationToSeconds returns null.

  • : The use of ?? parseFloat(val) is correct and necessary to handle cases where convertDurationToSeconds returns null.
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.ts

Length 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-matches

Length of output: 61



Script:

#!/bin/bash
# Search for the `convertDurationToSeconds` function definition using different patterns.
rg 'convertDurationToSeconds\s*=\s*\(' --files-with-matches

Length 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.ts

Length 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.ts

Length of output: 4945

packages/nocodb-sdk/src/lib/durationUtils.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between c936fca and 38fc248.

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 function convertDurationToSeconds correctly handles conversion from various formatted durations to seconds.

packages/nocodb-sdk/src/lib/durationUtils.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@rameshmane7218 rameshmane7218 left a 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
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 38fc248 and 3fba129.

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 against null (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 of convertDurationToSeconds 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 to parseFloat is consistent with the expected data types and formats.

Verification successful

The fallback mechanism using parseFloat after convertDurationToSeconds 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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 of any 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: Replace this with Model 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 of any 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

Commits

Files that changed from the base of the PR and between 3fba129 and 761bb5d.

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 of O(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 of O(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 against null (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! The durationOptions constant is well-defined and matches the PR objectives.


129-185: LGTM! The convertDurationToSeconds function handles various duration formats effectively.

packages/nocodb/src/db/BaseModelSqlv2.ts (1)

12-12: The import of convertDurationToSeconds is correctly added and aligns with the PR's objective to handle duration values.

packages/nocodb/src/db/BaseModelSqlv2.ts Show resolved Hide resolved
packages/nocodb-sdk/src/lib/durationUtils.ts Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Bug: csv upload into field set to duration fails
4 participants