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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CIS-1367] Remove unnecessary API calls from ChannelVC and ChannelListVC #1706

Merged
merged 1 commit into from Dec 21, 2021

Conversation

tbarbugli
Copy link
Member

@tbarbugli tbarbugli commented Dec 18, 2021

馃敆 Issue Link

CIS-1367

馃幆 Goal

Fix sending multiple unnecessary pagination requests when ChannelListVC and ChannelVC are shown.
This PR makes sure that only 1 API call is made to load both ChannelListVC and ChannelVC

馃帹 Changes

Before this PR ChannelListVC was calling the Query Channels endpoint 3 times!

ChannelListVC

  • Pagination was incorrectly firing the request to load another page
  • The make method was calling the setUp method (which calls synchronize)

ChannelVC

  • Pagination was incorrectly firing the request to load another page

鈽戯笍 Checklist

  • I have signed the Stream CLA (required)
  • This change follows zero 鈿狅笍 policy (required)
  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • Affected documentation updated (docusaurus, tutorial, CMS (task created)

@tbarbugli tbarbugli requested a review from a team as a code owner December 18, 2021 21:26
@codecov
Copy link

codecov bot commented Dec 18, 2021

Codecov Report

Merging #1706 (f321160) into develop (def5be4) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1706   +/-   ##
========================================
  Coverage    85.21%   85.21%           
========================================
  Files          231      231           
  Lines        11089    11089           
========================================
  Hits          9450     9450           
  Misses        1639     1639           
Flag Coverage 螖
llc-tests 85.21% <酶> (酶)

Flags with carried forward coverage won't be shown. Click here to find out 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 def5be4...f321160. Read the comment docs.

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, just some small comments regarding naming 馃憤 Let's wait for the green light from Adam and we will be good to 馃殺 Do we need a changelog entry for this, btw?

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.

Requesting change just to make sure we revert that code. Other than that LGTM 馃憤

Copy link
Contributor

@adamrushy adamrushy left a comment

Choose a reason for hiding this comment

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

Tested across the make function and removing the setUp call, all is working as expected and I also checked the Storyboard implementation and is all good. Just a conflict and a few other questions to answer then good to go 馃憤馃徏

@evsaev evsaev changed the title CIS-1367 Remove not necessary API calls [CIS-1367] Remove not necessary API calls Dec 21, 2021
@evsaev evsaev added 馃帹聽SDK: StreamChatUI Tasks related to the StreamChatUI SDK 馃悶聽Bug An issue or PR related to a bug labels Dec 21, 2021
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! 馃殌

@evsaev evsaev force-pushed the feature/reduce-api-calls branch 2 times, most recently from 02fb22a to b18ed9d Compare December 21, 2021 17:15
@evsaev evsaev changed the title [CIS-1367] Remove not necessary API calls [CIS-1367] Remove unnecessary API calls from ChannelVC and ChannelListVC Dec 21, 2021
@evsaev evsaev merged commit 6388879 into develop Dec 21, 2021
@evsaev evsaev deleted the feature/reduce-api-calls branch December 21, 2021 18:01
@adamrushy adamrushy mentioned this pull request Dec 23, 2021
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: StreamChatUI Tasks related to the StreamChatUI SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants