A little out my expertise for best practcies, but looks good

In D6924#212240, @tomek wrote:

In D6924#211911, @rohan wrote:

Ah I understand now, sorry. Yeah you're right, those are the three things we want to display client side.

1. Robotext: Will be handled by introducing a new message type and spec
2. Pin indicator: I think this is the one I'm a little unsure on since I've not begun working on the client-side yet so I wasn't sure on the best way to handle this. I'm not sure if updating RawMessageInfo to support something like RawPinnedMessageInfo and MessageInfo to support PinnedMessageInfo is the best way
3. A list of pinned messages: D7110 and D7112 should handle this, I can call the endpoint to retrieve all of the pinned messages for a given thread.

1. Makes sense!
2. We should spend some time thinking about it as it might affect the backend side. We have a lot of options, but most of them aren't good. Especially, we should consider how are we going to inform client A that a message was pinned on client B.
3. Here we also have some decisions to make:
   1. Do we plan to fetch pinned messages every time a user opens pinned messages view?
   2. Do we plan to fetch all the messages at once or we're going to fetch them while scrolling?
   3. Do we plan to have a couple of pinned messages on client and fetch more if necessary?
   4. Do we plan to update a view when a new message got pinned on other client?
   5. Do we plan to display pinned messages optimistically (so when a user pins a message and opens the view, should we display a message as pinned before server saves the pin)?

Answering these is crucial because it affects how the API looks like. We probably should prefer simplifying the solution but it still requires being sure what the requirements are.

Make response FetchPinnedMessagesResult mandatory

Planning changes to avoid putting it on reviewer's queue until the discussion in the comments is resolved

Return an emptyArray of pinnedMessages instead of null if there are none (feedback from D7112)

Do you think we can get rid of these alerts now? Having to click through the alerts is kind of getting annoying on iOS Simulator.

memoize MessageTooltipButtonAvatar

address comments

Addressed some of the comments

you almost certainly need to be looking at threadInfoFromRawThreadInfo.

Rebase on master

feedback

ran -> run

Important changes for media files permission and ability to launch MainActivity from SplachActivity

add import, improve query format

In D7118#212107, @ashoat wrote:

We should not be polluting the global namespace in D7101. You should not be able to use QueryExecResult without importing it unless it's in the Flow lib.

add import

The problem is that we would need to validate each query result manually, going through each column one-by-one and checking the result type. This isn’t something we do today for MySQL queries typically. If we feel strongly about doing input validation for SQL query results we could do with tcomb, but I’m not sure that the effort is worth it.

rebase

address code review

In D6924#211911, @rohan wrote:

In D6924#211756, @tomek wrote:

It seems like I wasn't clear enough in my comment, so let me explain it differently. Correct me in any place where I'm wrong. So on the client side we would want to display three things:

1. A robotext message in a thread telling that a message was pinned
2. A pin indicator next to a message
3. A list of pinned messages

As far as I understand, what you're describing handles only the 1st point. How do we plan to handle 2nd and 3rd?

Ah I understand now, sorry. Yeah you're right, those are the three things we want to display client side.

1. Robotext: Will be handled by introducing a new message type and spec
2. Pin indicator: I think this is the one I'm a little unsure on since I've not begun working on the client-side yet so I wasn't sure on the best way to handle this. I'm not sure if updating RawMessageInfo to support something like RawPinnedMessageInfo and MessageInfo to support PinnedMessageInfo is the best way
3. A list of pinned messages: D7110 and D7112 should handle this, I can call the endpoint to retrieve all of the pinned messages for a given thread.

Actually remove the remaining "only for flow" lines, not sure why this didn't commit before...

I don't think doing input validation for SQL results is necessary – the any is fine here. It would probably have been more clear how / why you're using any if these utility functions were introduced in the same diff where they are used.

It seems to work with all data URIs too!

[will add screenshot shortly]

In D7151#212209, @ashoat wrote:

Just tested and it looks like this solves rendering SVG data URIs for us!! I haven't tested other types, but PNG, JPEG, and SVG are all listed as supported in the docs. Thanks @bartek for the tip! This is way better than what I had planned to do on Android.

rebase before landing

Just tested and it looks like this solves rendering SVG data URIs for us!! I haven't tested other types, but PNG, JPEG, and SVG are all listed as supported in the docs. Thanks @bartek for the tip! This is way better than what I had planned to do on Android.

Done testing!! I think this is safe to land. I haven't tested blurhash or SVG / data URI support yet, but those are things that react-native-fast-image can't handle today anyways

That general approach looks great!! Great work figuring this out.

My last proposed solution to fix the chat bubble poking out of the message box is to make the message box a flex container and align the inner text messages (either flex-end or flex-start) depending on if the message came from the viewer or not. This solution is very similar to how we position the InlineEngagement component, which inspired me to implement the solution this way.

Create compound index, set my DB version back to 20 and re-run the migration to confirm no errors are present, and use EXPLAIN on the query in D7110 to confirm that the query uses the newly created thread_pinned index

add feature flags and fix poking chat bubble