[Reactions v2] New reactions query endpoint #3167
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nuno-vieira
changed the title
nuno-vieira
commented
Apr 24, 2024
nuno-vieira
commented
Apr 24, 2024
Sources/StreamChat/Controllers/MessageController/MessageController.swift
Outdated
Show resolved
Hide resolved
nuno-vieira
commented
Apr 24, 2024
Sources/StreamChat/Controllers/MessageController/MessageController.swift
Outdated
Show resolved
Hide resolved
nuno-vieira
commented
Apr 24, 2024
Generated by 🚫 Danger |
nuno-vieira
added
✅ Feature
nuno-vieira
commented
Apr 24, 2024
nuno-vieira
commented
Apr 24, 2024
nuno-vieira
force-pushed
the
add/new-reactions-v2-query-endpoint
branch
from
e719621
to
83ec4ff
Compare
StreamChat XCMetrics
|
laevandus
reviewed
Apr 29, 2024
|
|
||
| override func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { | ||
| let reaction = reactions[indexPath.row] | ||
| let cell = UITableViewCell(style: .value1, reuseIdentifier: "slack-reaction-cell") |
There was a problem hiding this comment.
It is a sample app, but just wondering why it was not using dequeueReusableCell(withIdentifier:for:). Not that it makes a huge difference here. I guess because there are only a couple of cells so it does not matter. Just thinking out aloud, feel free to ignore.
laevandus
reviewed
Apr 29, 2024
|
|
||
| import CoreData | ||
|
|
||
| class ReactionListUpdater: Worker { |
There was a problem hiding this comment.
Thinking if we should keep it next to the old load reactions method in MessageUpdater.
laevandus
approved these changes
Apr 29, 2024
| // At the moment, we do not allow changing the query sorting. | ||
| request.sortDescriptors = [.init(key: #keyPath(MessageReactionDTO.createdAt), ascending: false)] | ||
|
|
||
| let messageIdPredicate = NSPredicate(format: "message.id == %@", query.messageId) |
There was a problem hiding this comment.
there was a nasty bug here - the message id wasn't used in the predicate. Therefore, whenever a reaction type was present, it showed all the reactions for all messages.
martinmitrevski
approved these changes
Apr 29, 2024
testableapple
approved these changes
Apr 29, 2024
|
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔗 Issue Links
Resolves https://github.com/GetStream/ios-issues-tracking/issues/787
🎯 Goal
Adds the new reactions v2 query endpoint to be able to filter reactions based on type and author.
📝 Summary
ChatReactionListController🛠 Implementation
Creates a new
ChatReactionListControllerto follow the SDK pattern, where each query endpoint represents a controller.TODO:
ReactionListController.🎨 Showcase
Simulator.Screen.Recording.-.iPhone.14.Pro.-.2024-04-26.at.21.58.36.mp4
🧪 Manual Testing Notes
Note: Backend user enrichment is not working on the new query reactions endpoint. So this is not testable until it is fixed on the backend.
☑️ Contributor Checklist