Skip to content

Handle ACL on save #123

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

Merged
merged 1 commit into from
Dec 17, 2015
Merged

Handle ACL on save #123

merged 1 commit into from
Dec 17, 2015

Conversation

wangmengyan95
Copy link
Contributor

It seems we do not handle ACL on save. Normally for a save/update, the server just return createAt, objectId and updateAt, so it is fine. However, if the developers enable beforeSave for this class, the server will return the full object. In this situation, if we do not handle ACL, it will make object.getACL() return null.
I follow the way we handle ACL in _finishFetch to handle ACL. Not sure it is the best way to do that.

@wangmengyan95
Copy link
Contributor Author

@hallucinogen not sure andrew has time to review, so could you also take a look?
cc @peterdotjs .

@codecov-io
Copy link

Current coverage is 78.54%

Merging #123 into master will increase coverage by +0.16% as of 928fc44

@@            master    #123   diff @@
======================================
  Files           37      37       
  Stmts         2822    2824     +2
  Branches       677     678     +1
  Methods          0       0       
======================================
+ Hit           2212    2218     +6
+ Partial        194     193     -1
+ Missed         416     413     -3

Review entire Coverage Diff as of 928fc44

Powered by Codecov. Updated on successful CI builds.

@andrewimm
Copy link
Contributor

lgtm, ship it

wangmengyan95 added a commit that referenced this pull request Dec 17, 2015
@wangmengyan95 wangmengyan95 merged commit fdbcdc6 into master Dec 17, 2015
@wangmengyan95 wangmengyan95 deleted the wangmengyan.handle_ACL_on_save branch December 17, 2015 00:56
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.

4 participants