From 2ac706a9203eca0f7b1e131f59e8d7cae87385ff Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 17 May 2022 09:02:56 -0400 Subject: [PATCH 1/3] Clarify the current_user_participated flag from MSC3440. --- proposals/3440-threading-via-relations.md | 6 +- .../3816-clarify-thread-participation.md | 70 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 proposals/3816-clarify-thread-participation.md diff --git a/proposals/3440-threading-via-relations.md b/proposals/3440-threading-via-relations.md index 7692ee1dc56..af750ddf935 100644 --- a/proposals/3440-threading-via-relations.md +++ b/proposals/3440-threading-via-relations.md @@ -57,8 +57,10 @@ would include additional information in the `unsigned` field: The latest event should be serialised in the same form as the event itself; this includes adding any bundled aggregations for the event (and applying edits).[^1] * `count`: An integer counting the number of `m.thread` events -* `current_user_participated`: A flag set to `true` if the current logged in user - has participated in the thread +* `current_user_participated`: A boolean flag, which is set to `true` if the + current logged in user has participated in the thread. The user has participated if: + * They created the current event. + * They created an event with a `m.thread` relation targeting the current event. #### Rich replies in a thread diff --git a/proposals/3816-clarify-thread-participation.md b/proposals/3816-clarify-thread-participation.md new file mode 100644 index 00000000000..f20726a4dfe --- /dev/null +++ b/proposals/3816-clarify-thread-participation.md @@ -0,0 +1,70 @@ +# MSC3816: Clarify Thread Participation + +[MSC3440](/~https://github.com/matrix-org/matrix-doc/pull/3440) defines relations +for threads and includes a `current_user_participated` flag, which is not fully +defined: + +> A flag set to `true` if the current logged in user has participated in the thread + +Known implementations implement this as whether the currently logged in user has +sent an event with an `m.thread` relation targeting the event, but this does not +make sense if the requesting user was the original event in the thread. + +Consider `A` as the root event in a thread from `@alice:example.com`, and `B` +as a threaded reply from `@bob:example.com`. The bundled aggregations for `A` +would include: + +| Requester | `current_user_participated` | +|----------------------|-----------------------------| +| `@alice:example.com` | `false` | +| `@bob:example.com` | `true` | + +If `@alice:example.com` sends reply `C`, this would change: + +| Requester | `current_user_participated` | +|----------------------|-----------------------------| +| `@alice:example.com` | `true` | +| `@bob:example.com` | `true` | + +The proposed clarification is that `@alice:example.com` should have always have +participated in the thread (e.g. both tables would be `true` in the example above). + +## Proposal + +The definition of the `current_user_participated` flag from +[MSC3440](/~https://github.com/matrix-org/matrix-doc/pull/3440) is updated to be: + +> A boolean flag, which is set to `true` if the current logged in user has +> participated in the thread. The user has participated if: +> +> * They created the current event. +> * They created an event with a `m.thread` relation targeting the current event. + +This better matches the intention of this flag, which is that a client is able to +visually separate threads which might be of interest. + +## Potential issues + +The current implementations will need to be updated to take into account the +sender of the current event when generating bundled aggregations. This should be +trivial since all of the needed information is directly available. + +It is suggested to [the new `filter` parameters from MSC3440](/~https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3440-threading-via-relations.md#fetch-all-threads-in-a-room) +in order to list threads in a room that a user has participated in. There would +now be an inconsistency that threads where the current user sent the root event +but has not replied to the thread could not easily be fetched. A future MSC may +solve this problem. + +## Alternatives + +Do not clarify [MSC3440](/~https://github.com/matrix-org/matrix-spec-proposals/pull/3440) +and leave it up to implementations to define the behavior of the +`current_user_participated` flag. + +## Security considerations + +None + +## Unstable prefix + +None, the changes above shouldn't dramatically change behavior for clients. From f30c5997ca0010f6650adf744a0d0f9d37a224d3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 17 May 2022 09:09:59 -0400 Subject: [PATCH 2/3] Add a better link to the definition. --- proposals/3816-clarify-thread-participation.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/3816-clarify-thread-participation.md b/proposals/3816-clarify-thread-participation.md index f20726a4dfe..4a31fc38cd3 100644 --- a/proposals/3816-clarify-thread-participation.md +++ b/proposals/3816-clarify-thread-participation.md @@ -31,8 +31,8 @@ participated in the thread (e.g. both tables would be `true` in the example abov ## Proposal -The definition of the `current_user_participated` flag from -[MSC3440](/~https://github.com/matrix-org/matrix-doc/pull/3440) is updated to be: +The [definition of the `current_user_participated` flag](/~https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3440-threading-via-relations.md#event-format) +from the bundled aggregations for `m.thread` relations is updated: > A boolean flag, which is set to `true` if the current logged in user has > participated in the thread. The user has participated if: From feb15804777a29546d56441636a915a0391dabae Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 24 May 2022 15:37:19 -0400 Subject: [PATCH 3/3] Clarifications from review. Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- .../3816-clarify-thread-participation.md | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/proposals/3816-clarify-thread-participation.md b/proposals/3816-clarify-thread-participation.md index 4a31fc38cd3..32de2565ea7 100644 --- a/proposals/3816-clarify-thread-participation.md +++ b/proposals/3816-clarify-thread-participation.md @@ -1,16 +1,19 @@ # MSC3816: Clarify Thread Participation -[MSC3440](/~https://github.com/matrix-org/matrix-doc/pull/3440) defines relations -for threads and includes a `current_user_participated` flag, which is not fully -defined: +[MSC3440](/~https://github.com/matrix-org/matrix-doc/pull/3440) defines the `m.thread` relation +type, and the format of the serverside aggregation for them. The definition of the aggregation includes a +`current_user_participated` flag, which is not fully defined: > A flag set to `true` if the current logged in user has participated in the thread -Known implementations implement this as whether the currently logged in user has -sent an event with an `m.thread` relation targeting the event, but this does not -make sense if the requesting user was the original event in the thread. +In particular, it is unclear whether sending the initial event (i.e., the event which is the +target of the `m.thread` relation) counts as participating in the thread. -Consider `A` as the root event in a thread from `@alice:example.com`, and `B` +Known implementations do *not* count the initial event in this way, and instead +implement this as: "has the current user sent an event with an `m.thread` relation +targeting the event", but this has found to give poor user experience in practice. + +For example, consider `A` as the root event in a thread from `@alice:example.com`, and `B` as a threaded reply from `@bob:example.com`. The bundled aggregations for `A` would include: @@ -27,7 +30,7 @@ If `@alice:example.com` sends reply `C`, this would change: | `@bob:example.com` | `true` | The proposed clarification is that `@alice:example.com` should have always have -participated in the thread (e.g. both tables would be `true` in the example above). +participated in the thread (i.e. both tables would be `true` in the example above). ## Proposal @@ -49,7 +52,7 @@ The current implementations will need to be updated to take into account the sender of the current event when generating bundled aggregations. This should be trivial since all of the needed information is directly available. -It is suggested to [the new `filter` parameters from MSC3440](/~https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3440-threading-via-relations.md#fetch-all-threads-in-a-room) +MSC3440 proposes using [new `filter` parameters](/~https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3440-threading-via-relations.md#fetch-all-threads-in-a-room) in order to list threads in a room that a user has participated in. There would now be an inconsistency that threads where the current user sent the root event but has not replied to the thread could not easily be fetched. A future MSC may