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

handle import validation edge cases #1993

Merged
merged 2 commits into from
Sep 30, 2024
Merged

Conversation

bcb37
Copy link
Collaborator

@bcb37 bcb37 commented Sep 26, 2024

Fixes validation for imports of segments, feature flags and experiments for the following valid json file contents:

  • [empty file]
  • "abc"
  • ""

@zackcl
Copy link
Collaborator

zackcl commented Sep 27, 2024

I tested importing the JSON files you mentioned and they all seem to work fine and display the "Invalid JSON format" (or "Incompatible") message on the import modal, except when importing a JSON file with contents "abc" or "" on the "Import Segments" modal. When I import these files, I get a long weird error message instead of "Invalid JSON format".

Screenshot 2024-09-27 at 7 45 14 PM

@bcb37
Copy link
Collaborator Author

bcb37 commented Sep 27, 2024

I tested importing the JSON files you mentioned and they all seem to work fine and display the "Invalid JSON format" (or "Incompatible") message on the import modal, except when importing a JSON file with contents "abc" or "" on the "Import Segments" modal. When I import these files, I get a long weird error message instead of "Invalid JSON format".

Screenshot 2024-09-27 at 7 45 14 PM

This is expected, the way segments validation is set up. Those two examples are technically valid JSON, so the segment validator fills in the missing attributes as undefined values. The long message is reporting each of the missing attributes (which is all of them, in this case).

@bcb37 bcb37 merged commit 71aac44 into dev Sep 30, 2024
14 checks passed
@bcb37 bcb37 deleted the bugfix/import-validation-cases branch September 30, 2024 14:03
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.

3 participants