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

Kick was shown as a leave #7853

Closed
Half-Shot opened this issue Dec 13, 2018 · 12 comments · Fixed by matrix-org/matrix-react-sdk#3198
Closed

Kick was shown as a leave #7853

Half-Shot opened this issue Dec 13, 2018 · 12 comments · Fixed by matrix-org/matrix-react-sdk#3198
Assignees
Labels
A-Timeline P2 S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect X-Needs-Info This issue is blocked awaiting information from the reporter Z-Community-PR Issue is solved by a community member's PR

Comments

@Half-Shot
Copy link
Member

Half-Shot commented Dec 13, 2018

We kicked someone who was having trouble joining from their HS (looked like a split). However, the kick shows up as "X left the room" despite the event being submitted by another user:

{
  "type": "m.room.member",
  "content": {
    "membership": "leave",
    "reason": "Apparently you aren't even joined"
  },
  "sender": "@Half-Shot:half-shot.uk",
  "state_key": "@benpa:bpulse.org",
  "membership": "leave",
  "event_id": "$1544700825245czgIl:half-shot.uk",
  "origin_server_ts": 1544700825258,
  "unsigned": {
    "replaces_state": "$154099363819RsEzk:bpulse.org",
    "prev_content": {
      "membership": "leave"
    },
    "prev_sender": "@benpa:bpulse.org",
    "age": 552
  },
  "room_id": "!ruaviCwHdJSWfKcBam:half-shot.uk"
}
@lampholder
Copy link
Member

If you /kick a mxid that's neither a room member nor an invited room member, Riot renders this as that mxid's having left (I am certain I filed a bug for this in the past).

Is that what happened here?

@lampholder lampholder added the X-Needs-Info This issue is blocked awaiting information from the reporter label Dec 20, 2018
@Half-Shot
Copy link
Member Author

Oh right, that's probably it. The user was broken so I can believe the membership was also broken.

@t3chguy
Copy link
Member

t3chguy commented Feb 24, 2019

So I guess we need to decide how we'd like to show a NOTHING -> Leave (/kick of a user who wasn't even in the room)

@Half-Shot
Copy link
Member Author

Half-Shot commented Feb 24, 2019

Feeling is that if the user has no previous state then it shouldn't be shown (as it isn't really a state change).

However, it's a bit more worrying when the client is missing previous state. I guess we could query for state about the member in the absence of state, and show a kick/leave message if appropriate.

I don't think calls to /state/m.room.member/@foobar:matrix.org is expensive?

Doesn't sync actually contain a previous_content for state changes, which means we should always know if there was previous state and as such do the above reliably?

@turt2live
Copy link
Member

tbh I'm not sure why we even need to consider the prev content for a kick: a kick by definition is making a user leave who is not yourself.

@lampholder
Copy link
Member

Isn't @Half-Shot saying we need to consider the prev content to know whether it's a kick (causing somebody who is not you to leave) or a noop?

@lampholder lampholder added T-Defect P2 S-Minor Impairs non-critical functionality or suitable workarounds exist A-Timeline labels Mar 12, 2019
@turt2live
Copy link
Member

You don't though, because state_key !== sender ? kick : leave

@Half-Shot
Copy link
Member Author

tbh I'm not sure why we even need to consider the prev content for a kick: a kick by definition is making a user leave who is not yourself.

I think this is acceptable.

@t3chguy
Copy link
Member

t3chguy commented Mar 15, 2019

Because if they weren't really in the room then you aren't making them leave

@turt2live
Copy link
Member

That sounds like a spec problem, not a UI/UX problem. Not to mention the case of someone going out of their way to kick someone that wasn't in the room is fairly rare.

@t3chguy
Copy link
Member

t3chguy commented Mar 15, 2019

Not to mention the case of someone going out of their way to kick someone that wasn't in the room is fairly rare.

Yes but by mistake, such as a typo, I can imagine it happening

@turt2live
Copy link
Member

I'm still convinced that we should fix that particular case in the spec, not in the UI.

https://github.com/matrix-org/matrix-doc/issues/1928

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Timeline P2 S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect X-Needs-Info This issue is blocked awaiting information from the reporter Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants