Lite version: Fix number field validation for custom decimal separators by shervElmi · Pull Request #2248 · Strategy11/formidable-forms

shervElmi · 2025-02-17T16:29:01Z

Main PR:

https://github.com/Strategy11/formidable-pro/pull/5629#issuecomment-2663420187

coderabbitai · 2025-02-17T16:31:08Z

Walkthrough

The changes introduce a new static method in a currency helper class that normalizes decimal characters in number strings. Additionally, updates to a field type class now retrieve a custom decimal separator and use the new helper method when processing minimum and maximum numeric values. These modifications adjust how decimal values are interpreted for validation.

Changes

File	Change Summary
`classes/.../FrmCurrencyHelper.php`	Added a public static method `replace_custom_decimal_char` that replaces a custom decimal character in a number string with a period (`.`).
`classes/.../FrmFieldType.php`	Modified the `add_min_max` method to first retrieve a custom decimal separator and then use the new helper method to process `minnum` and `maxnum`.

Sequence Diagram(s)

sequenceDiagram
    participant FT as FrmFieldType::add_min_max
    participant Field as FrmField::get_option
    participant CH as FrmCurrencyHelper

    FT->>Field: get_option('custom_decimal_separator')
    FT->>Field: get_option('minnum')
    FT->>Field: get_option('maxnum')
    FT->>CH: replace_custom_decimal_char(minnum, custom_decimal_separator)
    FT->>CH: replace_custom_decimal_char(maxnum, custom_decimal_separator)

Possibly related PRs

Issuefix/ff style #2240: Directly utilizes the new replace_custom_decimal_char method in processing min/max values within the field type class.
Lite Version: Fix number field step validation #2245: Involves modifications in handling numeric values using a similar approach with the currency helper method.
Fix JS error when trying to validate readonly time field #1946: Relates to the replace functionality where ensuring string normalization is a core aspect of processing numeric values.

Suggested reviewers

Crabcyborg
lauramekaj1

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ad93b1c and c87464c.

📒 Files selected for processing (2)

classes/helpers/FrmCurrencyHelper.php (1 hunks)
classes/models/fields/FrmFieldType.php (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms (9)

GitHub Check: Cypress
GitHub Check: Cypress
GitHub Check: PHP 8 tests in WP trunk
GitHub Check: PHP 8 tests in WP trunk
GitHub Check: PHP 7.4 tests in WP trunk
GitHub Check: PHP 8 tests in WP trunk
GitHub Check: PHP 7.4 tests in WP trunk
GitHub Check: Cypress
GitHub Check: PHP 7.4 tests in WP trunk

🔇 Additional comments (2)

classes/helpers/FrmCurrencyHelper.php (1)

41-54: LGTM! Well-documented helper method.

The new helper method is well-documented, handles edge cases correctly, and follows a clean implementation pattern.

classes/models/fields/FrmFieldType.php (1)

1119-1126: LGTM! Proper handling of custom decimal separators.

The changes correctly handle custom decimal separators by:

Retrieving the custom decimal separator from field options.

Using the new helper method to normalize min/max values before validation.

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>, please review it.
- 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @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 using 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 generate docstrings to generate docstrings for this PR. (Beta)
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration 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.

Crabcyborg · 2025-02-17T16:41:36Z

 		}

-		$max = FrmField::get_option( $this->field, 'maxnum' );
+		$max = FrmCurrencyHelper::replace_custom_decimal_char( FrmField::get_option( $this->field, 'maxnum' ), $decimal_separator );


It would be nice if we could do this without requiring changes in Lite.

This is also rather dangerous, since we don't really want to use custom_decimal_separator in Lite even if it does exist, since the setting could no longer be available to someone who is no longer using Pro.

I’ve reverted these changes and will handle them in Pro. However, let’s keep replace_custom_decimal_char in the Lite version with this PR. Does that sound good to you?

If it isn't being used in Lite I don't see why it would make sense in Lite.

I thought it might be useful in the future, but you’re right. 👍🏻

@Crabcyborg, could you close this PR? I’ve made the necessary changes in the Pro version.

fix add_min_max which custom decimal separator

c87464c

shervElmi added run analysis run tests labels Feb 17, 2025

shervElmi requested a review from Crabcyborg February 17, 2025 16:29

shervElmi self-assigned this Feb 17, 2025

Crabcyborg reviewed Feb 17, 2025

View reviewed changes

Revert add_min_max changes

2b15fc1

Crabcyborg closed this Feb 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lite version: Fix number field validation for custom decimal separators#2248

Lite version: Fix number field validation for custom decimal separators#2248
shervElmi wants to merge 2 commits into
masterfrom
fix/number-field-validation-with-costom-decimal-char-pro

shervElmi commented Feb 17, 2025

Uh oh!

coderabbitai Bot commented Feb 17, 2025

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

Crabcyborg Feb 17, 2025

Uh oh!

shervElmi Feb 17, 2025

Uh oh!

Crabcyborg Feb 17, 2025

Uh oh!

shervElmi Feb 17, 2025

Uh oh!

shervElmi Feb 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

shervElmi commented Feb 17, 2025

Main PR:

Uh oh!

coderabbitai Bot commented Feb 17, 2025

Walkthrough

Changes

Sequence Diagram(s)

Possibly related PRs

Suggested reviewers

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

Crabcyborg Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

shervElmi Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

Crabcyborg Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

shervElmi Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

shervElmi Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

CodeRabbit Configuration File (`.coderabbit.yaml`)