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

Add FilterKey.members and support for equal operator to match an array #3536

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

laevandus
Copy link
Contributor

@laevandus laevandus commented Dec 18, 2024

🔗 Issue Links

Resolves IOS-609

🎯 Goal

Enable filtering channel list with $eq and an array of channel member ids

📝 Summary

  • Add FilterKey.members
  • Add support for $eq to match an array of values

🛠 Implementation

Filtering by members supports $in and $eq operators. We did not expose the latter. For supporting it, some changes were required for enabling Core Data to match channels with $eq and an array of values.

🎨 Showcase

🧪 Manual Testing Notes

  1. Log in with chewbacca
  2. Try different channel queries (R2-D2 Channels uses the new members equals query and returns all the channels where members equal to current user and r2-d2)

Example snippet

DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(5)) {
    Task {
        let query = ChannelListQuery(filter: .equal(.members, values: ["chewbacca", "luke_skywalker"]))
        let channelList = self.makeChannelList(with: query)
        try await channelList.get()
        let channels = channelList.state.channels
        for channel in channels {
            print(channel.cid, channel.memberCount, channel.lastActiveMembers.map(\.id).sorted())
        }
    }
}

☑️ Contributor Checklist

  • I have signed the Stream CLA (required)
  • This change should be manually QAed
  • Changelog is updated with client-facing changes
  • Changelog is updated with new localization keys
  • New code is covered by unit tests
  • Comparison screenshots added for visual changes
  • Affected documentation updated (docusaurus, tutorial, CMS)

@laevandus laevandus requested a review from a team as a code owner December 18, 2024 09:01
@laevandus laevandus added 🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK ✅ Feature An issue or PR related to a feature 🤞 Ready For QA A PR that is Ready for QA 🏗 Missing Feature Parity A feature that is not yet available on the iOS SDK. labels Dec 18, 2024
@laevandus laevandus force-pushed the feature/filter-equal-members branch from 100a4b7 to 8221ef2 Compare December 18, 2024 09:09
@Stream-SDK-Bot
Copy link
Collaborator

SDK Performance

target metric benchmark branch performance status
MessageList Hitches total duration 10 ms 6.68 ms 33.2% 🔼 🟢
Duration 2.6 s 2.56 s 1.54% 🔼 🟢
Hitch time ratio 4 ms per s 2.63 ms per s 34.25% 🔼 🟢
Frame rate 75 fps 78.09 fps 4.12% 🔼 🟢
Number of hitches 1 0.6 40.0% 🔼 🟢

@Stream-SDK-Bot
Copy link
Collaborator

Stream-SDK-Bot commented Dec 18, 2024

SDK Size

title develop branch diff status
StreamChat 7.03 MB 7.03 MB 0 KB 🟢
StreamChatUI 4.77 MB 4.77 MB 0 KB 🟢

Copy link
Contributor

@martinmitrevski martinmitrevski left a comment

Choose a reason for hiding this comment

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

LGTM ✅ Also, please update the docs in the LLC

@testableapple testableapple added 🟢 QAed A PR that was QAed and removed 🤞 Ready For QA A PR that is Ready for QA labels Dec 18, 2024
Comment on lines -49 to -50
// We don't want to expose `members` publicly because it can't be used with any other operator
// than `$in`. We expose it publicly via the `containMembers` filter helper.
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we expose something like equalMembers(userIds:) since only $eq and $in operator are supported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It feels more closer to public docs if we allow using it directly. But I am lacking some historic knowledge on channel list filters so not sure what is best here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With other keys we can create incompatible combinations.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, my guess is that for some properties, we started limiting the operators, especially for performance reasons. But for other properties we did not follow the same approach.

IMO, it would be better that programmatically you could only use the operators that you are allowed to, and that it would not require you to look into docs to see which operators are supported. It would be better to just limit every field but now it is kinda too late, since we already expose most of them :/

So yeah, maybe it is better we open up this one for consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we have that containMembers, maybe it is better then to have equalMembers. Hmm

@testableapple testableapple merged commit 57e8ea8 into develop Dec 18, 2024
12 checks passed
@testableapple testableapple deleted the feature/filter-equal-members branch December 18, 2024 11:06
@Stream-SDK-Bot Stream-SDK-Bot mentioned this pull request Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✅ Feature An issue or PR related to a feature 🏗 Missing Feature Parity A feature that is not yet available on the iOS SDK. 🟢 QAed A PR that was QAed 🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants