These are chat archives for reactioncommerce/reaction

28th
May 2015
Bogi
@boboci9
May 28 2015 11:54

Hi @aaronjudd I am still going through the functionality changes and I am stuck again, I found your hasPermission function which uses the @getShopId

 hasPermission: (permissions) ->
    # shop specific check
    if Roles.userIsInRole Meteor.userId(), permissions, @getShopId()
      return true
    # global roles check
    if Roles.userIsInRole Meteor.userId(), permissions, Roles.GLOBAL_GROUP
      return true

But what happens if I am a seller of a shop, I have the createProduct role added for that particular shop and I want to insert a product there? I will get throuwn back because of the permission check in the createProduct function

 unless ReactionCore.hasPermission('createProduct')
      throw new Meteor.Error 403, "Access Denied"

although I have the permission for the shop I am about the insert the product.

Bogi
@boboci9
May 28 2015 12:04
should I find a go around or I should think of a PR for the core to support this use case as well?
Arbab Abdul Waheed
@aawaheed
May 28 2015 14:37
hi @aaronjudd . I did see waffle board but as I said I am new to contributing so not sure how to get along :(
Goutham Veeramachaneni
@gouthamve
May 28 2015 14:59
Hi. Just pick an issue and try to fix it. I was like you sometime back new to meteor and I too decided to contribute to reaction. Just picked an issue which seemed easy and dived into the code. And whenever I faced a problem I asked here and I got my questions answered.
Arbab Abdul Waheed
@aawaheed
May 28 2015 15:08
Thnx @Gouthamve will do so :)
Aaron Judd
@aaronjudd
May 28 2015 15:14
@boboci9 it should be working perfectly for the scenario you describe - the user will be allowed to use createProduct if the permission was created for either the shop or global. currently only the admin user gets global roles - all others are assigned at the shop level. so generally every users permission check is going to be shop + permission. You can have multiple shops (permissions assigned) to allow access to add to multiple shops. (but views need revamping for multiple shop management, just talking permissions here)
Bogi
@boboci9
May 28 2015 16:16
but in my case the @getShopId is the main market place's shopId while the current user want's to create an order in the shop where he is a seller
so the check in my case should be if Roles.userIsInRole Meteor.userId(), permissions, sellerShopId
in this case there is only one maine shop, the domain mathes that, and the other shops are just vendors like Amazon
Aaron Judd
@aaronjudd
May 28 2015 16:19
yes, so they'd have permissions in two shops (or no permissions in the main marketplace)
Bogi
@boboci9
May 28 2015 16:19
they should have permissions only for their own shops no?
Aaron Judd
@aaronjudd
May 28 2015 16:20
correct - no need to assign any others
Bogi
@boboci9
May 28 2015 16:20
but in this case the @getShopId() is not the seller's shopId
Aaron Judd
@aaronjudd
May 28 2015 16:21
so we can add a param to pass shopId or get from current
but I think the major thing is "how does a user select a shop"
Bogi
@boboci9
May 28 2015 16:23
or we can check it with Roles.getGroupsForUser which will return the shopId if the user has the permission for any shop
Aaron Judd
@aaronjudd
May 28 2015 16:26
well I guess what I am getting at, is that I think the permissions are fine, but this hits the next issue -> which is exactly how do you know what shop the user is using currently - if "domain" isn't the unique identifier
yes, you are correct with getGroupsForUser (see shopMembers, I'm doing that there to indicate each site)
Bogi
@boboci9
May 28 2015 16:28
yes I created a getSellerShopId based on that, you commented out the old implementation in the reaction-core app.coffee it was there in the init
maybe we should think about that, that could return the id of the seller's shop if he owns any
Aaron Judd
@aaronjudd
May 28 2015 16:33
ah yes -> let me take a look at the logic you had (unless you have and idea of what you think should be done there)
Bogi
@boboci9
May 28 2015 16:34
yes I have actually :) I already wrote a function for myself but this requires that every seller can have only one shop, if he want's to create a new shop he needs a new user account, so you agree?
it's not in coffee but hopefully it helps
Aaron Judd
@aaronjudd
May 28 2015 16:34
I think users should be able to have multiple shops per user (I tested this in build the shopMembers)
Bogi
@boboci9
May 28 2015 16:35
ok than my function is not working for you, our specs say we should have only one shop per seller
Aaron Judd
@aaronjudd
May 28 2015 16:37
we could go that way from a UX standpoint, but I would think we'd just want to return whatever shops they had in getGroupsforUsers
Bogi
@boboci9
May 28 2015 16:37
then how would you know in which shop is the user activating?
Aaron Judd
@aaronjudd
May 28 2015 16:40
just brainstorming here: 1. domain=shop domain 2. UI / Dropdown to select shops 3. check your group against the stored shopId on the collection, if you've got the shopId, apply permissions
Bogi
@boboci9
May 28 2015 16:43
ok, and you would store the selected shopId in a session variable?
Aaron Judd
@aaronjudd
May 28 2015 16:44
if we just said: this product exists in shopId, getGroupsforUser has this shopId that could work for multiple shops, but at some point if you have multiple shops you're likely to want to filter shops - so yeah, we could store that as a session variable
Bogi
@boboci9
May 28 2015 16:45
yes mostly for create product, because then you don't have other shopId's to match yet
Aaron Judd
@aaronjudd
May 28 2015 16:45
reworking the getCurrentShop functionality to support that multiple shops in this way wouldn't be tough (this is why I reworked the permissions, so we can relay on them to control views, rather than the shopId)
yes - mostly we just need to know what shop you are working in when creating things, right?
Bogi
@boboci9
May 28 2015 16:47
that could work for me (with one shop/seller) as well, I just disregard the UI with the shop dropdown and when a seller logs in I just set the session variable that you are using
Aaron Judd
@aaronjudd
May 28 2015 16:49
and btw - this thinking needs to extend to i18n - we're going to need to be able to make clones of products with different i18n labeling, etc - you may just want to create two shops "EU vs US" for instance - this is the case I had in mind where you might want to admin multiple stores
Bogi
@boboci9
May 28 2015 16:51
I see, I will mention this use case to my project manager, I didn't think of this earlier
but then you would want to clone a product from one shop to another?
Aaron Judd
@aaronjudd
May 28 2015 16:52
yeah I could see that happening (although I don't want to think too much about that case yet -> that falls under the product inheritance / management task)
Bogi
@boboci9
May 28 2015 16:54
I see, ok I was just thinking it further, but we don't need that now for the permissions part
Aaron Judd
@aaronjudd
May 28 2015 16:54
the idea there is that if we maintain the product hierarchy you should be able to cascade updates to cloned products from a parent, no reason that couldn't be cross-shop as well (but there's some tricky logic that will need to be ironed out there)
Bogi
@boboci9
May 28 2015 16:58
so you will check for owner permision in the getCurrentShop? so that I know how to set up a seller
Aaron Judd
@aaronjudd
May 28 2015 16:59
in any case, I'd like to merge the 372 branches soon, (release as 0.6.0?) and then I can begin making the marketplace updates. I'm not going full blown on the functionality yet - I probably won't build much UI (we've got some UI help joining the team starting next week, so the UI should start moving forward quickly) - however I am committing to making all the core method/publications needed for the next release
Bogi
@boboci9
May 28 2015 17:02
ok I see, I just wanted to see how you will handle these things because I have to rewrite my packages as well to work together with the 372 branch
Aaron Judd
@aaronjudd
May 28 2015 17:03
re: a seller doesn't need to be admin or owner - they can have any set of permissions, so I guess you'll need to decide which default permissions you want your sellers to have.
Bogi
@boboci9
May 28 2015 17:05
I have a seller role set up for my sellers just in case but if wouldn't be a bad idea to consider an owner a seller, or you need that role for other purposes?
Aaron Judd
@aaronjudd
May 28 2015 17:10
you could do that- as it would be site specific (vs global) but owner/admin do have some default permissions that maybe they won't even need. I'd suggest that 'admin' would be the better pick of the two. Owner has all the permissions regardless, while 'admin' is the "administrator".
Bogi
@boboci9
May 28 2015 17:11
I see I have to rethink this than because I thought admin is the bigger fish, as in every permission check I mainly see admin not owner
Aaron Judd
@aaronjudd
May 28 2015 17:13
honestly I think owner is only appropriate on the global role, and I'd go with 'admin' - that's the one I decided made the most sense as a "generic" level that is higher than individual permissions
Bogi
@boboci9
May 28 2015 17:15
yes in my setup I kept both owner and admin only in the global role shpere and that's why I set the seller to be the role for the shop sellers so it won't interfere with any global setup, even if it's on shop level
I'm still thinking on what you said rewriting the getCurrentShop and not the getSellerShopId I think the getCurrentShop should stay by the domain still because that is the main shop no?
Aaron Judd
@aaronjudd
May 28 2015 17:17
well the global setups are as if they were their own shop - they get a __GLOBAL_ROLES__ entry in the roles collection, but are filtered out of the resulting publication so that you only see the shop entries
so it's totally ok to had 'admin' at the shop, and 'admin' as global
Bogi
@boboci9
May 28 2015 17:20
ok, I will recheck think in our setup as well
Bogi
@boboci9
May 28 2015 17:26
sorry for mixing up the ideas I just have all these questions in my had an I was so glad to catch you to see what you think about them so I 'm not sure if you read my last question
I'm still thinking on what you said rewriting the getCurrentShop and not the getSellerShopId I think the getCurrentShop should stay by the domain still because that is the main shop no?
Aaron Judd
@aaronjudd
May 28 2015 17:27
re: getCurrentShop - yes probably - but that logic is currently just domain: currentDomain - so it might not be that method, but somewhere in there it should be domain or selected shop
(or updating all to take a param)
Bogi
@boboci9
May 28 2015 17:29
yes I think the getCurrentShop should be the main shop only, while some other method should be returning the other shops the current user is involved in
Aaron Judd
@aaronjudd
May 28 2015 17:29
are you wanting for each shop to have it's own domain? (i've been wondering if that should just be for single tenant or not)
Bogi
@boboci9
May 28 2015 17:30
no we only have one domain, the sellers won't get a domain in our setup, it will be a marketplace like amazon
Aaron Judd
@aaronjudd
May 28 2015 17:33
I've been "sort of" using https://www.google.com/shopping/express/ as the guideline for a "marketplace"
Bogi
@boboci9
May 28 2015 17:37
I will check that out as well
that's their "select a store"
along with i18n, we'd only show the dropdowns if there were multiple languages, multiple shops (otherwise we'll just default, no need to show)
Bogi
@boboci9
May 28 2015 17:44
yes that would be a solution
BrianSkipworth
@BrianSkipworth
May 28 2015 23:32
hi @aaronjudd - I found this project through meteor home page, and wanted to test drive and see if my team can contribute. problem is I get 502 error trying to hit my shop at https://v24gavzc.reactioncommerce.com/
Aaron Judd
@aaronjudd
May 28 2015 23:37
@BrianSkipworth thanks, I'm taking a look at it now
Aaron Judd
@aaronjudd
May 28 2015 23:50
@BrianSkipworth you should be good to go now.