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 Peertube account subscribers extraction #575

Merged
merged 4 commits into from
Mar 25, 2021

Conversation

AudricV
Copy link
Member

@AudricV AudricV commented Mar 5, 2021

Fix Peertube account subscribers extraction by adding the subscribers of all channels of an account to the subscribers of this account.

Minimal subscriber count for Framasoft account has been increased to 750 in the test.

@AudricV AudricV added bug Issue is related to a bug peertube service, https://joinpeertube.org/ labels Mar 5, 2021
@TobiGr
Copy link
Contributor

TobiGr commented Mar 5, 2021

I already took a look at that one, too. But there seems to be a huge difference between the API result for this channel and the follower count on the website: 491 vs 952
To be honest, I have no idea, why we get different data here.

@AudricV
Copy link
Member Author

AudricV commented Mar 5, 2021

When I loaded the first time the website, it shows 491 followers first briefly and after 952. I don't understand why.

@B0pol
Copy link
Member

B0pol commented Mar 5, 2021

I GOT IT

I noticed something : it is what we've previously called a "parent channel". The channel, owner channel has 491 subscribers, but all the different sub-channels, video-channels have 461 subscribers if you sum them all. That makes 952 subscribers

The request that gives 491: https://framatube.org/api/v1/accounts/framasoft

To have the full subscriber count you have to make another request to get video channels (add /video-channels)
https://framatube.org/api/v1/accounts/framasoft/video-channels
and add all subscribers count.

That's what the web client does according to a quick look at the network tab of devtools

@AudricV AudricV force-pushed the fix-peertube-test branch from 057bdc5 to 830e1e0 Compare March 14, 2021 17:42
@AudricV AudricV changed the title Fix Peertube test Fix Peertube subscribers extraction Mar 14, 2021
@AudricV AudricV changed the title Fix Peertube subscribers extraction Fix Peertube account subscribers extraction Mar 14, 2021
@AudricV
Copy link
Member Author

AudricV commented Mar 14, 2021

@B0pol I implement what you said, this should be done now.
Tests are failing but it's not related to the subscribers extraction.

@Stypox Stypox mentioned this pull request Mar 24, 2021
7 tasks
@Stypox
Copy link
Member

Stypox commented Mar 25, 2021

@B0pol @TobiGr could you merge this if you think it's ok?

Copy link
Contributor

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Looks good to me. We should add a comment about what is done here and why it is done, so other people going through the code will know.

@AudricV AudricV force-pushed the fix-peertube-test branch from a4622c3 to ae28331 Compare March 25, 2021 16:56
@AudricV
Copy link
Member Author

AudricV commented Mar 25, 2021

A Peertube test is failing because of a new thumbnail URL, which changes at every time the test is run.

@TobiGr TobiGr merged commit d4f83a1 into TeamNewPipe:dev Mar 25, 2021
@AudricV AudricV deleted the fix-peertube-test branch March 25, 2021 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug peertube service, https://joinpeertube.org/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants