From 92668996d7918ee822b301b07c5eafeef658e47d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 15 Sep 2022 20:41:55 -0500 Subject: [PATCH] Add search to room directory landing page (#70) Part of https://github.com/matrix-org/matrix-public-archive/issues/6 --- public/css/room-directory.css | 14 ++- public/js/entry-client-hydrogen.js | 3 +- public/js/entry-client-room-directory.js | 3 +- server/lib/matrix-utils/fetch-public-rooms.js | 16 ++-- server/lib/status-error.js | 2 + server/routes/room-directory-routes.js | 4 +- server/routes/room-routes.js | 15 ++- shared/hydrogen-vm-render-script.js | 4 + shared/lib/url-creator.js | 5 +- shared/room-directory-vm-render-script.js | 5 +- shared/viewmodels/RoomDirectoryViewModel.js | 13 +++ shared/views/RoomCardView.js | 2 + shared/views/RoomDirectoryView.js | 96 ++++++++++++------- test/client-utils.js | 22 ++++- test/e2e-tests.js | 51 ++++++++++ 15 files changed, 194 insertions(+), 61 deletions(-) diff --git a/public/css/room-directory.css b/public/css/room-directory.css index e808e40..fcff473 100644 --- a/public/css/room-directory.css +++ b/public/css/room-directory.css @@ -3,10 +3,6 @@ } .RoomDirectoryView_header { - display: flex; - flex-direction: column; - align-items: center; - padding-top: 80px; padding-left: 10px; padding-bottom: 80px; padding-right: 10px; @@ -14,6 +10,13 @@ background-color: #fafafa; } +.RoomDirectoryView_headerForm { + display: flex; + flex-direction: column; + align-items: center; + padding-top: 80px; +} + .RoomDirectoryView_matrixLogo { width: 100%; max-width: 148px; @@ -50,6 +53,7 @@ width: 100%; height: 32px; padding-left: 32px; + padding-right: 8px; background: rgba(141, 151, 165, 0.15); border-radius: 8px; @@ -71,7 +75,7 @@ display: flex; flex-direction: column; align-items: center; - margin-bottom: 80px; + margin-bottom: 160px; } .RoomDirectoryView_paginationButtonCombo { diff --git a/public/js/entry-client-hydrogen.js b/public/js/entry-client-hydrogen.js index 251663e..3f43fdd 100644 --- a/public/js/entry-client-hydrogen.js +++ b/public/js/entry-client-hydrogen.js @@ -1,2 +1 @@ -import mounted from 'matrix-public-archive-shared/hydrogen-vm-render-script'; -console.log('mounted', mounted); +import 'matrix-public-archive-shared/hydrogen-vm-render-script'; diff --git a/public/js/entry-client-room-directory.js b/public/js/entry-client-room-directory.js index 40f5b67..3bf1b27 100644 --- a/public/js/entry-client-room-directory.js +++ b/public/js/entry-client-room-directory.js @@ -1,2 +1 @@ -import mounted from 'matrix-public-archive-shared/room-directory-vm-render-script'; -console.log('mounted', mounted); +import 'matrix-public-archive-shared/room-directory-vm-render-script'; diff --git a/server/lib/matrix-utils/fetch-public-rooms.js b/server/lib/matrix-utils/fetch-public-rooms.js index 9530563..cba8adf 100644 --- a/server/lib/matrix-utils/fetch-public-rooms.js +++ b/server/lib/matrix-utils/fetch-public-rooms.js @@ -10,19 +10,13 @@ const config = require('../config'); const matrixServerUrl = config.get('matrixServerUrl'); assert(matrixServerUrl); -async function fetchPublicRooms(accessToken, { server, paginationToken, limit } = {}) { +async function fetchPublicRooms(accessToken, { server, searchTerm, paginationToken, limit } = {}) { assert(accessToken); let qs = new URLSearchParams(); if (server) { qs.append('server', server); } - if (paginationToken) { - qs.append('since', paginationToken); - } - if (limit) { - qs.append('limit', limit); - } const publicRoomsEndpoint = urlJoin( matrixServerUrl, @@ -30,6 +24,14 @@ async function fetchPublicRooms(accessToken, { server, paginationToken, limit } ); const publicRoomsRes = await fetchEndpointAsJson(publicRoomsEndpoint, { + method: 'POST', + body: { + filter: { + generic_search_term: searchTerm, + }, + since: paginationToken, + limit, + }, accessToken, }); diff --git a/server/lib/status-error.js b/server/lib/status-error.js index 3d34e39..8ce8eb0 100644 --- a/server/lib/status-error.js +++ b/server/lib/status-error.js @@ -10,6 +10,8 @@ function StatusError(status, inputMessage) { } this.message = `${status} - ${message}`; + // This will be picked by the default Express error handler and assign the status code, + // https://expressjs.com/en/guide/error-handling.html#the-default-error-handler this.status = status; this.name = 'StatusError'; Error.captureStackTrace(this, StatusError); diff --git a/server/routes/room-directory-routes.js b/server/routes/room-directory-routes.js index ea44515..d5fae06 100644 --- a/server/routes/room-directory-routes.js +++ b/server/routes/room-directory-routes.js @@ -29,11 +29,13 @@ router.get( '/', asyncHandler(async function (req, res) { const paginationToken = req.query.page; + const searchTerm = req.query.search; const { rooms, nextPaginationToken, prevPaginationToken } = await fetchPublicRooms( matrixAccessToken, { //server: TODO, + searchTerm, paginationToken, // It would be good to grab more rooms than we display in case we need // to filter any out but then the pagination tokens with the homeserver @@ -54,7 +56,7 @@ router.get( rooms, nextPaginationToken, prevPaginationToken, - searchTerm: 'foobar (TODO)', + searchTerm, config: { basePath, matrixServerUrl, diff --git a/server/routes/room-routes.js b/server/routes/room-routes.js index 094d452..7bd1041 100644 --- a/server/routes/room-routes.js +++ b/server/routes/room-routes.js @@ -81,10 +81,14 @@ router.get( '/', asyncHandler(async function (req, res) { const roomIdOrAlias = req.params.roomIdOrAlias; - assert(roomIdOrAlias.startsWith('!') || roomIdOrAlias.startsWith('#')); + const isValidAlias = roomIdOrAlias.startsWith('!') || roomIdOrAlias.startsWith('#'); + if (!isValidAlias) { + throw new StatusError(404, `Invalid alias given: ${roomIdOrAlias}`); + } // In case we're joining a new room for the first time, - // let's avoid redirecting to our join event + // let's avoid redirecting to our join event by getting + // the time before we join and looking backwards. const dateBeforeJoin = Date.now(); // We have to wait for the room join to happen first before we can fetch @@ -99,7 +103,7 @@ router.get( direction: 'b', }); if (!originServerTs) { - throw new StatusError(404, 'Unable to find day with an history'); + throw new StatusError(404, 'Unable to find day with history'); } // Redirect to a day with messages @@ -127,7 +131,10 @@ router.get( timeoutMiddleware, asyncHandler(async function (req, res) { const roomIdOrAlias = req.params.roomIdOrAlias; - assert(roomIdOrAlias.startsWith('!') || roomIdOrAlias.startsWith('#')); + const isValidAlias = roomIdOrAlias.startsWith('!') || roomIdOrAlias.startsWith('#'); + if (!isValidAlias) { + throw new StatusError(404, `Invalid alias given: ${roomIdOrAlias}`); + } const { fromTimestamp, toTimestamp, hourRange, fromHour, toHour } = parseArchiveRangeFromReq(req); diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index 505aa56..656da75 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -123,6 +123,8 @@ function supressBlankAnchorsReloadingThePage() { // eslint-disable-next-line max-statements async function mountHydrogen() { + console.log('Mounting Hydrogen...'); + console.time('Completed mounting Hydrogen'); const appElement = document.querySelector('#app'); const platformConfig = {}; @@ -309,6 +311,8 @@ async function mountHydrogen() { addSupportClasses(); supressBlankAnchorsReloadingThePage(); + + console.timeEnd('Completed mounting Hydrogen'); } // N.B.: When we run this in a virtual machine (`vm`), it will return the last diff --git a/shared/lib/url-creator.js b/shared/lib/url-creator.js index a4d29ea..e03550a 100644 --- a/shared/lib/url-creator.js +++ b/shared/lib/url-creator.js @@ -17,8 +17,11 @@ class URLCreator { this._basePath = basePath; } - roomDirectoryUrl({ paginationToken } = {}) { + roomDirectoryUrl({ searchTerm, paginationToken } = {}) { let qs = new URLSearchParams(); + if (searchTerm) { + qs.append('search', searchTerm); + } if (paginationToken) { qs.append('page', paginationToken); } diff --git a/shared/room-directory-vm-render-script.js b/shared/room-directory-vm-render-script.js index 60e9c9d..2606ea1 100644 --- a/shared/room-directory-vm-render-script.js +++ b/shared/room-directory-vm-render-script.js @@ -17,7 +17,6 @@ assert(rooms); const nextPaginationToken = window.matrixPublicArchiveContext.nextPaginationToken; const prevPaginationToken = window.matrixPublicArchiveContext.prevPaginationToken; const searchTerm = window.matrixPublicArchiveContext.searchTerm; -assert(searchTerm); const config = window.matrixPublicArchiveContext.config; assert(config); assert(config.matrixServerUrl); @@ -27,6 +26,8 @@ assert(config.basePath); const matrixPublicArchiveURLCreator = new MatrixPublicArchiveURLCreator(config.basePath); async function mountHydrogen() { + console.log('Mounting Hydrogen...'); + console.time('Completed mounting Hydrogen'); const appElement = document.querySelector('#app'); const roomDirectoryViewModel = new RoomDirectoryViewModel({ @@ -34,6 +35,7 @@ async function mountHydrogen() { homeserverName: config.matrixServerName, matrixPublicArchiveURLCreator, rooms, + searchTerm, nextPaginationToken, prevPaginationToken, }); @@ -41,6 +43,7 @@ async function mountHydrogen() { const view = new RoomDirectoryView(roomDirectoryViewModel); appElement.replaceChildren(view.mount()); + console.timeEnd('Completed mounting Hydrogen'); } // N.B.: When we run this in a virtual machine (`vm`), it will return the last diff --git a/shared/viewmodels/RoomDirectoryViewModel.js b/shared/viewmodels/RoomDirectoryViewModel.js index 766d1fc..8c083bc 100644 --- a/shared/viewmodels/RoomDirectoryViewModel.js +++ b/shared/viewmodels/RoomDirectoryViewModel.js @@ -14,6 +14,7 @@ class RoomDirectoryViewModel extends ViewModel { homeserverName, matrixPublicArchiveURLCreator, rooms, + searchTerm, nextPaginationToken, prevPaginationToken, } = options; @@ -39,6 +40,7 @@ class RoomDirectoryViewModel extends ViewModel { }; }) ); + this._searchTerm = searchTerm; this._nextPaginationToken = nextPaginationToken; this._prevPaginationToken = prevPaginationToken; } @@ -51,9 +53,19 @@ class RoomDirectoryViewModel extends ViewModel { return this._matrixPublicArchiveURLCreator.roomDirectoryUrl(); } + get searchTerm() { + return this._searchTerm || ''; + } + + setSearchTerm(newSearchTerm) { + this._searchTerm = newSearchTerm; + this.emitChange('searchTerm'); + } + get nextPageUrl() { if (this._nextPaginationToken) { return this._matrixPublicArchiveURLCreator.roomDirectoryUrl({ + searchTerm: this.searchTerm, paginationToken: this._nextPaginationToken, }); } @@ -64,6 +76,7 @@ class RoomDirectoryViewModel extends ViewModel { get prevPageUrl() { if (this._prevPaginationToken) { return this._matrixPublicArchiveURLCreator.roomDirectoryUrl({ + searchTerm: this.searchTerm, paginationToken: this._prevPaginationToken, }); } diff --git a/shared/views/RoomCardView.js b/shared/views/RoomCardView.js index a85abc9..38fa0bb 100644 --- a/shared/views/RoomCardView.js +++ b/shared/views/RoomCardView.js @@ -32,6 +32,8 @@ class RoomCardView extends TemplateView { className: { RoomCardView: true, }, + 'data-room-id': vm.roomId, + 'data-testid': 'room-card', }, [ t.a( diff --git a/shared/views/RoomDirectoryView.js b/shared/views/RoomDirectoryView.js index 8819a6d..8ba31ed 100644 --- a/shared/views/RoomDirectoryView.js +++ b/shared/views/RoomDirectoryView.js @@ -7,6 +7,14 @@ const RoomCardView = require('./RoomCardView'); class RoomDirectoryView extends TemplateView { render(t, vm) { + // Make sure we don't overwrite the search input value if someone has typed + // before the JavaScript has loaded + const searchInputBeforeRendering = document.querySelector('.RoomDirectoryView_searchInput'); + if (searchInputBeforeRendering) { + const searchInputValueBeforeRendering = searchInputBeforeRendering.value; + vm.setSearchTerm(searchInputValueBeforeRendering); + } + const roomList = new ListView( { className: 'RoomDirectoryView_roomList', @@ -30,46 +38,62 @@ class RoomDirectoryView extends TemplateView { }, [ t.header({ className: 'RoomDirectoryView_header' }, [ - t.a( - { - className: 'RoomDirectoryView_matrixLogo', - title: 'Matrix Public Archive', - href: vm.roomDirectoryUrl, - }, - [t.view(new MatrixLogoView(vm))] - ), - t.h3( - { className: 'RoomDirectoryView_subHeader' }, - 'Browse thousands of rooms using Matrix...' - ), - t.div({ className: 'RoomDirectoryView_search' }, [ - t.svg( + t.form({ className: 'RoomDirectoryView_headerForm', method: 'GET' }, [ + t.a( { - className: 'RoomDirectoryView_searchIcon', - viewBox: '0 0 18 18', - fill: 'currentColor', - xmlns: 'http://www.w3.org/2000/svg', + className: 'RoomDirectoryView_matrixLogo', + title: 'Matrix Public Archive', + href: vm.roomDirectoryUrl, }, - [ - t.path({ - 'fill-rule': 'evenodd', - 'clip-rule': 'evenodd', - d: 'M12.1333 8.06667C12.1333 10.3126 10.3126 12.1333 8.06667 12.1333C5.82071 12.1333 4 10.3126 4 8.06667C4 5.82071 5.82071 4 8.06667 4C10.3126 4 12.1333 5.82071 12.1333 8.06667ZM12.9992 11.5994C13.7131 10.6044 14.1333 9.38463 14.1333 8.06667C14.1333 4.71614 11.4172 2 8.06667 2C4.71614 2 2 4.71614 2 8.06667C2 11.4172 4.71614 14.1333 8.06667 14.1333C9.38457 14.1333 10.6043 13.7131 11.5992 12.9993C11.6274 13.0369 11.6586 13.0729 11.6928 13.1071L14.2928 15.7071C14.6833 16.0977 15.3165 16.0977 15.707 15.7071C16.0975 15.3166 16.0975 14.6834 15.707 14.2929L13.107 11.6929C13.0728 11.6587 13.0368 11.6276 12.9992 11.5994Z', - }), - ] + [t.view(new MatrixLogoView(vm))] ), - t.input({ - className: 'RoomDirectoryView_searchInput', - placeholder: 'Search rooms (disabled, not implemented yet)', - disabled: true, - }), - ]), - t.div({ className: 'RoomDirectoryView_homeserverSelectSection' }, [ - t.div({}, 'Show: Matrix rooms on'), - t.select( - { className: 'RoomDirectoryView_homeserverSelector' }, - availableHomeserverOptionElements + t.h3( + { className: 'RoomDirectoryView_subHeader' }, + 'Browse thousands of rooms using Matrix...' ), + t.div({ className: 'RoomDirectoryView_search' }, [ + t.svg( + { + className: 'RoomDirectoryView_searchIcon', + viewBox: '0 0 18 18', + fill: 'currentColor', + xmlns: 'http://www.w3.org/2000/svg', + }, + [ + t.path({ + 'fill-rule': 'evenodd', + 'clip-rule': 'evenodd', + d: 'M12.1333 8.06667C12.1333 10.3126 10.3126 12.1333 8.06667 12.1333C5.82071 12.1333 4 10.3126 4 8.06667C4 5.82071 5.82071 4 8.06667 4C10.3126 4 12.1333 5.82071 12.1333 8.06667ZM12.9992 11.5994C13.7131 10.6044 14.1333 9.38463 14.1333 8.06667C14.1333 4.71614 11.4172 2 8.06667 2C4.71614 2 2 4.71614 2 8.06667C2 11.4172 4.71614 14.1333 8.06667 14.1333C9.38457 14.1333 10.6043 13.7131 11.5992 12.9993C11.6274 13.0369 11.6586 13.0729 11.6928 13.1071L14.2928 15.7071C14.6833 16.0977 15.3165 16.0977 15.707 15.7071C16.0975 15.3166 16.0975 14.6834 15.707 14.2929L13.107 11.6929C13.0728 11.6587 13.0368 11.6276 12.9992 11.5994Z', + }), + ] + ), + t.input({ + type: 'search', + className: 'RoomDirectoryView_searchInput', + placeholder: 'Search rooms', + name: 'search', + value: vm.searchTerm, + // Autocomplete is disabled because browsers share autocomplete + // suggestions across domains and this uses a very common + // `name="search"`. The name is important because it's what + // shows up in the query parameters when the `
` is submitted. I wish we could scope the + // autocomplete suggestions to the apps domain + // (https://github.com/whatwg/html/issues/8284). Trying some + // custom non-spec value here also doesn't seem to work (Chrome + // decides to autofill based on `name="search"`). + autocomplete: 'off', + autocapitalize: 'off', + 'data-testid': 'room-directory-search-input', + }), + ]), + t.div({ className: 'RoomDirectoryView_homeserverSelectSection' }, [ + t.div({}, 'Show: Matrix rooms on'), + t.select( + { className: 'RoomDirectoryView_homeserverSelector', name: 'homeserver' }, + availableHomeserverOptionElements + ), + ]), ]), ]), t.main({ className: 'RoomDirectoryView_mainContent' }, [ diff --git a/test/client-utils.js b/test/client-utils.js index b13e7b3..6d570ff 100644 --- a/test/client-utils.js +++ b/test/client-utils.js @@ -16,6 +16,19 @@ function getTxnId() { return `${new Date().getTime()}--${txnCount}`; } +// Basic slugify function, plenty of edge cases and should not be used for +// production. +function slugify(inputText) { + return ( + inputText + .toLowerCase() + // Replace whitespace with hyphens + .replace(/\s+/g, '-') + // Remove anything not alpha-numeric or hypen + .replace(/[^a-z0-9-]+/g, '') + ); +} + async function ensureUserRegistered({ matrixServerUrl, username }) { const registerResponse = await fetchEndpointAsJson( urlJoin(matrixServerUrl, '/_matrix/client/v3/register'), @@ -73,19 +86,23 @@ async function getTestClientForHs(testMatrixServerUrl) { } // Create a public room to test in -async function createTestRoom(client, overrideCreateOptions) { +async function createTestRoom(client, overrideCreateOptions = {}) { let qs = new URLSearchParams(); if (client.applicationServiceUserIdOverride) { qs.append('user_id', client.applicationServiceUserIdOverride); } + const roomName = overrideCreateOptions.name || 'the hangout spot'; + const roomAlias = slugify(roomName + getTxnId()); + const createRoomResponse = await fetchEndpointAsJson( urlJoin(client.homeserverUrl, `/_matrix/client/v3/createRoom?${qs.toString()}`), { method: 'POST', body: { preset: 'public_chat', - name: 'the hangout spot', + name: roomName, + room_alias_name: roomAlias, initial_state: [ { type: 'm.room.history_visibility', @@ -95,6 +112,7 @@ async function createTestRoom(client, overrideCreateOptions) { }, }, ], + visibility: 'public', ...overrideCreateOptions, }, accessToken: client.accessToken, diff --git a/test/e2e-tests.js b/test/e2e-tests.js index 0522af1..7a65bf2 100644 --- a/test/e2e-tests.js +++ b/test/e2e-tests.js @@ -507,6 +507,57 @@ describe('matrix-public-archive', () => { `will render a room with a sparse amount of messages (a few per day) with no contamination between days` ); + describe('Room directory', () => { + it('room search narrows down results', async () => { + const client = await getTestClientForHs(testMatrixServerUrl1); + // This is just an extra room to fill out the room directory and make sure + // that it does not appear when searching. + await createTestRoom(client); + + // Then create two rooms we will find with search + const timeToken = Date.now(); + const roomPlanetPrefix = `planet-${timeToken}`; + const roomSaturnId = await createTestRoom(client, { + name: `${roomPlanetPrefix}-saturn`, + }); + const roomMarsId = await createTestRoom(client, { + name: `${roomPlanetPrefix}-mars`, + }); + + // Browse the room directory without search to see many rooms + archiveUrl = matrixPublicArchiveURLCreator.roomDirectoryUrl(); + const roomDirectoryPageHtml = await fetchEndpointAsText(archiveUrl); + const dom = parseHTML(roomDirectoryPageHtml); + + const roomsOnPageWithoutSearch = [ + ...dom.document.querySelectorAll(`[data-testid="room-card"]`), + ].map((roomCardEl) => { + return roomCardEl.getAttribute('data-room-id'); + }); + + // Then browse the room directory again, this time with the search + // narrowing down results. + archiveUrl = matrixPublicArchiveURLCreator.roomDirectoryUrl({ + searchTerm: roomPlanetPrefix, + }); + const roomDirectoryWithSearchPageHtml = await fetchEndpointAsText(archiveUrl); + const domWithSearch = parseHTML(roomDirectoryWithSearchPageHtml); + + const roomsOnPageWithSearch = [ + ...domWithSearch.document.querySelectorAll(`[data-testid="room-card"]`), + ].map((roomCardEl) => { + return roomCardEl.getAttribute('data-room-id'); + }); + + // Assert that the rooms we searched for are visible + assert.deepStrictEqual(roomsOnPageWithSearch.sort(), [roomSaturnId, roomMarsId].sort()); + + // Sanity check that search does something. Assert that it's not showing + // the same results as if we didn't make any search. + assert.notDeepStrictEqual(roomsOnPageWithSearch.sort(), roomsOnPageWithoutSearch.sort()); + }); + }); + describe('access controls', () => { it('not allowed to view private room even when the archiver user is in the room', async () => { const client = await getTestClientForHs(testMatrixServerUrl1);