Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remote client discovery #1635

Merged
merged 15 commits into from
Jul 2, 2021
Merged

Remote client discovery #1635

merged 15 commits into from
Jul 2, 2021

Conversation

pcapriotti
Copy link
Contributor

@pcapriotti pcapriotti commented Jun 29, 2021 •
edited
Loading

This PR implements fetching of remote clients when sending messages involving remote users, and takes those clients into account when checking recipients and computing missing, deleted and redundant clients.

This supersedes #1623.

Checklist

  • Title of this PR explains the impact of the change.
  • The 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.
  • The CHANGELOG.md file in the Unreleased section has been updated to explain the change which will be included in the release notes.

@pcapriotti pcapriotti changed the base branch from develop to pcapriotti/send-message-refactoring June 29, 2021 13:51
@pcapriotti pcapriotti marked this pull request as draft June 29, 2021 13:51
@akshaymankar akshaymankar mentioned this pull request Jun 29, 2021
Closed
5 tasks
@pcapriotti pcapriotti force-pushed the pcapriotti/remote-client-disco branch 2 times, most recently from 15be360 to e95b358 Compare June 30, 2021 12:17
@pcapriotti pcapriotti marked this pull request as ready for review June 30, 2021 13:02
@pcapriotti pcapriotti force-pushed the pcapriotti/remote-client-disco branch from e95b358 to 58add66 Compare June 30, 2021 13:07
Base automatically changed from pcapriotti/send-message-refactoring to develop July 1, 2021 06:05
@pcapriotti pcapriotti force-pushed the pcapriotti/remote-client-disco branch from 58add66 to 966461d Compare July 1, 2021 06:28
akshaymankar
akshaymankar reviewed Jul 1, 2021
View reviewed changes
Copy link
Member

@akshaymankar akshaymankar left a comment

Choose a reason for hiding this comment

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

Couldn't finish reviewing, looks fine until now. Minor comments inline, other's please feel free to review 😄

services/galley/src/Galley/API/Message.hs Outdated Show resolved Hide resolved
services/galley/test/integration/API.hs Outdated Show resolved Hide resolved
services/galley/test/integration/API.hs Outdated Show resolved Hide resolved
jschaul
jschaul approved these changes Jul 1, 2021
View reviewed changes
Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

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

Implementation and test cases look good to me. Introducing assertMismatch helper is also a good idea.
Fine to do nitpicks on this or on a follow-up PR as you prefer.

services/galley/src/Galley/API/Message.hs Outdated Show resolved Hide resolved
@pcapriotti pcapriotti merged commit 736c20c into develop Jul 2, 2021
@pcapriotti pcapriotti deleted the pcapriotti/remote-client-disco branch July 2, 2021 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akshaymankar akshaymankar akshaymankar left review comments

@jschaul jschaul jschaul approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pcapriotti @akshaymankar @jschaul @smatting