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

Fix empty assignee in TaskHeader and update permission to mark Task complete/incomplete/cancel in Task report #20689

Merged
merged 16 commits into from
Jun 18, 2023
Merged
33 changes: 28 additions & 5 deletions src/components/TaskHeader.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {View} from 'react-native';
import PropTypes from 'prop-types';
import lodashGet from 'lodash/get';
import _ from 'underscore';
import {withOnyx} from 'react-native-onyx';
import reportPropTypes from '../pages/reportPropTypes';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import * as ReportUtils from '../libs/ReportUtils';
Expand All @@ -23,6 +24,7 @@ import Button from './Button';
import * as TaskUtils from '../libs/actions/Task';
import * as UserUtils from '../libs/UserUtils';
import PressableWithFeedback from './Pressable/PressableWithFeedback';
import ONYXKEYS from '../ONYXKEYS';

const propTypes = {
/** The report currently being looked at */
Expand All @@ -31,13 +33,25 @@ const propTypes = {
/** Personal details so we can get the ones for the report participants */
personalDetails: PropTypes.objectOf(participantPropTypes).isRequired,

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

...withLocalizePropTypes,
};

const defaultProps = {
session: {
email: null,
},
};

function TaskHeader(props) {
const title = ReportUtils.getReportName(props.report);
const assigneeName = ReportUtils.getDisplayNameForParticipant(props.report.managerID);
const assigneeAvatar = UserUtils.getAvatar(lodashGet(props.personalDetails, [props.report.managerID, 'avatar']), props.report.managerID);
const assigneeEmail = TaskUtils.getTaskAssigneeEmail(props.report);
const assigneeName = ReportUtils.getDisplayNameForParticipant(assigneeEmail);
const assigneeAvatar = UserUtils.getAvatar(lodashGet(props.personalDetails, [assigneeEmail, 'avatar']), assigneeEmail);
const isOpen = props.report.stateNum === CONST.REPORT.STATE_NUM.OPEN && props.report.statusNum === CONST.REPORT.STATUS.OPEN;
const isCompleted = props.report.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.report.statusNum === CONST.REPORT.STATUS.APPROVED;

Expand All @@ -60,7 +74,7 @@ function TaskHeader(props) {
>
<View style={[styles.flexRow, styles.alignItemsCenter, styles.justifyContentBetween, styles.pv3]}>
<View style={[styles.flexRow, styles.alignItemsCenter, styles.justifyContentBetween]}>
{!_.isEmpty(props.report.managerID) && (
{!_.isEmpty(assigneeEmail) && (
<>
<Avatar
source={assigneeAvatar}
Expand Down Expand Up @@ -93,7 +107,7 @@ function TaskHeader(props) {
) : (
<Button
success
isDisabled={TaskUtils.isTaskCanceled(props.report)}
isDisabled={TaskUtils.isTaskCanceled(props.report) || !TaskUtils.isTaskAssigneeOrTaskOwner(props.report, props.session.email)}
medium
text={props.translate('newTaskPage.markAsDone')}
onPress={() => TaskUtils.completeTask(props.report.reportID, title)}
Expand Down Expand Up @@ -124,6 +138,15 @@ function TaskHeader(props) {
}

TaskHeader.propTypes = propTypes;
TaskHeader.defaultProps = defaultProps;
TaskHeader.displayName = 'TaskHeader';

export default compose(withWindowDimensions, withLocalize)(TaskHeader);
export default compose(
withWindowDimensions,
withLocalize,
withOnyx({
session: {
key: ONYXKEYS.SESSION,
},
}),
)(TaskHeader);
2 changes: 1 addition & 1 deletion src/libs/ReportActionsUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ function hasCommentThread(reportAction) {
}

/**
* Returns the parentReportAction if the given report is a thread.
* Returns the parentReportAction if the given report is a thread/task.
*
* @param {Object} report
* @returns {Object}
Expand Down
43 changes: 43 additions & 0 deletions src/libs/actions/Task.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import ROUTES from '../../ROUTES';
import CONST from '../../CONST';
import DateUtils from '../DateUtils';
import * as UserUtils from '../UserUtils';
import * as ReportActionsUtils from '../ReportActionsUtils';

/**
* Clears out the task info from the store
Expand Down Expand Up @@ -626,6 +627,46 @@ function dismissModalAndClearOutTaskInfo() {
clearOutTaskInfo();
}

/**
* Returns Task assignee email
*
* @param {Object} taskReport
* @returns {String|null}
*/
function getTaskAssigneeEmail(taskReport) {
if (!taskReport) {
return null;
}

if (taskReport.managerID) {
return taskReport.managerID;
}

const reportAction = ReportActionsUtils.getParentReportAction(taskReport);
return lodashGet(reportAction, 'childManagerEmail', null);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mollfpr @joelbettner I just merged latest main to this PR and I realize that we're going to change from "email" -> "accountID" a lot of place.

For "childManagerEmail", do you know when will the BE return "childManagerAccountID"? for reportAction of taskReport? (as we optimistic build from FE side)?

App/src/libs/ReportUtils.js

Lines 1239 to 1240 in 7b00c47

reportAction.reportAction.childManagerEmail = taskAssignee;
reportAction.reportAction.childManagerAccountID = taskAssigneeAccountID;

Currently, I have a blocker that most of utils (get avatar/display name) is changed to accountID

function getDisplayNameForParticipant(accountID, shouldUseShortForm = false) {

So if we only have email, we can not get the display name/avatar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nvm, I found a way to get accountID for given login.

}

/**
* Returns Task owner email
*
* @param {Object} taskReport
* @returns {String|null}
*/
function getTaskOwnerEmail(taskReport) {
return lodashGet(taskReport, 'ownerEmail', null);
}

/**
* Check if current user is either task assignee or task owner
*
* @param {Object} taskReport
* @param {String} sessionEmail
* @returns {Boolean}
*/
function isTaskAssigneeOrTaskOwner(taskReport, sessionEmail) {
return sessionEmail === getTaskOwnerEmail(taskReport) || sessionEmail === getTaskAssigneeEmail(taskReport);
}

export {
createTaskAndNavigate,
editTaskAndNavigate,
Expand All @@ -645,4 +686,6 @@ export {
cancelTask,
isTaskCanceled,
dismissModalAndClearOutTaskInfo,
getTaskAssigneeEmail,
isTaskAssigneeOrTaskOwner,
};
18 changes: 15 additions & 3 deletions src/pages/home/HeaderView.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ const propTypes = {
guideCalendarLink: PropTypes.string,
}),

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

/** The report actions from the parent report */
// TO DO: Replace with HOC https://github.com/Expensify/App/issues/18769.
// eslint-disable-next-line react/no-unused-prop-types
Expand All @@ -68,6 +73,9 @@ const defaultProps = {
guideCalendarLink: null,
},
parentReport: {},
session: {
email: null,
},
};

function HeaderView(props) {
Expand All @@ -91,7 +99,8 @@ function HeaderView(props) {
const shouldShowCallButton = (isConcierge && guideCalendarLink) || (!isAutomatedExpensifyAccount && !isTaskReport);
const threeDotMenuItems = [];
if (isTaskReport) {
if (props.report.stateNum === CONST.REPORT.STATE_NUM.OPEN && props.report.statusNum === CONST.REPORT.STATUS.OPEN) {
const isTaskAssigneeOrTaskOwner = Task.isTaskAssigneeOrTaskOwner(props.report, props.session.email);
if (props.report.stateNum === CONST.REPORT.STATE_NUM.OPEN && props.report.statusNum === CONST.REPORT.STATUS.OPEN && isTaskAssigneeOrTaskOwner) {
threeDotMenuItems.push({
icon: Expensicons.Checkmark,
text: props.translate('newTaskPage.markAsDone'),
Expand All @@ -100,7 +109,7 @@ function HeaderView(props) {
}

// Task is marked as completed
if (props.report.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.report.statusNum === CONST.REPORT.STATUS.APPROVED) {
if (props.report.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.report.statusNum === CONST.REPORT.STATUS.APPROVED && isTaskAssigneeOrTaskOwner) {
threeDotMenuItems.push({
icon: Expensicons.Checkmark,
text: props.translate('newTaskPage.markAsIncomplete'),
Expand All @@ -109,7 +118,7 @@ function HeaderView(props) {
}

// Task is not closed
if (props.report.stateNum !== CONST.REPORT.STATE_NUM.SUBMITTED && props.report.statusNum !== CONST.REPORT.STATUS.CLOSED) {
if (props.report.stateNum !== CONST.REPORT.STATE_NUM.SUBMITTED && props.report.statusNum !== CONST.REPORT.STATUS.CLOSED && isTaskAssigneeOrTaskOwner) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure this one. Should we allow assignee to cancel the task?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think so. If this is a flow we do not want in the future we can create another issue for it. But I believe an assignee should be able to cancel a task.

threeDotMenuItems.push({
icon: Expensicons.Trashcan,
text: props.translate('common.cancel'),
Expand Down Expand Up @@ -258,5 +267,8 @@ export default compose(
parentReport: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report.parentReportID || report.reportID}`,
},
session: {
key: ONYXKEYS.SESSION,
},
}),
)(HeaderView);