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

Automatically try reloading failed images on network changes #3170

Merged
merged 7 commits into from
Aug 15, 2024

Conversation

stefanceriu
Copy link
Member

@stefanceriu stefanceriu commented Aug 15, 2024

This PR exposes a new mechanism in the ImageProvider that can hold off image loading resolution until the connectivity is re-established. It automatically handles cancelling tasks when views get deallocated and will bail out if the second attempt fails while on a reachable connection to avoid retrying indefinitely.

It also refactors the MediaProvider tests and moves towards a generated mock MediaLoader.

@stefanceriu stefanceriu requested a review from a team as a code owner August 15, 2024 12:08
@stefanceriu stefanceriu requested review from Velin92 and removed request for a team August 15, 2024 12:08
Copy link

github-actions bot commented Aug 15, 2024

Warnings
⚠️ Some of the commits are missing ticket numbers. Please consider squashing all commits that don't have a tracking number.
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 Danger Swift against 42d2f5a

Copy link

codecov bot commented Aug 15, 2024

Codecov Report

Attention: Patch coverage is 88.77888% with 34 lines in your changes missing coverage. Please review.

Project coverage is 77.26%. Comparing base (cf35056) to head (42d2f5a).
Report is 2 commits behind head on develop.

Files Patch % Lines
...tX/Sources/Other/SwiftUI/Views/LoadableImage.swift 77.14% 8 Missing ⚠️
...ources/Services/Media/Provider/MediaProvider.swift 80.00% 8 Missing ⚠️
...sts/Sources/MediaProvider/MediaProviderTests.swift 95.16% 3 Missing ⚠️
...ens/InviteUsersScreen/View/InviteUsersScreen.swift 0.00% 2 Missing ⚠️
...ources/Services/UserSession/UserSessionStore.swift 0.00% 2 Missing ⚠️
NSE/Sources/Other/NSEUserSession.swift 0.00% 2 Missing ⚠️
.../FlowCoordinators/UserSessionFlowCoordinator.swift 0.00% 1 Missing ⚠️
...mentX/Sources/Other/Extensions/ClientBuilder.swift 0.00% 1 Missing ⚠️
...urces/Other/Pills/PillAttachmentViewProvider.swift 75.00% 1 Missing ⚠️
...alSearchScreen/GlobalSearchScreenCoordinator.swift 0.00% 1 Missing ⚠️
... and 5 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3170      +/-   ##
===========================================
- Coverage    77.33%   77.26%   -0.07%     
===========================================
  Files          717      717              
  Lines        56141    56247     +106     
===========================================
+ Hits         43415    43459      +44     
- Misses       12726    12788      +62     
Flag Coverage Δ
unittests 68.39% <88.77%> (-0.08%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

Yeah this seems neat to me, let see what happens 😎

@pixlwave pixlwave added the pr-bugfix for bug fix label Aug 15, 2024
Copy link

sonarcloud bot commented Aug 15, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
36.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@stefanceriu stefanceriu merged commit f89c3e5 into develop Aug 15, 2024
6 of 8 checks passed
@stefanceriu stefanceriu deleted the stefan/3113-part2 branch August 15, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix for bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants