Accepting to unblock but the pattern of copy-paste is concerning

Please make sure @inka creates the task I requested before you land this

Looks like this is just a move diff so blindly accepting. Not familiar with that error myself, wondering what "initialization" means in this context (is it a circular dependency issue?)

Also throwing in @kuba, since he has been looking at chat-selectors.js recently too

Please apply the requested change before landing!

Looks great, just minor nits! Great work on the synchronization, really for the distraction looking at threadsafe collections

Thanks @bartek, your explanation makes sense... I agree now that doing something on the Java side doesn't make any sense. I think we can move the discussion to D7120 for the JS side and safely land this diff as-is

After gathering the discussion above my statement on this is that:

1. I would follow @bartek approach since as he stated it is a common practice he encountered in open source projects.
2. If there is no acceptance to this approach I will make this method resolve not to two but to three values: false, true, null. The promise resolved to null will be interpreted in JS code that the request is already in progress and another promise will shortly resolve to some meaningful value we can react to (by querying for device token or setting it to null). It will probably be an early return in JS. Alternatively to resolving to ?boolean we can resolve to string constants such as GRANTED, DENIED and REQUEST_ALREADY_IN_PROGRESS.

Talked to @varun offline, plan is to handle thread safety in a later diff

Collection should be threadsafe

Is this still meant to be a dummy / example diff that doesn't get landed?

Please make the requested changes before landing

No, I don't think so

I'm starting to wonder if it'd be better to revert my suggestion just to avoid all that fuss.

Awesome!! Thanks for taking the time to revise this a bunch, I know it was more complicated than initially expected

Thanks for explaining!

Really close!

Back to you

Ah but actually some of my other feedback hasn't been addressed yet

In retrospect, D7077 is the right place for this discussion – @inka, would you mind responding to my questions in this diff there?

In D7065#212101, @ashoat wrote:

Will this affect the height of the item? Have you made the necessary changes to the height determination code (textMessageItemHeight)?

I think we should include localMediaSelection, but not sure if a future diff adds that

Passing back to you with question about multiple files being uploaded to the same endpoint at once. I don't think we actually ever do this, so I'm wondering if we can simply throw an exception if we get multiple files in the same upload

That makes sense RE older clients. Thinking about it more, I'm not actually sure why we ever thought it made sense to "hide" the avatar field from older clients... they're going to eventually need the field, and if we hide it from them initially then they will need to rely on the state check mechanism to fix things after they upgrade codeVersions.

The current fetchPinnedMessageInfos query doesn't have the ability to paginate, but it can always be added in later. We could initially fetch all of the pinned messages at once from the server, but have a task to paginate the fetching

Yeah we can probably get rid of them, feel free to put up a diff

One last thing that would be great if you could test: what happens if you upload a single very "thin" photo? I want to make sure it doesn't expand to take the whole width and then becomes super tall. (Or at least, that there's no regression in this.)

Back to your queue for nits and fetchDerivedMessages

Adding @kuba here – he is looking at fetchDerivedMessages right now in D7141

Can you please amend the Test Plan to include:

Did not review this closely

I'm sorry, I completely missed that it was an old migration. My apologies for making assumptions and leaving an unnecessarily aggressive comment

On context on why this diff has been up for 2 weeks and hasn't been reviewed?

Note that I didn't bother adding a line here because @commapp/opaque-ke-wasm isn't actually consumed directly via Yarn workspaces, so we don't need its package.json in order to run yarn cleaninstall (apparently anyways, given that CI passes). Instead, we'll consume published versions of @commapp/opaque-ke-wasm as an NPM package.