Skip to content

Commit

Permalink
Implement MSC 4228 Search Redirection
Browse files Browse the repository at this point in the history
See matrix-org/matrix-spec-proposals#4228 for details.
Since this is tricky to test without server-side support, I have added a basic implementation
to the mock server in appiumtests/login-server.py

1. Start appiumtests/login-server.py
2. Start neochat with "--test --ignore-ssl-errors" options
3. Open "Explore Rooms"
4. Search for the exact string "forbidden"
5. See new error message provided by server
  • Loading branch information
TobiasFella committed Nov 23, 2024
1 parent 9391e44 commit d14d576
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 4 deletions.
9 changes: 9 additions & 0 deletions appiumtests/login-server.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,15 @@ def create_room():
next_sync_payload = "sync_response_new_room"
return response

@app.route("/_matrix/client/v3/publicRooms", methods=["POST"])
def public_rooms():
if request.get_json()["filter"]["generic_search_term"] == "forbidden":
data = dict()
data["errcode"] = "M_FORBIDDEN"
data["error"] = "You are not allowed to search for this. Go to https://wikipedia.org for more information"
return data, 403
return dict()



if __name__ == "__main__":
Expand Down
29 changes: 28 additions & 1 deletion src/models/publicroomlistmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,23 @@

using namespace Quotient;

class NeoChatQueryPublicRoomsJob : public QueryPublicRoomsJob
{
public:
explicit NeoChatQueryPublicRoomsJob(const QString &server = {},
std::optional<int> limit = std::nullopt,
const QString &since = {},
const std::optional<Filter> &filter = std::nullopt,
std::optional<bool> includeAllNetworks = std::nullopt,
const QString &thirdPartyInstanceId = {})
: QueryPublicRoomsJob(server, limit, since, filter, includeAllNetworks, thirdPartyInstanceId)
{
// TODO Remove once we can use libQuotient's job directly
// This is to make libQuotient happy about results not having the "chunk" field
setExpectedKeys({});
}
};

PublicRoomListModel::PublicRoomListModel(QObject *parent)
: QAbstractListModel(parent)
{
Expand Down Expand Up @@ -153,6 +170,8 @@ void PublicRoomListModel::next(int limit)
if (m_connection == nullptr || limit < 1) {
return;
}
m_redirectedText.clear();
Q_EMIT redirectedChanged();

if (job) {
qCDebug(PublicRoomList) << "Other job running, ignore";
Expand All @@ -163,7 +182,7 @@ void PublicRoomListModel::next(int limit)
if (m_showOnlySpaces) {
roomTypes += QLatin1String("m.space");
}
job = m_connection->callApi<QueryPublicRoomsJob>(m_server, limit, nextBatch, QueryPublicRoomsJob::Filter{m_searchText, roomTypes});
job = m_connection->callApi<NeoChatQueryPublicRoomsJob>(m_server, limit, nextBatch, QueryPublicRoomsJob::Filter{m_searchText, roomTypes});
Q_EMIT searchingChanged();

connect(job, &BaseJob::finished, this, [this] {
Expand All @@ -181,6 +200,9 @@ void PublicRoomListModel::next(int limit)
this->beginInsertRows({}, rooms.count(), rooms.count() + job->chunk().count() - 1);
rooms.append(job->chunk());
this->endInsertRows();
} else if (job->error() == BaseJob::ContentAccessError) {
m_redirectedText = job->jsonData()[u"error"_s].toString();
Q_EMIT redirectedChanged();
}

this->job = nullptr;
Expand Down Expand Up @@ -302,4 +324,9 @@ bool PublicRoomListModel::searching() const
return job != nullptr;
}

QString PublicRoomListModel::redirectedText() const
{
return m_redirectedText;
}

#include "moc_publicroomlistmodel.cpp"
9 changes: 9 additions & 0 deletions src/models/publicroomlistmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ class PublicRoomListModel : public QAbstractListModel
*/
Q_PROPERTY(bool searching READ searching NOTIFY searchingChanged)

/**
* @brief The text returned by the server after redirection
*/
Q_PROPERTY(QString redirectedText READ redirectedText NOTIFY redirectedChanged)

public:
/**
* @brief Defines the model roles.
Expand Down Expand Up @@ -113,6 +118,8 @@ class PublicRoomListModel : public QAbstractListModel
*/
Q_INVOKABLE void search(int limit = 50);

QString redirectedText() const;

private:
QPointer<NeoChatConnection> m_connection = nullptr;
QString m_server;
Expand All @@ -135,11 +142,13 @@ class PublicRoomListModel : public QAbstractListModel
QList<Quotient::PublicRoomsChunk> rooms;

Quotient::QueryPublicRoomsJob *job = nullptr;
QString m_redirectedText;

Q_SIGNALS:
void connectionChanged();
void serverChanged();
void searchTextChanged();
void showOnlySpacesChanged();
void searchingChanged();
void redirectedChanged();
};
3 changes: 3 additions & 0 deletions src/qml/ExploreRoomsPage.qml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ SearchPage {
signal roomSelected(string roomId, string displayName, url avatarUrl, string alias, string topic, int memberCount, bool isJoined)

title: i18nc("@action:title", "Explore Rooms")
customPlaceholderText: publicRoomListModel.redirectedText
customPlaceholderIcon: "data-warning"

Component.onCompleted: focusSearch()

Expand Down Expand Up @@ -93,6 +95,7 @@ SearchPage {

activeFocusOnTab: false // We handle moving to this item via up/down arrows, otherwise the tab order is wacky
text: i18n("Enter a Room Manually")
visible: publicRoomListModel.redirectedText.length === 0
icon.name: "compass"
icon.width: Kirigami.Units.gridUnit * 2
icon.height: Kirigami.Units.gridUnit * 2
Expand Down
24 changes: 21 additions & 3 deletions src/qml/SearchPage.qml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,17 @@ Kirigami.ScrollablePage {
*/
property bool showSearchButton: true

/**
* @brief Message to be shown in a custom placeholder.
* The custom placeholder will be shown if the text is not empty
*/
property alias customPlaceholderText: customPlaceholder.text

/**
* @brief icon for the custom placeholder
*/
property string customPlaceholderIcon: ""

/**
* @brief Force the search field to be focussed.
*/
Expand Down Expand Up @@ -167,18 +178,25 @@ Kirigami.ScrollablePage {
Kirigami.PlaceholderMessage {
id: noSearchMessage
anchors.centerIn: parent
visible: searchField.text.length === 0 && listView.count === 0
visible: searchField.text.length === 0 && listView.count === 0 && !root.showCustomPlaceholder && customPlaceholder.text.length === 0
}

Kirigami.PlaceholderMessage {
id: noResultMessage
anchors.centerIn: parent
visible: searchField.text.length > 0 && listView.count === 0 && !root.model.searching
visible: searchField.text.length > 0 && listView.count === 0 && !root.model.searching && customPlaceholder.text.length === 0
}

Kirigami.PlaceholderMessage {
id: customPlaceholder
anchors.centerIn: parent
visible: searchField.text.length > 0 && listView.count === 0 && !root.model.searching && text.length > 0
icon.name: root.customPlaceholderIcon
}

Kirigami.LoadingPlaceholder {
anchors.centerIn: parent
visible: searchField.text.length > 0 && listView.count === 0 && root.model.searching
visible: searchField.text.length > 0 && listView.count === 0 && root.model.searching && customPlaceholder.text.length === 0
}

Keys.onUpPressed: {
Expand Down

0 comments on commit d14d576

Please sign in to comment.