mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-09-21 09:05:10 -07:00
[DeckShare] Address review findings and harden the share flows
Gate every share entry point on login, de-duplicate the share-link and color-identity logic behind DeckShareUtils and an injected querier, and replace the silent tray/status-bar notices with always-visible dialogs. - abstract_tab_deck_editor: explain that sharing requires a connection instead of silently doing nothing when logged out - tab_deck_storage: disable the share action on disconnect, reject folder/deck mixes and the root folder with clear warnings, re-enable Create on every entry/response so a dropped connection cannot leave the button disabled - tab_deck_storage_visual: same login gate for the context-menu entry, visible success/error dialogs, and a symmetric in-flight guard - getDeckColorIdentity now takes a CardDatabaseQuerier, dropping the CardDatabaseManager singleton access and enabling unit tests
This commit is contained in:
parent
d91fe34d20
commit
0276eb303e
12 changed files with 103 additions and 53 deletions
|
|
@ -1,11 +1,11 @@
|
|||
#include "deck_color_identity.h"
|
||||
|
||||
#include <QSet>
|
||||
#include <libcockatrice/card/database/card_database_manager.h>
|
||||
#include <libcockatrice/card/database/card_database_querier.h>
|
||||
#include <libcockatrice/deck_list/deck_list.h>
|
||||
#include <libcockatrice/deck_list/tree/inner_deck_list_node.h>
|
||||
|
||||
QString getDeckColorIdentity(const DeckList &deck)
|
||||
QString getDeckColorIdentity(const DeckList &deck, const CardDatabaseQuerier *db)
|
||||
{
|
||||
const QStringList cardList = deck.getCardList({DECK_ZONE_MAIN, DECK_ZONE_SIDE});
|
||||
if (cardList.isEmpty()) {
|
||||
|
|
@ -15,7 +15,7 @@ QString getDeckColorIdentity(const DeckList &deck)
|
|||
QSet<QChar> colorSet; // A set to collect unique color symbols (e.g., W, U, B, R, G)
|
||||
|
||||
for (const QString &cardName : cardList) {
|
||||
CardInfoPtr currentCard = CardDatabaseManager::query()->getCardInfo(cardName);
|
||||
CardInfoPtr currentCard = db->getCardInfo(cardName);
|
||||
if (currentCard) {
|
||||
const QString colors = currentCard->getColors(); // returns something like "WUB"
|
||||
for (const QChar &color : colors) {
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
|
||||
#include <QString>
|
||||
|
||||
class CardDatabaseQuerier;
|
||||
class DeckList;
|
||||
|
||||
/**
|
||||
|
|
@ -11,7 +12,9 @@ class DeckList;
|
|||
*
|
||||
* Shared as a free function so the deck storage previews and the deck share
|
||||
* dialog compute identities identically.
|
||||
*
|
||||
* @param db Card database used to look up card color symbols.
|
||||
*/
|
||||
QString getDeckColorIdentity(const DeckList &deck);
|
||||
QString getDeckColorIdentity(const DeckList &deck, const CardDatabaseQuerier *db);
|
||||
|
||||
#endif // COCKATRICE_DECK_COLOR_IDENTITY_H
|
||||
|
|
|
|||
|
|
@ -67,6 +67,11 @@ void ShareBarWidget::setHintText(const QString &text, bool visible)
|
|||
hintLabel->setVisible(visible);
|
||||
}
|
||||
|
||||
void ShareBarWidget::setCreateEnabled(bool enabled)
|
||||
{
|
||||
createButton->setEnabled(enabled);
|
||||
}
|
||||
|
||||
void ShareBarWidget::focusName()
|
||||
{
|
||||
nameEdit->setFocus();
|
||||
|
|
|
|||
|
|
@ -42,6 +42,9 @@ public:
|
|||
/** @brief Sets the explainer hint text, showing it when @p visible is true. */
|
||||
void setHintText(const QString &text, bool visible);
|
||||
|
||||
/** @brief Enables or disables the create-share-link button (guards double submission). */
|
||||
void setCreateEnabled(bool enabled);
|
||||
|
||||
/** @brief Moves keyboard focus to the name field. */
|
||||
void focusName();
|
||||
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@
|
|||
#include <QPushButton>
|
||||
#include <QTimeZone>
|
||||
#include <QVBoxLayout>
|
||||
#include <libcockatrice/card/database/card_database_manager.h>
|
||||
#include <libcockatrice/deck_list/deck_list.h>
|
||||
#include <libcockatrice/network/client/abstract/abstract_client.h>
|
||||
#include <libcockatrice/protocol/pb/command_deck_share_create.pb.h>
|
||||
|
|
@ -36,11 +37,14 @@ DlgShareDeck::DlgShareDeck(AbstractClient *_client, const QSharedPointer<DeckLis
|
|||
buttonBox->button(QDialogButtonBox::Cancel)->setText(tr("Cancel"));
|
||||
connect(buttonBox, &QDialogButtonBox::accepted, this, &DlgShareDeck::actShare);
|
||||
connect(buttonBox, &QDialogButtonBox::rejected, this, &DlgShareDeck::reject);
|
||||
this->buttonBox = buttonBox;
|
||||
layout->addWidget(buttonBox);
|
||||
}
|
||||
|
||||
void DlgShareDeck::actShare()
|
||||
{
|
||||
buttonBox->button(QDialogButtonBox::Ok)->setEnabled(false);
|
||||
|
||||
Command_DeckShareCreate cmd;
|
||||
cmd.set_name(nameEdit->text().trimmed().toStdString());
|
||||
if (cmd.name().empty()) {
|
||||
|
|
@ -49,7 +53,7 @@ void DlgShareDeck::actShare()
|
|||
|
||||
DeckShareItem *item = cmd.add_items();
|
||||
item->set_deck_list(deck->writeToString_Native().toStdString());
|
||||
item->set_color_identity(getDeckColorIdentity(*deck).toStdString());
|
||||
item->set_color_identity(getDeckColorIdentity(*deck, CardDatabaseManager::query()).toStdString());
|
||||
|
||||
PendingCommand *pend = client->prepareSessionCommand(cmd);
|
||||
connect(pend, &PendingCommand::finished, this, &DlgShareDeck::shareFinished);
|
||||
|
|
@ -59,6 +63,7 @@ void DlgShareDeck::actShare()
|
|||
void DlgShareDeck::shareFinished(const Response &response, const CommandContainer & /*commandContainer*/)
|
||||
{
|
||||
if (response.response_code() != Response::RespOk) {
|
||||
buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true);
|
||||
QMessageBox::critical(this, tr("Share deck"),
|
||||
tr("Failed to create the share link (server response code %1).")
|
||||
.arg(QString::number(static_cast<int>(response.response_code()))));
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@
|
|||
class AbstractClient;
|
||||
class CommandContainer;
|
||||
class DeckList;
|
||||
class QDialogButtonBox;
|
||||
class QLineEdit;
|
||||
class Response;
|
||||
|
||||
|
|
@ -36,6 +37,7 @@ private:
|
|||
AbstractClient *client;
|
||||
QSharedPointer<DeckList> deck;
|
||||
QLineEdit *nameEdit;
|
||||
QDialogButtonBox *buttonBox;
|
||||
};
|
||||
|
||||
#endif // DLG_SHARE_DECK_H
|
||||
|
|
@ -388,6 +388,11 @@ bool AbstractTabDeckEditor::actSaveDeckAs()
|
|||
*/
|
||||
void AbstractTabDeckEditor::actShareDeck()
|
||||
{
|
||||
if (tabSupervisor->getClient()->getStatus() != StatusLoggedIn) {
|
||||
QMessageBox::information(this, tr("Share deck"), tr("You must be connected to the server to share a deck."));
|
||||
return;
|
||||
}
|
||||
|
||||
const QSharedPointer<DeckList> deck = deckStateManager->getDeckListShared();
|
||||
if (deck->isBlankDeck()) {
|
||||
return;
|
||||
|
|
|
|||
|
|
@ -1,30 +1,25 @@
|
|||
#include "tab_deck_storage.h"
|
||||
|
||||
#include "../../../client/settings/cache_settings.h"
|
||||
#include "../../../main.h"
|
||||
#include "../../deck_loader/deck_loader.h"
|
||||
#include "../../pixel_map_generator.h"
|
||||
#include "../deck_share/deck_share_utils.h"
|
||||
#include "../deck_share/share_bar_widget.h"
|
||||
#include "../interface/widgets/server/remote/remote_decklist_tree_widget.h"
|
||||
#include "../interface/widgets/utility/get_text_with_max.h"
|
||||
|
||||
#include <QAction>
|
||||
#include <QApplication>
|
||||
#include <QClipboard>
|
||||
#include <QDateTime>
|
||||
#include <QDebug>
|
||||
#include <QDesktopServices>
|
||||
#include <QFileSystemModel>
|
||||
#include <QGroupBox>
|
||||
#include <QGuiApplication>
|
||||
#include <QHBoxLayout>
|
||||
#include <QHeaderView>
|
||||
#include <QInputDialog>
|
||||
#include <QLineEdit>
|
||||
#include <QMainWindow>
|
||||
#include <QMessageBox>
|
||||
#include <QStatusBar>
|
||||
#include <QSystemTrayIcon>
|
||||
#include <QTimeZone>
|
||||
#include <QToolBar>
|
||||
#include <QTreeView>
|
||||
|
|
@ -249,12 +244,14 @@ void TabDeckStorage::setRemoteEnabled(bool enabled)
|
|||
aUpload->setEnabled(enabled);
|
||||
aOpenRemoteDeck->setEnabled(enabled);
|
||||
aDownload->setEnabled(enabled);
|
||||
aShareDecks->setEnabled(enabled);
|
||||
aNewFolder->setEnabled(enabled);
|
||||
aDeleteRemoteDeck->setEnabled(enabled);
|
||||
|
||||
if (enabled) {
|
||||
serverDirView->refreshTree();
|
||||
} else {
|
||||
setShareModeEnabled(false);
|
||||
serverDirView->clearTree();
|
||||
}
|
||||
}
|
||||
|
|
@ -670,6 +667,7 @@ void TabDeckStorage::setShareModeEnabled(bool enabled)
|
|||
{
|
||||
shareBar->setVisible(enabled);
|
||||
if (enabled) {
|
||||
shareBar->setCreateEnabled(true);
|
||||
shareBar->setName(tr("Shared decks"));
|
||||
onServerSelectionChanged();
|
||||
shareBar->focusName();
|
||||
|
|
@ -693,6 +691,17 @@ void TabDeckStorage::onServerSelectionChanged()
|
|||
++files;
|
||||
}
|
||||
}
|
||||
|
||||
QString hint;
|
||||
if (folders > 1) {
|
||||
hint = tr("Only one folder can be shared at a time.");
|
||||
} else if (folders > 0 && files > 0) {
|
||||
hint = tr("Share either a folder or decks, not both.");
|
||||
} else if (folders == 0 && files == 0) {
|
||||
hint = tr("Select folders or decks in the tree to share.");
|
||||
}
|
||||
shareBar->setHintText(hint, !hint.isEmpty());
|
||||
|
||||
QStringList parts;
|
||||
if (folders > 0) {
|
||||
parts << tr("%n folder(s)", "", folders);
|
||||
|
|
@ -706,6 +715,30 @@ void TabDeckStorage::onServerSelectionChanged()
|
|||
void TabDeckStorage::actShareSelection()
|
||||
{
|
||||
const auto selection = serverDirView->getCurrentSelection();
|
||||
QString sharedFolder;
|
||||
bool hasFile = false;
|
||||
bool hasFolder = false;
|
||||
for (const auto *node : selection) {
|
||||
if (const auto *dirNode = dynamic_cast<const RemoteDeckList_TreeModel::DirectoryNode *>(node)) {
|
||||
hasFolder = true;
|
||||
if (!sharedFolder.isEmpty()) {
|
||||
showShareNotice(tr("Only one folder can be shared at a time."), true);
|
||||
return;
|
||||
}
|
||||
sharedFolder = dirNode->getPath();
|
||||
} else {
|
||||
hasFile = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (hasFile && hasFolder) {
|
||||
showShareNotice(tr("Share either a folder or decks, not both."), true);
|
||||
return;
|
||||
}
|
||||
if (hasFolder && sharedFolder.isEmpty()) {
|
||||
showShareNotice(tr("The root folder cannot be shared."), true);
|
||||
return;
|
||||
}
|
||||
|
||||
Command_DeckShareCreate cmd;
|
||||
cmd.set_name(shareBar->name().toStdString());
|
||||
|
|
@ -713,19 +746,9 @@ void TabDeckStorage::actShareSelection()
|
|||
cmd.set_name(tr("Shared decks").toStdString());
|
||||
}
|
||||
|
||||
// Sharing a folder is exclusive with sharing individual decks (matches the picker's rule).
|
||||
for (const auto *node : selection) {
|
||||
if (const auto *dirNode = dynamic_cast<const RemoteDeckList_TreeModel::DirectoryNode *>(node)) {
|
||||
const QString path = dirNode->getPath();
|
||||
if (path.isEmpty()) {
|
||||
continue; // the root folder cannot be shared
|
||||
}
|
||||
cmd.set_folder_path(path.toStdString());
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (cmd.folder_path().empty()) {
|
||||
if (!sharedFolder.isEmpty()) {
|
||||
cmd.set_folder_path(sharedFolder.toStdString());
|
||||
} else {
|
||||
for (const auto *node : selection) {
|
||||
if (const auto *fileNode = dynamic_cast<const RemoteDeckList_TreeModel::FileNode *>(node)) {
|
||||
DeckShareItem *item = cmd.add_items();
|
||||
|
|
@ -735,10 +758,11 @@ void TabDeckStorage::actShareSelection()
|
|||
}
|
||||
|
||||
if (cmd.items_size() == 0 && cmd.folder_path().empty()) {
|
||||
showShareNotice(tr("Select decks to share."));
|
||||
showShareNotice(tr("Select decks to share."), true);
|
||||
return;
|
||||
}
|
||||
|
||||
shareBar->setCreateEnabled(false);
|
||||
PendingCommand *pend = client->prepareSessionCommand(cmd);
|
||||
connect(pend, &PendingCommand::finished, this, &TabDeckStorage::shareFromTreeFinished);
|
||||
client->sendCommand(pend);
|
||||
|
|
@ -746,29 +770,29 @@ void TabDeckStorage::actShareSelection()
|
|||
|
||||
void TabDeckStorage::shareFromTreeFinished(const Response &response, const CommandContainer & /*commandContainer*/)
|
||||
{
|
||||
shareBar->setCreateEnabled(true);
|
||||
if (response.response_code() != Response::RespOk) {
|
||||
qWarning() << "failed to create deck share:" << response.response_code();
|
||||
showShareNotice(tr("Failed to create the share link (server response code %1).")
|
||||
.arg(QString::number(static_cast<int>(response.response_code()))));
|
||||
.arg(QString::number(static_cast<int>(response.response_code()))),
|
||||
true);
|
||||
return;
|
||||
}
|
||||
const Response_DeckShareCreate &resp = response.GetExtension(Response_DeckShareCreate::ext);
|
||||
const QString token = QString::fromStdString(resp.token());
|
||||
const QDateTime expiry = QDateTime::fromSecsSinceEpoch(resp.expires_at(), QTimeZone::UTC).toLocalTime();
|
||||
const QDateTime expiry = QDateTime::fromSecsSinceEpoch(resp.expires_at(), QTimeZone::UTC);
|
||||
|
||||
const QString link = QString("cockatrice://opendeck?share=%1&hostname=%2&port=%3")
|
||||
.arg(token, client->serverName(), QString::number(client->serverPort()));
|
||||
QGuiApplication::clipboard()->setText(link);
|
||||
const QString link = DeckShareUtils::buildShareLink(client, token);
|
||||
DeckShareUtils::copyShareLinkToClipboard(link);
|
||||
|
||||
showShareNotice(tr("Share link copied to the clipboard.\nExpires on %1.").arg(expiry.toString()));
|
||||
showShareNotice(
|
||||
tr("Share link copied to the clipboard.\nExpires on %1.").arg(DeckShareUtils::formatShareExpiry(expiry)));
|
||||
setShareModeEnabled(false);
|
||||
}
|
||||
|
||||
void TabDeckStorage::showShareNotice(const QString &message)
|
||||
void TabDeckStorage::showShareNotice(const QString &message, bool warning)
|
||||
{
|
||||
if (trayIcon && trayIcon->isVisible()) {
|
||||
trayIcon->showMessage(tr("Deck share"), message);
|
||||
} else if (auto *mainWindow = qobject_cast<QMainWindow *>(window())) {
|
||||
mainWindow->statusBar()->showMessage(message, 10000);
|
||||
}
|
||||
QMessageBox box(warning ? QMessageBox::Warning : QMessageBox::Information, tr("Deck share"), message,
|
||||
QMessageBox::Ok, this);
|
||||
box.exec();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -45,7 +45,7 @@ private:
|
|||
|
||||
void setRemoteEnabled(bool enabled);
|
||||
|
||||
void showShareNotice(const QString &message);
|
||||
void showShareNotice(const QString &message, bool warning = false);
|
||||
|
||||
void setShareModeEnabled(bool enabled);
|
||||
|
||||
|
|
|
|||
|
|
@ -1,17 +1,15 @@
|
|||
#include "tab_deck_storage_visual.h"
|
||||
|
||||
#include "../../../../main.h"
|
||||
#include "../../../deck_loader/deck_loader.h"
|
||||
#include "../../cards/additional_info/deck_color_identity.h"
|
||||
#include "../../deck_share/deck_share_utils.h"
|
||||
#include "../../interface/widgets/visual_deck_storage/visual_deck_storage_widget.h"
|
||||
#include "../tab_supervisor.h"
|
||||
|
||||
#include <QMainWindow>
|
||||
#include <QDateTime>
|
||||
#include <QMessageBox>
|
||||
#include <QStatusBar>
|
||||
#include <QSystemTrayIcon>
|
||||
#include <QVBoxLayout>
|
||||
#include <libcockatrice/card/database/card_database_manager.h>
|
||||
#include <libcockatrice/deck_list/deck_list.h>
|
||||
#include <libcockatrice/network/client/abstract/abstract_client.h>
|
||||
#include <libcockatrice/protocol/pb/command_deck_share_create.pb.h>
|
||||
|
|
@ -82,6 +80,7 @@ void TabDeckStorageVisual::enterShareMode(const QStringList &preselectFiles)
|
|||
if (!shareDeckAvailable) {
|
||||
return; // sharing is gated on being logged in
|
||||
}
|
||||
shareBar->setCreateEnabled(true);
|
||||
visualDeckStorageWidget->setShareSelectable(true);
|
||||
visualDeckStorageWidget->setShareSelectedFiles(preselectFiles);
|
||||
shareBar->setName(tr("Shared decks"));
|
||||
|
|
@ -99,6 +98,10 @@ void TabDeckStorageVisual::exitShareMode()
|
|||
|
||||
void TabDeckStorageVisual::actShareDeck(const QString &filePath)
|
||||
{
|
||||
if (!shareDeckAvailable) {
|
||||
QMessageBox::information(this, tr("Share deck"), tr("You must be connected to the server to share a deck."));
|
||||
return;
|
||||
}
|
||||
enterShareMode({filePath});
|
||||
}
|
||||
|
||||
|
|
@ -145,9 +148,10 @@ void TabDeckStorageVisual::actShareSelected()
|
|||
}
|
||||
DeckShareItem *item = cmd.add_items();
|
||||
item->set_deck_list(deckOpt->deckList.writeToString_Native().toStdString());
|
||||
item->set_color_identity(getDeckColorIdentity(deckOpt->deckList).toStdString());
|
||||
item->set_color_identity(getDeckColorIdentity(deckOpt->deckList, CardDatabaseManager::query()).toStdString());
|
||||
}
|
||||
|
||||
shareBar->setCreateEnabled(false);
|
||||
PendingCommand *pend = tabSupervisor->getClient()->prepareSessionCommand(cmd);
|
||||
connect(pend, &PendingCommand::finished, this, &TabDeckStorageVisual::shareFinished);
|
||||
tabSupervisor->getClient()->sendCommand(pend);
|
||||
|
|
@ -155,10 +159,11 @@ void TabDeckStorageVisual::actShareSelected()
|
|||
|
||||
void TabDeckStorageVisual::shareFinished(const Response &response, const CommandContainer & /*commandContainer*/)
|
||||
{
|
||||
shareBar->setCreateEnabled(true);
|
||||
if (response.response_code() != Response::RespOk) {
|
||||
showShareNotice(tr("Failed to create the share link (server response code %1).")
|
||||
.arg(QString::number(static_cast<int>(response.response_code()))));
|
||||
exitShareMode();
|
||||
.arg(QString::number(static_cast<int>(response.response_code()))),
|
||||
true);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -183,11 +188,9 @@ void TabDeckStorageVisual::handleConnectionChanged(ClientStatus status)
|
|||
}
|
||||
}
|
||||
|
||||
void TabDeckStorageVisual::showShareNotice(const QString &message)
|
||||
void TabDeckStorageVisual::showShareNotice(const QString &message, bool warning)
|
||||
{
|
||||
if (trayIcon && trayIcon->isVisible()) {
|
||||
trayIcon->showMessage(tr("Deck share"), message);
|
||||
} else if (auto *mainWindow = qobject_cast<QMainWindow *>(window())) {
|
||||
mainWindow->statusBar()->showMessage(message, 10000);
|
||||
}
|
||||
QMessageBox box(warning ? QMessageBox::Warning : QMessageBox::Information, tr("Deck share"), message,
|
||||
QMessageBox::Ok, this);
|
||||
box.exec();
|
||||
}
|
||||
|
|
@ -67,7 +67,7 @@ private slots:
|
|||
void handleConnectionChanged(ClientStatus status);
|
||||
|
||||
private:
|
||||
void showShareNotice(const QString &message);
|
||||
void showShareNotice(const QString &message, bool warning = false);
|
||||
void updateShareHint();
|
||||
|
||||
VisualDeckStorageWidget *visualDeckStorageWidget;
|
||||
|
|
|
|||
|
|
@ -51,7 +51,7 @@ VisualDeckStorageWidget::VisualDeckStorageWidget(QWidget *parent) : QWidget(pare
|
|||
shareButton = new QToolButton(this);
|
||||
shareButton->setIcon(QPixmap("theme:icons/share"));
|
||||
shareButton->setFixedSize(32, 32);
|
||||
shareButton->setToolTip(tr("Share selected decks"));
|
||||
shareButton->setToolTip(tr("Select decks to share"));
|
||||
shareButton->setVisible(false);
|
||||
connect(shareButton, &QPushButton::clicked, this, &VisualDeckStorageWidget::shareRequested);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue