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

Fixed Validation error when creating custom fields' default values #11724

Conversation

inietov
Copy link
Collaborator

@inietov inietov commented Aug 24, 2022

Description

When a user creates custom fields, they can mark such fields as required. Then they can create fieldsets to customize the existent models. Then they can set default values for this custom fields, but when creating those default values we apply a validator so later when creating assets using that model, the default values are valid.

But it can be the case that the user doesn't want to put a default value to every custom field, and if the field is marked as required, then any of the default values can be assigned. This PR eliminates the rule 'required' on fields that have it, so it only apply the format requested by the user.

It doesn't affect the integrity of the system as this changes are at the Asset model level, so when a new asset using this model is actually created, the required rules are still validating the data.

I still have to find a way to not lose every default value when the validator fails, because in the case there's a lot of custom fields it can be tiresome to fill all the data everytime. But it's a little step in making this feature a little more usable.

Fixes freshdesk #30229

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Test Configuration:

  • PHP version: 7.4.16
  • MySQL version: 8.0.23
  • Webserver version: nginx/1.19.8
  • OS version: Debian 10

Checklist:

@snipe snipe merged commit 74f5980 into snipe:develop Aug 24, 2022
@snipe
Copy link
Owner

snipe commented Aug 24, 2022

Looks great, thank you @inietov!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants