feat(crypto): Support verification violation composer banner

[BillCarsonFr]

Draft: depends on matrix-org/matrix-js-sdk#4646

Following an initial early review, I have refactored the component to now use a viewModel 8325c0c
The update model has also been made a lot more simple, but also less optimized; as an example a UserTrustStatusChanged event will trigger a new computation of all the violations in the room. If we see performances problem this could be improoved

Following initial review,

Checklist

• Tests written for new code (and old code if feasible).
• New or updated public/exported symbols have accurate TSDoc documentation.
• Linter and other CI checks pass.
• I have licensed the changes to Element by completing the Contributor License Agreement (CLA)

[dbkr]

Could all of these exported things have tsdoc please?

[BillCarsonFr]

done 6b76fbc

[dbkr]

This could also do with some doc as it's not super obvious what it does.

[BillCarsonFr]

Done 6b76fbc

[dbkr]

For functions that are defined and then just used once on the next line, probably may as well just use an anonymous one inline.

[BillCarsonFr]

Done in 6b76fbc

[dbkr]

It's a bit weird to assign a different callback to the button depending on the state. Generally you would just have one callback which behaves accordingly. Also, you probably want to make it a useCallback and ideally be a bit more specific than onButtonClick (even though in this case there's only one button).

[BillCarsonFr]

Thx, I tried that d1509b9

[dbkr]

Thanks, this looks fine to me

[uhoreg]

Just a few nits, but looks reasonable.

[richvdh]

@BillCarsonFr as author and @uhoreg and @dbkr as reviewers: can any of you tell me what's going on here? On the first call to getUserVerificationStatus it blocks for 40ms, but on subsequent calls, it returns immediately: why does it sleep in the first place?

This test seems to be racy (#29266); I think we need something more resilient than sleeps for tens of ms.

[richvdh]

Looking further at what's happening in this PR, it looks like this test is no longer relevant. It's testing what happens when the UserTrustStatusChanged event says something different to getUserVerificationStatus, but the code no longer relies on the payload of UserTrustStatusChanged, so it feels like the test is redundant.

[richvdh]

(conclusion from discussion with @BillCarsonFr is that this test is indeed redundant)

[BillCarsonFr]

It was testing a race that was possible in the component before the refactoring to view model.

The old component was saying:

   // For each user, we assign a sequence number to each verification status
    // that we get, or fetch.
    //
    // Since fetching a verification status is asynchronous, we could get an
    // update in the middle of fetching the verification status, which could
    // mean that the status that we fetched is out of date.  So if the current
    // sequence number is not the same as the sequence number when we started
    // the fetch, then we drop our fetched result, under the assumption that the
    // update that we received is the most up-to-date version.  If it is in fact
    // not the most up-to-date version, then we should be receiving a new update
    // soon with the newer value, so it will fix itself in the end.

So this test was trying to trigger this flow, where the initial status fetch is resolved late thus returning an outdated value, this to ensure that the component detects it and get the up to date value

It is indeed redondant with the new model (was usefull to test no regression with the old component)