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

Channels: Fix for live videos #5027

Closed
wants to merge 2 commits into from

Conversation

iBicha
Copy link
Contributor

@iBicha iBicha commented Oct 26, 2024

Closes #5021

This is an alternative to #5026 to keep the sort options working

After digging into the proto, live stream sorting has different values than channel videos

@iBicha iBicha requested a review from a team as a code owner October 26, 2024 16:42
@iBicha iBicha requested review from syeopite and removed request for a team October 26, 2024 16:42
@absidue
Copy link
Contributor

absidue commented Oct 26, 2024

As this one fixes the problem properly, without removing functionality, I would suggest that this pull request should be considered the proper fix and that the other one be considered as the alternative or even closed now that there is a proper fix (so the opposite of what the titles currently say).

@iBicha
Copy link
Contributor Author

iBicha commented Oct 26, 2024

As this one fixes the problem properly, without removing functionality, I would suggest that this pull request should be considered the proper fix and that the other one be considered as the alternative or even closed now that there is a proper fix (so the opposite of what the titles currently say).

fair enough

@iBicha iBicha changed the title [Alternative] Fix for channel live videos Fix for channel live videos Oct 26, 2024
@SamantazFox
Copy link
Member

This is an alternative to 5026 to keep the sort options working

I'll second absidue here: please close the other one as this one is more complete

@SamantazFox SamantazFox added the need-testing This feature needs to be deployed and tested to see if it's working, and doesn't break something label Oct 31, 2024
@Fijxu
Copy link
Contributor

Fijxu commented Nov 1, 2024

Applied to my instance.
Test URL: https://inv.nadeko.net/channel/UC7_YxT-KID8kRbqZo7MyscQ/streams

@SamantazFox
Copy link
Member

Applied to my instance. Test URL: https://inv.nadeko.net/channel/UC7_YxT-KID8kRbqZo7MyscQ/streams

Thanks for testing it out :)

@SamantazFox SamantazFox added ready and removed need-testing This feature needs to be deployed and tested to see if it's working, and doesn't break something labels Nov 5, 2024
Copy link
Contributor

@lifo9 lifo9 left a comment

Choose a reason for hiding this comment

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

Hey,
thanks for finding out!
Protobuf values also changed for regular videos.
Tested on my local instance and it works now.

src/invidious/channels/videos.cr Show resolved Hide resolved
src/invidious/channels/videos.cr Show resolved Hide resolved
@SamantazFox
Copy link
Member

SamantazFox commented Nov 8, 2024

Hey, thanks for finding out! Protobuf values also changed for regular videos. Tested on my local instance and it works now.

Thanks! It is being addressed in #5059

@iBicha
Copy link
Contributor Author

iBicha commented Nov 8, 2024

Superseded by #5059

@iBicha iBicha closed this Nov 8, 2024
@SamantazFox SamantazFox reopened this Nov 8, 2024
@SamantazFox SamantazFox closed this Nov 8, 2024
@SamantazFox
Copy link
Member

Oh, wait, nevermind, your commits are in my own PR, no need to merge them separately!

@SamantazFox SamantazFox changed the title Fix for channel live videos Channels: Fix for live videos Nov 8, 2024
@SamantazFox
Copy link
Member

Thanks for your contribution btw!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] "Request contains an invalid argument." when going to channels
5 participants