Chat - On changing skin tone 6th time, emoji reduced to one

IuliiaHerets commented 3 weeks ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 9.0.31-14 Reproducible in staging?: Y Reproducible in production?: Y Issue reported by: Applause Internal Team

Action Performed:

Go to https://staging.new.expensify.com/home
Open a chat
Tap on emoticon plus reaction button( send a message & long press and react for the message so that you can see this button in conversation)
Now select thumbs up emoji
Change the skin tone
Now select thumbs up emoji in new skin tone
Repeat this and select thumbs up emoji in all 6 skin tone
Note in conversation we can see all 5 skin tone thumbs up emoji but on adding 6th skin tone it reduced to one emoji

Expected Result:

On changing skin tone 6th time, emoji must not be reduced to one.

Actual Result:

On changing skin tone 6th time, emoji reduced to one. 6th skin tone also unused one. It is not used 2nd time but reduced to one emoji.

Workaround:

Unknown

Platforms:

[x] Android: Native
[x] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/e8f14691-8bea-4c38-a44d-b553c6a94bc6

View all open jobs on GitHub

melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

IuliiaHerets commented 3 weeks ago

@garrettmknight FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

daledah commented 3 weeks ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

On changing skin tone 6th time, emoji reduced to one. 6th skin tone also unused one. It is not used 2nd time but reduced to one emoji.

What is the root cause of that problem?

The emoji reduced to one because we choose the emoji with skinTone = 0

And we have some issues with Onyx.merge method with array here https://github.com/Expensify/App/blob/92171d88b709ecd2d987fab85a0b824fa1bb0cd5/src/libs/actions/Report.ts#L2513-L2523

With skinTones = {1: '2024-09-10 22:34:22', -1: '2024-09-10 22:34:30'} and Onyx merged new skinTone = {0: '2024-09-10 22:34:39.470'} to this existed skinTones after that skinTones will be {0: '2024-09-10 22:34:39.470'}

What changes do you think we should make in order to solve the problem?

Change method merge here to set and we can calculate skinTones through existingReactions https://github.com/Expensify/App/blob/92171d88b709ecd2d987fab85a0b824fa1bb0cd5/src/libs/actions/Report.ts#L2513

 onyxMethod: Onyx.METHOD.SET,

Add new param existingReactions to function addEmojiReaction here and pass it from toggleEmojiReaction

https://github.com/Expensify/App/blob/92171d88b709ecd2d987fab85a0b824fa1bb0cd5/src/libs/actions/Report.ts#L2509

function addEmojiReaction(reportID: string, reportActionID: string, emoji: Emoji, skinTone: string | number = preferredSkinTone, existingReactions?: OnyxEntry<ReportActionReactions>) {

Calculate skinTones through existingReactions and use it in optimisticData

   const skinTones =
        existingReactions && existingReactions?.[emoji.name]?.users?.[currentUserAccountID]?.skinTones
            ? {...existingReactions?.[emoji.name]?.users?.[currentUserAccountID]?.skinTones, [skinTone ?? CONST.EMOJI_DEFAULT_SKIN_TONE]: createdAt}
            : {
                  [skinTone ?? CONST.EMOJI_DEFAULT_SKIN_TONE]: createdAt,
              };
    const optimisticData: OnyxUpdate[] = [
        {
            onyxMethod: Onyx.METHOD.SET,
            key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${reportActionID}`,
            value: {
                [emoji.name]: {
                    createdAt,
                    pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
                    users: {
                        [currentUserAccountID]: {
                            skinTones: skinTones,
                        },
                    },
                },
            },
        },
    ];

What alternative solutions did you explore? (Optional)

garrettmknight commented 3 weeks ago

I don't think this is worth fixing.

Expensify / App