Skip to content

Fixed youtube subscription import: ignore ones with invalid url and keep ones with empty title.#160

Merged
TobiGr merged 4 commits intoTeamNewPipe:devfrom
Stypox:invalid-youtube-subscription-fix
May 14, 2019
Merged

Fixed youtube subscription import: ignore ones with invalid url and keep ones with empty title.#160
TobiGr merged 4 commits intoTeamNewPipe:devfrom
Stypox:invalid-youtube-subscription-fix

Conversation

@Stypox
Copy link
Member

@Stypox Stypox commented Apr 26, 2019

  • I carefully read the contribution guidelines and agree to them.
  • I did test the API against NewPipe.
  • [no breaking change] I agree to ASAP create a PULL request for NewPipe for making in compatible when I changed the api.

Now when the YoutubeSubscriptionExtractor parses the YouTube subscription_manager file, it ignores channels that have no title, instead of throwing an error: that file can sometimes natively contain channels with no title (i.e. deleted channels).

I tested the changes inside the NewPipe app against the file provided in TeamNewPipe/NewPipe#2226. I also added a unit test.

I left untouched all the other "document has invalid entries" exceptions, since they are not involved in the empty-title issue. I think this is ok.

Fix TeamNewPipe/NewPipe#2226

Stypox added 2 commits April 26, 2019 18:54
(in the youtube subscription extractor)
Ignore subscriptions that have an empty title instead of throwing an error: the youtube subscription_manager XML file can sometimes contain those (i.e. deleted channels).
(youtube subscription extractor)
@mauriciocolli
Copy link
Contributor

mauriciocolli commented Apr 27, 2019

Thanks for the pull request!

I think we should remove the check for the title, and leave only the url one (the essential part).

Like I commented in the issue, I think YouTube does this for channels deleted OR suspended. The channels that are suspended can come back (rarely), so what if the user export before it comes back and tries to import after it comes back? It just won't show up.

I say we should let this entry go and let the importing service handle it (which is simply ignoring the failing entry at this moment), what do you think?


Edit: After thinking a little more about it, should we fail immediately after we don't find a invalid url? Or be more lenient about it (just ignore the entry)?

I'm leaned towards being more lenient.

@TobiGr
Copy link
Contributor

TobiGr commented Apr 27, 2019

After thinking a little more about it, should we fail immediately after we don't find a invalid url? Or be more lenient about it (just ignore the entry)?

Yes, we should ignore and not fail to import. Maybe return the number of imports/urls which caused a problem and then display a toast / dialog which or how many subscriptions could not be imported.

@Stypox
Copy link
Member Author

Stypox commented Apr 28, 2019

I don't think it would be much useful for the user to know how many subscriptions couldn't be imported. And that number will be 0 most of the times (if not always: a subscription with an empty title still has a valid url). I would just do what mauriciocolli suggested: ignoring the extremely rare invalid entries.

Stypox added 2 commits April 28, 2019 14:17
if a channel if deleted (thus it has an empty title), it is imported in NewPipe anyway, so that if it becomes undeleted in the future, it will be shown in the app.
Also modified the test for empty title, since now  subscriptions with empty title are not ignored anymore.
@Stypox Stypox changed the title Fixed youtube subscription import: subscriptions with empty title are ignored. Fixed youtube subscription import: ignore ones with invalid url and keep ones with empty title. Apr 28, 2019
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.

Unable to import really long YouTube subscription list

3 participants