refactor: remove removeAccountFromGroup meteor method and rewrite with GraphQL

[kieckhafer]

Resolves #5339
Impact: minor
Type: feature|refactor

Issue

removeAccountFromGroup still uses a meteor method to perform its actions.

Solution

Rewrite removeAccountFromGroup mutation to use GraphQL instead of meteor method.

• Remove removeAccountFromGroup meteor method from app

Breaking changes

Nothing in core code. Any custom code calling the group/removeUser Meteor method will break.

Testing

1. In Reaction Admin, invite a new user to a group
2. Remove that user from group (no need to set up email / confirm the user, you can remove them without them setting up their account)

[kieckhafer]

@aldeed updated this PR, removed [WIP].

[kieckhafer]

@aldeed I didn't add an internalCall check here, based on the comment that only admins should be able to make this change (the comment persists from the previous meteor code)

[aldeed]

If no group is found, then shopId would be undefined which I think would cause either an error or unexpected results from the userHasPermission call below. It would be better to do an explicit check before the permission check.

if (!group) throw new ReactionError("not-found", "Group not found");

[aldeed]

Remove the || {} and throw error if (!defaultCustomerGroupForShop). Otherwise it will just throw less clear errors when accessing defaultCustomerGroupForShop.permissions below.

[aldeed]

It's a bit strange that we decided to return the group at all, and the docs are not clear about which group is returned, but to me it seems more logical to expect the removed group back rather than the new group. So I would return group instead of defaultCustomerGroupForShop.

It's actually a poorly named mutation in general, because it's actually changing the account group (to "customer") rather than just removing them from a group. I guess that's because we wanted everybody to always be in exactly one group per shop. So I'd argue we don't really need this mutation. You can call addAccountToGroup with the "customer" group, and it will do mostly the same thing. (Even that one would be better named setAccountGroupForShop.)

There is also a slight difference between those two mutations, which is a bit worrying. When switching back to the "customer" group here, we simply overwrite the full roles list for the shop, setting it to match the customer group permissions. But in addAccountToGroup, we delete all roles from the previous group and then add all roles from the new group. The difference is that addAccountToGroup way would keep one-off roles that have been added directly to a user outside the group paradigm. I don't know if this is really what we want or if users should have only the permissions from the group they are in and no others.

All of that being said, it doesn't seem like any core code actually calls removeAccountFromGroup, so how about you address these comments here but then mark it deprecated in the gql schema so that we can remove it in the next major release? The roles logic in addAccountToGroup is fine for now but we may want to raise the question of whether extra roles should be allowed on users (fits in with the work of the identity+auth pod that is starting).

[kieckhafer]

@aldeed comments addressed