Habitica: allow guilds to be edited by staff and moderators

[Alys]

Staff and moderators should be able to edit public and private guilds in the same way that the guild owner can - i.e., moderators should have all the same permissions as the guild leader. Moderators are identified by having contributor.admin set to true.

This will not apply to parties, not even the party that the moderator or staff is a member of.

Something similar was done for challenges in https://github.com/HabitRPG/habitrpg/pull/5799

_FYI, this is intended to be used in only rare or essential circumstances:_

• _If a guild's title or descriptions have extreme violations of the Community Guidelines or Terms of Service, an admin would edit it immediately._
• _If a guild's title or descriptions have minor violations, an admin would edit it if the owner did not respond to an edit request within a reasonable timeframe._
• _Occasionally, a guild's title or descriptions might benefit from an important change unrelated to the Community Guidelines, and then an admin would edit it if the owner did not respond to an edit request (e.g., recently we modified the Newbies Guild to state that notifications are no longer sent)._

_When this change has been made, the wiki's Guilds page should be updated to contain a section similar to "Staff and Moderators' Permissions" in the Challenges page. It would be awesome if the dev who creates a PR for this could remind us of that. :)_

[colwem]

I'll work on this.

[colwem]

Can staff and moderators remove users? Can they remove the leader? Can they re-assign the leader? Do we need to make sure this only works for guilds or can these changes be applied to parties also?

[crookedneighbor]

There's middleware that should populates res.locals.group that should take care of that.

[colwem]

@crookedneighbor I don't follow. Yeah, res.locals.group is populated, but should we give admins the ability to run POST /groups/{gid}/removeMember on all gid? Or just POST /groups/{gid}

[Alys]

@colwem It's not as important for an admin to be able to remove a user from a group. We've never had to in the past, and if we needed to, we could do it in the database. However if it's easy to give us the ability to do that (it probably is) then please do.

It is correct that any user can invite any other user to a group or a party. It is not meant to be locked down to only the leader.

[Alys]

@colwem I've just noticed that you've edited one of your questions. It's best to always make a new comment instead of editing an existing one, because we don't get email notifications about edits. If there's a reason to edit an existing comment, you're welcome to do so but also create a new comment containing the change (you can delete the new comment after you've made it if you wish, since the only purpose of it is to get the full change into the email stream).

Staff and moderators should be able to re-assign the leader.

This issue is only for public and private guilds, not for parties.

Parties are considered private and admins should not have visibility of them. Obviously that doesn't include the actual party that the admin is in in the admin's role as a normal Habitica player, but I'm reluctant to give admins extra permissions in that case.

[colwem]

It's best to always make a new comment instead of editing an existing one, because we don't get email notifications about edits.

@Alys Thank you, I didn't realize that. I have a deeply ingrained habit of ninja editing posts from years of using reddit.com. I'll just have to be a little more cautious now.

It is correct that any user can invite any other user to a group or a party. It is not meant to be locked down to only the leader.

I didn't mean that other members of the group can invite people to their group, I mean it looks like anyone can invite anyone to any group. I don't see where it checks to make sure that the user giving the invite is actually part of the group.

[Alys]

@colwem I'm about to edit Guidance for Blacksmiths to include the tip about editing posts.

Also.. I've just edited your most recent comment to delete the second point you made, although it is NOT bad that you made it! Thank you VERY much for finding that and for telling us about it. We have it on record now in our emails, and I've removed it from here because I don't want anyone using that information before we've had a chance to fix that bug... If you have questions about why I did this, you're welcome to email [email protected] (just include a link to this issue to remind us what it is about).

[colwem]

@Alys I did some testing and I don't think it's actually a problem after all. I couldn't figure out how but it looks like it is preventing you from sending invites for groups you don't belong to.

[Alys]

@colwem It was a problem that the API could be used by anyone to issue an invitation to a private group but we've fixed that now with https://github.com/HabitRPG/habitrpg/commit/f69a29c152f2433ed7a90d3577fd59bc84b241ff and the fix has been deployed to production. Thank you for finding that bug! I've reinserted your original comment about it in your post above.

[colwem]

@Alys Ah ok. yeah, I saw the change, It was the conflict in #6775 that I was asking about yesterday. I also saw your addition of test/api/v2/groups/POST-groups_id_invite.test.js
which is currently being a big help to me as I try to write tests for #6775.

Also, thank you very much for updating the testing section in the blacksmith guidance wiki page. I think I've finally got it working right.

[Alys]

@colwem Are you still interested in working on this?

[Alys]

I'm opening this up to other contributors to work on (although @colwem if you're still working on it, if you comment here before anyone else takes it up, we'll reassign it to you).

I've edited the top post to include information that was discussed in the comments here and I've put strike-out on all the comments that are no longer needed because of that and because they applied to other, related code that's since been fixed.

[wdavew]

I'm new to open source contribution but have been learning JS and Node. Fixing this issue seems like it would just be a matter of adding a few more integration tests, changing the conditional statements in the updateGroup and removeGroupMember methods in the group controller, and then updating the group view accordingly. Does that sound accurate? If so I would be happy to take this on.

[crookedneighbor]

Sounds accurate.

[wdavew]

Great, working on it.