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-1071] Fix member removed from Channel still present in MemberListController #1323

Merged
merged 2 commits into from Aug 2, 2021

Conversation

b-onc
Copy link
Contributor

@b-onc b-onc commented Jul 27, 2021

This happened because we forgot to update the queries of Member when it's removed. A similar bug may be present in MemberAdded and MemberUpdated events. For those events, we need to re-fetch the queries, since Client doesn't have a way to apply filters.

@b-onc b-onc added 🐞 Bug An issue or PR related to a bug 🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK labels Jul 27, 2021
@b-onc b-onc requested review from tbarbugli and dmigach July 27, 2021 11:50
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2021

Codecov Report

Merging #1323 (9a5eb71) into main (0cbf1a0) will decrease coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1323      +/-   ##
==========================================
- Coverage   91.15%   91.01%   -0.14%     
==========================================
  Files         227      227              
  Lines        9763     9767       +4     
==========================================
- Hits         8899     8889      -10     
- Misses        864      878      +14     
Flag Coverage Δ
llc-tests 91.01% <100.00%> (+0.01%) ⬆️
llc-tests-ios12 ?

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

Impacted Files Coverage Δ
...rces/StreamChat/Database/DTOs/MemberModelDTO.swift 100.00% <ø> (ø)
...lient/EventMiddlewares/MemberEventMiddleware.swift 97.61% <100.00%> (+0.25%) ⬆️
...ocketClient/Engine/StarscreamWebSocketEngine.swift 0.00% <0.00%> (-30.96%) ⬇️
.../Utils/InternetConnection/InternetConnection.swift 54.05% <0.00%> (-0.91%) ⬇️
...ources/StreamChat/Database/DatabaseContainer.swift 96.42% <0.00%> (-0.72%) ⬇️
Sources/StreamChat/Workers/ChannelUpdater.swift 98.31% <0.00%> (+0.84%) ⬆️

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 e13ed8c...9a5eb71. Read the comment docs.

@b-onc b-onc force-pushed the cis-1071-memberlist-not-updated branch from 29f7b73 to eef3f0c Compare July 27, 2021 13:30
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!

@b-onc b-onc force-pushed the cis-1071-memberlist-not-updated branch 2 times, most recently from 63206b9 to e1386c4 Compare July 29, 2021 15:02
@tbarbugli
Copy link
Member

@dmigach @DominikBucher12 why are you approving a PR with broken CI?

@b-onc b-onc force-pushed the cis-1071-memberlist-not-updated branch 2 times, most recently from bc23e19 to 3c09ce3 Compare August 2, 2021 11:52
Bahadir Oncel added 2 commits August 2, 2021 13:53
This happened because we forgot to update the queries of Member when it's removed. A similar bug may be present in MemberAdded and MemberUpdated events. For those events, we need to re-fetch the queries, since Client doesn't have a way to apply filters.
The new test somehow keeps failing on CI.. perhaps dividing it into 2 will fix it?
@b-onc b-onc force-pushed the cis-1071-memberlist-not-updated branch from 3c09ce3 to 9a5eb71 Compare August 2, 2021 11:53
@b-onc b-onc merged commit 17c6271 into main Aug 2, 2021
@b-onc b-onc deleted the cis-1071-memberlist-not-updated branch August 2, 2021 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug An issue or PR related to a bug 🌐 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

5 participants