Show knock rooms in the list

[maheichyk]

We @nordeck are currently implementing the knock rooms behind the feature flag feature_ask_to_join (introduced in #11182).

This PR allows to see knocked rooms in the rooms list. Knock rooms have incon/notification showing the knock request status.

Epic: vector-im/element-web/issues/18655

Videos:

knock_approve.mp4

knock_cancel.mp4

knock_forget.mp4

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)

Type: enhancement

---

Here's what your changelog entry will look like:

✨ Features

• Show knock rooms in the list (#11573). Contributed by @maheichyk.

[daniellekirkwood]

the videos are helpful, thank you. Generally, this looks fine to me although I don't expect the user to have to "accept" a join invite once their knock has been granted.

In the first video the room is in the room list with the hand icon which is great but once 'accepted' the room jumps to the invite list and when clicking on it their asked to join the room?

[maheichyk]

@daniellekirkwood Yes, it happens because in order to add the user into the room we need to invite him first, although the user has already knocked. Synapse by default doesn't auto-accept the invitation and in this use case it would be really nice. Currently this auto-accept behavior should be possible via custom synapse module or widget. I think it is a very good point.

[maheichyk]

@daniellekirkwood another option could be to auto-accept these invites from Element client, but this should be done as a separate PR.

[maheichyk]

@t3chguy, @richvdh could you please review?

[daniellekirkwood]

@daniellekirkwood another option could be to auto-accept these invites from Element client, but this should be done as a separate PR.

that would be great, thank you

[weeman1337]

Thank you for your contribution @maheichyk .

Unfortunately it seems like we get „stuck rooms“ with this PR:

All of those rooms with the hand are either rejected knocks or rooms that I've forgotten.

You can reproduce it by

• User A: Knock at a room
• User B: Reject
• User A: Sees the red badge
• User A: Reload (probably several times)
• User A has now a stuck room

Also rooms that I've discarded from the archive tend to reappear. Just reload the app several times.

[maheichyk]

@weeman1337 thanks for the issue. Could you please clarify the expected behavior here: are rejected rooms supposed to disappear completely on this step or are they expected to be there but with reject icon (until user explicitly forgets the room)?

For the case above I would expect the room to keep the notification until I dismiss it on my side. Currently, after reload, it looks like a knocked-at-room at again.

The other case I was able to identify is after you dismissed a room where you have knocked at, it will go to archive. If you forget the room there and reload the app these rooms reappear as knocked-at-rooms in the room list. Here I would expect to never see the room again.

@weeman1337 matrix-js-sdk was changed to delete knocked rooms when knock membership changes, so if knock request is approved, cancelled or rejected, then the room is deleted from knocked rooms stored in the DB.

This change fixes the issue that knocks were "stuck". At the moment denied knocks will disappear after some time if Element is reloaded several times, but this is probably okay for initial implementation (@daniellekirkwood please correct me if I am wrong here), because there is some uncertainty how to implement this correctly and would be nice to improve it in separate PR.

It would be nice to move this PR forward.

[weeman1337]

Thank you for working further on this @maheichyk Unfortunately the issue seems still to exist:

I can see two errors in console. Probably this helps:

M_FORBIDDEN: MatrixError: [403] User @michael_test123:matrix.org not allowed to view events in room !fcxEbolLGVYuvOYLFz:matrix.org at token StreamToken(room_key=RoomStreamToken(topological=None, stream=4319719337, instance_map=immutabledict({})), presence_key=757284974, typing_key=17851800, receipt_key=2360235631, account_data_key=2467556192, push_rules_key=4593358, to_device_key=1046202977, device_list_key=8426059873, groups_key=0, un_partial_stated_rooms_key=172948) (https://matrix-client.matrix.org/_matrix/client/v3/rooms/!fcxEbolLGVYuvOYLFz%3Amatrix.org/members?not_membership=leave&at=s4319719337_757284974_17851800_2360235631_2467556192_4593358_1046202977_8426059873_0_172948)
    HTTPError errors.ts:34
    MatrixError errors.ts:63
    parseErrorResponse utils.ts:83
    requestOtherUrl fetch.ts:294
    request fetch.ts:204
    authedRequest fetch.ts:155
    members client.ts:8152
    loadMembersFromServer room.ts:961
    loadMembers room.ts:976
    loadMembersIfNeeded room.ts:1015
    getEncryptionTargetMembers room.ts:1714
    shieldStatusForRoom ShieldUtils.ts:35
    updateE2EStatus RoomView.tsx:1518
    RoomView_RoomView RoomView.tsx:1393
    RoomView_RoomView RoomView.tsx:770
    componentDidMount RoomView.tsx:962
    React 2
    unstable_runWithPriority scheduler.production.min.js:18
    React 4
    unstable_runWithPriority scheduler.production.min.js:18
    React 6
    setState MatrixChat.tsx:475
    viewRoom MatrixChat.tsx:1053
    MatrixChat MatrixChat.tsx:758
    invokeCallback dispatcher.ts:120
    MatrixDispatcher dispatcher.ts:97
    setTimeout handler*dispatch dispatcher.ts:170
    RoomTile RoomTile.tsx:241
    React 11
    unstable_runWithPriority scheduler.production.min.js:18
    React 3

Tried to remove unknown room from im.vector.fake.recent: !fcxEbolLGVYuvOYLFz:matrix.org

[weeman1337]

because there is some uncertainty how to implement this correctly and would be nice to improve it in separate PR.

What about removing the room from the client if it is discarded from the ui?

If this goes to a separate issue can you please create one and link it to the epic?

[weeman1337]

Another issue is that the room does sometimes not appear in the list:

There are some log items:

!fcxEbolLGVYuvOYLFz:matrix.org is current in RVS but missing from client - clearing sticky room [rageshake.ts:74:27](webpack:///src/rageshake/rageshake.ts)
Got room state event for unknown room !fcxEbolLGVYuvOYLFz:matrix.org! 5 [rageshake.ts:74:27](webpack:///src/rageshake/rageshake.ts)
    fnName rageshake.ts:74
    instrumentConsole instrument.js:124
    methodFactory logger.ts:48
    MatrixRTCSessionManager_MatrixRTCSessionManager MatrixRTCSessionManager.ts:105
    emit events.js:158
    emit typed-event-emitter.ts:89
    forSource ReEmitter.ts:55
    emit events.js:153
    emit typed-event-emitter.ts:89
    forSource ReEmitter.ts:55
    emit events.js:153
    emit typed-event-emitter.ts:89
    setStateEvents room-state.ts:415
    setStateEvents room-state.ts:402
    initialiseState event-timeline.ts:156
    injectRoomEvents sync.ts:1788
    processSyncResponse sync.ts:1524
    promiseMapSeries utils.ts:425
!fcxEbolLGVYuvOYLFz:matrix.org is reportedly new but is already known - assuming TagChange instead [rageshake.ts:74:27](webpack:///src/rageshake/rageshake.ts)
!fcxEbolLGVYuvOYLFz:matrix.org might be a reference change - attempting to update reference [rageshake.ts:74:27](webpack:///src/rageshake/rageshake.ts)
M_FORBIDDEN: MatrixError: [403] User @michael_test123:matrix.org not allowed to view events in room !fcxEbolLGVYuvOYLFz:matrix.org at token StreamToken(room_key=RoomStreamToken(topological=None, stream=4319768730, instance_map=immutabledict({})), presence_key=757284974, typing_key=17935268, receipt_key=2360272602, account_data_key=2467608598, push_rules_key=4593420, to_device_key=1046233990, device_list_key=8426187070, groups_key=0, un_partial_stated_rooms_key=172965) (https://matrix-client.matrix.org/_matrix/client/v3/rooms/!fcxEbolLGVYuvOYLFz%3Amatrix.org/members?not_membership=leave&at=s4319768730_757284974_17935268_2360272602_2467608598_4593420_1046233990_8426187070_0_172965)
    HTTPError errors.ts:34
    MatrixError errors.ts:63
    parseErrorResponse utils.ts:83
    requestOtherUrl fetch.ts:294

[weeman1337]

Looks good code-wise. Unfortunately, the feature seems to have its flaws. @daniellekirkwood how do you think we should proceed here? Should we create issues for the things I found in the comments before and merge it as it is?

[weeman1337]

The comment below can be removed now 🥳

[maheichyk]

thanks, removed

[weeman1337]

There is still a TODO comment here. Hard-coding any matrix ID should be okay here.

[maheichyk]

thanks for the point, deleted

[maheichyk]

Tried to remove unknown room from im.vector.fake.recent: !fcxEbolLGVYuvOYLFz:matrix.org

This happens in Element for invite -> reject -> forget flow when room is sticky: user rejects the room he is invited into, then selects (clicks) on this room in archive (so room is sticky) and clicks on forget in the room options menu, the same errors. In the case of knock the room is sticky and the same problem just appears when we click on forget. Would be good to resolve this as separate issue.

[weeman1337]

Tried to remove unknown room from im.vector.fake.recent: !fcxEbolLGVYuvOYLFz:matrix.org

This happens in Element for invite -> reject -> forget flow when room is sticky: user rejects the room he is invited into, then selects (clicks) on this room in archive (so room is sticky) and clicks on forget in the room options menu, the same errors. In the case of knock the room is sticky and the same problem just appears when we click on forget. Would be good to resolve this as separate issue.

👍 Okay. Can you please create one and add it to the epic?

[maheichyk]

because there is some uncertainty how to implement this correctly and would be nice to improve it in separate PR.

What about removing the room from the client if it is discarded from the ui?

If this goes to a separate issue can you please create one and link it to the epic?

When room is forgotten (including denied knock) it should be deleted from the client already (this is how it already done for normal room forget) but it doesn't work for denied because of the issue before.

[maheichyk]

👍 Okay. Can you please create one and add it to the epic?

I have created an issue: element-hq/element-web#26195

[maheichyk]

Another issue is that the room does sometimes not appear in the list:

I tried on PR deployment and cannot reproduce it unfortunately, would be nice to have an issue with steps to reproduce, but the root cause could be not directly related to the changes in this PR.

[daniellekirkwood]

Thank you for creating a new issue to track the bugs found here!

At the moment denied knocks will disappear after some time if Element is reloaded several times, but this is probably okay for initial implementation (@daniellekirkwood please correct me if I am wrong here), because there is some uncertainty how to implement this correctly and would be nice to improve it in separate PR.

I think this is acceptable for now. If the user chooses to do nothing when they have been rejected 🤷 that feels like an enhancement we can handle better later.

Though, if the user chooses to "forget" the room it should be moved to their "historical" list. If this is not happening today, I would consider that a bug.

I'm not sure our process on PRs that have bugs, @weeman1337 ? I think that new issue created handles the bug cases introduced here, are we ok to accept this PR with the understanding that @maheichyk will work on those bugs, or do we need a PR for the bugs before accepting this one?

[weeman1337]

I'm not sure our process on PRs that have bugs, @weeman1337 ? I think that new issue created handles the bug cases introduced here, are we ok to accept this PR with the understanding that @maheichyk will work on those bugs, or do we need a PR for the bugs before accepting this one?

Agree. @maheichyk already created issues.