Migrate Twitch badges to Helix #4537

ZonianMidian · 2023-04-11T19:27:29Z

Pull request checklist:

CHANGELOG.md was updated, if applicable

Description

The legacy badges endpoint will be closed for the first time on May 4 and permanently on June 1, so it is necessary to migrate this endpoint to the Helix API.
Official announcement: https://discuss.dev.twitch.tv/t/legacy-badges-endpoint-shutdown-details-and-timeline-june-2023/44621

src/providers/twitch/api/Helix.hpp

iProdigy · 2023-04-12T04:53:43Z

Did you forget to commit HelixGlobalBadges/HelixChannelBadges ?

ZonianMidian · 2023-04-12T08:09:08Z

Did you forget to commit HelixGlobalBadges/HelixChannelBadges ?

That's right, I hadn't done it yet. Recently added in b997784

src/providers/twitch/api/Helix.hpp

Co-authored-by: iProdigy <[email protected]>

iProdigy

nit: can remove Content-Type header as there is no request body

src/providers/twitch/api/Helix.cpp

iProdigy · 2023-04-12T08:33:47Z

Be sure to define virtual functions in IHelix for getGlobalBadges/getChannelBadges

…nianMidian/chatterino2 into helix-migration/twitch-badges

ZonianMidian · 2023-04-12T19:19:42Z

Be sure to define virtual functions in IHelix for getGlobalBadges/getChannelBadges

That's right, I was missing the definition of virtual variables in IHelix. Recently added in cd0c55d

src/providers/twitch/api/Helix.hpp

…Badges

tests/src/HighlightController.cpp

Co-authored-by: iProdigy <[email protected]>

Felanbird · 2023-04-13T15:20:50Z

Checking this PR from the artifact loads all badges as expected

pajlada

Looks good so far!
The two remaining steps I see are:

Modify the TwitchBadges.cpp function loadTwitchBadges to instead use the method you created getGlobalBadges()
Modify the TwitchChannel.cpp function refreshBadges to instead use the method you created getChannelBadges(broadcasterID)

After that's done, the PR is ready for testing! 🧇

dnsge · 2023-04-15T02:24:51Z

https://twitter.com/TwitchDev/status/1647017279123513344

Looks like the endpoint has been updated with some more info, not sure if anything in this PR needs to change. Perhaps the new info should parsed into our badge structs? Unclear if we would even use it.

ZonianMidian · 2023-04-15T02:38:18Z

Looks like the endpoint has been updated with some more info, not sure if anything in this PR needs to change. Perhaps the new info should parsed into our badge structs? Unclear if we would even use it.

I believe that at the time of starting this PR those data were already available since they were released together with the official announcement. I guess it was only now that it was made public on Twitter 🤔

Felanbird · 2023-04-15T03:35:06Z

I believe that at the time of starting this PR those data were already available since they were released together with the official announcement. I guess it was only now that it was made public on Twitter 🤔

This is correct, @iProdigy confirmed all the data available from the legacy endpoint was also available in the Helix response the day after the deprecation announcement.

…nianMidian/chatterino2 into helix-migration/twitch-badges

iProdigy · 2023-04-16T01:51:21Z

src/providers/twitch/TwitchBadges.cpp

+                }
+                break;
+            }
+            qCWarning(chatterinoTwitch) << errorMessage;
            QFile file(":/twitch-badges.json");


In a follow-up PR, we can convert twitch-badges.json to use the new helix structure (this would allow enable a refactor of parseTwitchBadges that leverages the new structs from this PR).

This prevents the parameter from being copied for each invocation - since we know the lifetime of the object this is not a problem

There's not really a big gain in any particular way, but it makes it, imo, more readable since it's less code

We don't use the default constructor, so we don't need it defined Bonus: This also adds a newline between the member variables & the constructor

The Helix API calls it data, but for us it makes more sense to call it `badgeSets`

also include identifiers in the message itself to make it more identifiable

get is already the default request type if one is not specified (see NetworkRequest.hpp)

This is done by capturing a weak pointer of the channel into the success & failure lambdas, then verifying that we're able to lock them when the request has finished

…tch-badges

pajlada

Thank you! As a first-time contributor, you can now add yourself to the contributors list that's shown inside the About page in Chatterino.

If you want this, you can open a new PR where you modify the resources/contributors.txt file and add yourself to the list. Make sure to read the comments at the top for instructions.

ZonianMidian added 3 commits April 11, 2023 14:21

Initial implementation of Helix.cpp & Helix.hpp

9abe788

Add changelog item

ad94591

Class declaration for errors

e538c4a

iProdigy reviewed Apr 12, 2023

View reviewed changes

src/providers/twitch/api/Helix.hpp Outdated Show resolved Hide resolved

ZonianMidian added 2 commits April 12, 2023 02:57

New implementations and fixes for Helix.cpp and Helix.hpp

b997784

Minor correction

cdaf33c

iProdigy reviewed Apr 12, 2023

View reviewed changes

src/providers/twitch/api/Helix.hpp Outdated Show resolved Hide resolved

Class correction for clickURL in Helix.hpp

e0db1e4

Co-authored-by: iProdigy <[email protected]>

iProdigy reviewed Apr 12, 2023

View reviewed changes

src/providers/twitch/api/Helix.cpp Outdated Show resolved Hide resolved

src/providers/twitch/api/Helix.cpp Outdated Show resolved Hide resolved

ZonianMidian added 3 commits April 12, 2023 14:00

Removed unnecessary Content-Type

ae4e27a

Merge branch 'helix-migration/twitch-badges' of https://github.com/Zo…

8c99efb

