Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed: dev: '0:undefined:' displayed in reaction emoji tooltip #23371

Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function ReportActionItemEmojiReactions(props) {
};

const onReactionListOpen = (event) => {
reactionListRef.current.showReactionList(event, popoverReactionListAnchor.current, reaction.emoji, props.reportActionID);
reactionListRef.current.showReactionList(event, popoverReactionListAnchor.current, reactionEmojiName, props.reportActionID);
};

return (
Expand Down
111 changes: 102 additions & 9 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -1563,16 +1563,18 @@ function hasAccountIDEmojiReacted(accountID, users, skinTone) {
* Adds a reaction to the report action.
* Uses the NEW FORMAT for "emojiReactions"
* @param {String} reportID
* @param {String} reportActionID
* @param {Object} reportAction
* @param {Object} emoji
* @param {String} emoji.name
* @param {String} emoji.code
* @param {String[]} [emoji.types]
* @param {Number} [skinTone]
*/
function addEmojiReaction(reportID, reportActionID, emoji, skinTone = preferredSkinTone) {
function addEmojiReaction(reportID, reportAction, emoji, skinTone = preferredSkinTone) {
const reportActionID = reportAction.reportActionID;
const createdAt = moment().utc().format(CONST.DATE.SQL_DATE_TIME);
const optimisticData = [

const reportActionsReactionsData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${reportActionID}`,
Expand All @@ -1591,6 +1593,50 @@ function addEmojiReaction(reportID, reportActionID, emoji, skinTone = preferredS
},
];

// We need to update the REPORT_ACTIONS on success
const originalReportID = ReportUtils.getOriginalReportID(reportID, reportAction);
const message = reportAction.message[0];

let reactionObject = message.reactions && _.find(message.reactions, (reaction) => reaction.emoji === emoji.name);
const needToInsertReactionObject = !reactionObject;
if (needToInsertReactionObject) {
reactionObject = {
emoji: emoji.name,
users: [],
};
} else {
// Make a copy of the reaction object so that we can modify it without mutating the original
reactionObject = {...reactionObject};
}

reactionObject.users = [...reactionObject.users, {accountID: currentUserAccountID, skinTone}];
let updatedReactions = [...(message.reactions || [])];
if (needToInsertReactionObject) {
updatedReactions = [...updatedReactions, reactionObject];
} else {
updatedReactions = _.map(updatedReactions, (reaction) => (reaction.emoji === emoji.name ? reactionObject : reaction));
}

const updatedMessage = {
...message,
reactions: updatedReactions,
};
const reportActionsData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${originalReportID}`,
value: {
[reportActionID]: {
message: [updatedMessage],
},
},
},
];
const optimisticData = [
...reportActionsReactionsData,
...reportActionsData,
];

const parameters = {
reportID,
skinTone,
Expand All @@ -1607,14 +1653,15 @@ function addEmojiReaction(reportID, reportActionID, emoji, skinTone = preferredS
* Removes a reaction to the report action.
* Uses the NEW FORMAT for "emojiReactions"
* @param {String} reportID
* @param {String} reportActionID
* @param {Object} reportAction
* @param {Object} emoji
* @param {String} emoji.name
* @param {String} emoji.code
* @param {String[]} [emoji.types]
*/
function removeEmojiReaction(reportID, reportActionID, emoji) {
const optimisticData = [
function removeEmojiReaction(reportID, reportAction, emoji) {
const reportActionID = reportAction.reportActionID;
const reportActionsReactionsData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${reportActionID}`,
Expand All @@ -1628,13 +1675,60 @@ function removeEmojiReaction(reportID, reportActionID, emoji) {
},
];

// We need to update the REPORT_ACTIONS on success
const message = reportAction.message[0];
const reactionObject = message.reactions && _.find(message.reactions, (reaction) => reaction.emoji === emoji.name);
if (!reactionObject) {
return;
}

const updatedReactionObject = {
...reactionObject,
};
updatedReactionObject.users = _.filter(reactionObject.users, (sender) => sender.accountID !== currentUserAccountID);
const updatedReactions = _.filter(
// Replace the reaction object either with the updated one or null if there are no users
_.map(message.reactions, (reaction) => {
if (reaction.emoji === emoji.name) {
if (updatedReactionObject.users.length === 0) {
return null;
}
return updatedReactionObject;
}
return reaction;
}),

// Remove any null reactions
(reportObject) => reportObject !== null,
);
const updatedMessage = {
...message,
reactions: updatedReactions,
};
const reportActionsData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`,
value: {
[reportActionID]: {
message: [updatedMessage],
},
},
},
];
const optimisticData = [
...reportActionsReactionsData,
...reportActionsData,
];

const parameters = {
reportID,
reportActionID,
emojiCode: emoji.name,
// This will be removed as part of https://github.com/Expensify/App/issues/19535
useEmojiReactions: true,
};

jfquevedol2198 marked this conversation as resolved.
Show resolved Hide resolved
API.write('RemoveEmojiReaction', parameters, {optimisticData});
}

Expand Down Expand Up @@ -1664,11 +1758,10 @@ function toggleEmojiReaction(reportID, reportAction, reactionObject, existingRea
const skinTone = emoji.types === undefined ? -1 : paramSkinTone;

if (existingReactionObject && hasAccountIDEmojiReacted(currentUserAccountID, existingReactionObject.users, skinTone)) {
removeEmojiReaction(reportID, reportAction.reportActionID, emoji);
removeEmojiReaction(reportID, reportAction, emoji);
return;
}

addEmojiReaction(reportID, reportAction.reportActionID, emoji, skinTone);
addEmojiReaction(reportID, reportAction, emoji, skinTone);
}

/**
Expand Down
11 changes: 9 additions & 2 deletions src/pages/home/report/ReactionList/PopoverReactionList.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,14 @@ class PopoverReactionList extends React.Component {
getSelectedReactionFromAction(reportAction, emojiName) {
const reactions = lodashGet(reportAction, ['message', 0, 'reactions'], []);
const reactionsWithCount = _.filter(reactions, (reaction) => reaction.users.length > 0);
return _.find(reactionsWithCount, (reaction) => reaction.emoji === emojiName);
const reaction = _.find(reactionsWithCount, (item) => item.emoji === emojiName);
if (reaction) {
return {
...reaction,
users: _.map(reaction.users, ({accountID, skinTone}) => ({accountID, skinTones: {skinTone}})),
};
}
return undefined;
}

/**
Expand Down Expand Up @@ -186,7 +193,7 @@ class PopoverReactionList extends React.Component {
const reactionUsers = _.map(selectedReaction.users, (sender) => sender.accountID);
const emoji = EmojiUtils.findEmojiByName(selectedReaction.emoji);
const emojiCodes = EmojiUtils.getUniqueEmojiCodes(emoji, selectedReaction.users);
const hasUserReacted = Report.hasAccountIDReacted(this.props.currentUserPersonalDetails.accountID, reactionUsers);
const hasUserReacted = Report.hasAccountIDEmojiReacted(this.props.currentUserPersonalDetails.accountID, reactionUsers);
const users = PersonalDetailsUtils.getPersonalDetailsByIDs(reactionUsers, this.props.currentUserPersonalDetails.accountID, true);
return {
emojiCount,
Expand Down
10 changes: 10 additions & 0 deletions tests/actions/ReportTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,8 @@ describe('actions/Report', () => {
return waitForPromisesToResolve();
})
.then(() => {
reportAction = _.first(_.values(reportActions));

// Expect the reaction to exist in the reportActionsReactions collection
expect(reportActionsReactions).toHaveProperty(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${reportActionID}`);

Expand All @@ -583,15 +585,21 @@ describe('actions/Report', () => {
expect(reportActionReaction[EMOJI.name].users[TEST_USER_ACCOUNT_ID]).toBeNull();
})
.then(() => {
reportAction = _.first(_.values(reportActions));

// Add the same reaction to the same report action with a different skintone
Report.toggleEmojiReaction(REPORT_ID, reportAction, EMOJI);
return waitForPromisesToResolve()
.then(() => {
reportAction = _.first(_.values(reportActions));

const reportActionReaction = reportActionsReactions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${reportActionID}`];
Report.toggleEmojiReaction(REPORT_ID, reportAction, EMOJI, reportActionReaction, EMOJI_SKIN_TONE);
return waitForPromisesToResolve();
})
.then(() => {
reportAction = _.first(_.values(reportActions));

// Expect the reaction to exist in the reportActionsReactions collection
expect(reportActionsReactions).toHaveProperty(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${reportActionID}`);

Expand Down Expand Up @@ -670,6 +678,8 @@ describe('actions/Report', () => {
return waitForPromisesToResolve();
})
.then(() => {
resultAction = _.first(_.values(reportActions));

// Now we toggle the reaction while the skin tone has changed.
// As the emoji doesn't support skin tones, the emoji
// should get removed instead of added again.
Expand Down