Skip to content

chore: code review modification suggestions #882

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
deepin-bot merged 1 commit into from
Nov 16, 2024
Merged

chore: code review modification suggestions #882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion panels/notification/bubble/bubblemodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ int BubbleModel::replaceBubbleIndex(const BubbleItem *bubble) const
if (bubble->replacesId() != NoReplaceId) {
for (int i = 0; i < m_bubbles.size(); i++) {
auto item = m_bubbles[i];
if (item->appName() != item->appName())
if (item->appName() != bubble->appName())
continue;

const bool firstItem = item->replacesId() == NoReplaceId && item->bubbleId() == bubble->replacesId();
Expand Down
2 changes: 1 addition & 1 deletion panels/notification/bubble/bubblemodel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class BubbleModel : public QAbstractListModel
} BubbleRole;

explicit BubbleModel(QObject *parent = nullptr);
~BubbleModel();
~BubbleModel() override;

public:
void push(BubbleItem *bubble);
Expand Down
95 changes: 50 additions & 45 deletions panels/notification/common/dbaccessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@
if (dataPaths.isEmpty()) {
const QString dataDir = QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation);
QDir dir(dataDir);
const auto dbSubfix = QString("%1/%2/data.db").arg(qApp->organizationName()).arg(qApp->applicationName());
const auto appPath = dir.absoluteFilePath(dbSubfix);
const auto dbSubFix = QString("%1/%2/data.db").arg(qApp->organizationName()).arg(qApp->applicationName());
const auto appPath = dir.absoluteFilePath(dbSubFix);
dataPaths << appPath;
QString path = dir.absoluteFilePath("deepin/dde-osd/data.db");
dataPaths << path;
Expand All @@ -75,15 +75,15 @@
QDir().mkpath(QFileInfo(file.fileName()).path());
}
if (!file.open(QIODevice::ReadWrite)) {
qDebug(notifyLog) << "Falied on open the data path:" << path << ", error:" << file.errorString();
qDebug(notifyLog) << "Failed on open the data path:" << path << ", error:" << file.errorString();
continue;
}
if (QFileInfo::exists(path)) {
return path;
}
}
qWarning(notifyLog) << "Doesn't exist the data path" << dataPaths;
return QString();
return {};
}

class Benchmark
Expand Down Expand Up @@ -147,21 +147,26 @@

QSqlQuery query(m_connection);

QString sqlCmd = QString("INSERT INTO %1 (").arg(TableName_v2);
sqlCmd += ColumnIcon + ",";
sqlCmd += ColumnSummary + ",";
sqlCmd += ColumnBody + ",";
sqlCmd += ColumnAppName + ",";
sqlCmd += ColumnAppId + ",";
sqlCmd += ColumnCTime + ",";
sqlCmd += ColumnAction + ",";
sqlCmd += ColumnHint + ",";
sqlCmd += ColumnReplacesId + ",";
sqlCmd += ColumnNotifyId + ",";
sqlCmd += ColumnProcessedType + ")";
sqlCmd += "VALUES (:icon, :summary, :body, :appName, :appId, :ctime, :action, :hint, :replacesId, :notifyId, :processedType)";
QString columns = QStringList{
ColumnIcon,
ColumnSummary,
ColumnBody,
ColumnAppName,
ColumnAppId,
ColumnCTime,
ColumnAction,
ColumnHint,
ColumnReplacesId,
ColumnNotifyId,
ColumnProcessedType
}.join(", ");

QString sqlCmd = QString("INSERT INTO %1 (%2) VALUES (%3)")
.arg(TableName_v2)
.arg(columns)
.arg(":icon, :summary, :body, :appName, :appId, :ctime, :action, :hint, :replacesId, :notifyId, :processedType");

query.prepare(sqlCmd);

Check warning on line 169 in panels/notification/common/dbaccessor.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Error code from the return value of function query.prepare() is not used.
query.bindValue(":icon", entity.appIcon());
query.bindValue(":summary", entity.summary());
query.bindValue(":body", entity.body());
Expand All @@ -180,7 +185,7 @@
}

// to get entity's id in database
int storageId = query.lastInsertId().toLongLong();
qint64 storageId = query.lastInsertId().toLongLong();

qDebug(notifyDBLog) << "Insert entity bubbleId:" << entity.bubbleId() << ", id:" << storageId;

Expand Down Expand Up @@ -433,22 +438,27 @@
{
QSqlQuery query(m_connection);

QString sql = QString("CREATE TABLE IF NOT EXISTS %1("
"%2 INTEGER PRIMARY KEY AUTOINCREMENT,").arg(TableName_v2, ColumnId);
sql += ColumnIcon + " TEXT,";
sql += ColumnSummary + " TEXT,";
sql += ColumnBody + " TEXT,";
sql += ColumnAppName + " TEXT,";
sql += ColumnAppId + " TEXT,";
sql += ColumnCTime + " TEXT,";
sql += ColumnAction + " TEXT,";
sql += ColumnHint + " TEXT,";
sql += ColumnReplacesId + " TEXT,";
sql += ColumnNotifyId + " TEXT,";
sql += ColumnTimeout + " TEXT,";
sql += ColumnProcessedType + " INTEGER)";
QStringList columns = {
QString("%1 INTEGER PRIMARY KEY AUTOINCREMENT").arg(ColumnId),
QString("%1 TEXT").arg(ColumnIcon),
QString("%1 TEXT").arg(ColumnSummary),
QString("%1 TEXT").arg(ColumnBody),
QString("%1 TEXT").arg(ColumnAppName),
QString("%1 TEXT").arg(ColumnAppId),
QString("%1 TEXT").arg(ColumnCTime),
QString("%1 TEXT").arg(ColumnAction),
QString("%1 TEXT").arg(ColumnHint),
QString("%1 TEXT").arg(ColumnReplacesId),
QString("%1 TEXT").arg(ColumnNotifyId),
QString("%1 TEXT").arg(ColumnTimeout),
QString("%1 INTEGER").arg(ColumnProcessedType)
};

QString sql = QString("CREATE TABLE IF NOT EXISTS %1(%2)")
.arg(TableName_v2)
.arg(columns.join(", "));

query.prepare(sql);

Check warning on line 461 in panels/notification/common/dbaccessor.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Error code from the return value of function query.prepare() is not used.

if (!query.exec()) {
qWarning(notifyDBLog) << "create table failed" << query.lastError().text();
Expand Down Expand Up @@ -476,7 +486,7 @@
}
}

