Update subscription view to be able to load videos from video cache per channel #3668

PikachuEXE · 2023-06-15T07:02:06Z

Pull Request Type

Bugfix
Feature Implementation
Documentation
Other

Related issue

A variant (?) of #3665

Description

#3665 restores the "original" behaviour (well the code that was there but disabled somehow)
This is a different video caching strategy
Videos (and potentially other info like live streams and shorts) are cached per channel instead of profile
The difference is that every profile can potentially just load from cache if all the channels of selected profile have required content in cache, without the limitation of only caching one profile at a time

Screenshots

Similar to 3665 one so no repeating here

Testing

Preparation:

Create following profiles (other than default)
- Profile A with most but not all channels
- Profile B with only some channels from A
- Profile C with some channels from A (1 is enough), some from the channel(s) excluded in A
- Profile D with no channel

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

A. Auto fetching disabled
A1. Loading from cache after all channel cached

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

A2a. Refresh subscriptions effects - With default profile loaded first

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 all videos loaded, no network request made for fetching videos
(due to only cache for channels in profile A refreshed, but cache for not refreshed channel are not purged)
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

A2b. Refresh subscriptions effects - Without default profile loaded first

Switch to default profile (if not already)
DO NOT load videos from remote
Switch to profile A
Load videos from remote
Switch to default profile
Ensure NO videos loaded
(due to not all channels are cached)
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
(due to Profile B with only some channels from A)

A3. After subscribing new channel(s)

Switch to default profile (if not already)
Load videos from remote
Ensure videos loaded
Visit a random not subscribed channel, subscribe it
Go back to subscriptions view
Ensure NO videos loaded (due to not all subscribed channel cached)

B. Auto fetching enabled
B1. Loading from subscriptionsCacheForAllSubscriptionsProfile

Same as A1 except Load videos from remote is done automatically

B2a. Refresh subscriptions effects - With default profile loaded first

Switch to default profile (if not already)
Ensure videos loaded
Switch to profile A
Load videos from remote
Switch to default profile
Ensure all videos loaded, no network request made for fetching videos
(due to only cache for channels in profile A refreshed, but cache for not refreshed channel are not purged)
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

B2b. Refresh subscriptions effects - Without default profile loaded first

Switch to non subscription view
Switch to profile A
Ensure videos loaded
Switch to default profile
Ensure videos loaded, with no. of requests matching no. of channels
(i.e. all videos are fetched from remote not from cache)
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
(due to Profile B with only some channels from A)

B3. After subscribing new channel(s)

Switch to default profile (if not already)
Ensure videos loaded
Visit a random not subscribed channel, subscribe it
Go back to subscriptions view
Ensure videos loaded, with no. of requests matching no. of channels
(i.e. all videos are fetched from remote not from cache)

Desktop

OS:
OS Version:
FreeTube version:

Additional context

Fetch Feed Automatically is enabled by default
To avoid expectation issue when enabled all videos are fetch from remote (if cache for any channel absent)

ChunkyProgrammer · 2023-06-20T10:30:00Z

There's a GitHub warning for src/renderer/views/Subscriptions/Subscriptions.js at line 11

src/renderer/views/Subscriptions/Subscriptions.js

efb4f5ff-1298-471a-8973-3d47447115dc · 2023-06-27T14:23:46Z

Just to be sure, this is how B1 should be tested right?

Switch to default profile (if not already)
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

Edit: Passed all testcases, just waiting on confirmation

ChunkyProgrammer

Remove unused import in src/renderer/views/Subscriptions/Subscriptions.js

PikachuEXE · 2023-06-28T01:19:17Z

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

@ChunkyProgrammer
Done

Now the changes includes those from #3673 (to solve code conflict in advance)
Please merge this after #3673 (that one only needs 1 more approval to be merged)

… again

… per channel

efb4f5ff-1298-471a-8973-3d47447115dc

lgtm

absidue · 2023-07-01T15:15:20Z

Tested this pull request and most situations work fine, however I found this one situation where it doesn't load from the cache:

Fetch feed automatically: OFF

Total channels: 50

Switch to profile 1: load the subscriptions (10 requests)
Switch to profile 2: load subscriptions (5 requests)

Don't do anything with profiles 3-8

Switch to all channels profile: load subscriptions (requests 50, expected 35 as I expected it to load the 15 previously loaded ones from the cache)

absidue · 2023-07-01T15:16:58Z

Is that behaviour expected or is that a bug?

PikachuEXE · 2023-07-02T00:36:47Z

@absidue
I consider manual reload as "fetch all subscription from remote"
It is what it means right now and I find it rare to change that
To alter that behaviour we need new settings which should not be part of this PR

absidue

Approving as your explanation makes sense.

PikachuEXE · 2023-07-02T14:50:25Z

Now #3673 requires even more changes arrrr

