Migrate /mods command to helix API #4103

cbclemmer · 2022-11-03T15:37:43Z

Pull request checklist:

CHANGELOG.md was updated, if applicable

Description

Modeled after the /chatters migration PR #4088.

/mods command no longer uses the irc command and now makes a helix api request.
mod list is paginated just like chatter list.

cbclemmer · 2022-11-03T15:40:01Z

@pajlada I tried to follow the same paradigm as your changes to the /chatters api request.

github-actions

clang-tidy made some suggestions

src/controllers/commands/CommandController.cpp

src/providers/twitch/api/Helix.cpp

src/controllers/commands/CommandController.cpp

Felanbird · 2022-11-03T18:49:34Z

This PR can get helixTimegate applied to it, as we know approximately what day the previous command will be deprecated.

The related code to copy can be found in the /vips PR https://github.com/Chatterino/chatterino2/pull/4053/files

cbclemmer · 2022-11-03T19:15:59Z

@Felanbird I added the timegate logic.

github-actions

clang-tidy made some suggestions

src/controllers/commands/CommandController.cpp

src/widgets/settingspages/GeneralPage.cpp

Felanbird

A few things I noticed:

Clickable/colorized names have been regressed.
https://github.com/Chatterino/chatterino2/pull/3187/files was the original implementation
Response should be sorted alphabetically
needs a test in tests/src/HighlightController.cpp
example from /vips:

    // /vips
    // The extra parenthesis around the failure callback is because its type contains a comma
    MOCK_METHOD(
        void, getChannelVIPs,
        (QString broadcasterID,
         ResultCallback<std::vector<HelixVip>> successCallback,
         (FailureCallback<HelixListVIPsError, QString> failureCallback)),
        (override));  // /vips

src/controllers/commands/CommandController.cpp

cbclemmer · 2022-11-04T17:35:23Z

@Felanbird All items from your previous comment should be fixed.

github-actions

clang-tidy made some suggestions

tests/src/HighlightController.cpp

src/controllers/commands/CommandController.cpp

Co-authored-by: Felanbird <41973452+Felanbird@users.noreply.github.com>

Felanbird

Functionality works as expected 👍

pajlada

Small change requests - last PR before Helix migration is done

src/singletons/Settings.hpp

src/providers/twitch/api/Helix.hpp

We've chosen a limit of 500, but this could be updated if necessary in the future

…st (max 100 as per helix docs)

pajlada · 2022-11-05T10:50:56Z

I tried to follow the same paradigm as your changes to the /chatters api request.

You followed the paradigm really well, and it turns out my implementation actually didn't work! I made it work now for sure, and I'll make a PR later to fix the Chatters command too

github-actions

clang-tidy made some suggestions

github-actions · 2022-11-05T11:20:56Z

src/providers/twitch/TwitchMessageBuilder.cpp

@@ -1657,6 +1657,62 @@ void TwitchMessageBuilder::listOfUsersSystemMessage(QString prefix,
    }
 }

+void TwitchMessageBuilder::listOfUsersSystemMessage(


warning: method 'listOfUsersSystemMessage' can be made static [readability-convert-member-functions-to-static]

Suggested change

void TwitchMessageBuilder::listOfUsersSystemMessage(

}static

github-actions · 2022-11-05T11:20:57Z

src/providers/twitch/TwitchMessageBuilder.cpp

@@ -1657,6 +1657,62 @@
    }
 }

+void TwitchMessageBuilder::listOfUsersSystemMessage(
+    QString prefix, const std::vector<HelixModerator> &users, Channel *channel,


warning: the parameter 'prefix' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change

QString prefix, const std::vector<HelixModerator> &users, Channel *channel,

sage(const &

github-actions · 2022-11-05T11:20:57Z

src/providers/twitch/TwitchMessageBuilder.cpp

@@ -1657,6 +1657,62 @@
    }
 }

+void TwitchMessageBuilder::listOfUsersSystemMessage(
+    QString prefix, const std::vector<HelixModerator> &users, Channel *channel,


warning: parameter 'users' is unused [misc-unused-parameters]

Suggested change

QString prefix, const std::vector<HelixModerator> &users, Channel *channel,

sage( /*users*/

github-actions · 2022-11-05T11:20:57Z

src/providers/twitch/api/Helix.cpp

+
+using namespace chatterino;
+
+static constexpr auto NUM_MODERATORS_TO_FETCH_PER_REQUEST = 100;


warning: 'NUM_MODERATORS_TO_FETCH_PER_REQUEST' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]

Suggested change

static constexpr auto NUM_MODERATORS_TO_FETCH_PER_REQUEST = 100;

constexpr auto NUM_MODERATORS_TO_FETCH_PER_REQUEST = 100;

github-actions · 2022-11-05T11:20:57Z

src/providers/twitch/api/Helix.cpp

@@ -1859,6 +1867,115 @@
        .execute();
 }

+void Helix::onFetchModeratorsSuccess(


warning: method 'onFetchModeratorsSuccess' can be made static [readability-convert-member-functions-to-static]

Suggested change

void Helix::onFetchModeratorsSuccess(

static void Helix::onFetchModeratorsSuccess(

github-actions · 2022-11-05T11:20:58Z

src/providers/twitch/api/Helix.cpp

+
+// https://dev.twitch.tv/docs/api/reference#get-moderators
+void Helix::fetchModerators(
+    QString broadcasterID, int first, QString after,


warning: the parameter 'broadcasterID' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change

QString broadcasterID, int first, QString after,

const QString& broadcasterID, int first, QString after,

github-actions · 2022-11-05T11:20:58Z

src/providers/twitch/api/Helix.cpp

+
+// https://dev.twitch.tv/docs/api/reference#get-moderators
+void Helix::fetchModerators(
+    QString broadcasterID, int first, QString after,


warning: the parameter 'after' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change

QString broadcasterID, int first, QString after,

QString broadcasterID, int first, const QString& after,

github-actions · 2022-11-05T11:20:58Z

src/providers/twitch/api/Helix.cpp

+                case 401: {
+                    if (message.startsWith("Missing scope",
+                                           Qt::CaseInsensitive))
+                    {


warning: repeated branch in conditional chain [bugprone-branch-clone]

{ ^

src/providers/twitch/api/Helix.cpp:1949: end of the original

} ^

src/providers/twitch/api/Helix.cpp:1951: clone 1 starts here

{ ^

src/providers/twitch/api/Helix.cpp:1955: clone 2 starts here

{ ^

github-actions · 2022-11-05T11:20:58Z

src/providers/twitch/api/Helix.cpp

@@ -2106,6 +2223,25 @@
                        fetchSuccess(fetchSuccess), failureCallback);
 }

+// https://dev.twitch.tv/docs/api/reference#get-moderators
+void Helix::getModerators(


warning: method 'getModerators' can be made static [readability-convert-member-functions-to-static]

Suggested change

void Helix::getModerators(

static void Helix::getModerators(

github-actions · 2022-11-05T11:20:59Z

src/providers/twitch/api/Helix.cpp

@@ -2106,6 +2223,25 @@
                        fetchSuccess(fetchSuccess), failureCallback);
 }

+// https://dev.twitch.tv/docs/api/reference#get-moderators
+void Helix::getModerators(
+    QString broadcasterID, int maxModeratorsToFetch,


warning: the parameter 'broadcasterID' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change

QString broadcasterID, int maxModeratorsToFetch,

const QString& broadcasterID, int maxModeratorsToFetch,

cbclemmer added 2 commits November 3, 2022 10:33

Override /mods command to use helix API

998225e

Add changelog item

f3856ef

Run clang format

c730ca7

github-actions bot reviewed Nov 3, 2022

View reviewed changes

Felanbird reviewed Nov 3, 2022

View reviewed changes

src/controllers/commands/CommandController.cpp Outdated Show resolved Hide resolved

cbclemmer mentioned this pull request Nov 3, 2022

Migrate /mods command to Helix API #3971

Closed

cbclemmer added 2 commits November 3, 2022 13:53

Change auth error message

55dd12b

Add timegate logic

6d4e543

github-actions bot reviewed Nov 3, 2022

View reviewed changes

src/controllers/commands/CommandController.cpp Show resolved Hide resolved

src/controllers/commands/CommandController.cpp Show resolved Hide resolved

src/widgets/settingspages/GeneralPage.cpp Show resolved Hide resolved

Felanbird requested changes Nov 3, 2022

View reviewed changes

src/controllers/commands/CommandController.cpp Outdated Show resolved Hide resolved

src/controllers/commands/CommandController.cpp Outdated Show resolved Hide resolved

Felanbird linked an issue Nov 4, 2022 that may be closed by this pull request

Migrate /mods command to Helix API #3971

Closed

Use TwitchMessageBuilder::listOfUsersSystemMessage function

597ff3e

clang-format

01c6ba0

github-actions bot reviewed Nov 4, 2022

View reviewed changes

tests/src/HighlightController.cpp Show resolved Hide resolved

Felanbird reviewed Nov 4, 2022

View reviewed changes

src/controllers/commands/CommandController.cpp Outdated Show resolved Hide resolved

Remove extra whitespace

008587d

Co-authored-by: Felanbird <41973452+Felanbird@users.noreply.github.com>

Felanbird approved these changes Nov 4, 2022

View reviewed changes

Felanbird requested a review from pajlada November 4, 2022 18:22

pajlada requested changes Nov 5, 2022

View reviewed changes

src/singletons/Settings.hpp Outdated Show resolved Hide resolved

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

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

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

pajlada added 6 commits November 5, 2022 11:18

fix "user not authorized" error message

764edf8

Fix recursion (AGAIN) - sorry!

115eec7

Fix getModerators comment

f1e2611

Add parameter to limit amount of moderators that are fetched

6add8f3

We've chosen a limit of 500, but this could be updated if necessary in the future

fetchModerators now accepts number of moderators to fetch per reque…

5c8e196

…st (max 100 as per helix docs)

fix spelling mistake in moderators helix timegate setting

bb1f107

pajlada added 3 commits November 5, 2022 11:53

Fix display-name-users not being clickable for CJK

521e8cc

Merge remote-tracking branch 'origin/master' into cc/Chatterino#3971

15c7255

Fix missing header

5799630

pajlada approved these changes Nov 5, 2022

View reviewed changes

pajlada enabled auto-merge (squash) November 5, 2022 11:01

pajlada merged commit e531161 into Chatterino:master Nov 5, 2022

github-actions bot reviewed Nov 5, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate /mods command to helix API #4103

Migrate /mods command to helix API #4103

cbclemmer commented Nov 3, 2022 •

edited

cbclemmer commented Nov 3, 2022

github-actions bot left a comment

Felanbird commented Nov 3, 2022

cbclemmer commented Nov 3, 2022

github-actions bot left a comment

Felanbird left a comment •

edited

cbclemmer commented Nov 4, 2022

github-actions bot left a comment

Felanbird left a comment

pajlada left a comment

pajlada commented Nov 5, 2022

github-actions bot left a comment

github-actions bot Nov 5, 2022

github-actions bot Nov 5, 2022

github-actions bot Nov 5, 2022

github-actions bot Nov 5, 2022

github-actions bot Nov 5, 2022

github-actions bot Nov 5, 2022

github-actions bot Nov 5, 2022

github-actions bot Nov 5, 2022

github-actions bot Nov 5, 2022

github-actions bot Nov 5, 2022

	QString prefix, const std::vector<HelixModerator> &users, Channel *channel,
	sage(const &

	QString prefix, const std::vector<HelixModerator> &users, Channel *channel,
	sage( /users/


		using namespace chatterino;

		static constexpr auto NUM_MODERATORS_TO_FETCH_PER_REQUEST = 100;

	static constexpr auto NUM_MODERATORS_TO_FETCH_PER_REQUEST = 100;
	constexpr auto NUM_MODERATORS_TO_FETCH_PER_REQUEST = 100;

	void Helix::onFetchModeratorsSuccess(
	static void Helix::onFetchModeratorsSuccess(

	QString broadcasterID, int first, QString after,
	const QString& broadcasterID, int first, QString after,

	QString broadcasterID, int first, QString after,
	QString broadcasterID, int first, const QString& after,

	QString broadcasterID, int maxModeratorsToFetch,
	const QString& broadcasterID, int maxModeratorsToFetch,

Migrate /mods command to helix API #4103

Migrate /mods command to helix API #4103

Conversation

cbclemmer commented Nov 3, 2022 • edited

Description

cbclemmer commented Nov 3, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

Felanbird commented Nov 3, 2022

cbclemmer commented Nov 3, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

Felanbird left a comment • edited

Choose a reason for hiding this comment

cbclemmer commented Nov 4, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

Felanbird left a comment

Choose a reason for hiding this comment

pajlada left a comment

Choose a reason for hiding this comment

pajlada commented Nov 5, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Nov 5, 2022

Choose a reason for hiding this comment

github-actions bot Nov 5, 2022

Choose a reason for hiding this comment

github-actions bot Nov 5, 2022

Choose a reason for hiding this comment

github-actions bot Nov 5, 2022

Choose a reason for hiding this comment

github-actions bot Nov 5, 2022

Choose a reason for hiding this comment

github-actions bot Nov 5, 2022

Choose a reason for hiding this comment

github-actions bot Nov 5, 2022

Choose a reason for hiding this comment

github-actions bot Nov 5, 2022

Choose a reason for hiding this comment

github-actions bot Nov 5, 2022

Choose a reason for hiding this comment

github-actions bot Nov 5, 2022

Choose a reason for hiding this comment

cbclemmer commented Nov 3, 2022 •

edited

Felanbird left a comment •

edited