…nianMidian/chatterino2 into helix-migration/twitch-badges

Virtual functions getGlobalBadges and getChannelBadges defined in IHelix

cd0c55d

Added missing

ca8653d

dnsge reviewed Apr 13, 2023

View reviewed changes

src/providers/twitch/api/Helix.hpp Outdated Show resolved Hide resolved

Added mock method to MockHelix for HelixGlobalBadges and HelixChannel…

8702f8a

…Badges

iProdigy reviewed Apr 13, 2023

View reviewed changes

tests/src/HighlightController.cpp Outdated Show resolved Hide resolved

Added extra parenthesis in mock method

88cb070

Co-authored-by: iProdigy <[email protected]>

pajlada requested changes Apr 13, 2023

View reviewed changes

ZonianMidian added 2 commits April 14, 2023 03:47

Correction of file formatting

a0841cb

Merge branch 'master' into helix-migration/twitch-badges

1218dc3

ZonianMidian added 2 commits April 14, 2023 23:51

Merge branch 'master' into helix-migration/twitch-badges

4f8fb94

Implementation of getGlobalsBadges in TwitchBadges.cpp

97c4f52

ZonianMidian added 5 commits April 15, 2023 18:23

Merge branch 'helix-migration/twitch-badges' of https://github.com/Zo…

dbff8bb

…nianMidian/chatterino2 into helix-migration/twitch-badges

Merge branch 'master' into helix-migration/twitch-badges

521f297

Fixing error message in TwitchBadges.cpp

30811e9

Merge branch 'helix-migration/twitch-badges' of https://github.com/Zo…

91677d6

…nianMidian/chatterino2 into helix-migration/twitch-badges

Implementation of getChannelBadges in TwitchChannel.cpp

eb59af8

ZonianMidian requested a review from pajlada April 16, 2023 01:43

iProdigy reviewed Apr 16, 2023

View reviewed changes

ZonianMidian and others added 17 commits April 15, 2023 20:55

README update on Helix to add the new endpoints

58b07dd

nit: const ref HelixBadgeVersion constructor parameter

20feb1c

This prevents the parameter from being copied for each invocation - since we know the lifetime of the object this is not a problem

nit: Rename json_versions to jsonVersions

17ca5ea

nit: rename badge_version to badgeVersion

17ac301

nit: use emplace_back to in-place construct badges

f50dae6

There's not really a big gain in any particular way, but it makes it, imo, more readable since it's less code

nit: Remove HelixGlobalBadges default constructor

1b4f04f

We don't use the default constructor, so we don't need it defined Bonus: This also adds a newline between the member variables & the constructor

nit: rename badges member data to badgeSets

85c3517

The Helix API calls it data, but for us it makes more sense to call it `badgeSets`

nit: Change the rare helix error cases to be warnings

3a84760

also include identifiers in the message itself to make it more identifiable

nit: add space inbetween using & the makeRequest/variable definitions

3672216

nit: don't specify the network request type to get

8ccfb99

get is already the default request type if one is not specified (see NetworkRequest.hpp)

fix: we must make sure the channel is alive when channel badges finish

35d90f3

This is done by capturing a weak pointer of the channel into the success & failure lambdas, then verifying that we're able to lock them when the request has finished

nit: Reformat Emote creation

ffeb297

nit: Remove const ref in emote creation

f1907c9

nit: add back scale for 1x url

f6c855b

nit: keep EmoteName{} for specifying an empty emote name

ef89d7c

nit: Don't recreate the global badges

1f31077

Merge remote-tracking branch 'origin/master' into helix-migration/twi…

cc50428

…tch-badges

pajlada approved these changes Apr 16, 2023

View reviewed changes

pajlada enabled auto-merge (squash) April 16, 2023 09:23

pajlada merged commit d6ef48d into Chatterino:master Apr 16, 2023

Felanbird mentioned this pull request Apr 16, 2023

Twitch badges (mods, subs, etc.) #4071

Closed

4 tasks

ilya-zlobintsev mentioned this pull request Jan 30, 2024

Add developer_name appdata field ilya-zlobintsev/chatterino2#3

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Twitch badges to Helix #4537

Migrate Twitch badges to Helix #4537

ZonianMidian commented Apr 11, 2023 •

edited

Loading

iProdigy commented Apr 12, 2023

ZonianMidian commented Apr 12, 2023

iProdigy left a comment

iProdigy commented Apr 12, 2023

ZonianMidian commented Apr 12, 2023

Felanbird commented Apr 13, 2023

pajlada left a comment

dnsge commented Apr 15, 2023

ZonianMidian commented Apr 15, 2023

Felanbird commented Apr 15, 2023

iProdigy Apr 16, 2023 •

edited

Loading

pajlada left a comment

Migrate Twitch badges to Helix #4537

Migrate Twitch badges to Helix #4537

Conversation

ZonianMidian commented Apr 11, 2023 • edited Loading

Description

iProdigy commented Apr 12, 2023

ZonianMidian commented Apr 12, 2023

iProdigy left a comment

Choose a reason for hiding this comment

iProdigy commented Apr 12, 2023

ZonianMidian commented Apr 12, 2023

Felanbird commented Apr 13, 2023

pajlada left a comment

Choose a reason for hiding this comment

dnsge commented Apr 15, 2023

ZonianMidian commented Apr 15, 2023

Felanbird commented Apr 15, 2023

iProdigy Apr 16, 2023 • edited Loading

Choose a reason for hiding this comment

pajlada left a comment

Choose a reason for hiding this comment

ZonianMidian commented Apr 11, 2023 •

edited

Loading

iProdigy Apr 16, 2023 •

edited

Loading