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

Add new validation rule: VALUE_OBJECT_CANNOT_CONTAIN_UNDEFINED #73

Closed
warpech opened this issue Jun 18, 2015 · 8 comments
Closed

Add new validation rule: VALUE_OBJECT_CANNOT_CONTAIN_UNDEFINED #73

warpech opened this issue Jun 18, 2015 · 8 comments
Assignees

Comments

@warpech
Copy link
Collaborator

warpech commented Jun 18, 2015

(Continued from Palindrom/Palindrom#21)

PuppetJs in debug mode (currently ON by default) validates outgoing patches:

   if(this.debug) {
      this.validateSequence(this.remoteObj, patches);
    }

in which case the error should be reported. I am glad that we both agree :)

@miyconst: Current validation is implemented here: https://github.com/Starcounter-Jack/JSON-Patch/blob/master/src/json-patch-duplex.js#L632

And it is throwing OPERATION_VALUE_REQUIRED for undefined values, which should be replaced with VALUE_OBJECT_CANNOT_CONTAIN_UNDEFINED.

It should not be mixed. The current check for undefined value (OPERATION_VALUE_REQUIRED) should be kept. A new check for undefined property of the value (if the value is an object) should be added after it (VALUE_OBJECT_CANNOT_CONTAIN_UNDEFINED).

@miyconst could you prepare a solution for this on a separate branch in https://github.com/Starcounter-Jack/JSON-Patch? Publish code on a separate branch for review. The changes should include test and update to README.md

@miyconst
Copy link
Collaborator

Here is the new branch:
https://github.com/Starcounter-Jack/JSON-Patch/tree/issues/73

The exception name is OPERATION_VALUE_CANNOT_CONTAIN_UNDEFINED, I added OPERATION_ to match all other exceptions and removed _OBJECT to shorten it somehow.

Three new validation specs added:

should return an error if an "add" operation "value" contains "undefined"
should return an error if a "replace" operation "value" contains "undefined"
should return an error if a "test" operation "value" contains "undefined"

with the following operation values:

{ foo: undefined }
{ foos: [undefined] }
{ foo: { bars: [undefined] }} }

@warpech
Copy link
Collaborator Author

warpech commented Jun 22, 2015

@tomalec can you review?

@tomalec
Copy link
Collaborator

tomalec commented Jun 22, 2015

Check itself LGTM, but we need that change to be made in .ts files as well.

miyconst pushed a commit that referenced this issue Jun 22, 2015
@miyconst
Copy link
Collaborator

Updated .ts files. How do you test TypeScript? There is nothing about it in the project.

@tomalec
Copy link
Collaborator

tomalec commented Jun 22, 2015

U should use tsc in command-line to generate js out of ts, then test js ;)

@miyconst
Copy link
Collaborator

@tomalec thanks, tested.

@tomalec
Copy link
Collaborator

tomalec commented Jun 22, 2015

cool, can you merge it to the master, and close the issue?

@miyconst
Copy link
Collaborator

Done. Released 0.5.3 version.

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

No branches or pull requests

3 participants