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-997] Fix over fetching previous messages #1110

Merged
merged 2 commits into from
Jul 9, 2021

Conversation

nuno-vieira
Copy link
Member

@nuno-vieira nuno-vieira commented May 25, 2021

Description of the pull request

When getting the previous messages, we would make unnecessary requests. Now we only load the previous messages if there's more to fetch.

The only thing left to do is add test cases. I will do it once the implementation approach is approved.

@nuno-vieira nuno-vieira added the 🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK label May 25, 2021
@nuno-vieira nuno-vieira changed the title Fix over fetching previous messages [CIS-997] Fix over fetching previous messages Jul 8, 2021
Copy link
Contributor

@b-onc b-onc left a comment

Choose a reason for hiding this comment

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

I think this approach is good 👍

@nuno-vieira nuno-vieira force-pushed the fix-over-fetching-previous-messages branch from bed70e4 to ccf5468 Compare July 9, 2021 14:45
@nuno-vieira nuno-vieira requested a review from b-onc July 9, 2021 14:45
@nuno-vieira nuno-vieira marked this pull request as ready for review July 9, 2021 14:45
@nuno-vieira nuno-vieira requested a review from tbarbugli July 9, 2021 14:46
@codecov-commenter
Copy link

codecov-commenter commented Jul 9, 2021

Codecov Report

Merging #1110 (57f44c0) into main (75ad2de) will decrease coverage by 0.01%.
The diff coverage is 95.65%.

❗ Current head 57f44c0 differs from pull request most recent head 5b4d624. Consider uploading reports for the commit 5b4d624 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1110      +/-   ##
==========================================
- Coverage   91.38%   91.36%   -0.02%     
==========================================
  Files         219      219              
  Lines        9421     9432      +11     
==========================================
+ Hits         8609     8618       +9     
- Misses        812      814       +2     
Flag Coverage Δ
llc-tests 91.21% <95.65%> (-0.02%) ⬇️
llc-tests-ios12 88.07% <95.65%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
Sources/StreamChat/Workers/ChannelUpdater.swift 97.93% <87.50%> (-2.07%) ⬇️
...trollers/ChannelController/ChannelController.swift 93.00% <100.00%> (+0.12%) ⬆️

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 75ad2de...5b4d624. Read the comment docs.

@nuno-vieira nuno-vieira force-pushed the fix-over-fetching-previous-messages branch from ccf5468 to d1c80fb Compare July 9, 2021 15:06
@nuno-vieira nuno-vieira force-pushed the fix-over-fetching-previous-messages branch from 57f44c0 to 401e2b1 Compare July 9, 2021 15:16
@nuno-vieira nuno-vieira force-pushed the fix-over-fetching-previous-messages branch from 401e2b1 to 5b4d624 Compare July 9, 2021 15:20
Copy link
Contributor

@evsaev evsaev left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@nuno-vieira nuno-vieira merged commit a44693b into main Jul 9, 2021
@nuno-vieira nuno-vieira deleted the fix-over-fetching-previous-messages branch July 9, 2021 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants