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

[CIS-805] Refresh channel list queries when current user membership changes #1016

Merged
merged 8 commits into from Apr 23, 2021

Conversation

VojtaStavik
Copy link
Contributor

This PR makes the existing queries for a channel invalidated when the current user membership changes. In other words - when you stop being a member of a channel, you won't see it in the list :)

@VojtaStavik VojtaStavik added the 🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK label Apr 23, 2021
Copy link
Contributor

@DominikBucher12 DominikBucher12 left a comment

Choose a reason for hiding this comment

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

LGTM, just 1 little comment which you can probably ignore

@@ -8,9 +8,8 @@ public protocol ConnectionEvent: Event {
var connectionId: String { get }
}

public struct HealthCheckEvent: ConnectionEvent, EventWithPayload, EventWithCurrentUserPayload {
public struct HealthCheckEvent: ConnectionEvent, EventWithPayload {
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉 Thanks <3

updateChannelListQuery(for: channelDTO)
database.write {
let dto = $0.channel(cid: try! ChannelId(cid: channelDTO.cid))
dto?.needsRefreshQueries = false
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what is gooing on here 😅 Maybe add little documentation? This makes me wonder why it doesn't need to refresh queries...

@Stream-iOS-Bot
Copy link
Collaborator

1 Error
🚫 Please start subject with capital letter.
f48ed94

Generated by 🚫 Danger

try session.saveMember(payload: memberPayload, channelId: memberEvent.cid)
case is MemberRemovedEvent:
guard let channel = session.channel(cid: memberEvent.cid) else {
try session.saveMember(payload: memberPayload, channelId: (event as! EventWithChannelId).cid)
Copy link
Member

Choose a reason for hiding this comment

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

event as! EventWithChannelId are we sure this is always true? Why the force unwrap? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

MemberRemovedEvent is always EventWithChannelId :) @nuno-vieira

@@ -72,7 +72,14 @@ final class NewChannelQueryUpdater<ExtraData: ExtraDataTypes>: Worker {
changes.forEach { change in
switch change {
case let .insert(channelDTO, _):
updateChannelListQuery(for: channelDTO)
database.write {
let dto = $0.channel(cid: try! ChannelId(cid: channelDTO.cid))
Copy link
Member

Choose a reason for hiding this comment

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

Is this force unwrap safe as well?

Copy link
Member

@nuno-vieira nuno-vieira left a comment

Choose a reason for hiding this comment

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

LGTM!

@codecov
Copy link

codecov bot commented Apr 23, 2021

Codecov Report

Merging #1016 (f48ed94) into main (bf452fe) will increase coverage by 0.14%.
The diff coverage is 99.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1016      +/-   ##
==========================================
+ Coverage   89.78%   89.93%   +0.14%     
==========================================
  Files         211      213       +2     
  Lines        8872     8943      +71     
==========================================
+ Hits         7966     8043      +77     
+ Misses        906      900       -6     
Flag Coverage Δ
llc-tests 89.76% <99.06%> (+0.14%) ⬆️
llc-tests-ios12 86.52% <99.06%> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...Chat/WebSocketClient/Events/ConnectionEvents.swift 94.11% <ø> (+23.52%) ⬆️
...lient/EventMiddlewares/MemberEventMiddleware.swift 92.10% <95.00%> (+12.93%) ⬆️
...Client/Endpoints/Payloads/ChannelListPayload.swift 100.00% <100.00%> (ø)
...lient/Endpoints/Payloads/MutedChannelPayload.swift 100.00% <100.00%> (ø)
Sources/StreamChat/ChatClient.swift 96.60% <100.00%> (+0.01%) ⬆️
Sources/StreamChat/Database/DTOs/ChannelDTO.swift 99.07% <100.00%> (+0.06%) ⬆️
...Middlewares/ChannelVisibilityEventMiddleware.swift 100.00% <100.00%> (ø)
...eamChat/WebSocketClient/Events/ChannelEvents.swift 93.93% <100.00%> (+0.18%) ⬆️
.../StreamChat/WebSocketClient/Events/EventType.swift 100.00% <100.00%> (ø)
...reamChat/WebSocketClient/Events/MemberEvents.swift 100.00% <100.00%> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb21543...f48ed94. Read the comment docs.

@VojtaStavik VojtaStavik merged commit d3d1bc8 into main Apr 23, 2021
@VojtaStavik VojtaStavik deleted the CIS-805-refresh-list-queries branch April 23, 2021 17:34
nuno-vieira pushed a commit that referenced this pull request Apr 23, 2021
…hanges (#1016)

* Expose `queries` field on `ChannelDTO`

* Add `needsRefreshQueries` field to `ChannelDTO`

* Make `NewChannelQueryUpdater` respect `needsRefreshQueries`

* Make it possible to create mock channels with query

* Streamline events protocols

* Invalidate `ChannelDTO` queries when current user changes membership

* Update CHANGELOG

* fixup! Streamline events protocols
nuno-vieira pushed a commit that referenced this pull request Apr 23, 2021
…hanges (#1016)

* Expose `queries` field on `ChannelDTO`

* Add `needsRefreshQueries` field to `ChannelDTO`

* Make `NewChannelQueryUpdater` respect `needsRefreshQueries`

* Make it possible to create mock channels with query

* Streamline events protocols

* Invalidate `ChannelDTO` queries when current user changes membership

* Update CHANGELOG

* fixup! Streamline events protocols
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants