Skip to content

Commit 01d255b

Browse files
committed
fix(search): address review feedback
Replace deprecated QML effects, rely on JsonApiJob self-deletion, and expose people-search retry as a slot. Align the reviewed constructor signature and correct model index assertions exposed by the focused tests. Signed-off-by: Rello <github@scherello.de> Assisted-by: Codex:GPT-5
1 parent cefa5a1 commit 01d255b

8 files changed

Lines changed: 26 additions & 22 deletions

src/gui/search/unifiedsearchpeoplemodel.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ UnifiedSearchPeopleModel::~UnifiedSearchPeopleModel() { cancel(); }
3232

3333
QVariant UnifiedSearchPeopleModel::data(const QModelIndex &index, int role) const
3434
{
35-
Q_ASSERT(!checkIndex(index, CheckIndexOption::IndexIsValid));
35+
Q_ASSERT(checkIndex(index, CheckIndexOption::IndexIsValid));
3636
const auto &person = _people.at(index.row());
3737
switch (role) {
3838
case UserIdRole: return person.id;
@@ -186,8 +186,8 @@ void UnifiedSearchPeopleModel::cancel()
186186
++_generation;
187187
if (_job) {
188188
disconnect(_job, nullptr, this, nullptr);
189-
if (const auto job = qobject_cast<JsonApiJob *>(_job.data()); job && job->reply() && job->reply()->isRunning()) job->reply()->abort();
190-
_job->deleteLater();
189+
if (const auto job = qobject_cast<JsonApiJob *>(_job.data()); job && job->reply() && job->reply()->isRunning())
190+
job->reply()->abort();
191191
_job.clear();
192192
}
193193
setBusy(false);

src/gui/search/unifiedsearchpeoplemodel.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ class UnifiedSearchPeopleModel : public QAbstractListModel
3434
[[nodiscard]] QString searchTerm() const;
3535
[[nodiscard]] bool busy() const;
3636
[[nodiscard]] QString errorString() const;
37-
Q_INVOKABLE void retry();
3837

3938
public Q_SLOTS:
4039
void setAccountState(AccountState *accountState);
4140
void setSearchTerm(const QString &searchTerm);
41+
void retry();
4242

4343
Q_SIGNALS:
4444
void accountStateChanged();

src/gui/search/unifiedsearchresultslistmodel.cpp

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,7 @@ QString navigationAppIconForResult(const OCC::AccountState *accountState,
177177
namespace OCC {
178178
Q_LOGGING_CATEGORY(lcUnifiedSearch, "nextcloud.gui.unifiedsearch", QtInfoMsg)
179179

180-
UnifiedSearchResultsListModel::UnifiedSearchResultsListModel(AccountState *accountState,
181-
QObject *parent,
182-
int debounceInterval,
183-
int revealInterval)
180+
UnifiedSearchResultsListModel::UnifiedSearchResultsListModel(AccountState *accountState, int debounceInterval, int revealInterval, QObject *parent)
184181
: QAbstractListModel(parent)
185182
, _accountState(accountState)
186183
{
@@ -254,7 +251,7 @@ UnifiedSearchResultsListModel::~UnifiedSearchResultsListModel()
254251

255252
QVariant UnifiedSearchResultsListModel::data(const QModelIndex &index, int role) const
256253
{
257-
Q_ASSERT(!checkIndex(index, QAbstractItemModel::CheckIndexOption::IndexIsValid));
254+
Q_ASSERT(checkIndex(index, QAbstractItemModel::CheckIndexOption::IndexIsValid));
258255
const auto &result = _results.at(index.row());
259256
switch (role) {
260257
case ProviderNameRole:
@@ -1071,7 +1068,6 @@ void UnifiedSearchResultsListModel::abortSearchJobs()
10711068
if (const auto job = qobject_cast<JsonApiJob *>(jobObject.data()); job && job->reply() && job->reply()->isRunning()) {
10721069
job->reply()->abort();
10731070
}
1074-
jobObject->deleteLater();
10751071
}
10761072
_activeSearchJobs.clear();
10771073
_inFlightSearchRequests = 0;
@@ -1090,7 +1086,6 @@ void UnifiedSearchResultsListModel::abortProviderDiscovery()
10901086
if (const auto job = qobject_cast<JsonApiJob *>(_providerDiscoveryJob.data()); job && job->reply() && job->reply()->isRunning()) {
10911087
job->reply()->abort();
10921088
}
1093-
_providerDiscoveryJob->deleteLater();
10941089
_providerDiscoveryJob.clear();
10951090
_providersLoading = false;
10961091
}

src/gui/systray.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,8 @@ void Systray::showSearchWindow(int userIndex)
461461
return;
462462
}
463463

464-
auto *const searchModel = new UnifiedSearchResultsListModel(accountState.data(), accountState.data());
464+
auto *const searchModel = new UnifiedSearchResultsListModel(accountState.data());
465+
searchModel->setParent(accountState.data());
465466
const QVariantMap initialProperties{
466467
{"account", QVariantMap{
467468
{"avatar", user->avatarUrl()},

src/gui/wizard/qml/WizardButton.qml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55

66
import QtQuick
77
import QtQuick.Controls.Basic as BasicControls
8+
import QtQuick.Effects
89
import QtQuick.Layouts
9-
import Qt5Compat.GraphicalEffects
1010
import Style
1111

1212
BasicControls.Button {
@@ -56,13 +56,13 @@ BasicControls.Button {
5656
Accessible.ignored: true
5757
}
5858

59-
ColorOverlay {
59+
MultiEffect {
6060
objectName: "wizardButtonLeadingIconTint"
6161
anchors.fill: leadingIconImage
6262
visible: root.tintIcon
6363
source: leadingIconImage
64-
color: root.iconTintColor
65-
cached: true
64+
colorization: 1.0
65+
colorizationColor: root.iconTintColor
6666
Accessible.ignored: true
6767
}
6868
}

src/gui/wizard/qml/WizardMenuItem.qml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55

66
import QtQuick
77
import QtQuick.Controls.Basic as BasicControls
8+
import QtQuick.Effects
89
import QtQuick.Layouts
9-
import Qt5Compat.GraphicalEffects
1010

1111
import Style
1212

@@ -42,13 +42,13 @@ BasicControls.MenuItem {
4242
Accessible.ignored: true
4343
}
4444

45-
ColorOverlay {
45+
MultiEffect {
4646
objectName: "wizardMenuItemIconTint"
4747
anchors.fill: menuIconImage
4848
visible: root.tintIcon
4949
source: menuIconImage
50-
color: root.iconTintColor
51-
cached: true
50+
colorization: 1.0
51+
colorizationColor: root.iconTintColor
5252
Accessible.ignored: true
5353
}
5454

test/qml/search/testsearch.qml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,8 @@ Item {
578578
const iconTint = findChild(wizardHoverButton, "wizardButtonLeadingIconTint")
579579
verify(iconTint !== null)
580580
compare(iconTint.visible, true)
581-
compare(iconTint.color.toString(), Style.wizardPrimaryText.toString())
581+
compare(iconTint.colorization, 1.0)
582+
compare(iconTint.colorizationColor.toString(), Style.wizardPrimaryText.toString())
582583
}
583584

584585
function test_trailingIconCannotOverlapLongText() {
@@ -618,7 +619,8 @@ Item {
618619
const iconTint = findChild(wizardMenuHoverItem, "wizardMenuItemIconTint")
619620
verify(iconTint !== null)
620621
compare(iconTint.visible, true)
621-
compare(iconTint.color.toString(), Style.wizardPrimaryText.toString())
622+
compare(iconTint.colorization, 1.0)
623+
compare(iconTint.colorizationColor.toString(), Style.wizardPrimaryText.toString())
622624
}
623625
}
624626

test/testunifiedsearchpeoplemodel.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ class TestUnifiedSearchPeopleModel : public QObject
3030
Q_OBJECT
3131

3232
private Q_SLOTS:
33+
void retryIsExposedAsSlot()
34+
{
35+
const auto methodIndex = OCC::UnifiedSearchPeopleModel::staticMetaObject.indexOfSlot("retry()");
36+
QVERIFY(methodIndex >= 0);
37+
}
38+
3339
void emptyQueryShowsSignedInUserWithoutRequest()
3440
{
3541
auto qnam = std::unique_ptr<FakeQNAM>(new FakeQNAM({}));

0 commit comments

Comments
 (0)