Skip to content

Federation: support conversation access updates #1808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 30, 2021

Conversation

pcapriotti
Copy link
Contributor

@pcapriotti pcapriotti commented Sep 24, 2021

This generalises the on-conversation-updated RPC to support notifications of conversation access updates, by adding a new constructor to the corresponding "action" type.

Note: An access update can potentially trigger member deletion. The previous code was performing the access update on the database, sending the corresponding notification, then deleting members, in that order. That meant that a failure in deleting members would result in the whole request failure, even though the access update was performed successfully. So now members are deleted asynchronously in a separate thread, similarly to how notifications to bots are sent.

This is part of https://wearezeta.atlassian.net/browse/SQCORE-885 (federated conversation metadata updates).

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • changelog.d contains the following bits of information:
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

@pcapriotti pcapriotti force-pushed the pcapriotti/propagate-access branch from 4ebea26 to 378019f Compare September 24, 2021 14:18
@pcapriotti pcapriotti force-pushed the pcapriotti/propagate-read-receipts branch from 8249bc8 to edb1772 Compare September 24, 2021 14:44
@pcapriotti pcapriotti force-pushed the pcapriotti/propagate-access branch 2 times, most recently from 84a9aa5 to fad0975 Compare September 27, 2021 06:30
@pcapriotti pcapriotti marked this pull request as ready for review September 27, 2021 06:50
Base automatically changed from pcapriotti/propagate-read-receipts to develop September 28, 2021 09:55
@pcapriotti pcapriotti force-pushed the pcapriotti/propagate-access branch from 2ed2132 to 3151928 Compare September 28, 2021 13:59
Comment on lines 252 to 253
modelConversationAccessDataEvent :: Doc.Model
modelConversationAccessDataEvent = Doc.defineModel "ConversationAccessDataEvent" $ do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was renamed by mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a0cf82c.

Doc.description "conversation access update event"
Doc.property "data" (Doc.ref modelConversationAccessUpdate) $ Doc.description "conversation access data"
Doc.property "data" (Doc.ref modelConversationAccessData) $ Doc.description "conversation access data"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was fine.

Also changed the name of `ConversationAccessUpdate` to
`ConversationAccessData`, since it is going to be used to keep track of
the current access and role, instead of just updates.
@pcapriotti pcapriotti force-pushed the pcapriotti/propagate-access branch from 3151928 to a0cf82c Compare September 29, 2021 12:48
@pcapriotti pcapriotti merged commit 2ac318c into develop Sep 30, 2021
@pcapriotti pcapriotti deleted the pcapriotti/propagate-access branch September 30, 2021 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants