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

Fix channel page ID handling #2457

Merged
merged 2 commits into from
Aug 8, 2022

Conversation

absidue
Copy link
Member

@absidue absidue commented Aug 6, 2022


Fix channel page ID handling

Pull Request Type

  • Bugfix

Related issue
closes #2420
fixes a race condition introduced by #2259

Description
This pull request resolves two issues relating to the handling of channel IDs on the channel page.

  1. Treat /channel/, /user/ and /c/ channel URLs differently, as a /user/ and /c/ URLs with the same id might point to different channels.
  2. Fixes the race condition introduced by Fix: wrong info displayed when quickly navigate from a channel to another #2259, that happens if fetching the info is faster than the video or playlist fetching. The issue is that after the info is fetched it overwrites the id property with the actual channel ID, so if you entered this URL https://www.youtube.com/c/linustechtips, the id property is set to linustechtips and when the info request returns it overwrites it with the channel ID UCXuqSBlHAE6Xw-yeJA0Tunw, as the videos and playlists functions still expect the id to be linustechtips they abort.

The fix for this was to introduce an originalId property that is only updated when the actual channel changes, which is then used for the abort checks.

Testing (for code that is not small enough to be easily understandable)
I tested the following URLs:

Desktop (please complete the following information):

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.17.0

@absidue absidue added the PR: waiting for review For PRs that are complete, tested, and ready for review label Aug 6, 2022
@PrestonN PrestonN enabled auto-merge (squash) August 6, 2022 10:18
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Do we have a place to stored these "magic values"?

I certainly won't be able to parse all the constant values for idType in the future.
Not to mention I can't parse them now.

Edit 1: Something like this?

@absidue
Copy link
Member Author

absidue commented Aug 6, 2022

@PikachuEXE As those "magic values" come from yt-channel-info, if we create an enum it should definitely be in yt-channel-info and then imported into FreeTube.

https://github.com/FreeTubeApp/yt-channel-info/blob/development/app/helper.js#L501

Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Tested locally

All 3 provided URLs returned different channels

Copy link
Member

@ChunkyProgrammer ChunkyProgrammer left a comment

Choose a reason for hiding this comment

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

Works well, also tested to make sure subpaths worked (ie c/name/playlsts)

@PrestonN PrestonN merged commit 6334df0 into FreeTubeApp:development Aug 8, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Aug 8, 2022
@absidue absidue deleted the channel-page-fixes branch August 8, 2022 09:27
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.

[Bug]: Custom channel URLs resolve by username which is incorrect
5 participants