-
Notifications
You must be signed in to change notification settings - Fork 11
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.
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
Follow tombstone and predecessor history #167
Follow tombstone and predecessor history #167
Conversation
…essor-history Conflicts: server/lib/matrix-utils/get-server-name-from-matrix-room-id-or-alias.js server/lib/parse-via-servers-from-user-input.js
And will implement the continuation timestamp stuff as part of `/jump` instead.
We shouldn't see the same messages after paginating
…tampsStartFromSameUtcX` back to `areTimestampsFromSameUtcX`
server/routes/room-routes.js
Outdated
// We choose `currentRangeStartTs` instead of `ts` (the jump point) because TODO: why? | ||
// and we don't choose `currentRangeEndTs` because TODO: why? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like I can't justify this 🤔 but it does seem correct from tested behavior.
I think my reasoning for using currentRangeEndTs
before was that we could keep paginating with day precision if we were showing more than a day for each page. But then this falls apart when you have a bunch of messages from the same hour that fill up multiple pages. As a note with this change from currentRangeEndTs
-> currentRangeStartTs
, we did have to update the can jump backward to the previous activity
test to use hour precision now.
Using ts
or currentRangeStartTs
makes sense for the reference point of being the jump point as in "The closest event is from the same hour we tried to jump from."
Using ts
makes various tests fail with various precision differences. Probably because of the - 1
/+ 1
stuff that we want to remove in the future anyway when MSC3999 lands.
But those don't seem like explanations for why (the truth kernel)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a link to this discussion thread so people can get some context. But I think I will opt to merging this PR without figuring this out. Will just have to contemplate this more and fill it out when an epiphany happens.
Follow tombstone and predecessor history
Fix #59
Other updates:
/roomid/room1/date/2022/01/03
format instead of trying to retrofit the weird alias stuff on there. Which also makes the fancy to actual URL utilities much more simple.archiveMessageLimit
in the test case because pages have different number of events depending on if we are against a boundary, hidden events, etc.Dev notes
/jump
Edge case: Someone can create a circular reference which will cause us to keep redirecting the user around in a loop. Do we care?ERR_TOO_MANY_REDIRECTS
in the browser will handle it/timestamp_to_event
? Like/timestamp_to_event?dir=f&ts=123&event_id=$123
meaning find the closest event fromts
looking forward afterevent_id
(the causality part, "A after B")m.room.create
popping up in/timestamp_to_event?dir=f
when we/jump
. If we seem.room.create
, then assume we've reached the end of the room and move to the successor/timestamp_to_event
/timestamp_to_event?event_id=$abc