-
-
Notifications
You must be signed in to change notification settings - Fork 168
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
Version 4 #379
Version 4 #379
Conversation
…feature/export-collections
And account for the new index creation method
@StorytellerCZ is there anything that isn't already implemented by the latest release that contains the async counterparts? |
Not 100% sure, but I think we have everything. I will remove the sync MongoDB calls in this branch. Or maybe for the client we can leave the sync methods? Thoughts? |
@StorytellerCZ I think it would make sense to leave sync methods for the client. This is consistent with other packages like Accounts isn't it ? Thanks |
Synchronous Mongo calls on the client side are alright in Meteor 3.0, IIRC. When is this PR expected to be merged in and published? I would love to test this in my application. |
Once this is merged: I will release a beta. |
I would suggest updating the
For obvious reasons, the method needs to become async to be able to count how many records are in the collection. |
@dallman2 PRs welcome. |
30b4352
to
4bb9684
Compare
roles/roles_client.js
Outdated
@@ -69,7 +65,7 @@ Object.assign(Roles, { | |||
unlessExists: false | |||
}, options) | |||
|
|||
const result = Meteor.roles.upsert({ _id: roleName }, { $setOnInsert: { children: [] } }) | |||
const result = RolesCollection.upsert({ _id: roleName }, { $setOnInsert: { children: [] } }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is breaking all our tests:
- If we directly replace
Meteor.Roles
it will not apply this internal collection. We also can no longer check if a role is created usingMeteor.roles.countDocuments
- If we use
hwillston:stub-collections
we can only test for integration and not manully stub specific methods for unit tests. This was possible whenMeteor.roles
was used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Meteor.roles
is still applied and unchanged, but you can now import he RolesCollection as well from the package. See roles_common_async.js
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jankapunkt please give this a look in the latest push, I have reverted to using the Meteor
object for collection access.
@jankapunkt @harryadel can I get a check on this from you, so that we can do the release? Many thanks! |
Anything else for release? @jankapunkt @harryadel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very few nitpicks, feel free to disregard if you don't agree.
Was missing subscription to get the data. Not sure why the sync client tests are able to still run. Maybe the data added persist in minimongo?
console.debug
for debugging messagesRolesCollection
andRoleAssignmentsCollection
can now be exported in addition to being accessed viaMeteor.roles
andMeteor.roleAssignment