[Server/Client] Unify report categories (#7345)

* [Server/Client] Unify report categories into one shared source of truth

* Lont.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

cockatrice/src/interface/widgets/dialogs/dlg_report_user.cpp

@@ -15,6 +15,7 @@
 #include <QVBoxLayout>
 #include <libcockatrice/protocol/pb/command_report.pb.h>
 #include <libcockatrice/protocol/pending_command.h>
+#include <libcockatrice/utility/report_categories.h>
 
 DlgReportUser::DlgReportUser(AbstractClient *_client,
                              const QString &_reportedUser,
@@ -56,22 +57,14 @@ DlgReportUser::DlgReportUser(AbstractClient *_client,
     auto *categoryGrid = new QGridLayout(categoryGroup);
 
     categoryBox = new QComboBox;
-    categoryBox->addItem(tr("Cheating / Unsporting behavior"), "cheating");
+    for (const QString &key : ReportCategories::keys()) {
-    categoryBox->setItemData(categoryBox->count() - 1,
+        const QString label = categoryLabel(key);
-                             tr("Using external tools, card marked manipulation, or exploiting game bugs"),
+        if (label.isEmpty()) {
-                             Qt::ToolTipRole);
+            continue; // skip keys without a dialog label
-    categoryBox->addItem(tr("Harassment / Abuse"), "harassment");
+        }
-    categoryBox->setItemData(categoryBox->count() - 1, tr("Threatening, bullying, or persistent unwanted contact"),
+        categoryBox->addItem(label, key);
-                             Qt::ToolTipRole);
+        categoryBox->setItemData(categoryBox->count() - 1, categoryToolTip(key), Qt::ToolTipRole);
-    categoryBox->addItem(tr("Hate speech"), "hate_speech");
+    }
-    categoryBox->setItemData(categoryBox->count() - 1,
-                             tr("Discriminatory language targeting race, gender, religion, etc."), Qt::ToolTipRole);
-    categoryBox->addItem(tr("Spam"), "spam");
-    categoryBox->setItemData(categoryBox->count() - 1, tr("Repeated unwanted messages or advertisements"),
-                             Qt::ToolTipRole);
-    categoryBox->addItem(tr("Other"), "other");
-    categoryBox->setItemData(categoryBox->count() - 1, tr("Any behavior not covered by the above categories"),
-                             Qt::ToolTipRole);
 
     categoryGrid->addWidget(new QLabel(tr("Category:")), 0, 0);
     categoryGrid->addWidget(categoryBox, 0, 1);
@@ -189,3 +182,55 @@ void DlgReportUser::reportResponse(const Response &response)
         QMessageBox::warning(this, tr("Submission Failed"), tr("Failed to submit report. Please try again."));
     }
 }
+
+QString DlgReportUser::categoryLabel(const QString &key)
+{
+    if (key == QLatin1String("cheating")) {
+        return tr("Cheating / Unsporting behavior");
+    }
+    if (key == QLatin1String("bug_abuse")) {
+        return tr("Bug Abuse");
+    }
+    if (key == QLatin1String("harassment")) {
+        return tr("Harassment / Abuse");
+    }
+    if (key == QLatin1String("verbal_abuse")) {
+        return tr("Verbal Abuse");
+    }
+    if (key == QLatin1String("hate_speech")) {
+        return tr("Hate speech");
+    }
+    if (key == QLatin1String("spam")) {
+        return tr("Spam");
+    }
+    if (key == QLatin1String("other")) {
+        return tr("Other");
+    }
+    return QString();
+}
+
+QString DlgReportUser::categoryToolTip(const QString &key)
+{
+    if (key == QLatin1String("cheating")) {
+        return tr("Using external tools, cheat programs, or exploiting game bugs");
+    }
+    if (key == QLatin1String("bug_abuse")) {
+        return tr("Exploiting a bug or glitch to gain an unfair advantage");
+    }
+    if (key == QLatin1String("harassment")) {
+        return tr("Threatening, bullying, or persistent unwanted contact");
+    }
+    if (key == QLatin1String("verbal_abuse")) {
+        return tr("Abusive or offensive language directed at another player");
+    }
+    if (key == QLatin1String("hate_speech")) {
+        return tr("Discriminatory language targeting race, gender, religion, etc.");
+    }
+    if (key == QLatin1String("spam")) {
+        return tr("Repeated unwanted messages or advertisements");
+    }
+    if (key == QLatin1String("other")) {
+        return tr("Any behavior not covered by the above categories");
+    }
+    return QString();
+}

cockatrice/src/interface/widgets/dialogs/dlg_report_user.h

@@ -26,6 +26,9 @@ private slots:
     void reportResponse(const Response &response);
 
 private:
+    static QString categoryLabel(const QString &key);
+    static QString categoryToolTip(const QString &key);
+
     AbstractClient *client;
     QString reportedUser;
     int gameId;

libcockatrice_utility/CMakeLists.txt

@@ -6,8 +6,12 @@ set(CMAKE_AUTOUIC ON)
 set(CMAKE_AUTORCC ON)
 
 set(UTILITY_SOURCES
-    libcockatrice/utility/cryptoutil.cpp libcockatrice/utility/expression.cpp libcockatrice/utility/levenshtein.cpp
+    libcockatrice/utility/cryptoutil.cpp
-    libcockatrice/utility/passwordhasher.cpp libcockatrice/utility/server_rate_limiter.cpp
+    libcockatrice/utility/expression.cpp
+    libcockatrice/utility/levenshtein.cpp
+    libcockatrice/utility/passwordhasher.cpp
+    libcockatrice/utility/report_categories.cpp
+    libcockatrice/utility/server_rate_limiter.cpp
     libcockatrice/utility/warning_categories.cpp
 )
 
@@ -20,6 +24,7 @@ set(UTILITY_HEADERS
     libcockatrice/utility/macros.h
     libcockatrice/utility/passwordhasher.h
     libcockatrice/utility/playmat_params.h
+    libcockatrice/utility/report_categories.h
     libcockatrice/utility/string_limits.h
     libcockatrice/utility/dice_limits.h
     libcockatrice/utility/counter_limits.h

libcockatrice_utility/libcockatrice/utility/report_categories.cpp

@@ -0,0 +1,16 @@
+#include "report_categories.h"
+
+namespace ReportCategories
+{
+const QStringList &keys()
+{
+    static const QStringList categories = {"cheating",    "bug_abuse", "harassment", "verbal_abuse",
+                                           "hate_speech", "spam",      "other"};
+    return categories;
+}
+
+bool isValid(const QString &key)
+{
+    return keys().contains(key, Qt::CaseInsensitive);
+}
+} // namespace ReportCategories

libcockatrice_utility/libcockatrice/utility/report_categories.h

@@ -0,0 +1,24 @@
+#ifndef REPORT_CATEGORIES_H
+#define REPORT_CATEGORIES_H
+
+#include <QString>
+#include <QStringList>
+
+/**
+ * Canonical report categories, shared by the client and the server.
+ *
+ * This is the single source of truth for the category keys exchanged over the
+ * wire (Command_Report.category) and stored in the reports table. The server
+ * only accepts keys from this list and the client report dialog only offers
+ * keys from this list, so the two sides cannot drift apart again.
+ */
+namespace ReportCategories
+{
+/** @brief The complete list of valid category keys, in canonical order. */
+const QStringList &keys();
+
+/** @brief Whether @a key is a valid report category (case-insensitive). */
+bool isValid(const QString &key);
+} // namespace ReportCategories
+
+#endif // REPORT_CATEGORIES_H

servatrice/src/serversocketinterface.cpp

@@ -128,6 +128,7 @@
 #include <libcockatrice/protocol/pb/serverinfo_user_alt.pb.h>
 #include <libcockatrice/protocol/pb/serverinfo_user_session.pb.h>
 #include <libcockatrice/utility/passwordhasher.h>
+#include <libcockatrice/utility/report_categories.h>
 #include <libcockatrice/utility/string_limits.h>
 #include <libcockatrice/utility/warning_categories.h>
 #include <server_response_containers.h>
@@ -3775,8 +3776,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReport(const Command_Re
         return Response::RespInvalidData;
     }
 
-    static const QStringList validCategories = {"cheating", "bug_abuse", "verbal_abuse", "other"};
+    if (!ReportCategories::isValid(category)) {
-    if (!validCategories.contains(category.toLower())) {
         return Response::RespInvalidData;
     }
 

tests/CMakeLists.txt

@@ -14,6 +14,7 @@ add_test(NAME server_rate_limiter_test COMMAND server_rate_limiter_test)
 add_test(NAME server_developer_role_test COMMAND server_developer_role_test)
 add_test(NAME server_game_join_test COMMAND server_game_join_test)
 add_test(NAME warning_categories_test COMMAND warning_categories_test)
+add_test(NAME report_categories_test COMMAND report_categories_test)
 add_test(NAME lag_monitor_test COMMAND lag_monitor_test)
 add_test(NAME latency_tracker_test COMMAND latency_tracker_test)
 add_test(NAME metrics_registry_test COMMAND metrics_registry_test)
@@ -37,6 +38,7 @@ add_executable(server_rate_limiter_test server_rate_limiter_test.cpp)
 add_executable(server_developer_role_test server_developer_role_test.cpp)
 add_executable(server_game_join_test server_game_join_test.cpp)
 add_executable(warning_categories_test warning_categories_test.cpp)
+add_executable(report_categories_test report_categories_test.cpp)
 add_executable(lag_monitor_test ${CMAKE_SOURCE_DIR}/cockatrice/src/client/lag_monitor.cpp lag_monitor_test.cpp)
 target_include_directories(lag_monitor_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src)
 add_executable(latency_tracker_test latency_tracker_test.cpp)
@@ -91,6 +93,7 @@ if(NOT GTEST_FOUND)
   add_dependencies(server_developer_role_test gtest)
   add_dependencies(server_game_join_test gtest)
   add_dependencies(warning_categories_test gtest)
+  add_dependencies(report_categories_test gtest)
   add_dependencies(lag_monitor_test gtest)
   add_dependencies(latency_tracker_test gtest)
   add_dependencies(metrics_registry_test gtest)
@@ -137,6 +140,9 @@ target_link_libraries(
 target_link_libraries(
   warning_categories_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
 )
+target_link_libraries(
+  report_categories_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
+)
 target_link_libraries(lag_monitor_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES})
 target_link_libraries(
   latency_tracker_test libcockatrice_network Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}

tests/report_categories_test.cpp

@@ -0,0 +1,54 @@
+#include "gtest/gtest.h"
+#include <QList>
+#include <QString>
+#include <QStringList>
+#include <libcockatrice/utility/report_categories.h>
+
+TEST(ReportCategoriesTest, CanonicalListIsNonEmptyAndStable)
+{
+    const QStringList categories = ReportCategories::keys();
+
+    ASSERT_FALSE(categories.isEmpty());
+    EXPECT_EQ("cheating", categories.at(0));
+    EXPECT_EQ("other", categories.last());
+}
+
+TEST(ReportCategoriesTest, CanonicalListContainsMergedClientAndServerCategories)
+{
+    const QStringList expected = {"cheating",    "bug_abuse", "harassment", "verbal_abuse",
+                                  "hate_speech", "spam",      "other"};
+    EXPECT_EQ(expected, ReportCategories::keys());
+    EXPECT_EQ(7, ReportCategories::keys().size());
+}
+
+TEST(ReportCategoriesTest, AllCanonicalKeysAreValid)
+{
+    for (const QString &category : ReportCategories::keys()) {
+        EXPECT_TRUE(ReportCategories::isValid(category)) << category.toStdString();
+    }
+}
+
+TEST(ReportCategoriesTest, LegacyCategoriesRemainValid)
+{
+    EXPECT_TRUE(ReportCategories::isValid("bug_abuse"));
+    EXPECT_TRUE(ReportCategories::isValid("verbal_abuse"));
+}
+
+TEST(ReportCategoriesTest, ValidationIsCaseInsensitive)
+{
+    EXPECT_TRUE(ReportCategories::isValid("Cheating"));
+    EXPECT_TRUE(ReportCategories::isValid("HATE_SPEECH"));
+}
+
+TEST(ReportCategoriesTest, UnknownAndEmptyCategoriesAreRejected)
+{
+    EXPECT_FALSE(ReportCategories::isValid(QString()));
+    EXPECT_FALSE(ReportCategories::isValid("hate speech"));
+    EXPECT_FALSE(ReportCategories::isValid("griefing"));
+}
+
+int main(int argc, char **argv)
+{
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}