Fixes https://linear.app/comm/issue/ENG-10075/adding-several-friends-at-once-is-broken-on-native
setOptions() wasn't called when onPressAdd changed. And onPressAdd changes when a new tag (user) is added. So the callback in
navigation button wasn't always updated when new tag was added thus resulting in the bug.
Details
Details
Go to Profile -> Friend list
Select 2 or more users and add them as friends
Click Save
Verify all users have friend requests sent
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Could you explain a bit more why this solves the issue? I have a couple of questions:
- setOptions() wasn't called when onPressAdd changed - The effect depends on onPressAdd and calls setOptions. Are you sure that setOptions isn't called when a new user is added?
- onPressAdd doesn't seem to depend on the selected users and it isn't obvious why it should always updated when new tag was added - why it has to be updated?
- Why saving onPressAdd in a ref solves the issue?
It looks to me like this solution is a workaround. I guess the bug is caused by an invalid condition setSaveButtonDisabled === undefined that is used to early return. We should probably have a different condition instead of introducing a ref.
Comment Actions
I think the meaning of the code before the changes was like this:
if there were tags before and now there aren’t then we need to disable the button
if there weren’t tags before and now there are tags then we need to enable the button
in other cases there is an early return and we don't call setOptions, because we want to avoid unecessary calls
Now what happens when user adds a second tag?
The effect runs, but there were tags before (1 tag) and now there are still tags (2 tags), so setSaveButtonDisabled is undefined (it was enabled and still needs to be enabled), so setOptions is not called (this answers your question 1), so in the header we still have an old onPressAdd with one tag instead of two
As for question 2, setOptions needs to be called when onPressAdd is updated, because onPressAdd callback has dependency on updateRelationshipsOnServer, and this function actually adds the users based on added tags and has a dependency on currentTags. If onPressAdd is not updated, then the old onPressAdd is called, which calls old updateRelationshipsOnServer with old tags.
As for question 3 I’m saving it in a ref just to compare the previous onPressAdd callback with the current onPressAdd callback to avoid calling setOptions unnecessarily. I added an additional condition to setSaveButtonDisabled === undefined: prevOnPressAdd.current === onPressAdd. So setOptions is called only when we need to disable/enable the button or onPressAdd callback changed.