Remove unknown MXIDs from invite suggestions

[weeman1337]

See element-hq/element-web#25446

closes element-hq/element-web#25446

PSF-2027

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)

element-web notes: The invite dialogue no longer suggests unknown Matrix-Ids

---

Here's what your changelog entry will look like:

🐛 Bug Fixes

• Remove unknown MXIDs from invite suggestions (#11055). Fixes Invite by mxId - Don't add unrecognised mxId in the ‘suggestions’ section element-hq/element-web#25446.

[richvdh]

Tell me about the investigation you've done into why this code was there in the first place and why we think that logic no longer applies?

[weeman1337]

Tell me about the investigation you've done into why this code was there in the first place and why we think that logic no longer applies?

Removing the suggestions was a product decision.

It might be a bit hidden behind „See https://github.com/vector-im/element-web/issues/25446“ in the description:

• the entire discussion
• comment, comment

[richvdh]

(I followed up in a DM)

[weeman1337]

Let's continue here. This is the issue solved by adding the MXID to the suggestions: element-hq/element-web#12419

You can still invite any unknown MXID by pressing „go“ or enter. Unknown MXIDs are only removed from the suggestion list. @turt2live as the author of the issue mentioned above: Do you see a problem here?

[turt2live]

We found that the comfort of seeing the exact match in the dialog before was important, so made sure to include it.

I'm not personally convinced we should spend time undoing that effort.

[richvdh]

I'm not personally convinced we should spend time undoing that effort.

Nor am I, though if people feel strongly otherwise, I won't object. It's just not clear to me that anyone has even thought about it.

@weeman1337: I think it's an important part of our job to think about what we're being asked to do, rather than just churn out code (or delete it...) like some sort of automaton.

[richvdh]

It looks to me like the code in question was added in 00cc253, which targets element-hq/element-web#12440. Unfortunately there's not much detail on that issue about the usecase in mind at that point.

@turt2live could you shed some more light on the cases when we expect /profile requests to fail?

[giomfo]

I'm not personally convinced we should spend time undoing that effort.

Nor am I, though if people feel strongly otherwise, I won't object. It's just not clear to me that anyone has even thought about it.

@weeman1337: I think it's an important part of our job to think about what we're being asked to do, rather than just churn out code (or delete it...) like some sort of automaton.

@richvdh : Hi, I totally disagree with this comment from you here. This requested change has been already discussed with Product and Design team here. I don't understand why you argue so much against it. Like @weeman1337 already said : after the suggested change, you can still invite any unknown/unrecognised MXID by pressing „go“ or enter.

[richvdh]

@weeman1337: I think it's an important part of our job to think about what we're being asked to do, rather than just churn out code (or delete it...) like some sort of automaton.

Rereading this again now, I think my comment comes across as harsher than intended, so let me publicly apologise to @weeman1337 and make plain that he is far from a mindless automaton and that he does a lot of very valuable work. We have discussed this PR privately and I hope Michael understands my point now.

Nevertheless I remain unconvinced that anybody has done much investigation into the reasons we made the active decision to include these MXIDs in the first place. Until we understand that, we cannot be sure we aren't introducing regressions. It may be that the reasons are lost in the mists of time - in which case fair enough, let's land the change and see what breaks - but I think we should at least make a reasonable attempt.

[turt2live]

@turt2live could you shed some more light on the cases when we expect /profile requests to fail?

There's the obvious cases of network trouble, but critically a homeserver admin can just disable access to user profiles without anyone knowing or being able to detect the difference. Using profiles is very much a hack and is only ~60% reliable at determining if a user exists, roughly speaking.

We don't currently have a way to detect if a user exists or not, and from a spec perspective this is largely considered a feature. Being able to invite non-existent users supports use cases like disaster recovery, where the user being invited doesn't have an account provisioned for them until they absolutely need to use it.

I'd encourage us to reconsider what we think of this feature, as "just press the Go button" is not discoverable and there's real-world reasons to show that you've entered a vaguely correct user ID. If anything, it feels like the lack of profile on the suggestion would give an indication that you may have typo'd the user ID, but otherwise have the right idea for what to put into the box.

Overall, I can't say my opinions are strong ones, but I do think that we need to spend a bit more time thinking through this problem space.

[giomfo]

@daniellekirkwood wdyt about the last comment from Travis here? Should we cancel/suspend the change suggested here?

[richvdh]

Let's take this discussion back to the issue where the change was agreed (element-hq/element-web#25446).

[richvdh]

Product team confirm that they have thought about this, and consider the "Invite" button discoverable enough, so let's ship it.