From 08254cbb4931ab67a5a399701bb572fd1a670af6 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 2 Nov 2022 04:27:30 -0500 Subject: [PATCH] Add a way to jump forwards and backwards to more activity in the room (seamless navigation) (#114) Fix https://github.com/matrix-org/matrix-public-archive/issues/46 Follow-up to https://github.com/matrix-org/matrix-public-archive/pull/71 Summary: - Changes the "Jump to next activity in room" to actually continue you to the next 100 messages ahead. Previously, it only jumped you to the single next event in the room which meant a lot of backwards overlap each time. - Jumping this direction will also start your scroll position at the top of the timeline to continue reading seamlessly `?continue=top` - Adds "Jump to previous activity in room" to the top of the timeline to continue reading the previous part of the conversation. [1]: There is a caveat with seamless here which is also commented on in the code: > XXX: This is flawed in the fact that when we go `/messages?dir=b` it could backfill messages which will fill up the response before we perfectly connect and continue from the position they were jumping from before. When `/messages?dir=f` backfills, we won't have this problem anymore because any messages backfilled in the forwards direction would be picked up the same going backwards. (need forwards fill MSC) --- config/config.default.json | 4 +- package-lock.json | 14 +- package.json | 2 +- public/css/styles.css | 34 ++- server/lib/fetch-endpoint.js | 1 + .../fetch-events-from-timestamp-backwards.js | 45 +--- .../get-messages-response-from-event-id.js | 52 ++++ server/routes/room-routes.js | 82 +++++- shared/hydrogen-vm-render-script.js | 4 + shared/lib/custom-tile-utilities.js | 18 +- shared/lib/url-creator.js | 18 +- shared/viewmodels/ArchiveRoomViewModel.js | 38 ++- ...JumpToNextActivitySummaryTileViewModel.js} | 7 +- ...pToPreviousActivitySummaryTileViewModel.js | 38 +++ shared/views/ArchiveRoomView.js | 7 +- ...s => JumpToNextActivitySummaryTileView.js} | 15 +- .../JumpToPreviousActivitySummaryTileView.js | 44 ++++ test/e2e-tests.js | 247 +++++++++++++++++- test/{ => lib}/client-utils.js | 4 +- test/lib/test-error.js | 13 + 20 files changed, 592 insertions(+), 95 deletions(-) create mode 100644 server/lib/matrix-utils/get-messages-response-from-event-id.js rename shared/viewmodels/{NotEnoughEventsFromDaySummaryTileViewModel.js => JumpToNextActivitySummaryTileViewModel.js} (82%) create mode 100644 shared/viewmodels/JumpToPreviousActivitySummaryTileViewModel.js rename shared/views/{NotEnoughEventsFromDaySummaryTileView.js => JumpToNextActivitySummaryTileView.js} (78%) create mode 100644 shared/views/JumpToPreviousActivitySummaryTileView.js rename test/{ => lib}/client-utils.js (98%) create mode 100644 test/lib/test-error.js diff --git a/config/config.default.json b/config/config.default.json index 6166a3d..05dda90 100644 --- a/config/config.default.json +++ b/config/config.default.json @@ -3,7 +3,9 @@ "basePath": "http://localhost:3050", "matrixServerUrl": "http://localhost:8008/", "matrixServerName": "localhost", - "archiveMessageLimit": 500, + // Set this to 100 since that is the max that Synapse will backfill even if you do a + // `/messges?limit=1000` and we don't want to miss messages in between. + "archiveMessageLimit": 100, "requestTimeoutMs": 25000, "logOutputFromChildProcesses": false, //"jaegerTracesEndpoint": "http://localhost:14268/api/traces", diff --git a/package-lock.json b/package-lock.json index 6fc30b2..980f6cf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21,7 +21,7 @@ "@opentelemetry/semantic-conventions": "^1.3.1", "dompurify": "^2.3.9", "express": "^4.17.2", - "hydrogen-view-sdk": "npm:@mlm/hydrogen-view-sdk@^0.20.0-scratch", + "hydrogen-view-sdk": "npm:@mlm/hydrogen-view-sdk@^0.21.0-scratch", "json5": "^2.2.1", "linkedom": "^0.14.17", "matrix-public-archive-shared": "file:./shared/", @@ -3640,9 +3640,9 @@ }, "node_modules/hydrogen-view-sdk": { "name": "@mlm/hydrogen-view-sdk", - "version": "0.20.0-scratch", - "resolved": "https://registry.npmjs.org/@mlm/hydrogen-view-sdk/-/hydrogen-view-sdk-0.20.0-scratch.tgz", - "integrity": "sha512-5WktCwE3b8BgQ2ICx37c91hWrFJbAg+rUU1DTFeT/sFpRP+USqBHfz5IDrd4EcgZk7/FiMpV1lzpsJ5Wfip1NQ==", + "version": "0.21.0-scratch", + "resolved": "https://registry.npmjs.org/@mlm/hydrogen-view-sdk/-/hydrogen-view-sdk-0.21.0-scratch.tgz", + "integrity": "sha512-TxGl1AhzfCLkcea2wnVLGTW8pGZOlqFSLQK8nUCN/gRZQmMOWf60l5ZUCfR4HAZHZkUGW7VKR6XKkYMjojmOwg==", "dependencies": { "@matrix-org/olm": "https://gitlab.matrix.org/api/v4/projects/27/packages/npm/@matrix-org/olm/-/@matrix-org/olm-3.2.8.tgz", "another-json": "^0.2.0", @@ -8091,9 +8091,9 @@ } }, "hydrogen-view-sdk": { - "version": "npm:@mlm/hydrogen-view-sdk@0.20.0-scratch", - "resolved": "https://registry.npmjs.org/@mlm/hydrogen-view-sdk/-/hydrogen-view-sdk-0.20.0-scratch.tgz", - "integrity": "sha512-5WktCwE3b8BgQ2ICx37c91hWrFJbAg+rUU1DTFeT/sFpRP+USqBHfz5IDrd4EcgZk7/FiMpV1lzpsJ5Wfip1NQ==", + "version": "npm:@mlm/hydrogen-view-sdk@0.21.0-scratch", + "resolved": "https://registry.npmjs.org/@mlm/hydrogen-view-sdk/-/hydrogen-view-sdk-0.21.0-scratch.tgz", + "integrity": "sha512-TxGl1AhzfCLkcea2wnVLGTW8pGZOlqFSLQK8nUCN/gRZQmMOWf60l5ZUCfR4HAZHZkUGW7VKR6XKkYMjojmOwg==", "requires": { "@matrix-org/olm": "https://gitlab.matrix.org/api/v4/projects/27/packages/npm/@matrix-org/olm/-/@matrix-org/olm-3.2.8.tgz", "another-json": "^0.2.0", diff --git a/package.json b/package.json index 26701a6..2911e10 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "@opentelemetry/semantic-conventions": "^1.3.1", "dompurify": "^2.3.9", "express": "^4.17.2", - "hydrogen-view-sdk": "npm:@mlm/hydrogen-view-sdk@^0.20.0-scratch", + "hydrogen-view-sdk": "npm:@mlm/hydrogen-view-sdk@^0.21.0-scratch", "json5": "^2.2.1", "linkedom": "^0.14.17", "matrix-public-archive-shared": "file:./shared/", diff --git a/public/css/styles.css b/public/css/styles.css index 7181239..2b93f5d 100644 --- a/public/css/styles.css +++ b/public/css/styles.css @@ -284,31 +284,47 @@ summary { /* Some custom timeline, tiles stuff */ -.NotEnoughEventsFromDaySummaryTileView { - margin-top: 40px; - padding: 20px 12px; +.JumpToPreviousActivitySummaryTileView, +.JumpToNextActivitySummaryTileView { + padding: calc(20px - 1em) 0; background: rgba(46, 48, 51, 0.1); border-top: 1px solid rgba(46, 48, 51, 0.38); + border-bottom: 1px solid rgba(46, 48, 51, 0.38); } -.NotEnoughEventsFromDaySummaryTileView_summaryMessage { - margin-top: 0; +.JumpToPreviousActivitySummaryTileView { + /* no margin so it's easier to notice when you scroll up */ +} + +.JumpToNextActivitySummaryTileView { + margin-top: 40px; +} + +.JumpToNextActivitySummaryTileView_summaryMessage { + margin-top: 1em; + margin-left: 12px; + margin-bottom: 0; + margin-right: 12px; + font-size: 1.17em; } -.NotEnoughEventsFromDaySummaryTileView_nextActivityLink { +.JumpToActivitySummaryTileView_activityLink { + display: inline-block; + padding: 1em 12px; + text-decoration: none; font-weight: bold; } -.NotEnoughEventsFromDaySummaryTileView_nextActivityLink:hover, -.NotEnoughEventsFromDaySummaryTileView_nextActivityLink:focus { +.JumpToActivitySummaryTileView_activityLink:hover, +.JumpToActivitySummaryTileView_activityLink:focus { color: #0098d4; text-decoration: underline; } -.NotEnoughEventsFromDaySummaryTileView_nextActivityIcon { +.JumpToActivitySummaryTileView_activityIcon { margin-left: 1ch; vertical-align: bottom; } diff --git a/server/lib/fetch-endpoint.js b/server/lib/fetch-endpoint.js index 158e2c4..e615a2a 100644 --- a/server/lib/fetch-endpoint.js +++ b/server/lib/fetch-endpoint.js @@ -66,6 +66,7 @@ async function fetchEndpointAsJson(endpoint, options) { } module.exports = { + HTTPResponseError, fetchEndpoint, fetchEndpointAsText, fetchEndpointAsJson, diff --git a/server/lib/matrix-utils/fetch-events-from-timestamp-backwards.js b/server/lib/matrix-utils/fetch-events-from-timestamp-backwards.js index f3c1fae..3943279 100644 --- a/server/lib/matrix-utils/fetch-events-from-timestamp-backwards.js +++ b/server/lib/matrix-utils/fetch-events-from-timestamp-backwards.js @@ -1,12 +1,11 @@ 'use strict'; const assert = require('assert'); -const urlJoin = require('url-join'); - -const { fetchEndpointAsJson } = require('../fetch-endpoint'); -const timestampToEvent = require('./timestamp-to-event'); const { traceFunction } = require('../../tracing/trace-utilities'); +const timestampToEvent = require('./timestamp-to-event'); +const getMessagesResponseFromEventId = require('./get-messages-response-from-event-id'); + const config = require('../config'); const matrixServerUrl = config.get('matrixServerUrl'); assert(matrixServerUrl); @@ -62,38 +61,14 @@ async function fetchEventsFromTimestampBackwards({ accessToken, roomId, ts, limi }; } - // We only use this endpoint to get a pagination token we can use with - // `/messages`. - // - // We add `limit=0` here because we want to grab the pagination token right - // (before/after) the event. - // - // Add `filter={"lazy_load_members":true}` so that this endpoint responds - // without timing out by returning just the state for the sender of the - // included event. Otherwise, the homeserver returns all state in the room at - // that point in time which in big rooms, can be 100k member events that we - // don't care about anyway. Synapse seems to timeout at about the ~5k state - // event mark. - const contextEndpoint = urlJoin( - matrixServerUrl, - `_matrix/client/r0/rooms/${encodeURIComponent(roomId)}/context/${encodeURIComponent( - eventIdForTimestamp - )}?limit=0&filter={"lazy_load_members":true}` - ); - const contextResData = await fetchEndpointAsJson(contextEndpoint, { - accessToken, - }); - - // Add `filter={"lazy_load_members":true}` to only get member state events for - // the messages included in the response - const messagesEndpoint = urlJoin( - matrixServerUrl, - `_matrix/client/r0/rooms/${encodeURIComponent(roomId)}/messages?dir=b&from=${encodeURIComponent( - contextResData.end - )}&limit=${limit}&filter={"lazy_load_members":true}` - ); - const messageResData = await fetchEndpointAsJson(messagesEndpoint, { + const messageResData = await getMessagesResponseFromEventId({ accessToken, + roomId, + eventId: eventIdForTimestamp, + // We go backwards because that's the direction that backfills events (Synapse + // doesn't backfill in the forward direction) + dir: 'b', + limit, }); const stateEventMap = {}; diff --git a/server/lib/matrix-utils/get-messages-response-from-event-id.js b/server/lib/matrix-utils/get-messages-response-from-event-id.js new file mode 100644 index 0000000..0892f3b --- /dev/null +++ b/server/lib/matrix-utils/get-messages-response-from-event-id.js @@ -0,0 +1,52 @@ +'use strict'; + +const assert = require('assert'); +const urlJoin = require('url-join'); + +const { fetchEndpointAsJson } = require('../fetch-endpoint'); + +const config = require('../config'); +const matrixServerUrl = config.get('matrixServerUrl'); +assert(matrixServerUrl); + +async function getMessagesResponseFromEventId({ accessToken, roomId, eventId, dir, limit }) { + // We only use this endpoint to get a pagination token we can use with + // `/messages`. + // + // We add `limit=0` here because we want to grab the pagination token right + // (before/after) the event. + // + // Add `filter={"lazy_load_members":true}` so that this endpoint responds + // without timing out by returning just the state for the sender of the + // included event. Otherwise, the homeserver returns all state in the room at + // that point in time which in big rooms, can be 100k member events that we + // don't care about anyway. Synapse seems to timeout at about the ~5k state + // event mark. + const contextEndpoint = urlJoin( + matrixServerUrl, + `_matrix/client/r0/rooms/${encodeURIComponent(roomId)}/context/${encodeURIComponent( + eventId + )}?limit=0&filter={"lazy_load_members":true}` + ); + const contextResData = await fetchEndpointAsJson(contextEndpoint, { + accessToken, + }); + + // Add `filter={"lazy_load_members":true}` to only get member state events for + // the messages included in the response + const messagesEndpoint = urlJoin( + matrixServerUrl, + `_matrix/client/r0/rooms/${encodeURIComponent( + roomId + )}/messages?dir=${dir}&from=${encodeURIComponent( + contextResData.end + )}&limit=${limit}&filter={"lazy_load_members":true}` + ); + const messageResData = await fetchEndpointAsJson(messagesEndpoint, { + accessToken, + }); + + return messageResData; +} + +module.exports = getMessagesResponseFromEventId; diff --git a/server/routes/room-routes.js b/server/routes/room-routes.js index 402f754..4686fab 100644 --- a/server/routes/room-routes.js +++ b/server/routes/room-routes.js @@ -10,10 +10,12 @@ const StatusError = require('../lib/status-error'); const timeoutMiddleware = require('./timeout-middleware'); const redirectToCorrectArchiveUrlIfBadSigil = require('./redirect-to-correct-archive-url-if-bad-sigil-middleware'); +const { HTTPResponseError } = require('../lib/fetch-endpoint'); const fetchRoomData = require('../lib/matrix-utils/fetch-room-data'); const fetchEventsFromTimestampBackwards = require('../lib/matrix-utils/fetch-events-from-timestamp-backwards'); const ensureRoomJoined = require('../lib/matrix-utils/ensure-room-joined'); const timestampToEvent = require('../lib/matrix-utils/timestamp-to-event'); +const getMessagesResponseFromEventId = require('../lib/matrix-utils/get-messages-response-from-event-id'); const renderHydrogenVmRenderScriptToPageHtml = require('../hydrogen-render/render-hydrogen-vm-render-script-to-page-html'); const MatrixPublicArchiveURLCreator = require('matrix-public-archive-shared/lib/url-creator'); @@ -157,25 +159,76 @@ router.get( assert(!Number.isNaN(ts), '?ts query parameter must be a number'); const dir = req.query.dir; assert(['f', 'b'].includes(dir), '?dir query parameter must be [f|b]'); + const scrollStartPosition = req.query.continue; // We have to wait for the room join to happen first before we can use the jump to // date endpoint const roomId = await ensureRoomJoined(matrixAccessToken, roomIdOrAlias, req.query.via); - // Find the closest day to today with messages - const { originServerTs } = await timestampToEvent({ - accessToken: matrixAccessToken, - roomId, - ts: ts, - direction: dir, - }); + let originServerTs; + let eventIdForTimestamp; + + try { + // Find the closest day to today with messages + ({ eventId: eventIdForTimestamp, originServerTs } = await timestampToEvent({ + accessToken: matrixAccessToken, + roomId, + ts: ts, + direction: dir, + })); + + // The goal is to go forward 100 messages, so that when we view the room at that + // point going backwards 100 message, we end up at the perfect sam continuation spot + // in the room. + // + // XXX: This is flawed in the fact that when we go `/messages?dir=b` it could + // backfill messages which will fill up the response before we perfectly connect and + // continue from the position they were jumping from before. When `/messages?dir=f` + // backfills, we won't have this problem anymore because any messages backfilled in + // the forwards direction would be picked up the same going backwards. + if (dir === 'f') { + // Use `/messages?dir=f` and get the `end` pagination token to paginate from. And + // then start the scroll from the top of the page so they can continue. + const archiveMessageLimit = config.get('archiveMessageLimit'); + const messageResData = await getMessagesResponseFromEventId({ + accessToken: matrixAccessToken, + roomId, + eventId: eventIdForTimestamp, + dir: 'f', + limit: archiveMessageLimit, + }); + + originServerTs = messageResData.chunk[messageResData.chunk.length - 1]?.origin_server_ts; + } + } catch (err) { + const is404Error = err instanceof HTTPResponseError && err.response.status === 404; + // Only throw if it's something other than a 404 error. 404 errors are fine, they + // just mean there is no more messages to paginate in that room. + if (!is404Error) { + throw err; + } + } + + // If we can't find any more messages to paginate to, just progress the date by a + // day in whatever direction they wanted to go so we can display the empty view for + // that day. if (!originServerTs) { - throw new StatusError(404, 'Unable to find day with history'); + const tsDate = new Date(ts); + const yyyy = tsDate.getUTCFullYear(); + const mm = tsDate.getUTCMonth(); + const dd = tsDate.getUTCDate(); + + const newDayDelta = dir === 'f' ? 1 : -1; + originServerTs = Date.UTC(yyyy, mm, dd + newDayDelta); } // Redirect to a day with messages res.redirect( - matrixPublicArchiveURLCreator.archiveUrlForDate(roomIdOrAlias, new Date(originServerTs)) + // TODO: Add query parameter that causes the client to start the scroll at the top + // when jumping forwards so they can continue reading where they left off. + matrixPublicArchiveURLCreator.archiveUrlForDate(roomIdOrAlias, new Date(originServerTs), { + scrollStartPosition, + }) ); }) ); @@ -199,6 +252,17 @@ router.get( const { fromTimestamp, toTimestamp, hourRange, fromHour, toHour } = parseArchiveRangeFromReq(req); + // Just 404 if anyone is trying to view the future, no need to waste resources on that + const nowTs = Date.now(); + if (fromTimestamp > nowTs) { + throw new StatusError( + 404, + `You can't view the history of a room on a future day (${new Date( + fromTimestamp + ).toISOString()} > ${new Date(nowTs).toISOString()}). Go back` + ); + } + // If the hourRange is defined, we force the range to always be 1 hour. If // the format isn't correct, redirect to the correct hour range if (hourRange && toHour !== fromHour + 1) { diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index 22a07c3..91be577 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -55,6 +55,9 @@ async function mountHydrogen() { console.time('Completed mounting Hydrogen'); const appElement = document.querySelector('#app'); + const qs = new URLSearchParams(window?.location?.search); + const scrollStartPosition = qs.get('continue'); + const platformConfig = {}; const assetPaths = {}; const platform = new Platform({ @@ -123,6 +126,7 @@ async function mountHydrogen() { // The timestamp from the URL that was originally visited dayTimestampFrom: fromTimestamp, dayTimestampTo: toTimestamp, + scrollStartPosition, events, stateEventMap, shouldIndex, diff --git a/shared/lib/custom-tile-utilities.js b/shared/lib/custom-tile-utilities.js index 06cadf1..e7c7b16 100644 --- a/shared/lib/custom-tile-utilities.js +++ b/shared/lib/custom-tile-utilities.js @@ -4,13 +4,17 @@ const { tileClassForEntry, viewClassForTile } = require('hydrogen-view-sdk'); -const NotEnoughEventsFromDaySummaryTileViewModel = require('matrix-public-archive-shared/viewmodels/NotEnoughEventsFromDaySummaryTileViewModel'); -const NotEnoughEventsFromDaySummaryTileView = require('matrix-public-archive-shared/views/NotEnoughEventsFromDaySummaryTileView'); +const JumpToPreviousActivitySummaryTileViewModel = require('matrix-public-archive-shared/viewmodels/JumpToPreviousActivitySummaryTileViewModel'); +const JumpToPreviousActivitySummaryTileView = require('matrix-public-archive-shared/views/JumpToPreviousActivitySummaryTileView'); +const JumpToNextActivitySummaryTileViewModel = require('matrix-public-archive-shared/viewmodels/JumpToNextActivitySummaryTileViewModel'); +const JumpToNextActivitySummaryTileView = require('matrix-public-archive-shared/views/JumpToNextActivitySummaryTileView'); function customTileClassForEntry(entry) { switch (entry.eventType) { - case 'org.matrix.archive.not_enough_events_from_day_summary': - return NotEnoughEventsFromDaySummaryTileViewModel; + case 'org.matrix.archive.jump_to_previous_activity_summary': + return JumpToPreviousActivitySummaryTileViewModel; + case 'org.matrix.archive.jump_to_next_activity_summary': + return JumpToNextActivitySummaryTileViewModel; default: return tileClassForEntry(entry); } @@ -18,8 +22,10 @@ function customTileClassForEntry(entry) { function customViewClassForTile(vm) { switch (vm.shape) { - case 'org.matrix.archive.not_enough_events_from_day_summary:shape': - return NotEnoughEventsFromDaySummaryTileView; + case 'org.matrix.archive.jump_to_previous_activity_summary:shape': + return JumpToPreviousActivitySummaryTileView; + case 'org.matrix.archive.jump_to_next_activity_summary:shape': + return JumpToNextActivitySummaryTileView; default: return viewClassForTile(vm); } diff --git a/shared/lib/url-creator.js b/shared/lib/url-creator.js index 9dafaa1..c34b94e 100644 --- a/shared/lib/url-creator.js +++ b/shared/lib/url-creator.js @@ -65,7 +65,7 @@ class URLCreator { return `${urlJoin(this._basePath, `${urlPath}`)}${qsToUrlPiece(qs)}`; } - archiveUrlForDate(roomIdOrAlias, date, { viaServers = [] } = {}) { + archiveUrlForDate(roomIdOrAlias, date, { viaServers = [], scrollStartPosition } = {}) { assert(roomIdOrAlias); assert(date); @@ -73,6 +73,9 @@ class URLCreator { [].concat(viaServers).forEach((viaServer) => { qs.append('via', viaServer); }); + if (scrollStartPosition) { + qs.append('continue', scrollStartPosition); + } const urlPath = this._getArchiveUrlPathForRoomIdOrAlias(roomIdOrAlias); @@ -83,7 +86,15 @@ class URLCreator { return `${urlJoin(this._basePath, `${urlPath}/date/${urlDate}`)}${qsToUrlPiece(qs)}`; } - archiveJumpUrlForRoom(roomIdOrAlias, { ts, dir }) { + archiveJumpUrlForRoom( + roomIdOrAlias, + { + ts, + dir, + // where the scroll position should continue from ['top'|'bottom'] + scrollStartPosition, + } + ) { assert(roomIdOrAlias); assert(ts); assert(dir); @@ -91,6 +102,9 @@ class URLCreator { let qs = new URLSearchParams(); qs.append('ts', ts); qs.append('dir', dir); + if (scrollStartPosition) { + qs.append('continue', scrollStartPosition); + } const urlPath = this._getArchiveUrlPathForRoomIdOrAlias(roomIdOrAlias); diff --git a/shared/viewmodels/ArchiveRoomViewModel.js b/shared/viewmodels/ArchiveRoomViewModel.js index b2c1bad..2e3e0ae 100644 --- a/shared/viewmodels/ArchiveRoomViewModel.js +++ b/shared/viewmodels/ArchiveRoomViewModel.js @@ -56,6 +56,7 @@ function makeEventEntryFromEventJson(eventJson, memberEvent) { } class ArchiveRoomViewModel extends ViewModel { + // eslint-disable-next-line max-statements constructor(options) { super(options); const { @@ -63,6 +64,7 @@ class ArchiveRoomViewModel extends ViewModel { room, dayTimestampFrom, dayTimestampTo, + scrollStartPosition, events, stateEventMap, shouldIndex, @@ -80,6 +82,7 @@ class ArchiveRoomViewModel extends ViewModel { this._room = room; this._dayTimestampFrom = dayTimestampFrom; this._dayTimestampTo = dayTimestampTo; + this._scrollStartPosition = scrollStartPosition === 'top' ? 'top' : 'bottom'; this._currentTopPositionEventEntry = null; this._matrixPublicArchiveURLCreator = new MatrixPublicArchiveURLCreator(basePath); this._basePath = basePath; @@ -247,6 +250,10 @@ class ArchiveRoomViewModel extends ViewModel { return this._currentTopPositionEventEntry; } + get scrollStartPosition() { + return this._scrollStartPosition; + } + get shouldShowRightPanel() { return this._shouldShowRightPanel; } @@ -298,8 +305,7 @@ class ArchiveRoomViewModel extends ViewModel { // in the timeline _addJumpSummaryEvents(inputEventList) { const events = [...inputEventList]; - // Add a summary item to the bottom of the timeline that explains if we found - // events on the day requested. + const hasEventsFromGivenDay = events[events.length - 1]?.origin_server_ts >= this._dayTimestampFrom; let daySummaryKind; @@ -310,9 +316,35 @@ class ArchiveRoomViewModel extends ViewModel { } else if (!hasEventsFromGivenDay) { daySummaryKind = 'no-events-in-day'; } + + // Add a summary item to the top of the timeline that allows you to jump to more + // previous activity. Also explain that you might have hit the beginning of the room. + // + // As long as there are events shown, have a button to jump to more previous activity + if (daySummaryKind !== 'no-events-at-all') { + events.unshift({ + event_id: getFakeEventId(), + type: 'org.matrix.archive.jump_to_previous_activity_summary', + room_id: this._room.id, + // Even though this isn't used for sort, just using the time where the event + // would logically be (at the start of the day) + origin_server_ts: events[0].origin_server_ts - 1, + content: { + canonicalAlias: this._room.canonicalAlias, + // The start of the range to use as a jumping off point to the previous activity + rangeStartTimestamp: events[0].origin_server_ts - 1, + // This is a bit cheating but I don't know how else to pass this kind of + // info to the Tile viewmodel + basePath: this._basePath, + }, + }); + } + + // Add a summary item to the bottom of the timeline that explains if we found events + // on the day requested. Also allow the user to jump to the next activity in the room. events.push({ event_id: getFakeEventId(), - type: 'org.matrix.archive.not_enough_events_from_day_summary', + type: 'org.matrix.archive.jump_to_next_activity_summary', room_id: this._room.id, // Even though this isn't used for sort, just using the time where the event // would logically be. diff --git a/shared/viewmodels/NotEnoughEventsFromDaySummaryTileViewModel.js b/shared/viewmodels/JumpToNextActivitySummaryTileViewModel.js similarity index 82% rename from shared/viewmodels/NotEnoughEventsFromDaySummaryTileViewModel.js rename to shared/viewmodels/JumpToNextActivitySummaryTileViewModel.js index 1dd49d9..3cf7f4e 100644 --- a/shared/viewmodels/NotEnoughEventsFromDaySummaryTileViewModel.js +++ b/shared/viewmodels/JumpToNextActivitySummaryTileViewModel.js @@ -5,7 +5,7 @@ const { SimpleTile } = require('hydrogen-view-sdk'); const MatrixPublicArchiveURLCreator = require('matrix-public-archive-shared/lib/url-creator'); const assert = require('../lib/assert'); -class NotEnoughEventsFromDaySummaryTileViewModel extends SimpleTile { +class JumpToNextActivitySummaryTileViewModel extends SimpleTile { constructor(entry, options) { super(entry, options); this._entry = entry; @@ -16,7 +16,7 @@ class NotEnoughEventsFromDaySummaryTileViewModel extends SimpleTile { } get shape() { - return 'org.matrix.archive.not_enough_events_from_day_summary:shape'; + return 'org.matrix.archive.jump_to_next_activity_summary:shape'; } get daySummaryKind() { @@ -38,9 +38,10 @@ class NotEnoughEventsFromDaySummaryTileViewModel extends SimpleTile { { ts: this.rangeEndTimestamp, dir: 'f', + scrollStartPosition: 'top', } ); } } -module.exports = NotEnoughEventsFromDaySummaryTileViewModel; +module.exports = JumpToNextActivitySummaryTileViewModel; diff --git a/shared/viewmodels/JumpToPreviousActivitySummaryTileViewModel.js b/shared/viewmodels/JumpToPreviousActivitySummaryTileViewModel.js new file mode 100644 index 0000000..acb2f2b --- /dev/null +++ b/shared/viewmodels/JumpToPreviousActivitySummaryTileViewModel.js @@ -0,0 +1,38 @@ +'use strict'; + +const { SimpleTile } = require('hydrogen-view-sdk'); + +const MatrixPublicArchiveURLCreator = require('matrix-public-archive-shared/lib/url-creator'); +const assert = require('../lib/assert'); + +class JumpToPreviousActivitySummaryTileViewModel extends SimpleTile { + constructor(entry, options) { + super(entry, options); + this._entry = entry; + + const basePath = this._entry?.content?.['basePath']; + assert(basePath); + this._matrixPublicArchiveURLCreator = new MatrixPublicArchiveURLCreator(basePath); + } + + get shape() { + return 'org.matrix.archive.jump_to_previous_activity_summary:shape'; + } + + // The start of the range to use as a jumping off point to the previous activity + get rangeStartTimestamp() { + return this._entry?.content?.['rangeStartTimestamp']; + } + + get jumpToPreviousActivityUrl() { + return this._matrixPublicArchiveURLCreator.archiveJumpUrlForRoom( + this._entry?.content?.['canonicalAlias'] || this._entry.roomId, + { + ts: this.rangeStartTimestamp, + dir: 'b', + } + ); + } +} + +module.exports = JumpToPreviousActivitySummaryTileViewModel; diff --git a/shared/views/ArchiveRoomView.js b/shared/views/ArchiveRoomView.js index 3a4c0cb..85c27ea 100644 --- a/shared/views/ArchiveRoomView.js +++ b/shared/views/ArchiveRoomView.js @@ -146,7 +146,12 @@ class ArchiveRoomView extends TemplateView { t.main({ className: 'ArchiveRoomView_mainArea' }, [ t.view(new RoomHeaderView(vm)), t.main({ className: 'ArchiveRoomView_mainBody' }, [ - t.view(new TimelineView(vm.timelineViewModel, customViewClassForTile)), + t.view( + new TimelineView(vm.timelineViewModel, { + viewClassForTile: customViewClassForTile, + stickToBottom: vm.scrollStartPosition === 'bottom', + }) + ), t.view(new DisabledComposerView(vm)), ]), ]), diff --git a/shared/views/NotEnoughEventsFromDaySummaryTileView.js b/shared/views/JumpToNextActivitySummaryTileView.js similarity index 78% rename from shared/views/NotEnoughEventsFromDaySummaryTileView.js rename to shared/views/JumpToNextActivitySummaryTileView.js index 69b0a4f..8517416 100644 --- a/shared/views/NotEnoughEventsFromDaySummaryTileView.js +++ b/shared/views/JumpToNextActivitySummaryTileView.js @@ -2,7 +2,7 @@ const { TemplateView } = require('hydrogen-view-sdk'); -class NotEnoughEventsFromDaySummaryTileView extends TemplateView { +class JumpToNextActivitySummaryTileView extends TemplateView { render(t, vm) { const kind = vm.daySummaryKind; let selectedDayString = 'the day you selected'; @@ -18,12 +18,12 @@ class NotEnoughEventsFromDaySummaryTileView extends TemplateView { } else if (kind === 'some-events-in-day') { daySummaryMessage = null; } else { - throw new Error(`Unknown kind=${kind} passed to NotEnoughEventsFromDaySummaryTileView`); + throw new Error(`Unknown kind=${kind} passed to JumpToNextActivitySummaryTileView`); } return t.div( { - className: 'NotEnoughEventsFromDaySummaryTileView', + className: 'JumpToNextActivitySummaryTileView', 'data-event-id': vm.eventId, }, [ @@ -32,7 +32,7 @@ class NotEnoughEventsFromDaySummaryTileView extends TemplateView { (t /*, vm*/) => t.p( { - className: 'NotEnoughEventsFromDaySummaryTileView_summaryMessage', + className: 'JumpToNextActivitySummaryTileView_summaryMessage', 'data-testid': `not-enough-events-summary-kind-${kind}`, }, daySummaryMessage @@ -40,14 +40,15 @@ class NotEnoughEventsFromDaySummaryTileView extends TemplateView { ), t.a( { - className: 'NotEnoughEventsFromDaySummaryTileView_nextActivityLink', + className: 'JumpToActivitySummaryTileView_activityLink', href: vm.jumpToNextActivityUrl, + 'data-testid': 'jump-to-next-activity-link', }, [ 'Jump to the next activity in the room', t.svg( { - className: 'NotEnoughEventsFromDaySummaryTileView_nextActivityIcon', + className: 'JumpToActivitySummaryTileView_activityIcon', xmlns: 'http://www.w3.org/2000/svg', width: '16', height: '16', @@ -68,4 +69,4 @@ class NotEnoughEventsFromDaySummaryTileView extends TemplateView { } } -module.exports = NotEnoughEventsFromDaySummaryTileView; +module.exports = JumpToNextActivitySummaryTileView; diff --git a/shared/views/JumpToPreviousActivitySummaryTileView.js b/shared/views/JumpToPreviousActivitySummaryTileView.js new file mode 100644 index 0000000..a86b795 --- /dev/null +++ b/shared/views/JumpToPreviousActivitySummaryTileView.js @@ -0,0 +1,44 @@ +'use strict'; + +const { TemplateView } = require('hydrogen-view-sdk'); + +class JumpToPreviousActivitySummaryTileView extends TemplateView { + render(t, vm) { + return t.div( + { + className: 'JumpToPreviousActivitySummaryTileView', + 'data-event-id': vm.eventId, + }, + [ + t.a( + { + className: 'JumpToActivitySummaryTileView_activityLink', + href: vm.jumpToPreviousActivityUrl, + 'data-testid': 'jump-to-previous-activity-link', + }, + [ + 'Jump to previous activity in the room', + t.svg( + { + className: 'JumpToActivitySummaryTileView_activityIcon', + xmlns: 'http://www.w3.org/2000/svg', + width: '16', + height: '16', + viewBox: '0 0 16 16', + fill: 'currentColor', + 'aria-hidden': 'true', + }, + [ + t.path({ + d: 'M0 4v8a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V4a2 2 0 0 0-2-2H2a2 2 0 0 0-2 2Zm4.271 1.055a.5.5 0 0 1 .52.038L8 7.386V5.5a.5.5 0 0 1 .79-.407l3.5 2.5a.5.5 0 0 1 0 .814l-3.5 2.5A.5.5 0 0 1 8 10.5V8.614l-3.21 2.293A.5.5 0 0 1 4 10.5v-5a.5.5 0 0 1 .271-.445Z', + }), + ] + ), + ] + ), + ] + ); + } +} + +module.exports = JumpToPreviousActivitySummaryTileView; diff --git a/test/e2e-tests.js b/test/e2e-tests.js index 3c0bd37..2b6c654 100644 --- a/test/e2e-tests.js +++ b/test/e2e-tests.js @@ -24,7 +24,8 @@ const { createMessagesInRoom, updateProfile, uploadContent, -} = require('./client-utils'); +} = require('./lib/client-utils'); +const TestError = require('./lib/test-error'); const testMatrixServerUrl1 = config.get('testMatrixServerUrl1'); const testMatrixServerUrl2 = config.get('testMatrixServerUrl2'); @@ -111,7 +112,8 @@ describe('matrix-public-archive', () => { // Use a fixed date at the start of the UTC day so that the tests are // consistent. Otherwise, the tests could fail when they start close to // midnight and it rolls over to the next day. - const archiveDate = new Date(Date.UTC(2022, 0, 3)); + // January 15th, 2022 + const archiveDate = new Date(Date.UTC(2022, 0, 15)); let archiveUrl; let numMessagesSent = 0; afterEach(() => { @@ -508,7 +510,7 @@ describe('matrix-public-archive', () => { const expectedEventIdsToBeDisplayed = [eventId]; // Visit the archive on the day ahead of where there are messages - const visitArchiveDate = new Date(Date.UTC(2022, 0, 5)); + const visitArchiveDate = new Date(Date.UTC(2022, 0, 20)); assert( visitArchiveDate > archiveDate, 'The date we visit the archive (`visitArchiveDate`) should be after where the messages were sent (`archiveDate`)' @@ -592,7 +594,6 @@ describe('matrix-public-archive', () => { const surroundEventIds = await createMessagesInRoom({ client, roomId: roomId, - // This is larger than the `archiveMessageLimit` we set numMessages: 2, prefix: 'events in room', timestamp: previousArchiveDate.getTime(), @@ -602,7 +603,6 @@ describe('matrix-public-archive', () => { const eventIdsOnDay = await createMessagesInRoom({ client, roomId: roomId, - // This is larger than the `archiveMessageLimit` we set numMessages: 2, prefix: 'events in room', timestamp: archiveDate.getTime(), @@ -624,6 +624,226 @@ describe('matrix-public-archive', () => { expectedEventIdsToBeDisplayed ); }); + + it('404 when trying to view a future day', async () => { + const client = await getTestClientForHs(testMatrixServerUrl1); + const roomId = await createTestRoom(client); + + try { + const TWO_DAY_MS = 2 * 24 * 60 * 60 * 1000; + await fetchEndpointAsText( + matrixPublicArchiveURLCreator.archiveUrlForDate( + roomId, + new Date(Date.now() + TWO_DAY_MS) + ) + ); + assert.fail( + new TestError( + `We expect the request to fail with a 404 since you can't view the future in the archive but it succeeded` + ) + ); + } catch (err) { + if (err instanceof TestError) { + throw err; + } + + assert.strictEqual( + err?.response?.status, + 404, + `Expected err.response.status=${err?.response?.status} to be 404 but error was: ${err.stack}` + ); + } + }); + + describe('Jump forwards and backwards', () => { + let client; + let roomId; + let previousDayToEventMap; + beforeEach(async () => { + // Set this low so we can easily create more than the limit + config.set('archiveMessageLimit', 3); + + client = await getTestClientForHs(testMatrixServerUrl1); + roomId = await createTestRoom(client); + + // Create enough surround messages on previous days that overflow the page limit + // but don't overflow the limit on a single day basis. + // + // We create 4 days of messages so we can see a seamless continuation from + // page1 to page2. The page limit is 3 but each page will show 4 messages + // because we fetch one extra to determine overflow. + // + // 1 <-- 2 <-- 3 <-- 4 <-- 5 <-- 6 <-- 7 <-- 8 + // [day 1] [day 2] [day 3] [day 4] + // [1st page ] [2nd page ] + previousDayToEventMap = new Map(); + for (let i = 1; i < 5; i++) { + // The date should be just past midnight so we don't run into inclusive + // bounds showing messages from more days than we expect in the tests. + const previousArchiveDate = new Date(Date.UTC(2022, 0, i, 1, 0, 0, 1)); + assert( + previousArchiveDate < archiveDate, + `The previousArchiveDate=${previousArchiveDate} should be before the archiveDate=${archiveDate}` + ); + const eventIds = await createMessagesInRoom({ + client, + roomId, + numMessages: 2, + prefix: `day ${i} - events in room`, + timestamp: previousArchiveDate.getTime(), + }); + previousDayToEventMap.set(previousArchiveDate, eventIds); + } + }); + + it('can jump forward to the next activity', async () => { + // `previousDayToEventMap` maps each day to the events in that day (2 events + // per day). The page limit is 3 but each page will show 4 messages because we + // fetch one extra to determine overflow. + // + // 1 <-- 2 <-- 3 <-- 4 <-- 5 <-- 6 <-- 7 <-- 8 + // [day 1] [day 2] [day 3] [day 4] + // [1st page ] [2nd page ] + const previousArchiveDates = Array.from(previousDayToEventMap.keys()); + assert.strictEqual( + previousArchiveDates.length, + 4, + `This test expects to work with 4 days of history, each with 2 messages and a page limit of 3 messages previousArchiveDates=${previousArchiveDates}` + ); + + // Fetch messages for the 1st page (day 2 backwards) + const firstPageArchiveUrl = matrixPublicArchiveURLCreator.archiveUrlForDate( + roomId, + previousArchiveDates[1] + ); + // Set this for debugging if the test fails here + archiveUrl = firstPageArchiveUrl; + const firstPageArchivePageHtml = await fetchEndpointAsText(firstPageArchiveUrl); + const firstPageDom = parseHTML(firstPageArchivePageHtml); + + const eventIdsOnFirstPage = [...firstPageDom.document.querySelectorAll(`[data-event-id]`)] + .map((eventEl) => { + return eventEl.getAttribute('data-event-id'); + }) + .filter((eventId) => { + // Only return valid events. Filter out our `fake-event-id-xxx--x` events + return eventId.startsWith('$'); + }); + + // Assert that the first page contains 4 events (day 2 and day 1) + assert.deepEqual(eventIdsOnFirstPage, [ + // All of day 1 + ...previousDayToEventMap.get(previousArchiveDates[0]), + // All of day 2 + ...previousDayToEventMap.get(previousArchiveDates[1]), + ]); + + // Follow the next activity link. Aka, fetch messages for the 2nd page (day 3 + // onwards, seamless continuation from the 1st page). + const nextActivityLinkEl = firstPageDom.document.querySelector( + '[data-testid="jump-to-next-activity-link"]' + ); + const nextActivityLink = nextActivityLinkEl.getAttribute('href'); + // Set this for debugging if the test fails here + archiveUrl = nextActivityLink; + const nextActivityArchivePageHtml = await fetchEndpointAsText(nextActivityLink); + const nextActivityDom = parseHTML(nextActivityArchivePageHtml); + + // Assert that it's a smooth continuation to more messages with no overlap + const eventIdsOnNextDay = [ + ...nextActivityDom.document.querySelectorAll(`[data-event-id]`), + ] + .map((eventEl) => { + return eventEl.getAttribute('data-event-id'); + }) + .filter((eventId) => { + // Only return valid events. Filter out our `fake-event-id-xxx--x` events + return eventId.startsWith('$'); + }); + + // Assert that the 2nd page contains 4 events (day 3 and day 4) + assert.deepEqual(eventIdsOnNextDay, [ + // All of day 3 + ...previousDayToEventMap.get(previousArchiveDates[2]), + // All of day 4 + ...previousDayToEventMap.get(previousArchiveDates[3]), + ]); + }); + + it('can jump backward to the previous activity', async () => { + // `previousDayToEventMap` maps each day to the events in that day (2 events + // per day). The page limit is 3 but each page will show 4 messages because we + // fetch one extra to determine overflow. + // + // 1 <-- 2 <-- 3 <-- 4 <-- 5 <-- 6 <-- 7 <-- 8 + // [day 1] [day 2] [day 3] [day 4] + // [2nd page ] [1st page ] + const previousArchiveDates = Array.from(previousDayToEventMap.keys()); + assert.strictEqual( + previousArchiveDates.length, + 4, + `This test expects to work with 4 days of history, each with 2 messages and a page limit of 3 messages previousArchiveDates=${previousArchiveDates}` + ); + + // Fetch messages for the 1st page (day 4 backwards) + const firstPageArchiveUrl = matrixPublicArchiveURLCreator.archiveUrlForDate( + roomId, + previousArchiveDates[3] + ); + // Set this for debugging if the test fails here + archiveUrl = firstPageArchiveUrl; + const firstPageArchivePageHtml = await fetchEndpointAsText(firstPageArchiveUrl); + const firstPageDom = parseHTML(firstPageArchivePageHtml); + + const eventIdsOnFirstPage = [...firstPageDom.document.querySelectorAll(`[data-event-id]`)] + .map((eventEl) => { + return eventEl.getAttribute('data-event-id'); + }) + .filter((eventId) => { + // Only return valid events. Filter out our `fake-event-id-xxx--x` events + return eventId.startsWith('$'); + }); + + // Assert that the first page contains 4 events (day 4 and day 3) + assert.deepEqual(eventIdsOnFirstPage, [ + // All of day 3 + ...previousDayToEventMap.get(previousArchiveDates[2]), + // All of day 4 + ...previousDayToEventMap.get(previousArchiveDates[3]), + ]); + + // Follow the previous activity link. Aka, fetch messages for the 2nd page (day 2 + // backwards, seamless continuation from the 1st page). + const previousActivityLinkEl = firstPageDom.document.querySelector( + '[data-testid="jump-to-previous-activity-link"]' + ); + const previousActivityLink = previousActivityLinkEl.getAttribute('href'); + // Set this for debugging if the test fails here + archiveUrl = previousActivityLink; + const previousActivityArchivePageHtml = await fetchEndpointAsText(previousActivityLink); + const previousActivityDom = parseHTML(previousActivityArchivePageHtml); + + // Assert that it's a smooth continuation to more messages with no overlap + const eventIdsOnPreviousDay = [ + ...previousActivityDom.document.querySelectorAll(`[data-event-id]`), + ] + .map((eventEl) => { + return eventEl.getAttribute('data-event-id'); + }) + .filter((eventId) => { + // Only return valid events. Filter out our `fake-event-id-xxx--x` events + return eventId.startsWith('$'); + }); + + // Assert that the 2nd page contains 4 events (day 2 and day 1) + assert.deepEqual(eventIdsOnPreviousDay, [ + // All of day 1 + ...previousDayToEventMap.get(previousArchiveDates[0]), + // All of day 2 + ...previousDayToEventMap.get(previousArchiveDates[1]), + ]); + }); + }); }); describe('Room directory', () => { @@ -734,10 +954,19 @@ describe('matrix-public-archive', () => { archiveUrl = matrixPublicArchiveURLCreator.archiveUrlForRoom(roomId); await fetchEndpointAsText(archiveUrl); assert.fail( - 'We expect the request to fail with a 403 since the archive should not be able to view a private room' + new TestError( + 'We expect the request to fail with a 403 since the archive should not be able to view a private room but it succeeded' + ) ); } catch (err) { - assert.strictEqual(err.response.status, 403); + if (err instanceof TestError) { + throw err; + } + assert.strictEqual( + err.response.status, + 403, + `Expected err.response.status=${err?.response?.status} to be 403 but error was: ${err.stack}` + ); } }); @@ -782,8 +1011,8 @@ describe('matrix-public-archive', () => { const controller = new AbortController(); const { signal } = controller; - // We have to use this over `fetch` because `fetch` does not allow us to manually - // follow redirects and get the resultant URL, see + // We have to use this sometimes over `fetch` because `fetch` does not allow us to + // manually follow redirects and get the resultant URL, see // https://github.com/whatwg/fetch/issues/763 function httpRequest(url) { return new Promise((resolve, reject) => { diff --git a/test/client-utils.js b/test/lib/client-utils.js similarity index 98% rename from test/client-utils.js rename to test/lib/client-utils.js index 3a024c4..45d2c26 100644 --- a/test/client-utils.js +++ b/test/lib/client-utils.js @@ -2,9 +2,9 @@ const assert = require('assert'); const urlJoin = require('url-join'); -const { fetchEndpointAsJson, fetchEndpoint } = require('../server/lib/fetch-endpoint'); +const { fetchEndpointAsJson, fetchEndpoint } = require('../../server/lib/fetch-endpoint'); -const config = require('../server/lib/config'); +const config = require('../../server/lib/config'); const matrixAccessToken = config.get('matrixAccessToken'); assert(matrixAccessToken); const testMatrixServerUrl1 = config.get('testMatrixServerUrl1'); diff --git a/test/lib/test-error.js b/test/lib/test-error.js new file mode 100644 index 0000000..8cfaf2e --- /dev/null +++ b/test/lib/test-error.js @@ -0,0 +1,13 @@ +'use strict'; + +// This is used to distinguish between an `AssertionError` within our app code from an +// `AssertionError` in the tests +class TestError extends Error { + constructor(message) { + super(message); + this.name = this.constructor.name; + Error.captureStackTrace(this, this.constructor); + } +} + +module.exports = TestError;