From a51f564b3c21fb34586d52492d81ec16cb3beae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Wed, 7 Oct 2026 22:56:00 +0200 Subject: [PATCH] [Report] Add report reopen action for closed reports - New Command_ReportReopen moderator command (extension 1206) with server handler that reopens resolved/dismissed reports, resets resolution state and notified, and records the reopening note in the audit log - Reopen... button in the report queue action bar, gated to resolved and dismissed reports, with optional note prompt and tooltip - Success confirmations for Assign, Resolve/Dismiss and Reopen now survive the list refresh via a pending status message instead of being clobbered - Dispatch/auth coverage in server_moderator_role_test --- .../src/interface/widgets/tabs/tab_report.cpp | 57 ++++++- .../src/interface/widgets/tabs/tab_report.h | 4 + .../libcockatrice/protocol/pb/CMakeLists.txt | 1 + .../protocol/pb/command_report_reopen.proto | 10 ++ .../protocol/pb/moderator_commands.proto | 1 + servatrice/src/serversocketinterface.cpp | 39 +++++ servatrice/src/serversocketinterface.h | 2 + tests/CMakeLists.txt | 7 + tests/server_moderator_role_test.cpp | 150 ++++++++++++++++++ 9 files changed, 269 insertions(+), 2 deletions(-) create mode 100644 libcockatrice_protocol/libcockatrice/protocol/pb/command_report_reopen.proto create mode 100644 tests/server_moderator_role_test.cpp diff --git a/cockatrice/src/interface/widgets/tabs/tab_report.cpp b/cockatrice/src/interface/widgets/tabs/tab_report.cpp index 0b9108f9b..aaf44742d 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_report.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_report.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -223,12 +224,14 @@ TabReport::TabReport(TabSupervisor *_tabSupervisor, AbstractClient *_client) : T resolveButton = new QPushButton; resolveWithNoteButton = new QPushButton; dismissButton = new QPushButton; + reopenButton = new QPushButton; viewReplayButton = new QPushButton; joinGameButton = new QPushButton; connect(assignButton, &QPushButton::clicked, this, &TabReport::assignReport); connect(resolveButton, &QPushButton::clicked, this, [this]() { resolveReport(false, false); }); connect(resolveWithNoteButton, &QPushButton::clicked, this, [this]() { resolveReport(false, true); }); connect(dismissButton, &QPushButton::clicked, this, [this]() { resolveReport(true, true); }); + connect(reopenButton, &QPushButton::clicked, this, &TabReport::reopenReport); connect(viewReplayButton, &QPushButton::clicked, this, &TabReport::viewReplay); connect(joinGameButton, &QPushButton::clicked, this, &TabReport::joinGame); @@ -239,6 +242,7 @@ TabReport::TabReport(TabSupervisor *_tabSupervisor, AbstractClient *_client) : T actionBar->addWidget(resolveButton); actionBar->addWidget(resolveWithNoteButton); actionBar->addWidget(dismissButton); + actionBar->addWidget(reopenButton); actionBar->addSpacing(20); actionBar->addWidget(viewReplayButton); actionBar->addWidget(joinGameButton); @@ -305,6 +309,8 @@ void TabReport::retranslateUi() resolveButton->setText(tr("Resolve")); resolveWithNoteButton->setText(tr("Resolve with note...")); dismissButton->setText(tr("Dismiss...")); + reopenButton->setText(tr("Reopen...")); + reopenButton->setToolTip(tr("Available for resolved or dismissed reports.")); viewReplayButton->setText(tr("View Replay")); joinGameButton->setText(tr("Join Game")); } @@ -336,6 +342,7 @@ void TabReport::reportListResponse(const Response &response) refreshButton->setEnabled(true); if (response.response_code() != Response::RespOk) { + pendingStatusMessage.clear(); statusLabel->setText(tr("Failed to load reports.")); return; } @@ -349,6 +356,11 @@ void TabReport::reportListResponse(const Response &response) applyFilters(); updateStats(); + if (!pendingStatusMessage.isEmpty()) { + statusLabel->setText(pendingStatusMessage); + pendingStatusMessage.clear(); + } + if (selectedReportIdBeforeRefresh >= 0) { bool found = false; for (int row = 0; row < table->rowCount(); ++row) { @@ -569,6 +581,7 @@ void TabReport::updateActionStates() resolveButton->setEnabled(status == "open" || status == "assigned"); resolveWithNoteButton->setEnabled(status == "open" || status == "assigned"); dismissButton->setEnabled(status == "open" || status == "assigned"); + reopenButton->setEnabled(status == "resolved" || status == "dismissed"); const bool hasGameId = report.game_id() > 0; const bool hasReplay = hasGameId && report.has_replay_id() && report.replay_id() > 0; @@ -582,6 +595,7 @@ void TabReport::setActionsEnabled(bool enabled) resolveButton->setEnabled(enabled); resolveWithNoteButton->setEnabled(enabled); dismissButton->setEnabled(enabled); + reopenButton->setEnabled(enabled); viewReplayButton->setEnabled(false); joinGameButton->setEnabled(false); commentButton->setEnabled(enabled); @@ -635,7 +649,7 @@ void TabReport::assignReport() void TabReport::assignResponse(const Response &response) { if (response.response_code() == Response::RespOk) { - statusLabel->setText(tr("Assigned.")); + pendingStatusMessage = tr("Assigned."); refreshList(); } else { statusLabel->setText(tr("Assignment failed.")); @@ -679,7 +693,46 @@ void TabReport::resolveReport(bool dismissed, bool promptNote) void TabReport::resolveResponse(const Response &response) { if (response.response_code() == Response::RespOk) { - statusLabel->setText(tr("Done.")); + pendingStatusMessage = tr("Done."); + refreshList(); + } else { + statusLabel->setText(tr("Action failed.")); + updateActionStates(); + } +} + +void TabReport::reopenReport() +{ + const int reportId = selectedReportId(); + if (reportId < 0) { + return; + } + + bool ok; + QString note = QInputDialog::getText(this, tr("Reopen Report"), tr("Reopening note (optional):"), QLineEdit::Normal, + QString(), &ok); + if (!ok) { + return; + } + + setActionsEnabled(false); + statusLabel->setText(tr("Reopening...")); + + Command_ReportReopen cmd; + cmd.set_report_id(reportId); + if (!note.isEmpty()) { + cmd.set_reopen_note(note.toStdString()); + } + + PendingCommand *pend = client->prepareModeratorCommand(cmd); + connect(pend, &PendingCommand::finished, this, &TabReport::reopenResponse); + client->sendCommand(pend); +} + +void TabReport::reopenResponse(const Response &response) +{ + if (response.response_code() == Response::RespOk) { + pendingStatusMessage = tr("Reopened."); refreshList(); } else { statusLabel->setText(tr("Action failed.")); diff --git a/cockatrice/src/interface/widgets/tabs/tab_report.h b/cockatrice/src/interface/widgets/tabs/tab_report.h index b62a99ce2..e49a5e144 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_report.h +++ b/cockatrice/src/interface/widgets/tabs/tab_report.h @@ -53,6 +53,8 @@ private slots: void reportDetailsResponse(const Response &response); void requestStats(); void statsResponse(const Response &response); + void reopenReport(); + void reopenResponse(const Response &response); private: int selectedReportId() const; @@ -84,6 +86,7 @@ private: QPushButton *resolveButton; QPushButton *resolveWithNoteButton; QPushButton *dismissButton; + QPushButton *reopenButton; QPushButton *viewReplayButton; QPushButton *joinGameButton; QLabel *statusLabel; @@ -105,6 +108,7 @@ private: QLabel *userContextNotesLabel; QLabel *userContextRecentReportsLabel; QString lastRequestedUser; + QString pendingStatusMessage; QGroupBox *statsGroup; QLabel *statsTotalLabel; QLabel *statsTrendLabel; diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt b/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt index bc4814d5e..4e4f0e190 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt @@ -55,6 +55,7 @@ set(PROTO_FILES command_report_details.proto command_report_list.proto command_report_my_list.proto + command_report_reopen.proto command_report_resolve.proto command_report_stats.proto command_report_user_info.proto diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/command_report_reopen.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/command_report_reopen.proto new file mode 100644 index 000000000..1f9c707e8 --- /dev/null +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/command_report_reopen.proto @@ -0,0 +1,10 @@ +syntax = "proto2"; +import "moderator_commands.proto"; + +message Command_ReportReopen { + extend ModeratorCommand { + optional Command_ReportReopen ext = 1206; + } + required int32 report_id = 1; + optional string reopen_note = 2; +} diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/moderator_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/moderator_commands.proto index 4f1e80c27..5298d3dad 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/moderator_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/moderator_commands.proto @@ -26,6 +26,7 @@ message ModeratorCommand { REPLAY_DOWNLOAD_BY_GAME_ID = 1203; REPORT_USER_INFO = 1204; REPORT_STATS = 1205; + REPORT_REOPEN = 1206; } extensions 100 to max; } diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index a8d13675d..2a79184e2 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -75,6 +75,7 @@ #include #include #include +#include #include #include #include @@ -391,6 +392,8 @@ Response::ResponseCode AbstractServerSocketInterface::processExtendedModeratorCo return cmdReportAssign(cmd.GetExtension(Command_ReportAssign::ext), rc); case ModeratorCommand::REPORT_RESOLVE: return cmdReportResolve(cmd.GetExtension(Command_ReportResolve::ext), rc); + case ModeratorCommand::REPORT_REOPEN: + return cmdReportReopen(cmd.GetExtension(Command_ReportReopen::ext), rc); case ModeratorCommand::VIEWLOG_HISTORY: return cmdGetLogHistory(cmd.GetExtension(Command_ViewLogHistory::ext), rc, true); case ModeratorCommand::GRANT_REPLAY_ACCESS: @@ -2748,6 +2751,42 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportMyList(const Comm return Response::RespOk; } +Response::ResponseCode AbstractServerSocketInterface::cmdReportReopen(const Command_ReportReopen &cmd, + ResponseContainer & /*rc*/) +{ + if (!sqlInterface->checkSql()) { + return Response::RespInternalError; + } + + int reportId = cmd.report_id(); + QString note = textFromStdString(cmd.reopen_note()); + + QSqlQuery *query = sqlInterface->prepareQuery("UPDATE {prefix}_reports " + "SET status = 'open', resolution_note = NULL, " + "resolution_time = NULL, resolved_by = NULL, assigned_to = NULL, " + "notified = 0 " + "WHERE id = :id AND status IN ('resolved', 'dismissed')"); + + query->bindValue(":id", reportId); + + if (!sqlInterface->execSqlQuery(query)) { + return Response::RespInternalError; + } + + if (query->numRowsAffected() == 0) { + return Response::RespInvalidData; + } + + QString auditDetails = QString("Report #%1 reopened").arg(reportId); + if (!note.isEmpty()) { + auditDetails += QString(". Reason: %1").arg(note.left(200)); + } + sqlInterface->addAuditRecord(QString::number(reportId), this->getAddress(), + QString::fromStdString(userInfo->clientid()), "REPORT_REOPENED", auditDetails, true); + + return Response::RespOk; +} + Response::ResponseCode AbstractServerSocketInterface::cmdReportDetails(const Command_ReportDetails &cmd, ResponseContainer &rc) { diff --git a/servatrice/src/serversocketinterface.h b/servatrice/src/serversocketinterface.h index 36b900d28..24cd0d44d 100644 --- a/servatrice/src/serversocketinterface.h +++ b/servatrice/src/serversocketinterface.h @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -148,6 +149,7 @@ private: Response::ResponseCode cmdUpdateServerMessage(const Command_UpdateServerMessage &cmd, ResponseContainer &rc); Response::ResponseCode cmdReportAssign(const Command_ReportAssign &cmd, ResponseContainer &); Response::ResponseCode cmdReportResolve(const Command_ReportResolve &cmd, ResponseContainer &); + Response::ResponseCode cmdReportReopen(const Command_ReportReopen &cmd, ResponseContainer &); Response::ResponseCode cmdReportUserInfo(const Command_ReportUserInfo &cmd, ResponseContainer &rc); Response::ResponseCode cmdReportStats(const Command_ReportStats &cmd, ResponseContainer &rc); Response::ResponseCode cmdRegisterAccount(const Command_Register &cmd, ResponseContainer &rc); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b51d10560..0a5c6c4c3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -11,6 +11,7 @@ add_test(NAME playmat_resolver_test COMMAND playmat_resolver_test) add_test(NAME server_card_counter_test COMMAND server_card_counter_test) add_test(NAME server_counter_test COMMAND server_counter_test) add_test(NAME server_rate_limiter_test COMMAND server_rate_limiter_test) +add_test(NAME server_moderator_role_test COMMAND server_moderator_role_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) @@ -39,6 +40,7 @@ add_executable(deck_hash_performance_test deck_hash_performance_test.cpp) add_executable(server_card_counter_test server_card_counter_test.cpp) add_executable(server_counter_test server_counter_test.cpp) add_executable(server_rate_limiter_test server_rate_limiter_test.cpp) +add_executable(server_moderator_role_test server_moderator_role_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) @@ -133,6 +135,7 @@ if(NOT GTEST_FOUND) add_dependencies(server_card_counter_test gtest) add_dependencies(server_counter_test gtest) add_dependencies(server_rate_limiter_test gtest) + add_dependencies(server_moderator_role_test gtest) add_dependencies(server_developer_role_test gtest) add_dependencies(server_game_join_test gtest) add_dependencies(warning_categories_test gtest) @@ -176,6 +179,10 @@ target_link_libraries( target_link_libraries( server_rate_limiter_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES} ) +target_link_libraries( + server_moderator_role_test libcockatrice_network libcockatrice_rng Threads::Threads ${GTEST_BOTH_LIBRARIES} + ${TEST_QT_MODULES} +) target_link_libraries( server_developer_role_test libcockatrice_network libcockatrice_rng Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES} diff --git a/tests/server_moderator_role_test.cpp b/tests/server_moderator_role_test.cpp new file mode 100644 index 000000000..ab754bfdd --- /dev/null +++ b/tests/server_moderator_role_test.cpp @@ -0,0 +1,150 @@ +/** @file server_moderator_role_test.cpp + * @brief Tests for the moderator staff role authorization and dispatch. + * @ingroup Tests + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +// The server_remote library references the global RNG, which is normally +// defined by the servatrice/client executable main(). Provide a stub so the +// unit test can link against it. +RNG_Abstract *rng = nullptr; + +namespace +{ + +class TestModeratorHandler : public Server_ProtocolHandler +{ +public: + explicit TestModeratorHandler(Server *_server) : Server_ProtocolHandler(_server, nullptr) + { + } + + QString getAddress() const override + { + return {}; + } + QString getConnectionType() const override + { + return {}; + } + + // Buffer the last response code sent to the client so tests can assert on + // the outcome of processCommandContainer(). + Response::ResponseCode lastResponseCode = Response::RespNothing; + int dispatchCount = 0; + +protected: + void transmitProtocolItem(const ServerMessage &item) override + { + if (item.message_type() == ServerMessage::RESPONSE) { + lastResponseCode = item.response().response_code(); + } + } + + Response::ResponseCode + processExtendedModeratorCommand(int cmdType, const ModeratorCommand &, ResponseContainer &) override + { + ++dispatchCount; + // Fail closed for anything not explicitly handled. + if (cmdType != ModeratorCommand::REPORT_REOPEN) { + return Response::RespFunctionNotAllowed; + } + return Response::RespOk; + } +}; + +class ModeratorRoleTest : public ::testing::Test +{ +protected: + Server server; + TestModeratorHandler handler{&server}; + + void setUserLevel(uint32_t level) + { + ServerInfo_User user; + user.set_user_level(level); + handler.setUserInfo(user); + } +}; + +TEST_F(ModeratorRoleTest, RejectsWhenNotLoggedIn) +{ + CommandContainer cont; + cont.add_moderator_command(); + handler.processCommandContainer(cont); + EXPECT_EQ(handler.lastResponseCode, Response::RespLoginNeeded); + EXPECT_EQ(handler.dispatchCount, 0); +} + +TEST_F(ModeratorRoleTest, RejectsPlainUser) +{ + setUserLevel(ServerInfo_User::IsUser | ServerInfo_User::IsRegistered); + + CommandContainer cont; + cont.add_moderator_command(); + handler.processCommandContainer(cont); + EXPECT_EQ(handler.lastResponseCode, Response::RespLoginNeeded); + EXPECT_EQ(handler.dispatchCount, 0); +} + +TEST_F(ModeratorRoleTest, DispatchesToModeratorCommandForModerator) +{ + setUserLevel(ServerInfo_User::IsModerator); + + CommandContainer cont; + ModeratorCommand *cmd = cont.add_moderator_command(); + cmd->MutableExtension(Command_ReportReopen::ext); + handler.processCommandContainer(cont); + EXPECT_EQ(handler.lastResponseCode, Response::RespOk); + EXPECT_EQ(handler.dispatchCount, 1); +} + +TEST_F(ModeratorRoleTest, DispatchesToModeratorCommandForAdmin) +{ + setUserLevel(ServerInfo_User::IsModerator | ServerInfo_User::IsAdmin); + + CommandContainer cont; + ModeratorCommand *cmd = cont.add_moderator_command(); + cmd->MutableExtension(Command_ReportReopen::ext); + handler.processCommandContainer(cont); + EXPECT_EQ(handler.lastResponseCode, Response::RespOk); + EXPECT_EQ(handler.dispatchCount, 1); +} + +TEST_F(ModeratorRoleTest, RejectsDeveloperThatIsNotModerator) +{ + setUserLevel(ServerInfo_User::IsDeveloper); + + CommandContainer cont; + cont.add_moderator_command(); + handler.processCommandContainer(cont); + EXPECT_EQ(handler.lastResponseCode, Response::RespLoginNeeded); + EXPECT_EQ(handler.dispatchCount, 0); +} + +TEST_F(ModeratorRoleTest, FailClosedForUnknownModeratorCommand) +{ + setUserLevel(ServerInfo_User::IsModerator); + + CommandContainer cont; + cont.add_moderator_command(); // no extension set -> getPbExtension() returns -1 + handler.processCommandContainer(cont); + EXPECT_EQ(handler.lastResponseCode, Response::RespFunctionNotAllowed); + EXPECT_EQ(handler.dispatchCount, 1); +} + +} // namespace + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}