[Marketplace] Feature - Permissions should be settable by placing a user into a predefined group #2448
Conversation
* Modifies Shops publication to return all shops with matching domain by removing limit * Rewrite of shop selector dropdown * Use Shops publication instead of SellerShops publication in dropdown * Remove requirement for marketplace ownership to see shop selector * Change Reaction.ShopId when shop selector is changed * TODO: Convert shop selector to React
* Eliminates console warnings
impactmass
changed the title
impactmass
changed the title
lib/collections/schemas/accounts.js
Outdated
| @@ -107,6 +107,11 @@ export const Accounts = new SimpleSchema({ | |||
| type: Profile, | |||
| optional: true | |||
| }, | |||
| groups: { | |||
| type: [String], | |||
There was a problem hiding this comment.
This is an array of group ids, correct? Would be nice to have comments here explaining what goes here and what the default means.
lib/collections/schemas/accounts.js
Outdated
| groups: { | ||
| type: [String], | ||
| optional: true, | ||
| defaultValue: [] |
There was a problem hiding this comment.
Does this default to a user having no permissions?
There was a problem hiding this comment.
No. It's just means at the schema level, they don't belong to a group yet. Although, on account creation, the array then receives either "guest" or "customer".
| spyOnMethod("createGroup", shop._id); | ||
|
|
||
| Meteor.call("group/createGroup", sampleGroup, shop._id); | ||
| const group = Groups.findOne({ shopId: shop._id }); |
There was a problem hiding this comment.
How would we find this group if there were more than one for this particular shop? By _id?
There was a problem hiding this comment.
Finding groups belonging to a shop can be done with the slug property. More can be seen in addDefaultRoles.app-test.js. This is just doing a findOne because just one group was added in the beforeEach.
| const newGroupData = Object.assign({}, sampleGroup, { permissions: ["new-permissions"] }); | ||
| Meteor.call("group/updateGroup", group._id, newGroupData, shop._id); | ||
| updatedUser = Meteor.users.findOne({ _id: user._id }); | ||
| expect(updatedUser.roles[shop._id]).to.include.members(newGroupData.permissions); |
There was a problem hiding this comment.
Might be worth adding a test to make sure the permissions that the old group had, but the new group does not have are no longer included.
server/methods/core/groups.js
Outdated
| throw new Meteor.Error(400, "Bad request"); | ||
| } | ||
|
|
||
| return { status: 200 }; |
There was a problem hiding this comment.
We should probably return the _id of the created group here as well?
| * Reaction Permission Group Methods | ||
| */ | ||
| Meteor.methods({ | ||
| /** |
There was a problem hiding this comment.
Great job with jsdoc here.
| @@ -19,6 +19,7 @@ Template.registerHelper("reactionTemplate", function (options) { | |||
| const shopId = options.hash.shopId || Reaction.getShopId(); | |||
| // get shop info, defaults to current | |||
| const Shop = Collections.Shops.findOne(shopId); | |||
| const defaultRoles = Collections.Groups.findOne({ slug: "customer", shopId }).permissions; | |||
There was a problem hiding this comment.
Do we need to ensure that defaultRoles are loaded at this point? (e.g. subscription.ready())? Maybe this is done already in another place
There was a problem hiding this comment.
This helper is only used in the checkout right now it seems, and even if we load the checkout directly default roles are ready, so I think we're good to go here.
Permission groups. Implements #2184
This PR has the Meteor methods to manage permission groups. It specifically adds
group/createGroup,group/updateGroup,group/addUser,group/removeUser.The UI for this is being handled separately (as that involved changing a lot of the current Accounts section of admin dashboard).
To test:
_ Take a look at the tests, to have a sense of how the methods work
_ The methods can be tested from the browser console of a running Reaction (via
Meteor.calland passing the needed values). See the jsdocs added for the descriptions of the arguments.group/createGroupfrom console, passing in the required options.group/addUseras describedOther notes:
_ Groups exits as a schema
_ Group Ids are referenced on Accounts to track which users belong to a group