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

Fix #325 - Required validation #326

Open
wants to merge 1 commit into
base: hotfix
Choose a base branch
from

Conversation

yunusyerli1
Copy link
Contributor

Description

Just changed order of below lines: #325
data.field.definition.required = required;
data.field.formControl.setValidators(validators);
data.field.formControl.updateValueAndValidity({onlySelf: true, emitEvent: true});
record.formGroup.updateValueAndValidity({onlySelf: true, emitEvent: true});

Motivation and Context

How To Test This

Choose Service as Missed Collection in Case Module.
Select Yes in Further Action Required field.
Fill Outcome field.
Click Save and Continue button.
Check Outcome field again.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Final checklist

  • My code follows the code style of this project found here.
  • My change requires a change to the documentation.
  • I have read the How to Contribute guidelines.

@clemente-raposo
Copy link
Contributor

Hi @yunusyerli1, I think has already been fixed. Do you confirm?

@yunusyerli1
Copy link
Contributor Author

yunusyerli1 commented Dec 15, 2023

Hi @yunusyerli1, I think has already been fixed. Do you confirm?

Yes, ı do confirm but there are repetitive lines.

@clemente-raposo clemente-raposo added Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member Status: Requires Testing Status: Passed Code Review labels Dec 15, 2023
@johnM2401 johnM2401 added the Status:Requires Updates Issues & PRs which requires input or update from the author label Dec 18, 2023
@johnM2401
Copy link

Hey @yunusyerli1 !

It looks like your "How to Test this" sections contains instructions that refer to an already customized instance, so I'm not 100% sure how to confirm the initial issue or proposed fix.

Is this just validation for Dynamic Dropdowns, or dependant fields?


Could you add some more generic replication/testing steps, that would be more for a vanilla Suite8 environment?
(or a brief description on what is required to test)

Thanks!

@yunusyerli1
Copy link
Contributor Author

Hey @yunusyerli1 !

It looks like your "How to Test this" sections contains instructions that refer to an already customized instance, so I'm not 100% sure how to confirm the initial issue or proposed fix.

Is this just validation for Dynamic Dropdowns, or dependant fields?

Could you add some more generic replication/testing steps, that would be more for a vanilla Suite8 environment? (or a brief description on what is required to test)

Thanks!

This is for dynamic dependants of a dropdown validation. It was customized instance with new module with some different services.
So, honestly, ı cant say how to test this.
Regards.

@johnM2401
Copy link

Hey!
Thanks for getting back

hmmmm, in that case, do you see this having any possible issues worth testing for, @clemente-raposo ?
It seems low impact so would be happy to approve as-is, if there's no concerns.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member Status: Passed Code Review Status: Requires Testing Status:Requires Updates Issues & PRs which requires input or update from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants