These will have to be updated to the proper number when we release a new app version. Not yet sure how to enforce that

This function calculated permissions for members and admins of community roots. There are three types of community roots: COMMUNITY_ROOT, COMMUNITY_ANNOUNCEMENT_ROOT and GENESIS

member permissions are calculated by getThreadPermissionBlobFromUserSurfacedPermissions based on memberUserSurfacedPermissions. So member permissions are changed by updating addMembersPermissions in thread-permission-types.js, which are used by getThreadPermissionBlobFromUserSurfacedPermissions.

admin permissions are specified in this function in baseAdminPermissions. So they are changed directly in this function

Because we have to add the ADD_MEMBERS permission back for old clients, but for GENESIS only admin members have it, I opted to keep it in GENESIS at all times.

1. This greatly simplifies the code in rawThreadInfoFromServerThreadInfo
2. Before multikeyservers are launched ENG-7134 will be implemented, so secondary keyservers will not have GENESIS. So this is only for the auth keyserver, on which the only admin is Ashoat.
3. All users are members of the auth keyserver, so the problem we are trying to solve by disallowing adding users to communities doesn't apply to GENESIS

Please see this discussion for an explanation why this source is used

Non-admin members don't have a ADD_MEMBERS permission in GENESIS - only the admin. Both non-admin and admin members have this permission in other community roots. So this allows me to not have to check in filterThreadPermissions in rawThreadInfoFromServerThreadInfo whether the permissions I am editing are supposed to be used for admins, or other members.

Don't forget to change this when you want this to ship. If you land this with FUTURE_CODE_VERSION and forget to update to NEXT_CODE_VERSION, this work will never ship

I spent over 30 minutes trying to understand this. I really feel like you could have spent like 5-10min improving your explanation here, and it would have saved me a lot of time.

Concretely: there's no explanation here for why you're leaving admin permissions for GENESIS unchanged. Is it because something breaks if you remove the ADD_MEMBERS permission for GENESIS? Or is it because it only affects my UX (not user-facing) and you wanted to avoid the extra work that would be necessary?

I'm going to assume that it's the latter and accept this, but please think carefully about this feedback. In the future, I'd appreciate it if you could spend more time on your wording and explanations in diff comments. Think about what context I might be missing, and try rewording your comment multiple times.

This should be changed when we run migrations in D12062.
I think this code can be landed before we change this to NEXT_CODE_VERSION . It works all right without changing this - clients will get ADD_MEMBERS permission for both old community roots that actually have it, and for new community roots that won't have it. But the UI (changes from next diffs in this stack) will not allow adding the users anyway, so this is not a problem. Once D12062 is landed, clients and keyserver will migrate the data, and then the keyserver should stop sending to the updated clients the ADD_MEMBERS permission. I commented on D12062 to make sure we change this flag in the diff that adds those migrations.

From your question

You justified this change earlier by mentioning "This greatly simplifies the code in rawThreadInfoFromServerThreadInfo". Can you explain this in more detail? I'm wondering what exactly is special about the GENESIS case here

I thought that

This greatly simplifies the code in rawThreadInfoFromServerThreadInfo

is the only part of my explanation which you didn't understand. So I explained how is the implementation simplified by this.

Or is it because it only affects my UX (not user-facing) and you wanted to avoid the extra work that would be necessary?

Yes, the reason is that this only affects your account in production, while not being less correct than before. We are disallowing adding users to community roots, because the keyservers don't know user relationships, and this knowledge is needed to know who can add whom to chats. But the auth keyserver knows user relationships. So this issue does not apply to GENESIS. So for GENESIS it is still equally correct as it was before.