…4380) * Implement first draft of last subscription refresh timestamp * Update styling to be a top bar * Update styling to be banner-compatible, & increase banner X button size on mobile * Update subscription refresh timestamp to be relative * Implement refresh timestamps for Shorts, Live, and Community tabs * Extract refresh widget to its own component * Add Trending and Popular refresh widgets with timestamps * Fix justifying when no timestamp exists * Move timestamps to utils store * Remove unneeded ref classes and currentLocale computed property * Add page-specific titles for each feed type * Implement showing least recent cache date per profile * Update styling property placement & match top nav box shadow on ft-refresh-widget * Implement showing timestamp for profile only if all channel subscriptions can be found in cache * Disable refresh button instead of removing it or the widget from the DOM * Increase top banner's top margin * Update channel caching calls to provide timestamps * Modify updateCacheByChannel functions to have default timestamp of new Date() * Fix 30-day month relative date calculation scenarios through new optional parameter * Rectify Case 3 (see #3668) * Add back missing line in Popular.js

github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jun 15, 2023

FreeTubeBot enabled auto-merge (squash) June 15, 2023 07:02

This was referenced Jun 14, 2023

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

Closed

Add DeArrow support for ft-list-video titles #3688

Merged

ChunkyProgrammer reviewed Jun 20, 2023

View reviewed changes

src/renderer/views/Subscriptions/Subscriptions.js Outdated Show resolved Hide resolved

PikachuEXE mentioned this pull request Jun 26, 2023

[Local API] Update subscription view to show live videos #3659

Closed

4 tasks

ChunkyProgrammer requested review from absidue, efb4f5ff-1298-471a-8973-3d47447115dc, ChunkyProgrammer and MarmadileManteater June 27, 2023 17:15

ChunkyProgrammer requested changes Jun 27, 2023

View reviewed changes

PikachuEXE force-pushed the pika/feature/subscription-view/load-videos-from-cache-per-channel branch from e5454e3 to 3310780 Compare June 28, 2023 01:19

petaded and others added 5 commits June 28, 2023 09:20

Changes from PR FreeTubeApp#3673

56aff33

* Update subscription view to be able to load videos from video cache…

82d3345

… again

* Update subscription view to be able to load videos from video cache…

51da2f6

… per channel

* Remove meaningless argument allowUseOfChannelCache

dc85b01

$ Remove unused imports

ca8a8a7

PikachuEXE force-pushed the pika/feature/subscription-view/load-videos-from-cache-per-channel branch from 3310780 to ca8a8a7 Compare June 28, 2023 01:20

efb4f5ff-1298-471a-8973-3d47447115dc approved these changes Jun 29, 2023

View reviewed changes

absidue approved these changes Jul 2, 2023

View reviewed changes

efb4f5ff-1298-471a-8973-3d47447115dc requested a review from ChunkyProgrammer July 2, 2023 14:17

ChunkyProgrammer approved these changes Jul 2, 2023

View reviewed changes

FreeTubeBot merged commit 242c93e into FreeTubeApp:development Jul 2, 2023

github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Jul 2, 2023

PikachuEXE deleted the pika/feature/subscription-view/load-videos-from-cache-per-channel branch July 2, 2023 14:51

PikachuEXE mentioned this pull request Jul 2, 2023

Fix Hide Channels and Hide Premier settings #3673

Merged

4 tasks

This was referenced Nov 25, 2023

Show when Subscriptions / Trending / Most Popular were last updated #4380

Merged

Feat/different subscription behavior kommunarr/FreeTube#2

Closed

kommunarr added a commit to kommunarr/FreeTube that referenced this pull request Apr 14, 2024

Rectify Case 3 (see FreeTubeApp#3668)

5c5ed08

kommunarr added a commit to kommunarr/FreeTube that referenced this pull request Apr 14, 2024

Rectify Case 3 (see FreeTubeApp#3668)

9bf8108

PikachuEXE mentioned this pull request Sep 16, 2024

Implement persistent subscription cache #5185

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update subscription view to be able to load videos from video cache per channel #3668

Update subscription view to be able to load videos from video cache per channel #3668

PikachuEXE commented Jun 15, 2023

ChunkyProgrammer commented Jun 20, 2023

efb4f5ff-1298-471a-8973-3d47447115dc commented Jun 27, 2023 •

edited

Loading

ChunkyProgrammer left a comment

PikachuEXE commented Jun 28, 2023

efb4f5ff-1298-471a-8973-3d47447115dc left a comment

absidue commented Jul 1, 2023

absidue commented Jul 1, 2023

PikachuEXE commented Jul 2, 2023

absidue left a comment

PikachuEXE commented Jul 2, 2023

Update subscription view to be able to load videos from video cache per channel #3668

Update subscription view to be able to load videos from video cache per channel #3668

Conversation

PikachuEXE commented Jun 15, 2023

Pull Request Type

Related issue

Description

Screenshots

Testing

Desktop

Additional context

ChunkyProgrammer commented Jun 20, 2023

efb4f5ff-1298-471a-8973-3d47447115dc commented Jun 27, 2023 • edited Loading

ChunkyProgrammer left a comment

Choose a reason for hiding this comment

PikachuEXE commented Jun 28, 2023

efb4f5ff-1298-471a-8973-3d47447115dc left a comment

Choose a reason for hiding this comment

absidue commented Jul 1, 2023

absidue commented Jul 1, 2023

PikachuEXE commented Jul 2, 2023

absidue left a comment

Choose a reason for hiding this comment

PikachuEXE commented Jul 2, 2023

efb4f5ff-1298-471a-8973-3d47447115dc commented Jun 27, 2023 •

edited

Loading