-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Hide member only videos (Local API) #7208
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
Hide member only videos (Local API) #7208
Conversation
My understanding was that we were going to always hide them from the channel and subscription pages, by skipping them during the parsing, like we do with the members-only section on the channel home tab, without a setting or labelling them. There is no point showing videos that you have no way of watching in FreeTube (you require a Google account that is paying a subscription to the channel in question). So that begs the question, is this just a temporary PR to learn how to identify members-only videos and you'll revert the changes and implement the skipping in a follow up or have you decided to change plans? |
I think it depends on the channel I might just hide them if IV API supports it already |
The plan is definitely to go back to state FT was in before. Dont show member only content anywhere like explained in #6437 At this point we have many features that are only on Local API that arent available on IV. I think the route needs to be:
|
I will wait for #7187 to avoid merge conflict... |
e449ab8
to
e1533bf
Compare
Updated to hide member only videos |
I got videos with |
Yes, hide it. Nice catch. How it looks like on YT Edit: maybe also add it to https://github.com/FreeTubeApp/FreeTube/blob/development/src/renderer/helpers/api/local.js#L911 2nd edit:
I wonder how it will present it in FT when it becomes available to everyone |
Just normal video Also I never see member only/first videos in home tab |
The channel home tab has a dedicated member's only section and we already skip parsing that entire section (unlike the approach in this PR which parses members-only stuff and then filters it out right at the end), so I'm pretty sure they won't show any other members-only stuff there. The only place I've seen members-only and members-first is on the channel videos tab, which is is also used on the subscriptions page. |
FYI: PR on IV side iv-org/invidious#5222. Do we need to do something after that PR has merged? |
Yes and that PR does not include member first currently, already commented there |
Added Member first handling but not tested yet, see updated testing section |
Updated but the member first channel example doesn't have video a.t.m. so I can't test yet... |
* development: (27 commits) [Dev] Bump package version from 0.23.3 to 0.23.4 (FreeTubeApp#7255) Update youtubei.js to latest (FreeTubeApp#7252) Translated using Weblate (Bulgarian) Translated using Weblate (Norwegian Bokmål) Translated using Weblate (Norwegian Bokmål) Translated using Weblate (Danish) Translated using Weblate (Awadhi) Translated using Weblate (Flemish (West)) Translated using Weblate (Afrikaans) Translated using Weblate (Odia) Translated using Weblate (Urdu) Translated using Weblate (Georgian) Translated using Weblate (Azerbaijani) Translated using Weblate (Khmer (Central)) Translated using Weblate (Nepali) Translated using Weblate (Persian) Translated using Weblate (Bengali) Translated using Weblate (Bosnian) Translated using Weblate (Hindi) Translated using Weblate (Korean) ...
Anyone got more example channels with member first videos...? |
Nope couldnt find any but i trust the code |
I got a bug in video searching in channel, fixing today |
Probably because you left all the other code in when you added in my suggestion, instead of using it as a replacement as it was intended to be. |
* development: Ensure there is at most one power save blocker per open window (FreeTubeApp#7247) Change author name (FreeTubeApp#7216) Migrate ExternalPlayerSettings and SponsorBlockSettings to the composition API (FreeTubeApp#7267) Migrate the FtButton component to the composition API (FreeTubeApp#7266) Cleanup event listener pass-through inline handlers (FreeTubeApp#7268) Feature/add Windows/MacOS Taskbar dock/jumplist task 'New Window' (FreeTubeApp#7049) Translated using Weblate (Hebrew) Translated using Weblate (Norwegian Bokmål) Translated using Weblate (Greek)
Updated, channel searching code was updated to use Member first re-tested like #7208 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding extra properties and creating extra arrays all over FreeTube for something that only appears on the channel videos tab (and therefore also on the subscription videos tab), to me at least it makes a lot more sense to do the filtering in the
parseLocalChannelVideos
function in thesrc/renderer/helpers/api/local.js
This is still not properly addressed, yes you copied my provided code snippet but didn't actually remove any of the other code. I know you might not care about performance but I do, so I would feel uncomfortable approving and merging something that adds filtering all over FreeTube when it is only needed in one single place. I'm using just that code snippet in my custom build and it is working great. We could always merge this as is and then I could do a follow up PR to clean it up, but that seems wrong too, so it is probably better to get it right in this pull request.
If you think about it the fact that member's only videos only appear on the channel page makes a lot of sense, there it's a "if you liked other videos by this channel you may also like their members only videos" but on the hashtag page, nobody is going to decide to sign up for a paid subscription for a channel they've not watched any videos from, just to watch one members only video.
src/renderer/helpers/api/local.js
Outdated
hasCaptions: video.has_captions | ||
isMemberOnly: video.badges.some(badge => badge.label === 'Members only'), | ||
isMemberFirst: video.badges.some(badge => badge.label === 'Members first'), | ||
hasCaptions: video.has_captions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert
videoList = videoList.filter(item => { | ||
return !item.isMemberOnly && !item.isMemberFirst | ||
}) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
latestVideos.value = latestVideos.value.concat(parseLocalChannelVideos(continuation.videos, id.value, channelName.value)) | ||
latestVideos.value = latestVideos.value.concat( | ||
parseLocalChannelVideos(continuation.videos, id.value, channelName.value) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
latestLive.value = latestLive.value.concat(parseLocalChannelVideos(continuation.videos, id.value, channelName.value)) | ||
latestLive.value = latestLive.value.concat( | ||
parseLocalChannelVideos(continuation.videos, id.value, channelName.value) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
.filter(item => item.type === 'Video' || (!hideChannelPlaylists.value && item.type === 'Playlist')) | ||
.map(item => { | ||
.reduce((items, item) => { | ||
if (item.type === 'Video') { | ||
return parseLocalListVideo(item) | ||
} else { | ||
return parseLocalListPlaylist(item, id.value, channelName.value) | ||
const video = parseLocalListVideo(item) | ||
if (video.isMemberOnly || video.isMemberFirst) { return null } | ||
|
||
items.push(video) | ||
} else if (!hideChannelPlaylists.value && item.type === 'Playlist') { | ||
items.push(parseLocalListPlaylist(item, id.value, channelName.value)) | ||
} | ||
}) | ||
return items | ||
}, []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
videos.value = hashtagData.videos.map((video) => parseLocalListVideo(video)) | ||
videos.value = hashtagData.videos.reduce((results, video) => { | ||
const v = parseLocalListVideo(video) | ||
if (v.isMemberOnly || v.isMemberFirst) { | ||
results.push(v) | ||
} | ||
return results | ||
}, []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
const newVideos = continuation.videos.map((video) => parseLocalListVideo(video)) | ||
const newVideos = continuation.videos.reduce((results, video) => { | ||
const v = parseLocalListVideo(video) | ||
if (v.isMemberOnly || v.isMemberFirst) { | ||
results.push(v) | ||
} | ||
return results | ||
}, []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
src/renderer/views/Watch/Watch.js
Outdated
forbiddenTitles.some((text) => video.title?.toLowerCase().includes(text.toLowerCase())) | ||
forbiddenTitles.some((text) => video.title?.toLowerCase().includes(text.toLowerCase())) || | ||
(video.isMemberOnly || video.isMemberFirst) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
Updated, was mostly trying to quick fix the bug I introduce so I can get a custom build that works I know I suck in many ways, performance side included. So I have no problem receiving criticism on implementations and act if necessary. |
This was indeed not necessary to say. A review can be critical but shouldnt be personal.
Didnt notice any of them but if you do then you also dont suck in many ways :) |
Ive been using this PR and its been working well, i haven't been doing performance testing though, just general use. Do we have a known workflow for performance testing here? if someone has one they use it might be good to share and refer to later. I couldn't find anything on testing in the Using at this commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really sorry for how I expressed myself, you definitely didn't deserve those harsh words.
@ChunkyProgrammer your power is needed |
* development: (35 commits) Hide member only videos (Local API) (FreeTubeApp#7208) Translated using Weblate (Hebrew) Translated using Weblate (Spanish (Mexico)) Bump electron-builder from 26.0.14 to 26.0.15 (FreeTubeApp#7362) Bump mikefarah/yq from 4.45.1 to 4.45.2 (FreeTubeApp#7355) Bump autolinker from 4.1.1 to 4.1.5 (FreeTubeApp#7359) Bump the babel group with 3 updates (FreeTubeApp#7360) Bump npm-run-all2 from 7.0.2 to 8.0.1 (FreeTubeApp#7361) Bump the eslint group with 3 updates (FreeTubeApp#7356) Bump shaka-player from 4.14.9 to 4.14.10 (FreeTubeApp#7357) Translated using Weblate (Icelandic) Translated using Weblate (German) Translated using Weblate (French) Translated using Weblate (Basque) Translated using Weblate (Italian) Translated using Weblate (Chinese (Simplified Han script)) Translated using Weblate (German) Translated using Weblate (Portuguese (Brazil)) Translated using Weblate (Estonian) Translated using Weblate (Portuguese (Brazil)) ...
Pull Request Type
Related issue
Related to #6425 but IV API not supported yet so won't close it
Description
There are member only videos in subscription/channel video list
This PR hides them
IV API has no such attribute so can't implement for it
Screenshots
Badge version screenshot (previous implementation, now for showing example member only videos)


Testing
A. Member only
B. Member first
Same as above but it might be time limited (you can only test during specific time period each day, e.g. https://www.youtube.com/channel/UCy8E4XShrcph5WWLg05zETQ only has 1 member first video per day several hours before 18:00 UTC+8/10:00 UTC)
Desktop
Additional context