-
Notifications
You must be signed in to change notification settings - Fork 23
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
DM updates - default to not displaying dm groups #1046
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you covered this somewhere but is there a way to override the filter out functionality? So for these pure V3 clients that they can still get conversations?
xmtp_mls/src/subscriptions.rs
Outdated
convo_callback(convo) | ||
let provider = client.context.mls_provider()?; | ||
// Don't execute callback for dms unless include_dm is true | ||
if include_dm || convo.metadata(provider)?.conversation_type != ConversationType::Dm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be better to do this filter in stream_conversations
itself rather than the callback method -- the callback methods should retain the same behavior as the Non-callback variants, but just accept a function instead. this makes the logic for both easier to follow
Here is a place you could insert a check/filter for DM Group (i.e another filter/filter map call just above that, or even just save the iterator and if the dm_group
flag is on, filter the iterator again)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call 👌 , fixed here fc02806
Co-authored-by: Andrew Plaza <[email protected]>
2ed8b2c
to
333b3c2
Compare
333b3c2
to
6fb0385
Compare
@nplasterer For streams to include Dm groups, we just need to pass in true to libxmtp/bindings_ffi/src/mls.rs Line 864 in 5d58c4c
For libxmtp/bindings_ffi/src/mls.rs Line 839 in eb506f4
|
* Adds functions for creating a DM group (#901) * update bindings cargo locks * Added dm group functionality * dm members can update all metadata * fix tests * fix indentation * fix test imports * gen protos back to main --------- Co-authored-by: cameronvoell <[email protected]> * Private Preferences DB (#946) * create the database migration for the private preference work * update the table to be focused on consent * first pass at database storage structure * update the get method for consent records * fix up the set method * add a test * fix up the test * fix up the clippy error with consent record * fix up the clippy error with consent record * fix up all clippy issues * cargo fmt * Validate dm group metadata + permissions from welcome (#1075) * validate dm group before creating from welcome * lint fix * lint fix --------- Co-authored-by: cameronvoell <[email protected]> * fixes after merge * DM updates - default to not displaying dm groups (#1046) * bindings create_dm function * find groups by default does not include dm groups * fmt fix * dont execute callbacks when dm group welcomes are streamed * Update bindings_ffi/src/mls.rs Co-authored-by: Andrew Plaza <[email protected]> * fixed bad merge * filter dms in stream_conversations * surface include_dm_groups in bindings list function more clearly --------- Co-authored-by: cameronvoell <[email protected]> Co-authored-by: Andrew Plaza <[email protected]> * fix merge conflicts * cargo clippy * Remove tracing from test * Fix test * try and fix the tests * fix up the test --------- Co-authored-by: cameronvoell <[email protected]> Co-authored-by: Naomi Plasterer <[email protected]> Co-authored-by: Andrew Plaza <[email protected]> Co-authored-by: Ry Racherbaumer <[email protected]>
See #1074
As a pre-requisite for dual sending 1 to 1 messages, we will be creating DM groups that we do not want to show up for users until we switch to V3 only clients. This PR filters DM groups from list and stream conversations functions that are used in bindings and downstream SDKs