Skip to content
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

Invites are collapsed incorrectly #4102

Closed
ara4n opened this issue Mar 4, 2021 · 2 comments · Fixed by #6300
Closed

Invites are collapsed incorrectly #4102

ara4n opened this issue Mar 4, 2021 · 2 comments · Fixed by #6300
Assignees
Labels
A-Invitations A-Invite A-Timeline Z-WTF WTF issues: High impact, Low Effort

Comments

@ara4n
Copy link
Member

ara4n commented Mar 4, 2021

I created a room and invited two people to it. The timeline says:

5D8D64FA-6C10-4851-8D03-16630093445C

which is wrong: there are 2 memberships, and the avatars should show the invited user not the invitee. “2 users invited” would be correct.

Johennes added a commit to Johennes/matrix-ios-kit that referenced this issue Mar 23, 2021
As reported in element-hq/element-ios#4102, membership changes are sometimes
wrongly collapsed. This happens when events of the same collapsible series
are processed in different batches. For forward directed events, only the
start of a collapsible series is tracked in `collapsingCellDataSeriess`. As
a result, when the event that corresponds to the beginning of the series
was processed in a preceding event batch, the series will be missing in
`collapsingCellDataSeriess` and its summary string won't get updated.

This commit fixes the situation by adding the start of the series to
`collapsingCellDataSeriess` when it is missing and the series is appended
to.

Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
@Johennes
Copy link
Contributor

I have a partial fix that addresses the wrong number in matrix-org/matrix-ios-kit#789. The issue only occurs when the collapsed events are processed in different batches (which, for instance, happens when you invite multiple people one by one).

I've only very briefly looked into the avatar issue. The app is currently using the sender's avatar. For invites, it looks like the only place in the event where we could get the invitee's username from is state_key.

"{
  "sender": "@alice:matrix.org",
  "content": {
    "displayname": "bob",
    "membership":"invite"
  },
  "origin_server_ts": 1616488993287,
  "state_key": "@bob:matrix.org", <=====
  "room_id": "...",
  "event_id": "...",
  "type": "m.room.member",
  "unsigned": {
    "age": 1204610,
    "prev_sender": "@alice:matrix.org",
    "prev_content": ...,
    "replaces_state": "..."
  }
}

Johennes added a commit to Johennes/element-ios that referenced this issue Mar 25, 2021
This commit switches to displaying the target user's avatar for collapsed membership
changes which addresses the avatar issue reported in element-hq#4102.

Depends on: matrix-org/matrix-ios-kit#793

Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
@Johennes
Copy link
Contributor

I have another incremental fix that addresses the wrong avatars in matrix-org/matrix-ios-kit#793 and #4148.

The logic around combining membership changes and building the summary string is still not as advanced as on Element web but bring that on par is an endeavor of its own.

Johennes added a commit to Johennes/element-ios that referenced this issue Mar 26, 2021
This commit switches to displaying the target user's avatar for collapsed membership
changes which addresses the avatar issue reported in element-hq#4102.

Depends on: matrix-org/matrix-ios-kit#793

Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
Johennes added a commit to Johennes/element-ios that referenced this issue Apr 14, 2021
This replaces the generic "x membership changes" messages with detailed
summaries analogous to element-web. The implementation uses the same code
used on element-web, executed in a `JSContext`.

Closes: element-hq#4102
Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
Johennes added a commit to Johennes/element-ios that referenced this issue Apr 14, 2021
This replaces the generic "x membership changes" messages with detailed
summaries analogous to element-web. The implementation uses the same code
used on element-web, executed in a `JSContext`.

Closes: element-hq#4102
Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
Johennes added a commit to Johennes/element-ios that referenced this issue Apr 16, 2021
This replaces the generic "x membership changes" messages with detailed
summaries analogous to element-web. The implementation uses the same code
used on element-web, executed in a `JSContext`.

Closes: element-hq#4102
Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
@daniellekirkwood daniellekirkwood added the Z-WTF WTF issues: High impact, Low Effort label Feb 7, 2022
stefanceriu pushed a commit that referenced this issue Feb 7, 2022
This commit switches to displaying the target user's avatar for collapsed membership
changes which addresses the avatar issue reported in #4102.

Depends on: matrix-org/matrix-ios-kit#793

Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
stefanceriu pushed a commit that referenced this issue Feb 7, 2022
This commit switches to displaying the target user's avatar for collapsed membership
changes which addresses the avatar issue reported in #4102.

Depends on: matrix-org/matrix-ios-kit#793

Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
stefanceriu pushed a commit that referenced this issue Feb 7, 2022
This commit switches to displaying the target user's avatar for collapsed membership
changes which addresses the avatar issue reported in #4102.

Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
stefanceriu pushed a commit that referenced this issue Feb 7, 2022
This commit switches to displaying the target user's avatar for collapsed membership
changes which addresses the avatar issue reported in #4102.

Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
@gileluard gileluard self-assigned this Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Invitations A-Invite A-Timeline Z-WTF WTF issues: High impact, Low Effort
Projects
None yet
4 participants