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

Allow op to be a non-object #217

Merged
merged 2 commits into from
Jul 23, 2018
Merged

Conversation

gkubisa
Copy link
Contributor

@gkubisa gkubisa commented Jun 25, 2018

On the client side, non-object ops are processed successfully.

When ops are received by a server, they must not be null.

sharedb/lib/ot.js

Lines 30 to 32 in 68bde00

} else if (op.op == null) {
return {code: 4010, message: 'Missing op, create, or del'};
}

However, when ops are applied by the server, they must be object.

if (typeof edit !== 'object') return {code: 5004, message: 'Missing op'};

This PR fixes the inconsistency in validation and allows ops to be non-objects.

@coveralls
Copy link

coveralls commented Jun 25, 2018

Coverage Status

Coverage remained the same at 96.492% when pulling 90b46d6 on Teamwork:fix-op-validation into 68bde00 on share:master.

Copy link
Contributor

@curran curran left a comment

Choose a reason for hiding this comment

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

Looks like a great improvement to me!

@ericyhwang
Copy link
Contributor

@nateps from the PR review meeting says this is good to merge.

Though, I noticed there's a package-lock.json file committed, and Share's not using package lock files yet. @gkubisa mind removing that real quick before we merge?

@gkubisa
Copy link
Contributor Author

gkubisa commented Jul 12, 2018

I've removed package-lock.json for now but I think we should start using it. I don't think it causes any real issues but it keeps the repo more stable for CI and ShareDB devs.

@nateps
Copy link
Contributor

nateps commented Jul 23, 2018

Awesome, thanks for the contribution! Looks great. ⚡️

@nateps nateps merged commit be3f161 into share:master Jul 23, 2018
@gkubisa gkubisa deleted the fix-op-validation branch July 24, 2018 07:32
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.

5 participants