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.

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

[FS-1249] Do Not List MLS Self-conversation in client API v1 and v2 #2872

Merged
merged 4 commits into from
Nov 28, 2022

Conversation

mdimjasevic
Copy link
Contributor

Currently the call to POST /conversation/list-ids will list the MLS self-conversation in client API v1 and v2 if a v3 client created the conversation. Filter out the conversation in versions below v3.

Tracked by https://wearezeta.atlassian.net/browse/FS-1249.

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@mdimjasevic mdimjasevic temporarily deployed to cachix November 25, 2022 16:11 Inactive
@mdimjasevic mdimjasevic temporarily deployed to cachix November 25, 2022 16:11 Inactive
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Nov 25, 2022
Copy link
Contributor

@elland elland left a comment

Choose a reason for hiding this comment

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

Should I take over to add the GTC filter?

@@ -398,6 +406,22 @@ conversationIdsPageFromV2 lusr Public.GetMultiTablePageRequest {..} = do
mtpPagingState = Public.ConversationPagingState table (LBS.toStrict . C.unPagingState <$> pwsState)
}

-- MLS self-conversation of this user
selfConvId = mlsSelfConvId (tUnqualified lusr)
isNotSelfConv = (/= selfConvId) . qUnqualified
Copy link
Contributor

Choose a reason for hiding this comment

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

How about the global team conversation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd suggest to do that in a separate PR. I suppose it won't be hard to add it on top of this (assuming it gets merged), but you would know best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants