(feat): remove ability for non-marketplace owner to switch shops

[kieckhafer]

This update changes the shop selector dropdown to only allow marketplace owners to switch shops. If you are a single shop owner, the selector will no longer be displayed, and you will no longer be allowed to switch active shops.

To test:

• Login as Reaction admin
• Invite new shop owner
• Make new shop active
• See you can switch back and forth between shops as Marketplace owner
• Make sure the last shop you land on is the original Marketplace shop (this will test to make sure the setting isn't saved in Browser storage)

THEN:

• Login as new shop
• See shop selector is not displayed, AND you are able to make all admin actions on your shop (i.e. the active shop is your shop, not the marketplace shop)

[kieckhafer]

@spencern this works as-is in this PR, however, please see comment on the original ticket (#3184 (comment)) for perhaps an update we can make to make it a little better.

[spencern]

@kieckhafer left some comments over there on #3184 - we can pair on this if you like.

What I think we need to make 100% certain of in this PR is that on login a user's activeShop is set correctly, and on logout it's set back to the primary shop correctly.

[spencern]

Works really well, but I'd like to change the method used in getShopsForUser as reduce would be a better fit than find for this use case.

I've also committed a change to use the same getShopsForUser to determine which shops should be shown in the shop selector if the user is an admin for multiple.

[spencern]

While this works, it's not really how find is designed to work and may be confusing. Array.prototype.find should return a value (the first value that returns true from the provided callback function). reduce would be a better choice here as it's designed to loop through an array and return a single value (array, or otherwise) derived from the original input.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce

e.g. something like this

const user = Meteor.user(userId);
const shopIds = Object.keys(user.roles);

// Reduce shopIds to shopsWithPermission
const shopIdsWithRoles = shopIds.reduce((shopsWithPermission, shopId) => {
  // Get list of roles user has for this shop
  const rolesUserHas = user.roles[shopId];

  // Find first role that is included in the passed in roles array, otherwise hasRole is undefined
  const hasRole = rolesUserHas.find((roleUserHas) => roles.includes(roleUserHas));

  // if we found the role, then the user has permission for this shop. Add shopId to shopsWithPermission array
  if (hasRole) {
    shopsWithPermission.push(shopId);
  }
  return shopsWithPermission;
}, []);

return shopIdsWithRoles;

[kieckhafer]

Ok. I know we discussed reduce earlier, but once coding, I saw this find inside the hasDashboardAccessForAnyShop, and figured I'd keep the code as similar as I could to other code we'd written.

[spencern]

This comment could better reflect the new idea that we show it when a user has admin/dashboard permission for multiple shops

[spencern]

update this comment for user with admin/dashboard permissions in multiple shops

[kieckhafer]

@spencern updated comments, and to use reduce.

One last thing I was noticing... Marketplace owners will always have the switcher, even if there is only one shop, because of the __global_roles__ group that's attached to them.

Should we account for this, or is this OK to always show for the main owner?

[spencern]

@kieckhafer good catch, we should probably exclude the __global_roles__ group from this calculation

[kieckhafer]

@spencern updated to remove __global_roles__.

I made the check on line 324 by just checking to see if shopId !== "__global_roles__".

Is it better practice to do the filtering of the shopIds prior to passing them into the reduce method?

getShopsForUser(roles, userId = Meteor.userId()) {
    // Get full user object, and get shopIds of all shops they are attached to
    const user = Meteor.user(userId);
    const shopIds = Object.keys(user.roles);
    // Remove "__global_roles__" from the list of shopIds, as this function will always return true for
    // marketplace admins if that "id" is left in the check
    const filteredShopIds = shopIds.filter(shopId => shopId !== "__global_roles__");

    // Reduce shopIds to shopsWithPermission, using the roles passed in to this function
    const shopIdsWithRoles = filteredShopIds.reduce((shopsWithPermission, shopId) => {
      // Get list of roles user has for this shop
      const rolesUserHas = user.roles[shopId];

      // Find first role that is included in the passed in roles array, otherwise hasRole is undefined
      const hasRole = rolesUserHas.find((roleUserHas) => roles.includes(roleUserHas));

      // if we found the role, then the user has permission for this shop. Add shopId to shopsWithPermission array
      if (hasRole) {
        shopsWithPermission.push(shopId);
      }
      return shopsWithPermission;
    }, []);

    return shopIdsWithRoles;
  },

[spencern]

@kieckhafer yeah, that would be better because it would not loop through all roles unnecessarily for the __global_roles__ group.

[kieckhafer]

@spencern cool, updated.

[spencern]

Nice. 👍