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 broken yt likes in comments #628

Merged
merged 16 commits into from
May 29, 2021

Conversation

litetex
Copy link
Member

@litetex litetex commented May 20, 2021

  • I carefully read the contribution guidelines and agree to them.
  • I have tested the API against NewPipe.
  • I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.

This PR fixes TeamNewPipe/NewPipe#6323 and does some minor clean up / code de-duplications.
Related PR in NewPipe project: TeamNewPipe/NewPipe#6337

⚠️ The build currently fails, because of outdated mock data
If someone of the maintainers could update the mock data (PR is editable) or tell me how to do it, the test should work again.

⚠️
Youtube now translates the voteCount (ex. likeCount) into the corresponding language. So in English 3033 is displayed in 3K while in German/Deutsch it's displayed as 3.033.

However this doesn't seem to work correctly in the app (e.g. when the "Default Content Language" is set) , because SUPPORTED_LANGUGAGES only contains "en-GB":

// https://www.youtube.com/picker_ajax?action_language_json=1
private static final List<Localization> SUPPORTED_LANGUAGES = Localization.listFrom(
"en-GB"
/*"af", "am", "ar", "az", "be", "bg", "bn", "bs", "ca", "cs", "da", "de",
"el", "en", "en-GB", "es", "es-419", "es-US", "et", "eu", "fa", "fi", "fil", "fr",
"fr-CA", "gl", "gu", "hi", "hr", "hu", "hy", "id", "is", "it", "iw", "ja",
"ka", "kk", "km", "kn", "ko", "ky", "lo", "lt", "lv", "mk", "ml", "mn",
"mr", "ms", "my", "ne", "nl", "no", "pa", "pl", "pt", "pt-PT", "ro", "ru",
"si", "sk", "sl", "sq", "sr", "sr-Latn", "sv", "sw", "ta", "te", "th", "tr",
"uk", "ur", "uz", "vi", "zh-CN", "zh-HK", "zh-TW", "zu"*/
);

When using tests I managed to get it working using this demo test:

YoutubeCommentsExtractorTest.java
....

public static class TEST {
        private final static String url = "https://www.youtube.com/watch?v=QqsLTNkzvaY";
        private static YoutubeCommentsExtractor extractor;

        @BeforeClass
        public static void setUp() throws Exception {
            YoutubeParsingHelper.resetClientVersionAndKey();
            YoutubeParsingHelper.setNumberGenerator(new Random(1));
            NewPipe.init(DownloaderTestImpl.init(null));
            extractor = (YoutubeCommentsExtractor) YouTube
                    .getCommentsExtractor(url);
            // Force lang
            extractor.forceLocalization(Localization.fromLocale(Locale.GERMANY));
            extractor.fetchPage();
        }

        @Test
        public void testGetCommentsAllData() throws IOException, ExtractionException {
            final InfoItemsPage<CommentsInfoItem> comments = extractor.getInitialPage();

            DefaultTests.defaultTestListOfItems(YouTube, comments.getItems(), comments.getErrors());

            CommentsInfoItem comment = comments.getItems().get(0);
            assertTrue("First comment isn't pinned", comment.isPinned());
            System.out.println(comment.getTextualVoteCount()); // Prints the votes for demo purposes
        }
    }

@XiangRongLin
Copy link
Collaborator

If someone of the maintainers could update the mock data (PR is editable) or tell me how to do it, the test should work again.

Start from the doc in DownloaderFactory. If that is not enough report back, so i can adjust it.

Copy link
Member

@B0pol B0pol left a comment

Choose a reason for hiding this comment

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

don't remove getLikeCount(), convert the string to int using helpers method (Utils.mixedNumberWordToLong() should do the job)

@litetex
Copy link
Member Author

litetex commented May 21, 2021

Restored the likeCount and wrote some test.
However some tests - which are not related to the changed code - still seem to fail. These may have been broken a longer time, taking a look at GH actions.

@XiangRongLin
Copy link
Collaborator

I should have added that only the mocks for tests related to you changes need to be updated. Especially since as you noted, there is broken stuff that you "uncover" like this, causing the build in your PR to fail

@litetex
Copy link
Member Author

litetex commented May 21, 2021

@XiangRongLin
Give me a moment I will remove the related commit and add only the affected files...
Should be fixed now 😄

Copy link
Collaborator

@XiangRongLin XiangRongLin left a comment

Choose a reason for hiding this comment

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

Can you also check the changes against newpipe. The default methods in interfaces should work on android, but i would like to have that verified.
Best if you can just upload an apk, so the people from the issue can test it out

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@litetex litetex force-pushed the fix-broken-yt-liked-comments branch from 1ac09e7 to 12fb18c Compare May 24, 2021 16:05
@litetex
Copy link
Member Author

litetex commented May 24, 2021

@XiangRongLin

Screenshots

Results when using NewPipe (dev branch) now (without changes to use the new textualVoteCount):
grafik grafik

Testing

The untested (I'm just using android studio 😉) apk (built with gradlew.bat assembleDebug lintDebug testDebugUnitTest --stacktrace):
debug.zip

Note: I couldn't find any instructions how to build it (in a hurry) and therefore used the CI instructions

If it doesn't work build it yourself with this manual:

@XiangRongLin XiangRongLin requested a review from B0pol May 26, 2021 15:54
Copy link
Member

@B0pol B0pol left a comment

Choose a reason for hiding this comment

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

About NewPipe, you checked

[x] I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.

There is no API change so it will work, but NewPipe could be improved.

You could check if it is from YouTube service, therefore you round the count using Localization.shortCount().

It is non-blocking but will be appreciated.

@B0pol B0pol merged commit ff11c2d into TeamNewPipe:dev May 29, 2021
@litetex litetex deleted the fix-broken-yt-liked-comments branch May 30, 2021 17:47
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.

Likes aren't shown in the YT videos comments
3 participants