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

Update subscription view to be able to load videos from video cache again #3665

Conversation

PikachuEXE
Copy link
Collaborator

@PikachuEXE PikachuEXE commented Jun 14, 2023

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

There is another caching strategy in #3668

Description

This is a change as base for adding live stream tab for subscription view
The original plan is to only refactor the code first but some existing code/feature seems disabled (intentionally or accidentally I am not sure)

Load videos from video cache

When videos for all subscriptions loaded (main profile), there is some code (though disabled due to bug) to cache all loaded videos and use those when active profile changed

The flow in this PR is like:

  • Loading videos from remote would put those profile ID, videos (and other stuff) in a local cache for active profile (now called subscriptionsCacheForActiveProfile)
    • Additionally if the active profile is default profile it would also store the videos (only) into another cache (now subscriptionsCacheForAllSubscriptionsProfile)
    • Else (non default profile), it will clear subscriptionsCacheForAllSubscriptionsProfile
  • Switching to another profile would load videos from cache subscriptionsCacheForActiveProfile if profile ID matches
  • Else if subscriptionsCacheForAllSubscriptionsProfile has videos cached it would load videos from it with filtering (video channel is in active profile)
  • Else load videos automatically if preference set
  • Else do nothing

Screenshots

For loading from subscriptionsCacheForAllSubscriptionsProfile

Screen.Recording.2023-06-14.at.14.28.21.mov

Testing

Preparation:

  • Create 3+ profiles (other than default)
  • 2 with overlapping (or not) channels (A, B), 1 with no channels (C)

Before starting each test please reopen window/app/refresh to ensure local state is reset

A. Auto fetching disabled
A1. Loading from subscriptionsCacheForAllSubscriptionsProfile

  • Switch to default profile (if not already)
  • Ensure no video loaded
  • Load videos from remote (via button, KB shortcut whatever)
  • Ensure videos loaded
  • Switch to profile A
  • Ensure correct videos loaded, no network request made for fetching videos
  • Switch to profile B
  • Ensure correct videos loaded, no network request made for fetching videos
  • Switch to profile C
  • Ensure correct (no) videos loaded, no network request made for fetching videos
  • Switch to default profile
  • Ensure correct (all) videos loaded, no network request made for fetching videos

A2. Auto clearing of subscriptionsCacheForAllSubscriptionsProfile & Loading from subscriptionsCacheForActiveProfile

  • Switch to default profile (if not already)
  • Load videos from remote
  • Ensure videos loaded
  • Switch to profile A
  • Load videos from remote
  • Switch to default profile
  • Ensure NO videos loaded
  • Switch to profile A
  • Ensure correct videos loaded, no network request made for fetching videos
  • Switch to profile B
  • Ensure NO videos loaded

B. Auto fetching enabled
B1. Loading from subscriptionsCacheForAllSubscriptionsProfile

  • Same as A1 except Load videos from remote is done automatically

B2. Auto clearing of subscriptionsCacheForAllSubscriptionsProfile & Loading from subscriptionsCacheForActiveProfile

  • Switch to default profile (if not already)
  • Ensure videos loaded
  • Switch to profile A
  • Load videos from remote (manually)
  • Switch to profile B
  • Ensure correct videos loaded, with network request made for fetching videos
  • Switch to default profile
  • Ensure correct videos loaded, with network request made for fetching videos

Desktop

  • OS:
  • OS Version:
  • FreeTube version:

Additional context

Refactoring code and typing test cases & testing might be more tiring than actual implementation

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jun 14, 2023
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) June 14, 2023 06:48
@github-actions github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Jul 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 2, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@efb4f5ff-1298-471a-8973-3d47447115dc

So this can be closed?

@PikachuEXE
Copy link
Collaborator Author

Replaced by merged #3668

@PikachuEXE PikachuEXE closed this Jul 2, 2023
auto-merge was automatically disabled July 2, 2023 14:51

Pull request was closed

@PikachuEXE PikachuEXE deleted the feature/subscription-view/restore-load-videos-from-all-subscription-profile-video-cache branch July 2, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants