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

[Bug]: 'Required' Field Validation in JSON Form Not Working Correctly If Fields Are Hidden #28018

Closed
1 task done
ame-appsmith opened this issue Oct 12, 2023 · 2 comments · Fixed by #37128
Closed
1 task done
Assignees
Labels
Bug Something isn't working Community Reported issues reported by community members High This issue blocks a user from building or impacts a lot of users JSON Form Issue / features related to the JSON form wiget Needs Triaging Needs attention from maintainers to triage Production QA Pod Issues under the QA Pod QA Needs QA attention Widget Validation Issues related to widget property validation Widgets & Accelerators Pod Issues related to widgets & Accelerators Widgets Product This label groups issues related to widgets

Comments

@ame-appsmith
Copy link

ame-appsmith commented Oct 12, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Description

If we have 2 fields and want to alternatively make them required and visible, based on the selection from a dropdown, the Required validation does not work as expected and the Submit button stays disabled.
It seems that once a field was marked as Required and it gets programmatically hidden, the Required mark does not get removed programmatically as it should (it ignores the JS condition).
See the screen recording and sample app for more details.
Front thread

Steps To Reproduce

  1. Use a JSON Form having one select "Type" (options Red and Blue) and two Input widgets labeled "Red" and "Blue".
  2. If the value of "Type" is "Red", then the "Red" field should be visible and required; also the "Blue" field should be non-required and hidden.
  3. If the value of "Type" is "Blue", then the "Blue" field should be visible and required; also the "Red" field should be non-required and hidden.
  4. For the Red Field in the required js put {{formData.Type == "Red"}} and visibility js put {{formData.Type == "Red"}}
  5. For the Blue Field in the required js put {{formData.Type == "Blue"}} and visibility js put {{formData.Type == "Blue"}}
  6. Now if the Type is selected as "Red", only the Red field should be visible and required, but this is not happening if we switch between red and blue in the type, it also considers both fields required validation.

Public Sample App

https://app.appsmith.com/app/jsonform-validation-required/page1-65280f6880b32971dff37e3c

Environment

Production

Issue video log

https://www.loom.com/share/50c7b4b6562544dfa598216aa82094fc

Version

Cloud v1.9.40

@ame-appsmith ame-appsmith added Bug Something isn't working Community Reported issues reported by community members Needs Triaging Needs attention from maintainers to triage Widget Validation Issues related to widget property validation JSON Form Issue / features related to the JSON form wiget labels Oct 12, 2023
@github-actions github-actions bot added the Widgets Product This label groups issues related to widgets label Oct 12, 2023
@Nikhil-Nandagopal Nikhil-Nandagopal added the High This issue blocks a user from building or impacts a lot of users label Jan 23, 2024
@Nikhil-Nandagopal
Copy link
Contributor

Seems to be working!

@ame-appsmith
Copy link
Author

It still does not work after resetting the form (see sample app and reproduction steps in the screen recording).

@ame-appsmith ame-appsmith reopened this Apr 19, 2024
@Nikhil-Nandagopal Nikhil-Nandagopal added the Widgets & Accelerators Pod Issues related to widgets & Accelerators label Aug 6, 2024
rahulbarwal added a commit that referenced this issue Oct 30, 2024
This commit fixes an edge case where form validation was not triggered when the child component was updated. The issue was reported in the GitHub issue #28018. This commit adds a trigger to ensure that form validation is performed correctly.

Refactor the `Form` component in `app/client/src/widgets/JSONFormWidget/component/Form.tsx` to include the `trigger` function from the `methods` object. This ensures that form validation is triggered when the child component is updated.

Closes #28018
@dvj1988 dvj1988 closed this as completed in b23ba1d Nov 6, 2024
@appsmith-bot appsmith-bot added the QA Needs QA attention label Nov 6, 2024
@github-actions github-actions bot added the QA Pod Issues under the QA Pod label Nov 6, 2024
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this issue Nov 20, 2024
…org#37128)

## Description
<ins>Problem</ins>

Form validation was not triggered when the child component was updated,
resulting in inconsistent data consistency.

<ins>Root cause</ins>

The `Form` component in
`app/client/src/widgets/JSONFormWidget/component/Form.tsx` did not
include the `trigger` function from the `methods` object, preventing
form validation from being triggered on child component updates.

<ins>Solution</ins>

This PR adds the `trigger` function from the `methods` object to the
`Form` component, ensuring form validation is triggered correctly when
the child component is updated.

* Adds unit tests for `Form` component as well


Fixes appsmithorg#28018
_or_  
Fixes `Issue URL`
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.JSONForm"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/11697880527>
> Commit: 1c38b05
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11697880527&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.JSONForm`
> Spec:
> <hr>Wed, 06 Nov 2024 06:06:41 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Enhanced form validation lifecycle management with the introduction of
the `useUnmountFieldValidation` hook for better handling of field
validation upon unmounting.
- Improved testability of the form component through the inclusion of
`data-testid` attributes for the submit and reset buttons.

- **Bug Fixes**
- Resolved edge cases in form validation, ensuring components function
correctly with changing props and handle empty schemas gracefully.

- **Tests**
- Introduced a comprehensive suite of unit tests for the `Form`
component, covering various scenarios including validation and
visibility management.
- Added tests for the new `useUnmountFieldValidation` hook to ensure
correct validation behavior during unmounting.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Community Reported issues reported by community members High This issue blocks a user from building or impacts a lot of users JSON Form Issue / features related to the JSON form wiget Needs Triaging Needs attention from maintainers to triage Production QA Pod Issues under the QA Pod QA Needs QA attention Widget Validation Issues related to widget property validation Widgets & Accelerators Pod Issues related to widgets & Accelerators Widgets Product This label groups issues related to widgets
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants