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

Remove all uses of actorEmail as part of personalDetails migration #21874

Merged
merged 15 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/libs/E2E/apiMocks/openReport.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ export default () => ({
text: '123 Ios',
},
],
actorEmail: 'fake3@gmail.com',
actorAccountID: 10773236,
message: [
{
Expand Down
2 changes: 1 addition & 1 deletion src/libs/ReportActionsUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ function isConsecutiveActionMadeByPreviousActor(reportActions, actionIndex) {
return false;
}

return currentAction.actorEmail === previousAction.actorEmail;
return currentAction.actorAccountID === previousAction.actorAccountID;
}

/**
Expand Down
19 changes: 6 additions & 13 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ function sortReportsByLastRead(reports) {
*/
function canEditReportAction(reportAction) {
return (
reportAction.actorEmail === currentUserEmail &&
reportAction.actorAccountID === currentUserAccountID &&
reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT &&
!isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})) &&
!ReportActionsUtils.isDeletedAction(reportAction) &&
Expand Down Expand Up @@ -238,7 +238,7 @@ function canDeleteReportAction(reportAction, reportID) {
) {
return false;
}
if (reportAction.actorEmail === currentUserEmail) {
if (reportAction.actorAccountID === currentUserAccountID) {
return true;
}
const report = lodashGet(allReports, `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {});
Expand Down Expand Up @@ -735,12 +735,12 @@ function getIcons(report, personalDetails, defaultIcon = null, isPayer = false,
if (isChatThread(report)) {
const parentReportAction = ReportActionsUtils.getParentReportAction(report);

const actorEmail = lodashGet(parentReportAction, 'actorEmail', '');
const actorAccountID = lodashGet(parentReportAction, 'actorAccountID', '');
const actorAccountID = lodashGet(parentReportAction, 'actorAccountID', 0);
const actorDisplayName = lodashGet(allPersonalDetails, [actorAccountID, 'displayName'], '');
const actorIcon = {
id: actorAccountID,
source: UserUtils.getAvatar(lodashGet(personalDetails, [actorAccountID, 'avatar']), actorAccountID),
name: actorEmail,
name: actorDisplayName,
type: CONST.ICON_TYPE_AVATAR,
};

Expand Down Expand Up @@ -1225,7 +1225,6 @@ function buildOptimisticAddCommentReportAction(text, file) {
reportAction: {
reportActionID: NumberUtils.rand64(),
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
actorEmail: currentUserEmail,
actorAccountID: currentUserAccountID,
person: [
{
Expand Down Expand Up @@ -1458,7 +1457,6 @@ function buildOptimisticIOUReportAction(type, amount, currency, comment, partici
return {
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
actorAccountID: currentUserAccountID,
actorEmail: currentUserEmail,
automatic: false,
avatar: lodashGet(currentUserPersonalDetails, 'avatar', UserUtils.getDefaultAvatar(currentUserAccountID)),
isAttachment: false,
Expand Down Expand Up @@ -1497,7 +1495,6 @@ function buildOptimisticReportPreview(reportID, iouReportID, payeeAccountID) {
originalMessage: {
linkedReportID: iouReportID,
},
actorEmail: currentUserEmail,
actorAccountID: currentUserAccountID,
};
}
Expand All @@ -1512,7 +1509,6 @@ function buildOptimisticTaskReportAction(taskReportID, actionName, message = '')
return {
actionName,
actorAccountID: currentUserAccountID,
actorEmail: currentUserEmail,
automatic: false,
avatar: lodashGet(currentUserPersonalDetails, 'avatar', UserUtils.getDefaultAvatar(currentUserAccountID)),
isAttachment: false,
Expand Down Expand Up @@ -1611,7 +1607,6 @@ function buildOptimisticCreatedReportAction(ownerEmail) {
actionName: CONST.REPORT.ACTIONS.TYPE.CREATED,
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
actorAccountID: currentUserAccountID,
actorEmail: currentUserEmail,
message: [
{
type: CONST.REPORT.MESSAGE.TYPE.TEXT,
Expand Down Expand Up @@ -1651,7 +1646,6 @@ function buildOptimisticEditedTaskReportAction(ownerEmail) {
actionName: CONST.REPORT.ACTIONS.TYPE.TASKEDITED,
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
actorAccountID: currentUserAccountID,
actorEmail: currentUserEmail,
message: [
{
type: CONST.REPORT.MESSAGE.TYPE.TEXT,
Expand Down Expand Up @@ -1816,7 +1810,6 @@ function buildOptimisticTaskReport(ownerEmail, ownerAccountID, assigneeAccountID
description,
ownerEmail,
ownerAccountID,
// managerEmail: assignee,
managerID: assigneeAccountID,
type: CONST.REPORT.TYPE.TASK,
parentReportID,
Expand Down Expand Up @@ -2139,7 +2132,7 @@ function shouldShowFlagComment(reportAction, report) {
!isArchivedRoom(report) &&
!chatIncludesChronos(report) &&
!isConciergeChatReport(report.reportID) &&
reportAction.actorEmail !== CONST.EMAIL.CONCIERGE
reportAction.actorAccountID !== CONST.ACCOUNT_ID.CONCIERGE
);
}

Expand Down
18 changes: 13 additions & 5 deletions src/libs/actions/Task.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ import * as UserUtils from '../UserUtils';
import * as PersonalDetailsUtils from '../PersonalDetailsUtils';
import * as ReportActionsUtils from '../ReportActionsUtils';

let currentUserEmail;
let currentUserAccountID;
Onyx.connect({
key: ONYXKEYS.SESSION,
callback: (val) => {
currentUserEmail = lodashGet(val, 'email', '');
currentUserAccountID = lodashGet(val, 'accountID', 0);
},
});

/**
* Clears out the task info from the store
*/
Expand All @@ -24,8 +34,6 @@ function clearOutTaskInfo() {
* Assign a task to a user
* Function title is createTask for consistency with the rest of the actions
* and also because we can create a task without assigning it to anyone
* @param {String} currentUserEmail
* @param {Number} currentUserAccountID
* @param {String} parentReportID
* @param {String} title
* @param {String} description
Expand All @@ -34,7 +42,7 @@ function clearOutTaskInfo() {
*
*/

function createTaskAndNavigate(currentUserEmail, currentUserAccountID, parentReportID, title, description, assignee, assigneeAccountID = 0) {
function createTaskAndNavigate(parentReportID, title, description, assignee, assigneeAccountID = 0) {
// Create the task report
const optimisticTaskReport = ReportUtils.buildOptimisticTaskReport(currentUserEmail, currentUserAccountID, assigneeAccountID, parentReportID, title, description);

Expand Down Expand Up @@ -251,7 +259,7 @@ function reopenTask(taskReportID, taskTitle) {
statusNum: CONST.REPORT.STATUS.OPEN,
lastVisibleActionCreated: reopenedTaskReportAction.created,
lastMessageText: message,
lastActorEmail: reopenedTaskReportAction.actorEmail,
lastActorEmail: currentUserEmail,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB, I think we should still keep it as reopenedTaskReportAction.actorEmail
In case we change ReportUtils::buildOptimisticTaskReportAction() in future and reopenedTaskReportAction get's updated, this direct reference might introduce some bugs.
Same below

Copy link
Contributor Author

@Beamanator Beamanator Jul 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I'm keen to completely get rid of actorEmail AND we're getting rid of lastActorEmail in this PR: #22008

So I think we'll keep it like this for now 🙏

lastActorAccountID: reopenedTaskReportAction.actorAccountID,
lastReadTime: reopenedTaskReportAction.created,
},
Expand Down Expand Up @@ -586,7 +594,7 @@ function cancelTask(taskReportID, taskTitle, originalStateNum, originalStatusNum
value: {
lastVisibleActionCreated: optimisticCancelReportAction.created,
lastMessageText: message,
lastActorEmail: optimisticCancelReportAction.actorEmail,
lastActorEmail: currentUserEmail,
lastActorAccountID: optimisticCancelReportAction.actorAccountID,
},
},
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/ContextMenu/ContextMenuActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ export default [
!isArchivedRoom &&
!isChronosReport &&
!ReportUtils.isConciergeChatReport(reportID) &&
reportAction.actorEmail !== CONST.EMAIL.CONCIERGE,
reportAction.actorAccountID !== CONST.ACCOUNT_ID.CONCIERGE,
onPress: (closePopover, {reportID, reportAction}) => {
if (closePopover) {
hideContextMenu(false, () => Navigation.navigate(ROUTES.getFlagCommentRoute(reportID, reportAction.reportActionID)));
Expand Down
4 changes: 2 additions & 2 deletions src/pages/home/report/ReportActionItemSingle.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ const showWorkspaceDetails = (reportID) => {
function ReportActionItemSingle(props) {
const actorAccountID = props.action.actorAccountID;
let {displayName} = props.personalDetailsList[actorAccountID] || {};
const {avatar, pendingFields} = props.personalDetailsList[actorAccountID] || {};
let actorHint = lodashGet(props.action, 'actorEmail', '').replace(CONST.REGEX.MERGED_ACCOUNT_PREFIX, '');
const {avatar, login, pendingFields} = props.personalDetailsList[actorAccountID] || {};
let actorHint = (login || displayName).replace(CONST.REGEX.MERGED_ACCOUNT_PREFIX, '');
const isWorkspaceActor = ReportUtils.isPolicyExpenseChat(props.report) && !actorAccountID;
let avatarSource = UserUtils.getAvatar(avatar, actorAccountID);

Expand Down
19 changes: 1 addition & 18 deletions src/pages/tasks/NewTaskPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,6 @@ const propTypes = {
}),
),

/** Current user session */
session: PropTypes.shape({
email: PropTypes.string.isRequired,
}),

/** All reports shared with the user */
reports: PropTypes.objectOf(reportPropTypes),

Expand All @@ -63,7 +58,6 @@ const defaultProps = {
task: {},
personalDetails: {},
reports: {},
session: {},
};

function NewTaskPage(props) {
Expand Down Expand Up @@ -120,15 +114,7 @@ function NewTaskPage(props) {
return;
}

TaskUtils.createTaskAndNavigate(
props.session.email,
props.session.accountID,
parentReport.reportID,
props.task.title,
props.task.description,
props.task.assignee,
props.task.assigneeAccountID,
);
TaskUtils.createTaskAndNavigate(parentReport.reportID, props.task.title, props.task.description, props.task.assignee, props.task.assigneeAccountID);
}

if (!Permissions.canUseTasks(props.betas)) {
Expand Down Expand Up @@ -206,9 +192,6 @@ export default compose(
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
session: {
key: ONYXKEYS.SESSION,
},
}),
withLocalize,
)(NewTaskPage);
2 changes: 0 additions & 2 deletions tests/actions/IOUTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ describe('actions/IOU', () => {
const iouAction = {
reportActionID: NumberUtils.rand64(),
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
actorEmail: RORY_EMAIL,
actorAccountID: RORY_ACCOUNT_ID,
created: DateUtils.getDBTime(),
originalMessage: {
Expand Down Expand Up @@ -835,7 +834,6 @@ describe('actions/IOU', () => {
const julesExistingIOUAction = {
reportActionID: NumberUtils.rand64(),
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
actorEmail: RORY_EMAIL,
actorAccountID: RORY_ACCOUNT_ID,
created: DateUtils.getDBTime(),
originalMessage: {
Expand Down
3 changes: 0 additions & 3 deletions tests/actions/ReportTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ describe('actions/Report', () => {
const REPORT_ACTION = {
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
actorAccountID: TEST_USER_ACCOUNT_ID,
actorEmail: TEST_USER_LOGIN,
automatic: false,
avatar: 'https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/avatar_3.png',
message: [{type: 'COMMENT', html: 'Testing a comment', text: 'Testing a comment'}],
Expand Down Expand Up @@ -230,7 +229,6 @@ describe('actions/Report', () => {
1: {
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
actorAccountID: USER_2_ACCOUNT_ID,
actorEmail: USER_2_LOGIN,
automatic: false,
avatar: 'https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/avatar_3.png',
message: [{type: 'COMMENT', html: 'Comment 1', text: 'Comment 1'}],
Expand Down Expand Up @@ -316,7 +314,6 @@ describe('actions/Report', () => {
const USER_1_BASE_ACTION = {
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
actorAccountID: USER_1_ACCOUNT_ID,
actorEmail: USER_1_LOGIN,
automatic: false,
avatar: 'https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/avatar_3.png',
person: [{type: 'TEXT', style: 'strong', text: 'Test User'}],
Expand Down
19 changes: 9 additions & 10 deletions tests/ui/UnreadIndicatorsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,15 @@ function signInAndGetAppWithUnreadChat() {
},
],
},
1: TestHelper.buildTestReportComment(USER_B_EMAIL, MOMENT_TEN_MINUTES_AGO.clone().add(10, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '1'),
2: TestHelper.buildTestReportComment(USER_B_EMAIL, MOMENT_TEN_MINUTES_AGO.clone().add(20, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '2'),
3: TestHelper.buildTestReportComment(USER_B_EMAIL, reportAction3CreatedDate, USER_B_ACCOUNT_ID, '3'),
4: TestHelper.buildTestReportComment(USER_B_EMAIL, MOMENT_TEN_MINUTES_AGO.clone().add(40, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '4'),
5: TestHelper.buildTestReportComment(USER_B_EMAIL, MOMENT_TEN_MINUTES_AGO.clone().add(50, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '5'),
6: TestHelper.buildTestReportComment(USER_B_EMAIL, MOMENT_TEN_MINUTES_AGO.clone().add(60, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '6'),
7: TestHelper.buildTestReportComment(USER_B_EMAIL, MOMENT_TEN_MINUTES_AGO.clone().add(70, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '7'),
8: TestHelper.buildTestReportComment(USER_B_EMAIL, MOMENT_TEN_MINUTES_AGO.clone().add(80, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '8'),
9: TestHelper.buildTestReportComment(USER_B_EMAIL, reportAction9CreatedDate, USER_B_ACCOUNT_ID, '9'),
1: TestHelper.buildTestReportComment(MOMENT_TEN_MINUTES_AGO.clone().add(10, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '1'),
2: TestHelper.buildTestReportComment(MOMENT_TEN_MINUTES_AGO.clone().add(20, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '2'),
3: TestHelper.buildTestReportComment(reportAction3CreatedDate, USER_B_ACCOUNT_ID, '3'),
4: TestHelper.buildTestReportComment(MOMENT_TEN_MINUTES_AGO.clone().add(40, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '4'),
5: TestHelper.buildTestReportComment(MOMENT_TEN_MINUTES_AGO.clone().add(50, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '5'),
6: TestHelper.buildTestReportComment(MOMENT_TEN_MINUTES_AGO.clone().add(60, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '6'),
7: TestHelper.buildTestReportComment(MOMENT_TEN_MINUTES_AGO.clone().add(70, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '7'),
8: TestHelper.buildTestReportComment(MOMENT_TEN_MINUTES_AGO.clone().add(80, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID, '8'),
9: TestHelper.buildTestReportComment(reportAction9CreatedDate, USER_B_ACCOUNT_ID, '9'),
});
Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {
[USER_B_ACCOUNT_ID]: TestHelper.buildPersonalDetails(USER_B_EMAIL, USER_B_ACCOUNT_ID, 'B'),
Expand Down Expand Up @@ -321,7 +321,6 @@ describe('Unread Indicators', () => {
},
[commentReportActionID]: {
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
actorEmail: USER_C_EMAIL,
actorAccountID: USER_C_ACCOUNT_ID,
person: [{type: 'TEXT', style: 'strong', text: 'User C'}],
created: NEW_REPORT_FIST_MESSAGE_CREATED_MOMENT.format(MOMENT_FORMAT),
Expand Down
4 changes: 1 addition & 3 deletions tests/utils/TestHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,17 +196,15 @@ function setPersonalDetails(login, accountID) {
}

/**
* @param {String} actorEmail
* @param {String} created
* @param {Number} actorAccountID
* @param {String} actionID
* @returns {Object}
*/
function buildTestReportComment(actorEmail, created, actorAccountID, actionID = null) {
function buildTestReportComment(created, actorAccountID, actionID = null) {
const reportActionID = actionID || NumberUtils.rand64();
return {
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
actorEmail,
person: [{type: 'TEXT', style: 'strong', text: 'User B'}],
created,
message: [{type: 'COMMENT', html: `Comment ${actionID}`, text: `Comment ${actionID}`}],
Expand Down