Don't re-fetch thread root if we already have it #4088

dbkr · 2024-02-29T15:36:43Z

The root event of a thread used to arrive with the pagination request, but this was unspecced and so got changed to simply fetch the root event. In many (almost all) cases this shouldn't be necessary because the thread should already have its root event: re-use it if it's already there. This is only in pagination, so there's no reason to believe that the root event would have changed and needs to be re-fetched.

This removes a number of duplicate calls to the /event/ endpoint from the tests.

Checklist

Tests written for new code (and old code if feasible)
Linter and other CI checks pass
Sign-off given on the changes (see CONTRIBUTING.md)

The root event of a thread used to arrive with the pagination request, but this was unspecced and so got changed to simply fetch the root event. In many (almost all) cases this shouldn't be necessary because the thread should already have its root event: re-use it if it's already there. This is only in pagination, so there's no reason to believe that the root event would have changed and needs to be re-fetched. This removes a number of duplicate calls to the /event/ endpoint from the tests.

dbkr · 2024-02-29T15:39:47Z

spec/integ/matrix-client-event-timeline.spec.ts

@@ -1643,7 +1635,7 @@ describe("MatrixClient event timelines", function () {
                            ...THREAD_ROOT.unsigned!["m.relations"],
                            "io.element.thread": {
                                ...THREAD_ROOT.unsigned!["m.relations"]!["io.element.thread"],
-                                count: 2,
+                                count: 1,


This is a weird one. The event that's being added to the thread here is a reaction and so should not increment this number since it's not a thread-type relation. The test later asserts that the thread reply count is still 1: essentially that this value was ignored. As far as I can see, it was only ignored (and therefore passing) by some race condition in the test. Because there's one less async call, the new value now propagates through. As far as I can see this is just the wrong number here and should be 1.

Another question is whether it's still useful for the test to assert the reply count later. I've left it in for now.

t3chguy

Looks sane

richvdh · 2024-03-06T13:00:56Z

This removes a number of duplicate calls to the /event/ endpoint from the tests.

And from the actual application, right? RIGHT?

dbkr · 2024-03-06T14:10:29Z

This removes a number of duplicate calls to the /event/ endpoint from the tests.

And from the actual application, right? RIGHT?

Indeed, yes, should be slightly fewer requests.

* v31.5.0-rc.0 * Don't re-fetch thread root if we already have it (matrix-org#4088) The root event of a thread used to arrive with the pagination request, but this was unspecced and so got changed to simply fetch the root event. In many (almost all) cases this shouldn't be necessary because the thread should already have its root event: re-use it if it's already there. This is only in pagination, so there's no reason to believe that the root event would have changed and needs to be re-fetched. This removes a number of duplicate calls to the /event/ endpoint from the tests. * Fix race condition with sliding sync extensions (matrix-org#4089) * Fix race condition with sliding sync extensions * Fix types on sliding-sync spec test * Prettier fixes * Add `.m.rule.is_room_mention` push rule to DEFAULT_OVERRIDE_RULES (matrix-org#4100) * Add intentional mentions push rules to DEFAULT_OVERRIDE_RULES Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Export types describing all specced media event formats (matrix-org#4092) * Export types describing all specced media event formats Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate PR Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Move types to a dedicated export Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add readme entry Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Update dependency typedoc to v0.25.11 (matrix-org#4102) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * v31.5.0 * Resetting package fields for development * Temporarily disable broken step in the release process Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * fix automatic DM avatar with functional members (matrix-org#4017) * fix automatic DM avatar with functional members * update comments * lint * add tests for functional members * keep functional members out of the public API - remove public API for functional members, reverting most of 0ce2d82, f9b41f6, e65fb24 - remove tests for functional members public API c114bf5 - add shared functional members getter for both room name and avatar fallback generation * filter functional members from more candidates - remove from hero(es) - remove from previous members * add tests for fallback avatars with functional members * Add docstring for getFunctionalMembers Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * inline getInvitedAndJoinedFunctionalMemberCount * update comments for getAvatarFallbackMember * use correct list of heroes in getAvatarFallbackMember * remove redundant type annotation * optimize performance of invitedAndJoinedFunctionalMemberCount * calculate nonFunctionalMemberCount in one step instead of iterating redundantly * clean up functional member tests with review feedback * lint * Update src/models/room.ts Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * apply feedback about comments * non-functional per review, lint --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: RiotRobot <releases@riot.im> Co-authored-by: David Baker <dbkr@users.noreply.github.com> Co-authored-by: Daniel Salinas <zzorba@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Kim Brose <2803622+HarHarLinks@users.noreply.github.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

dbkr added the T-Task Tasks for the team like planning label Feb 29, 2024

github-actions bot deployed to PR Documentation Preview February 29, 2024 15:38 View deployment

dbkr commented Feb 29, 2024

View reviewed changes

dbkr marked this pull request as ready for review February 29, 2024 15:42

dbkr requested a review from a team as a code owner February 29, 2024 15:42

dbkr requested review from richvdh and t3chguy February 29, 2024 15:42

t3chguy approved these changes Mar 5, 2024

View reviewed changes

dbkr added this pull request to the merge queue Mar 6, 2024

Merged via the queue into develop with commit 27dd856 Mar 6, 2024
28 checks passed

dbkr deleted the dbkr/use_thread_root_if_present_2 branch March 6, 2024 14:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't re-fetch thread root if we already have it #4088

Don't re-fetch thread root if we already have it #4088

dbkr commented Feb 29, 2024

dbkr Feb 29, 2024 •

edited

Loading

t3chguy left a comment

richvdh commented Mar 6, 2024

dbkr commented Mar 6, 2024

Don't re-fetch thread root if we already have it #4088

Don't re-fetch thread root if we already have it #4088

Conversation

dbkr commented Feb 29, 2024

Checklist

dbkr Feb 29, 2024 • edited Loading

Choose a reason for hiding this comment

t3chguy left a comment

Choose a reason for hiding this comment

richvdh commented Mar 6, 2024

dbkr commented Mar 6, 2024

dbkr Feb 29, 2024 •

edited

Loading