bool DBAccessor::isAttributeValid(const QString &tableName, const QString &attributeName)
bool DBAccessor::isAttributeValid(const QString &tableName, const QString &attributeName) const
{
QSqlQuery query(m_connection);

Expand All @@ -487,25 +497,20 @@
if (query.exec(sqlCmd)) {
QSqlRecord record = query.record();
int index = record.indexOf(attributeName);
if (index == -1) {
return false;
} else {
return true;
}
} else {
qDebug(notifyDBLog) << sqlCmd << ",lastError:" << query.lastError().text();
return false;
return index != -1;
}
} else { // table not exist
qDebug(notifyDBLog) << sqlCmd << ",lastError:" << query.lastError().text();
return false;
}
} else { // sql error
qDebug(notifyDBLog) << sqlCmd << ",lastError:" << query.lastError().text();
// table not exist
return false;
}
// sql error
qDebug(notifyDBLog) << sqlCmd << ",lastError:" << query.lastError().text();
return false;
}

bool DBAccessor::addAttributeToTable(const QString &tableName, const QString &attributeName, const QString &type)
bool DBAccessor::addAttributeToTable(const QString &tableName, const QString &attributeName, const QString &type) const
{
QSqlQuery query(m_connection);

Expand Down
4 changes: 2 additions & 2 deletions panels/notification/common/dbaccessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ class DBAccessor : public DataAccessor
private:
void tryToCreateTable();

bool isAttributeValid(const QString &tableName, const QString &attributeName);
bool addAttributeToTable(const QString &tableName, const QString &attributeName, const QString &type);
bool isAttributeValid(const QString &tableName, const QString &attributeName) const;
bool addAttributeToTable(const QString &tableName, const QString &attributeName, const QString &type) const;
void updateProcessTypeValue();

private:
Expand Down
2 changes: 1 addition & 1 deletion panels/notification/server/configs/org.deepin.dde.shell.notification.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"value": true,
"serial": 0,
"flags": [],
"name": "dndode",
"name": "dndMode",
"name[zh_CN]": "勿扰模式",
"description": "System notification do not disturb mode",
"description[zh_CN]": "系统通知勿扰模式",
Expand Down
15 changes: 4 additions & 11 deletions panels/notification/server/notificationmanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,20 +322,14 @@ bool NotificationManager::isDoNotDisturb() const
QTime startTime = QTime::fromString(m_setting->systemValue(NotificationSetting::StartTime).toString());
QTime endTime = QTime::fromString(m_setting->systemValue(NotificationSetting::EndTime).toString());

bool dndMode = false;
bool dndMode = true;
if (startTime < endTime) {
dndMode = startTime <= currentTime && endTime >= currentTime;
} else if (startTime > endTime) {
dndMode = startTime <= currentTime || endTime >= currentTime;
} else {
dndMode = true;
}

if (dndMode && m_setting->systemValue(NotificationSetting::OpenByTimeInterval).toBool()) {
return dndMode;
} else {
return false;
}
return dndMode && m_setting->systemValue(NotificationSetting::OpenByTimeInterval).toBool();
}

void NotificationManager::tryPlayNotificationSound(const NotifyEntity &entity, const QString &appId, bool dndMode) const
Expand Down Expand Up @@ -437,8 +431,7 @@ void NotificationManager::doActionInvoked(const NotifyEntity &entity, const QStr
while (i != hints.constEnd()) {
QStringList args = i.value().toString().split(",");
if (!args.isEmpty()) {
QString cmd = args.first(); //命令
args.removeFirst();
QString cmd = args.takeFirst(); //命令
if (i.key() == "x-deepin-action-" + actionId) {
QProcess::startDetached(cmd, args); //执行相关命令
}
Expand Down Expand Up @@ -478,7 +471,7 @@ void NotificationManager::onHandingPendingEntities()
m_lastTimeoutPoint = std::numeric_limits<qint64>::max();
}

for (const auto item : timeoutEntities) {
for (const auto &item : timeoutEntities) {
qDebug(notifyLog) << "Expired for the notification " << item.id() << item.appName();
notificationClosed(item.id(), item.bubbleId(), NotifyEntity::Expired);
}
Expand Down
2 changes: 1 addition & 1 deletion panels/notification/server/notificationsetting.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ enum Roles {

NotificationSetting::NotificationSetting(QObject *parent)
: QObject(parent)
, m_impl(Dtk::Core::DConfig::create("org.deepin.dde.shell", "org.deepin.dde.shell.notification"))
, m_impl(Dtk::Core::DConfig::create("org.deepin.dde.shell", "org.deepin.dde.shell.notification", QString(), this))
{
invalidAppItemCached();
connect(m_impl, &Dtk::Core::DConfig::valueChanged, this, [this] (const QString &key) {
Expand Down
Loading