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

parametersResolution: Handle ENUM properties #818

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

Conversation

thim81
Copy link
Contributor

@thim81 thim81 commented Jul 26, 2024

Linked to #817

@thim81
Copy link
Contributor Author

thim81 commented Jul 27, 2024

@VShingala The PR is ready for your review.

The issue originated in the resolveSchema function, which is solved in the PR.
There are a number of tests added.

There is an unexpected side-effect in the Validation tests, which I can't figure out what would be the expected result, perhaps you can assist?

ValidateV2.test.js (Line 577)

 expect(_.includes(['Bulldog', 'Retriever', 'Timberwolf', 'Grizzly', 'Husky'],
          propertyMismatchMap.BODY.suggestedFix.suggestedValue.breeds[2])).to.eql(true);

In the past this returned a random ENUM value with parametersResolution: schema, while now it will return <string>.

I find it strange the "ValidateV2" fails, since I've build it to only execute the conversion when the operation is "CONVERSION".

@thim81 thim81 changed the title parametersResolution: Added tests parametersResolution: Handle ENUM properties Jul 27, 2024
@thim81
Copy link
Contributor Author

thim81 commented Sep 11, 2024

@VShingala Picking this PR up again.

Can you have a look at my previous question ☝️ , and what would be the expected behaviour?

  1. Return schema type ()?
  2. Return one of the ENUM values?

@VShingala
Copy link
Member

@thim81 Although we've only updated conversion logic, validation logic/tests works hands in hands with it. i.e. tests failing are due to converted collection not getting validated after this change.

In validation workflow, we suggest changes based on schema. Till now these suggestions were of actual enums but from now they'll be "<string>" because the utility to suggest changes based on schema is used from functions present in schemaUtilsV2.

@thim81
Copy link
Contributor Author

thim81 commented Sep 16, 2024

hi @VShingala

So if I understand properly, I can update the validation test to check for the <string>?

For me it is not really clear, how to fix the validation tests, without impacting the expected behaviour.

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.

2 participants