make promises map thread-safe

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

i also missed jon's feedback from earlier so i'll address those comments now

Collection should be threadsafe

yeah fair i should just split this out into two diffs

fix

address last piece of feedback

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

remove unnecessary include

address feedback

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!

rebase and land

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?

don't include avatar if null (this should also now be landable as is)

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

Let's avoid setting avatar unless it exists, and only set it for a given UserInfo when that user calls the upstream methods to actually set their avatar. (At which point UPDATE_USER updates should help us avoid state inconsistencies.)

There shouldn't need to be any native release associated with this landing then, correct?

rebase and land

rebase and land

Refactor code to make more readable check for ability to request notifications permission

More robust check as to whether issue notifications permission request.

1. Replace method overloading with different names.
2. Simplify check for Android OS version.
3. Add comment to explain hardcoded request code for notifications permission.

Add intent-filter for MainActivity to be started from SplashActivity in debug build

I don't have experience in writing tests in jest but looks reasonable

Makes sense! Thanks for clarifying and answering the questions!

Add a comment explaining why we don't check subthread_permissions here and also ORDER BY m.pin_time DESC to fetch pinned messages from newest to oldest

address review

Discussed the revision mainly offline with @ashoat, so I'll copy the changes we discussed from our chat here, as well as outline any additional changes.

Address review

In D6924#212627, @ashoat wrote:

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 agreed, I've made a task to track: https://linear.app/comm/issue/ENG-3397/add-pagination-to-fetching-message-pins-for-a-thread

Separately, I'm a bit confused about how the client will determine eg. how many messages are pinned for a given chat. Is this info going to be fetched on-demand when the user opens the chat? (In which case it would appear "dynamically" after being fetched.) Or are we looking to "cache" this information for each thread, so that we correctly display it when opened?

I think it would be good to avoid having to fetch anything "dynamically" when a chat is opened. Depending on what information we show by default (I don't recall the designs exactly but it might just be a number of pinned messages), we might want to consider modifying ThreadInfo to store this information.

That's a good point, it probably would be good to store the number in ThreadInfo. I've similarly made a follow-up task: https://linear.app/comm/issue/ENG-3396/store-the-number-of-pinned-messages-in-threadinfo