Skip to content
This repository has been archived by the owner on Sep 7, 2020. It is now read-only.

Fix #489: Json Validator #513

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

GaelGRIFFON
Copy link

Fix #489: Json Validator

@CLAassistant
Copy link

CLAassistant commented Apr 12, 2018

CLA assistant check
All committers have signed the CLA.

@@ -59,6 +59,14 @@ export class Validator {
)
)
},
Json: (str) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting an object in str so JSON.parse will always failed

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Json: (str) => {
      if(typeof str === 'string') {
          try {
              JSON.parse(str);
          } catch (e) {
              return false;
          }
          return true;
      }
      return true;
    },

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @adrienbarreau

I don't understand why you get an object in str, you should get a string on Json format.

If you have an object, the test should fail...
Did I miss something?

Copy link

@mcmar mcmar May 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GaelGRIFFON Your code is assuming that the input to the Json function will be a string, but actually, it's an object. That's because the exporter already exports it as a javascript object.
The exporter serializes everything as a json file and Json fields "could" be serialized as a string in Json format, but they're not. They're serialized as a plain old javascript object. Or null. Or undefined. Or 0. Or literally any serializable js type. We already know that it's valid json because the original 000001.json file was successfully parsed. So at the time the import validator is running, this could be any type and it would be "valid json". Give that, here's my proposed solution:

Json: () => true,

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK got it!
Thx you

@mcmar
Copy link

mcmar commented May 21, 2018

@marktani @timsuchanek Can one of you please merge this? It's breaking a critical production feature for lots of people.

Because already parsed
@timsuchanek timsuchanek removed their assignment Jun 11, 2018
@benseitz
Copy link

benseitz commented Oct 24, 2018

@dpetrick Since this is a critical bug, I agree this should be merged even if the graphcool-framework isn't a priority project for prisma.

@vamshi9666
Copy link

any workarounds ?

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

Successfully merging this pull request may close these issues.

Field type Json has no validator defined
7 participants