This differential adds user to thick sidebar when another user mentions them in a message
Details
- Create thick sidebar
- Send message mentioning a user
- Ensure that they are added to the sidebar
Diff Detail
- Repository
- rCOMM Comm
- Branch
- marcin/eng-9100
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Setting @kamil to blocking on this as he recently refactored this logic – I want to make sure the changes still make sense to him. Sorry for any annoyance!
Before I start reviewing - can you rebase to the newest master?
You using code that was removed in D13329 so I guess this will change significantly.
native/input/input-state-container.react.js | ||
---|---|---|
576 | this is very ugly but recent changes on master (extracting sending message to hook) and the fact that we have this issue: https://linear.app/comm/issue/ENG-9103/update-processandsenddmoperation-to-support-multiple-dm-ops-at-once forces us to this change |
It appears that the only thing this diff is doing is having textMessageCreationSideEffectsFunc called after the text message is created for thick threads, but before the text message is created for thin threads.
Two questions:
- Why do the side effects need to happen after the text message is created for thick threads?
- Should we just have the side effects run after the text message for thin threads as well?
This is explained in detail here
- Should we just have the side effects run after the text message for thin threads as well?
I haven't investigated this direction since current approach has been around for some time so I concluded that it is better not to change it we don't have to. However I think that we should keep the current approach for thin threads. If sending message succeeds but adding to thread fails the user receives a message form thread they don't belong to which is strange UX. We should update approach for thick threads as soon as it is possible ( after this is done).