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

MLIBZ-1549: _id required, but not _kmd and _acl #234

Merged
merged 3 commits into from Sep 8, 2017

Conversation

Projects
None yet
3 participants
@heyzooi
Contributor

heyzooi commented Aug 29, 2017

Description

Make _id required, but not _kmd and _acl

Changes

  • Entity now validates if _id is present

Tests

  • Unit test to check if is ok if _kmd or _acl is missing
  • Unit test to check if _id is missing in a find(byId:) throws an error
  • Unit test to check if _id is missing in a find() returns an empty array

@heyzooi heyzooi self-assigned this Aug 29, 2017

@heyzooi heyzooi requested a review from tejasranade Aug 29, 2017

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Aug 30, 2017

Codecov Report

Merging #234 into develop will increase coverage by 0.29%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #234      +/-   ##
===========================================
+ Coverage    90.76%   91.06%   +0.29%     
===========================================
  Files           65       65              
  Lines         7692     7855     +163     
===========================================
+ Hits          6982     7153     +171     
+ Misses         710      702       -8
Flag Coverage Δ
#Mac 88.39% <100%> (+0.36%) ⬆️
#iOS 89.94% <100%> (+0.31%) ⬆️
Impacted Files Coverage Δ
Kinvey/Kinvey/GetOperation.swift 100% <100%> (ø) ⬆️
Kinvey/Kinvey/FindOperation.swift 98.06% <100%> (+0.2%) ⬆️
Kinvey/Kinvey/Kinvey.swift 100% <100%> (+0.9%) ⬆️
Kinvey/Kinvey/Error.swift 91.66% <0%> (-1.34%) ⬇️
Kinvey/Kinvey/PushOperation.swift 89.71% <0%> (+0.94%) ⬆️
Kinvey/Kinvey/Client.swift 92.61% <0%> (+0.98%) ⬆️
Kinvey/Kinvey/Persistable.swift 83.27% <0%> (+1.31%) ⬆️
Kinvey/Kinvey/Realtime.swift 74.62% <0%> (+1.49%) ⬆️
Kinvey/Kinvey/RealmCache.swift 89.58% <0%> (+2.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43955c9...e23e053. Read the comment docs.

codecov-io commented Aug 30, 2017

Codecov Report

Merging #234 into develop will increase coverage by 0.29%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #234      +/-   ##
===========================================
+ Coverage    90.76%   91.06%   +0.29%     
===========================================
  Files           65       65              
  Lines         7692     7855     +163     
===========================================
+ Hits          6982     7153     +171     
+ Misses         710      702       -8
Flag Coverage Δ
#Mac 88.39% <100%> (+0.36%) ⬆️
#iOS 89.94% <100%> (+0.31%) ⬆️
Impacted Files Coverage Δ
Kinvey/Kinvey/GetOperation.swift 100% <100%> (ø) ⬆️
Kinvey/Kinvey/FindOperation.swift 98.06% <100%> (+0.2%) ⬆️
Kinvey/Kinvey/Kinvey.swift 100% <100%> (+0.9%) ⬆️
Kinvey/Kinvey/Error.swift 91.66% <0%> (-1.34%) ⬇️
Kinvey/Kinvey/PushOperation.swift 89.71% <0%> (+0.94%) ⬆️
Kinvey/Kinvey/Client.swift 92.61% <0%> (+0.98%) ⬆️
Kinvey/Kinvey/Persistable.swift 83.27% <0%> (+1.31%) ⬆️
Kinvey/Kinvey/Realtime.swift 74.62% <0%> (+1.49%) ⬆️
Kinvey/Kinvey/RealmCache.swift 89.58% <0%> (+2.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43955c9...e23e053. Read the comment docs.

@tejasranade

@heyzooi Changes look good. Aren't there more places to check for _kmd? For example, what about delta set caching? What happens if my entities don't have _kmd?

@heyzooi

This comment has been minimized.

Show comment
Hide comment
@heyzooi

heyzooi Aug 30, 2017

Contributor

@tejasranade interesting question... I will add a unit test for that as well

Contributor

heyzooi commented Aug 30, 2017

@tejasranade interesting question... I will add a unit test for that as well

@heyzooi

This comment has been minimized.

Show comment
Hide comment
@heyzooi

heyzooi Sep 8, 2017

Contributor

@tejasranade delta set use case is also handled now

Contributor

heyzooi commented Sep 8, 2017

@tejasranade delta set use case is also handled now

@tejasranade

LGTM

@tejasranade tejasranade merged commit bd2cbe3 into develop Sep 8, 2017

3 checks passed

codebeat 8 issues resolved and 4 introduced
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@tejasranade tejasranade deleted the feature/MLIBZ-1549-id_required_but_kmd_acl_not branch Sep 8, 2017

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