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

MLIBZ-2213 Subscribe to user's personal collection channel #172

Merged
merged 3 commits into from Jan 10, 2018

Conversation

@georgiwe
Copy link
Contributor

commented Nov 30, 2017

This fixes MLIBZ-2213.

Description

Fixes the lack of updates for entities, for which the user has inclusive permissions to view.

Changes

Added a subscription to the user's personal channel, so they receive collection updates, according to ACL settings for them.

@georgiwe georgiwe self-assigned this Nov 30, 2017

@georgiwe georgiwe requested a review from thomasconner Nov 30, 2017

@thomasconner
Copy link
Contributor

left a comment

Should we add some unit tests?

@georgiwe

This comment has been minimized.

Copy link
Contributor Author

commented Dec 1, 2017

I amended the expects of existing tests. But all of the live service tests are being skipped - they have not been "migrated to the mono repo". They use es7 imports, paths have not been changed, etc.

@georgiwe georgiwe force-pushed the live-acl branch from 56a15a4 to e8d5e3e Dec 1, 2017

Merge branch 'MLIBZ-1999_Mono_Repo' into live-acl
* MLIBZ-1999_Mono_Repo:
  Moved source code to src folder so that babel will transpile the source code when the packages are bundled
  Export MobileIdentityConnect module
  Export a few modulse for kinvey-js-sdk

# Conflicts:
#	package.json
#	packages/kinvey-live/package.json
#	src/core/live/collection/live-collection-manager.js
#	yarn.lock
@tejasranade

This comment has been minimized.

Copy link
Member

commented Dec 11, 2017

@thomasconner @georgiwe it looks like a couple of updates have been pushed up to this PR since the last comment. Does it address the concerns with tests? Is more required on this PR before merge?

@thomasconner

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2017

Some tests are working for Live Service but not all of them. I would need to spend more time with them but given that @georgiwe created them before he might be able to get them working quicker then I would. This isn't really an issue with the PR so it doesn't appear that there is more required for this PR unless we are able to get unit tests to work for just the changes made in this PR and work on getting the rest of the live service unit tests running in a separate PR.

@georgiwe

This comment has been minimized.

Copy link
Contributor Author

commented Dec 12, 2017

While running the tests for live service is not part of this PR, they are part of the mono repo story.

And since I'm not a fan of doing things twice, I'd like to do them once we've settled on infrastructure for running the tests. I've logged MLIBZ-2245 for "test infrastructure" and I'd rather we did this after that, which I think (at least in part) should also be part of the move to mono repo.

@thomasconner thomasconner changed the base branch from MLIBZ-1999_Mono_Repo to master Dec 14, 2017

@thomasconner thomasconner merged commit d90796e into master Jan 10, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@thomasconner thomasconner deleted the live-acl branch Jan 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.