HomePhabricator
Diffusion Comm e092bcc39dcd

[keyserver/lib] Update the change_role message spec to include a role name…

Tags
None
Referenced Files
F636037: Deleted Role - Unshimmed.png
Jul 31 2023, 6:15 PM
File Not Attached
F636039: Existing Role - Unshimmed.png
Jul 31 2023, 6:15 PM
File Not Attached
F636038: Shimmed Change Role.png
Jul 31 2023, 6:15 PM
File Not Attached
F636036: After (DB).png
Jul 31 2023, 6:15 PM
File Not Attached
F636035: Before (DB).png
Jul 31 2023, 6:15 PM
File Not Attached
Subscribers
None

Description

[keyserver/lib] Update the change_role message spec to include a role name field in the DB

Summary:
While writing the code to edit/delete roles, I noticed that the message spec for the change_roles message type causes the app to crash if a role is deleted. The reason for this is when a user's role
is changed, we write the following information to the DB in the content field:

{"userIDs":["83813"],"newRole":"84094"}

To generate the robotext for this message in chat (so we can show users something like 'jack assigned max the "Moderators" role'), we introspect into threadInfo.roles and try to access the role info using the newRole ID. When a role is deleted, we delete it from the database, and so the threadInfo will no longer have this role info and the app will crash. To resolve this, we should now store the roleName associated with the role change in the DB to use as a fallback option.

We want to continue trying to access the role name via the threadInfo (since older clients will not have this roleName field), but fallback onto it if needed. The reason this should not cause a problem is that older clients are guaranteed to have the role stored in threadInfo since at the moment, only Members and Admins are roles on Comm, and neither are deletable.

As for the code, the following is done:

If the client has the newest code version, show them the robotext accessing the role either through threadInfo or the content field as a fallback.

If the client does not, then we just shim the message.

Depends on D8478

Test Plan:
Checked the before and after to verify that the new field was inserted into the DB when a role is changed. Then, confirmed that messages are shimmed successfully if my client's version is not meeting the minimum code version. Lastly, made sure that the app continues to display role names in the robotext even if it is deleted.

content field of the database when changing a user role (before):

Before (DB).png (140×2 px, 124 KB)

content field of the database when changing a user role (after):

After (DB).png (264×2 px, 215 KB)

Generated robotext for clients that do not meet the minimum code verison:

Shimmed Change Role.png (2×1 px, 1 MB)

Robotext for clients meeting the minimum code version with an existing custom role:

Existing Role - Unshimmed.png (2×1 px, 1 MB)

Robotext for clients meeting the minimum code version after the role has been deleted:

Deleted Role - Unshimmed.png (2×1 px, 1 MB)

For reference, the error I would get opening the thread after the role was deleted before this diff:

Simulator Screen Shot - iPhone 14 Pro - 2023-07-13 at 12.48.54.png (2×1 px, 259 KB)

Reviewers: ginsu, ashoat

Reviewed By: ashoat

Subscribers: ashoat, tomek

Differential Revision: https://phab.comm.dev/D8502