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/33420: Allow member to edit task #35219

Merged
merged 7 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
28 changes: 3 additions & 25 deletions src/components/ReportActionItem/TaskPreview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,14 @@ import * as Task from '@userActions/Task';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Policy, Report, ReportAction} from '@src/types/onyx';
import type {Report, ReportAction} from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';

type PolicyRole = {
/** The role of current user */
role: Task.PolicyValue | undefined;
};

type TaskPreviewOnyxProps = {
/* Onyx Props */

/* current report of TaskPreview */
taskReport: OnyxEntry<Report>;

/** The policy of root parent report */
rootParentReportpolicy: OnyxEntry<PolicyRole>;
};

type TaskPreviewProps = WithCurrentUserPersonalDetailsProps &
Expand All @@ -70,17 +62,7 @@ type TaskPreviewProps = WithCurrentUserPersonalDetailsProps &
checkIfContextMenuActive: () => void;
};

function TaskPreview({
taskReport,
taskReportID,
action,
contextMenuAnchor,
chatReportID,
checkIfContextMenuActive,
currentUserPersonalDetails,
rootParentReportpolicy,
isHovered = false,
}: TaskPreviewProps) {
function TaskPreview({taskReport, taskReportID, action, contextMenuAnchor, chatReportID, checkIfContextMenuActive, currentUserPersonalDetails, isHovered = false}: TaskPreviewProps) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const personalDetails = usePersonalDetails() || CONST.EMPTY_OBJECT;
Expand Down Expand Up @@ -123,7 +105,7 @@ function TaskPreview({
style={[styles.mr2]}
containerStyle={[styles.taskCheckbox]}
isChecked={isTaskCompleted}
disabled={!Task.canModifyTask(taskReport, currentUserPersonalDetails.accountID, rootParentReportpolicy?.role)}
disabled={!Task.canModifyTask(taskReport, currentUserPersonalDetails.accountID)}
onPress={Session.checkIfActionIsAllowed(() => {
if (isTaskCompleted) {
Task.reopenTask(taskReport);
Expand Down Expand Up @@ -151,9 +133,5 @@ export default withCurrentUserPersonalDetails(
taskReport: {
key: ({taskReportID}) => `${ONYXKEYS.COLLECTION.REPORT}${taskReportID}`,
},
rootParentReportpolicy: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID ?? '0'}`,
selector: (policy: Policy | null) => ({role: policy?.role}),
},
})(TaskPreview),
);
16 changes: 3 additions & 13 deletions src/components/ReportActionItem/TaskView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,11 @@ import * as Task from '@userActions/Task';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {PersonalDetailsList, Policy, Report} from '@src/types/onyx';
import type {PersonalDetailsList, Report} from '@src/types/onyx';

type TaskViewOnyxProps = {
/** All of the personal details for everyone */
personalDetails: OnyxEntry<PersonalDetailsList>;

/** The policy for the current route */
policy: Pick<Policy, 'role'> | null;
};

type TaskViewProps = TaskViewOnyxProps &
Expand All @@ -47,7 +44,7 @@ type TaskViewProps = TaskViewOnyxProps &
shouldShowHorizontalRule: boolean;
};

function TaskView({report, policy, shouldShowHorizontalRule, ...props}: TaskViewProps) {
function TaskView({report, shouldShowHorizontalRule, ...props}: TaskViewProps) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
useEffect(() => {
Expand All @@ -61,7 +58,7 @@ function TaskView({report, policy, shouldShowHorizontalRule, ...props}: TaskView
);
const isCompleted = ReportUtils.isCompletedTaskReport(report);
const isOpen = ReportUtils.isOpenTaskReport(report);
const canModifyTask = Task.canModifyTask(report, props.currentUserPersonalDetails.accountID, policy?.role);
const canModifyTask = Task.canModifyTask(report, props.currentUserPersonalDetails.accountID);
const disableState = !canModifyTask;
const isDisableInteractive = !canModifyTask || !isOpen;
const personalDetails = usePersonalDetails() || CONST.EMPTY_OBJECT;
Expand Down Expand Up @@ -199,13 +196,6 @@ const TaskViewWithOnyx = withOnyx<TaskViewProps, TaskViewOnyxProps>({
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
policy: {
key: ({report}) => {
const rootParentReport = ReportUtils.getRootParentReport(report);
return `${ONYXKEYS.COLLECTION.POLICY}${rootParentReport ? rootParentReport.policyID : '0'}`;
},
selector: (policy: OnyxEntry<Policy>) => (policy ? {role: policy.role} : null),
},
})(TaskView);

export default withCurrentUserPersonalDetails(TaskViewWithOnyx);
13 changes: 2 additions & 11 deletions src/components/TaskHeaderActionButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,22 @@ import Button from './Button';
type TaskHeaderActionButtonOnyxProps = {
/** Current user session */
session: OnyxEntry<OnyxTypes.Session>;

/** The policy of root parent report */
policy: OnyxEntry<OnyxTypes.Policy>;
};

type TaskHeaderActionButtonProps = TaskHeaderActionButtonOnyxProps & {
/** The report currently being looked at */
report: OnyxTypes.Report;
};

function TaskHeaderActionButton({report, session, policy}: TaskHeaderActionButtonProps) {
function TaskHeaderActionButton({report, session}: TaskHeaderActionButtonProps) {
const {translate} = useLocalize();
const styles = useThemeStyles();

return (
<View style={[styles.flexRow, styles.alignItemsCenter, styles.justifyContentEnd]}>
<Button
success
isDisabled={!Task.canModifyTask(report, session?.accountID ?? 0, policy?.role)}
isDisabled={!Task.canModifyTask(report, session?.accountID ?? 0)}
medium
text={translate(ReportUtils.isCompletedTaskReport(report) ? 'task.markAsIncomplete' : 'task.markAsComplete')}
onPress={Session.checkIfActionIsAllowed(() => (ReportUtils.isCompletedTaskReport(report) ? Task.reopenTask(report) : Task.completeTask(report)))}
Expand All @@ -48,10 +45,4 @@ export default withOnyx<TaskHeaderActionButtonProps, TaskHeaderActionButtonOnyxP
session: {
key: ONYXKEYS.SESSION,
},
policy: {
key: ({report}) => {
const rootParentReport = ReportUtils.getRootParentReport(report);
return `${ONYXKEYS.COLLECTION.POLICY}${rootParentReport ? rootParentReport.policyID : '0'}`;
},
},
})(TaskHeaderActionButton);
13 changes: 2 additions & 11 deletions src/libs/actions/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ function getTaskOwnerAccountID(taskReport: OnyxEntry<OnyxTypes.Report>): number
/**
* Check if you're allowed to modify the task - anyone that has write access to the report can modify the task
*/
function canModifyTask(taskReport: OnyxEntry<OnyxTypes.Report>, sessionAccountID: number, policyRole: PolicyValue | undefined): boolean {
function canModifyTask(taskReport: OnyxEntry<OnyxTypes.Report>, sessionAccountID: number): boolean {
if (ReportUtils.isCanceledTaskReport(taskReport)) {
return false;
}
Expand All @@ -885,16 +885,7 @@ function canModifyTask(taskReport: OnyxEntry<OnyxTypes.Report>, sessionAccountID
return true;
}

const parentReport = ReportUtils.getParentReport(taskReport);

if (policyRole && !isEmptyObject(parentReport) && (ReportUtils.isChatRoom(parentReport) || ReportUtils.isPolicyExpenseChat(parentReport)) && policyRole !== CONST.POLICY.ROLE.ADMIN) {
return false;
}

// If you don't have access to the task report (maybe haven't opened it yet), check if you can access the parent report
// - If the parent report is an #admins only room
// - If you are a policy admin
return !isEmptyObject(parentReport) && ReportUtils.isAllowedToComment(parentReport);
return !isEmptyObject(taskReport) && ReportUtils.isAllowedToComment(taskReport);
}

function clearTaskErrors(reportID: string) {
Expand Down
16 changes: 1 addition & 15 deletions src/pages/home/HeaderView.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,6 @@ const propTypes = {
accountID: PropTypes.number,
}),

/** The policy of root parent report */
rootParentReportPolicy: PropTypes.shape({
/** The role of current user */
role: PropTypes.string,
}),

/** The current policy of the report */
policy: PropTypes.shape({
/** The policy name */
Expand All @@ -88,7 +82,6 @@ const defaultProps = {
accountID: 0,
},
policy: {},
rootParentReportPolicy: {},
};

function HeaderView(props) {
Expand Down Expand Up @@ -124,7 +117,7 @@ function HeaderView(props) {
// these users via alternative means. It is possible to request a call with Concierge so we leave the option for them.
const threeDotMenuItems = [];
if (isTaskReport && !isCanceledTaskReport) {
const canModifyTask = Task.canModifyTask(props.report, props.session.accountID, lodashGet(props.rootParentReportPolicy, 'role', ''));
const canModifyTask = Task.canModifyTask(props.report, props.session.accountID);

// Task is marked as completed
if (ReportUtils.isCompletedTaskReport(props.report) && canModifyTask) {
Expand Down Expand Up @@ -362,13 +355,6 @@ export default memo(
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY}${report ? report.policyID : '0'}`,
selector: (policy) => _.pick(policy, ['name', 'avatar', 'pendingAction']),
},
rootParentReportPolicy: {
key: ({report}) => {
const rootParentReport = ReportUtils.getRootParentReport(report);
return `${ONYXKEYS.COLLECTION.POLICY}${rootParentReport ? rootParentReport.policyID : '0'}`;
},
selector: (policy) => _.pick(policy, ['role']),
},
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
Expand Down
1 change: 0 additions & 1 deletion src/pages/home/report/ReportActionItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,6 @@ function ReportActionItem(props) {
<ReportPreview
iouReportID={ReportActionsUtils.getIOUReportIDFromReportActionPreview(props.action)}
chatReportID={props.report.reportID}
policyID={props.report.policyID}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we removing this? Does this only affect tasks?

Copy link
Contributor Author

@DylanDylann DylanDylann Feb 2, 2024

Choose a reason for hiding this comment

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

@puneetlath Yes. In the PR, we pass policyID to get policy.role because we need pass policy.role to canModifyTask function. In this PR, we revert that change

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch!
@DylanDylann you confused this with TaskPreview

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, I updated

containerStyles={props.displayAsGroup ? [] : [styles.mt2]}
action={props.action}
isHovered={hovered}
Expand Down
19 changes: 1 addition & 18 deletions src/pages/tasks/TaskAssigneeSelectorModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,6 @@ const propTypes = {
report: reportPropTypes,
}),

/** The policy of root parent report */
rootParentReportPolicy: PropTypes.shape({
/** The role of current user */
role: PropTypes.string,
}),

...withLocalizePropTypes,
};

Expand All @@ -72,7 +66,6 @@ const defaultProps = {
session: {},
route: {},
task: {},
rootParentReportPolicy: {},
};

function TaskAssigneeSelectorModal(props) {
Expand Down Expand Up @@ -209,7 +202,7 @@ function TaskAssigneeSelectorModal(props) {
);

const isOpen = ReportUtils.isOpenTaskReport(report);
const canModifyTask = Task.canModifyTask(report, props.currentUserPersonalDetails.accountID, lodashGet(props.rootParentReportPolicy, 'role', ''));
const canModifyTask = Task.canModifyTask(report, props.currentUserPersonalDetails.accountID);
const isTaskNonEditable = ReportUtils.isTaskReport(report) && (!canModifyTask || !isOpen);

return (
Expand Down Expand Up @@ -264,14 +257,4 @@ export default compose(
key: ONYXKEYS.SESSION,
},
}),
withOnyx({
rootParentReportPolicy: {
key: ({reports, route}) => {
const report = reports[`${ONYXKEYS.COLLECTION.REPORT}${route.params?.reportID || '0'}`];
const rootParentReport = ReportUtils.getRootParentReport(report);
return `${ONYXKEYS.COLLECTION.POLICY}${rootParentReport ? rootParentReport.policyID : '0'}`;
},
selector: (policy) => _.pick(policy, ['role']),
},
}),
)(TaskAssigneeSelectorModal);
19 changes: 1 addition & 18 deletions src/pages/tasks/TaskDescriptionPage.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import {useFocusEffect} from '@react-navigation/native';
import ExpensiMark from 'expensify-common/lib/ExpensiMark';
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import React, {useCallback, useRef} from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
import FormProvider from '@components/Form/FormProvider';
import InputWrapper from '@components/Form/InputWrapper';
Expand All @@ -31,19 +28,12 @@ const propTypes = {
/** The report currently being looked at */
report: reportPropTypes,

/** The policy of parent report */
rootParentReportPolicy: PropTypes.shape({
/** The role of current user */
role: PropTypes.string,
}),

/* Onyx Props */
...withLocalizePropTypes,
};

const defaultProps = {
report: {},
rootParentReportPolicy: {},
};

const parser = new ExpensiMark();
Expand Down Expand Up @@ -74,7 +64,7 @@ function TaskDescriptionPage(props) {
const focusTimeoutRef = useRef(null);

const isOpen = ReportUtils.isOpenTaskReport(props.report);
const canModifyTask = Task.canModifyTask(props.report, props.currentUserPersonalDetails.accountID, lodashGet(props.rootParentReportPolicy, 'role', ''));
const canModifyTask = Task.canModifyTask(props.report, props.currentUserPersonalDetails.accountID);
const isTaskNonEditable = ReportUtils.isTaskReport(props.report) && (!canModifyTask || !isOpen);

useFocusEffect(
Expand Down Expand Up @@ -148,12 +138,5 @@ export default compose(
report: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`,
},
rootParentReportPolicy: {
key: ({report}) => {
const rootParentReport = ReportUtils.getRootParentReport(report);
return `${ONYXKEYS.COLLECTION.POLICY}${rootParentReport ? rootParentReport.policyID : '0'}`;
},
selector: (policy) => _.pick(policy, ['role']),
},
}),
)(TaskDescriptionPage);
18 changes: 1 addition & 17 deletions src/pages/tasks/TaskTitlePage.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import React, {useCallback, useRef} from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
Expand All @@ -26,19 +24,12 @@ const propTypes = {
/** The report currently being looked at */
report: reportPropTypes,

/** The policy of parent report */
rootParentReportPolicy: PropTypes.shape({
/** The role of current user */
role: PropTypes.string,
}),

/* Onyx Props */
...withLocalizePropTypes,
};

const defaultProps = {
report: {},
rootParentReportPolicy: {},
};

function TaskTitlePage(props) {
Expand Down Expand Up @@ -79,7 +70,7 @@ function TaskTitlePage(props) {

const inputRef = useRef(null);
const isOpen = ReportUtils.isOpenTaskReport(props.report);
const canModifyTask = Task.canModifyTask(props.report, props.currentUserPersonalDetails.accountID, lodashGet(props.rootParentReportPolicy, 'role', ''));
const canModifyTask = Task.canModifyTask(props.report, props.currentUserPersonalDetails.accountID);
const isTaskNonEditable = ReportUtils.isTaskReport(props.report) && (!canModifyTask || !isOpen);

return (
Expand Down Expand Up @@ -139,12 +130,5 @@ export default compose(
report: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`,
},
rootParentReportPolicy: {
key: ({report}) => {
const rootParentReport = ReportUtils.getRootParentReport(report);
return `${ONYXKEYS.COLLECTION.POLICY}${rootParentReport ? rootParentReport.policyID : '0'}`;
},
selector: (policy) => _.pick(policy, ['role']),
},
}),
)(TaskTitlePage);
Loading