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.

Sign up for GitHub

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: Make all task titles/descriptions read-only to all but the author/assignee. #53749

Merged
merged 27 commits into from
Dec 23, 2024
Merged

fix: Make all task titles/descriptions read-only to all but the author/assignee. #53749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8fcb133
fix: Make all task titles/descriptions read-only to all but the autho…
Krishna2323 Dec 9, 2024
737d469
fix 'withOnyx' is deprecated warning.
Krishna2323 Dec 9, 2024
0e46d44
fix 'withOnyx' is deprecated warning in TaskHeaderActionButton.
Krishna2323 Dec 9, 2024
fd36917
Merge branch 'main' into krishna2323/issue/52979_pr2
Krishna2323 Dec 11, 2024
eb1df5b
update cursor style for task view.
Krishna2323 Dec 11, 2024
b78d7bc
fix es-lint.
Krishna2323 Dec 11, 2024
6ff76aa
Merge branch 'Expensify:main' into krishna2323/issue/52979_pr2
Krishna2323 Dec 12, 2024
59ccd0c
disable interaction when task is completed.
Krishna2323 Dec 12, 2024
4d9471e
remove console log.
Krishna2323 Dec 12, 2024
9317132
minor update.
Krishna2323 Dec 12, 2024
2d66f39
minor update.
Krishna2323 Dec 12, 2024
880cbed
use interactive variable.
Krishna2323 Dec 12, 2024
7dd9409
update canModifyTask function.
Krishna2323 Dec 12, 2024
5609459
update canModifyTask function.
Krishna2323 Dec 12, 2024
2a02141
use taskOwnerAccountID & taskAssigneeAccountID for task previews.
Krishna2323 Dec 12, 2024
9fb9594
revert back cursor style changes for checkbox.
Krishna2323 Dec 12, 2024
6b7bd35
Merge branch 'main' into krishna2323/issue/52979_pr2
Krishna2323 Dec 13, 2024
a13be8a
revert the new behaviour for tasks.
Krishna2323 Dec 15, 2024
bc6034d
Merge branch 'Expensify:main' into krishna2323/issue/52979_pr2
Krishna2323 Dec 15, 2024
1f0c7ac
revert changes for header task action button.
Krishna2323 Dec 16, 2024
5a26e76
Merge branch 'Expensify:main' into krishna2323/issue/52979_pr2
Krishna2323 Dec 16, 2024
5877b0e
permit only the author to make changes in task title and discription.
Krishna2323 Dec 16, 2024
b31f66e
update 'canActionTask' to include additional checks.
Krishna2323 Dec 16, 2024
52a2cea
fix eslint.
Krishna2323 Dec 16, 2024
65b2f41
revert changes to dfeault values.
Krishna2323 Dec 16, 2024
e438da3
minor update.
Krishna2323 Dec 16, 2024
56a7c3a
Merge branch 'Expensify:main' into krishna2323/issue/52979_pr2
Krishna2323 Dec 21, 2024
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
9 changes: 5 additions & 4 deletions src/components/ReportActionItem/TaskPreview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ function TaskPreview({taskReportID, action, contextMenuAnchor, chatReportID, che
? taskReport?.stateNum === CONST.REPORT.STATE_NUM.APPROVED && taskReport.statusNum === CONST.REPORT.STATUS_NUM.APPROVED
: action?.childStateNum === CONST.REPORT.STATE_NUM.APPROVED && action?.childStatusNum === CONST.REPORT.STATUS_NUM.APPROVED;
const taskTitle = Str.htmlEncode(TaskUtils.getTaskTitleFromReport(taskReport, action?.childReportName ?? ''));
const taskAssigneeAccountID = Task.getTaskAssigneeAccountID(taskReport) ?? action?.childManagerAccountID ?? -1;
const taskAssigneeAccountID = Task.getTaskAssigneeAccountID(taskReport) ?? action?.childManagerAccountID ?? CONST.DEFAULT_NUMBER_ID;
const taskOwnerAccountID = taskReport?.ownerAccountID ?? action?.actorAccountID ?? CONST.DEFAULT_NUMBER_ID;
const hasAssignee = taskAssigneeAccountID > 0;
const personalDetails = usePersonalDetails();
const avatar = personalDetails?.[taskAssigneeAccountID]?.avatar ?? Expensicons.FallbackAvatar;
Expand Down Expand Up @@ -106,12 +107,12 @@ function TaskPreview({taskReportID, action, contextMenuAnchor, chatReportID, che
<Checkbox
style={[styles.mr2]}
isChecked={isTaskCompleted}
disabled={!Task.canModifyTask(taskReport, currentUserPersonalDetails.accountID) || !Task.canActionTask(taskReport, currentUserPersonalDetails.accountID)}
disabled={!Task.canActionTask(taskReport, currentUserPersonalDetails.accountID, taskOwnerAccountID, taskAssigneeAccountID)}
onPress={Session.checkIfActionIsAllowed(() => {
if (isTaskCompleted) {
Task.reopenTask(taskReport);
Task.reopenTask(taskReport, taskReportID);
} else {
Task.completeTask(taskReport);
Task.completeTask(taskReport, taskReportID);
}
})}
accessibilityLabel={translate('task.task')}
Expand Down
35 changes: 19 additions & 16 deletions src/components/ReportActionItem/TaskView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import OfflineWithFeedback from '@components/OfflineWithFeedback';
import {usePersonalDetails} from '@components/OnyxProvider';
import PressableWithSecondaryInteraction from '@components/PressableWithSecondaryInteraction';
import Text from '@components/Text';
import withCurrentUserPersonalDetails from '@components/withCurrentUserPersonalDetails';
import type {WithCurrentUserPersonalDetailsProps} from '@components/withCurrentUserPersonalDetails';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useLocalize from '@hooks/useLocalize';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
Expand All @@ -27,27 +26,28 @@ import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
import type {Report} from '@src/types/onyx';

type TaskViewProps = WithCurrentUserPersonalDetailsProps & {
type TaskViewProps = {
/** The report currently being looked at */
report: Report;
};

function TaskView({report, ...props}: TaskViewProps) {
function TaskView({report}: TaskViewProps) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
const personalDetails = usePersonalDetails();
useEffect(() => {
Task.setTaskReport(report);
}, [report]);
const personalDetails = usePersonalDetails();
const taskTitle = convertToLTR(report.reportName ?? '');
const assigneeTooltipDetails = ReportUtils.getDisplayNamesWithTooltips(
OptionsListUtils.getPersonalDetailsForAccountIDs(report.managerID ? [report.managerID] : [], personalDetails),
false,
);
const isCompleted = ReportUtils.isCompletedTaskReport(report);
const isOpen = ReportUtils.isOpenTaskReport(report);
const canModifyTask = Task.canModifyTask(report, props.currentUserPersonalDetails.accountID);
const canActionTask = Task.canActionTask(report, props.currentUserPersonalDetails.accountID);
const isCompleted = ReportUtils.isCompletedTaskReport(report);
const canModifyTask = Task.canModifyTask(report, currentUserPersonalDetails.accountID);
const canActionTask = Task.canActionTask(report, currentUserPersonalDetails.accountID);
const disableState = !canModifyTask;
const isDisableInteractive = !canModifyTask || !isOpen;
const {translate} = useLocalize();
Expand Down Expand Up @@ -77,10 +77,10 @@ function TaskView({report, ...props}: TaskViewProps) {
styles.ph5,
styles.pv2,
StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered, pressed, false, disableState, !isDisableInteractive), true),
isDisableInteractive && !disableState && styles.cursorDefault,
isDisableInteractive && styles.cursorDefault,
]}
disabled={disableState}
accessibilityLabel={taskTitle || translate('task.task')}
disabled={isDisableInteractive}
>
{({pressed}) => (
<OfflineWithFeedback pendingAction={report.pendingFields?.reportName}>
Expand All @@ -104,7 +104,7 @@ function TaskView({report, ...props}: TaskViewProps) {
containerBorderRadius={8}
caretSize={16}
accessibilityLabel={taskTitle || translate('task.task')}
disabled={!canModifyTask || !canActionTask}
disabled={!canActionTask}
/>
<View style={[styles.flexRow, styles.flex1]}>
<Text
Expand All @@ -114,7 +114,7 @@ function TaskView({report, ...props}: TaskViewProps) {
{taskTitle}
</Text>
</View>
{isOpen && (
{!isDisableInteractive && (
<View style={styles.taskRightIconContainer}>
<Icon
additionalStyles={[styles.alignItemsCenter]}
Expand All @@ -135,12 +135,13 @@ function TaskView({report, ...props}: TaskViewProps) {
description={translate('task.description')}
title={report.description ?? ''}
onPress={() => Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report.reportID, Navigation.getReportRHPActiveRoute()))}
shouldShowRightIcon={isOpen}
shouldShowRightIcon={!isDisableInteractive}
disabled={disableState}
wrapperStyle={[styles.pv2, styles.taskDescriptionMenuItem]}
shouldGreyOutWhenDisabled={false}
numberOfLinesTitle={0}
interactive={!isDisableInteractive}
shouldUseDefaultCursorWhenDisabled
/>
</OfflineWithFeedback>
<OfflineWithFeedback pendingAction={report.pendingFields?.managerID}>
Expand All @@ -153,23 +154,25 @@ function TaskView({report, ...props}: TaskViewProps) {
avatarSize={CONST.AVATAR_SIZE.SMALLER}
titleStyle={styles.assigneeTextStyle}
onPress={() => Navigation.navigate(ROUTES.TASK_ASSIGNEE.getRoute(report.reportID, Navigation.getReportRHPActiveRoute()))}
shouldShowRightIcon={isOpen}
shouldShowRightIcon={!isDisableInteractive}
disabled={disableState}
wrapperStyle={[styles.pv2]}
isSmallAvatarSubscriptMenu
shouldGreyOutWhenDisabled={false}
interactive={!isDisableInteractive}
titleWithTooltips={assigneeTooltipDetails}
shouldUseDefaultCursorWhenDisabled
/>
) : (
<MenuItemWithTopDescription
description={translate('task.assignee')}
onPress={() => Navigation.navigate(ROUTES.TASK_ASSIGNEE.getRoute(report.reportID, Navigation.getReportRHPActiveRoute()))}
shouldShowRightIcon={isOpen}
shouldShowRightIcon={!isDisableInteractive}
disabled={disableState}
wrapperStyle={[styles.pv2]}
shouldGreyOutWhenDisabled={false}
interactive={!isDisableInteractive}
shouldUseDefaultCursorWhenDisabled
/>
)}
</OfflineWithFeedback>
Expand All @@ -180,4 +183,4 @@ function TaskView({report, ...props}: TaskViewProps) {

TaskView.displayName = 'TaskView';

export default withCurrentUserPersonalDetails(TaskView);
export default TaskView;
23 changes: 7 additions & 16 deletions src/components/TaskHeaderActionButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,25 @@
import React from 'react';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import * as ReportUtils from '@libs/ReportUtils';
import * as TaskUtils from '@libs/TaskUtils';
import * as Session from '@userActions/Session';
import * as Task from '@userActions/Task';
import ONYXKEYS from '@src/ONYXKEYS';
import CONST from '@src/CONST';
import type * as OnyxTypes from '@src/types/onyx';
import Button from './Button';
import {useSession} from './OnyxProvider';

type TaskHeaderActionButtonOnyxProps = {
/** Current user session */
session: OnyxEntry<OnyxTypes.Session>;
};

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

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

if (!ReportUtils.canWriteInReport(report)) {
return null;
Expand All @@ -34,7 +29,7 @@ function TaskHeaderActionButton({report, session}: TaskHeaderActionButtonProps)
<View style={[styles.flexRow, styles.alignItemsCenter, styles.justifyContentEnd]}>
<Button
success
isDisabled={!Task.canModifyTask(report, session?.accountID ?? -1) || !Task.canActionTask(report, session?.accountID ?? -1)}
isDisabled={!Task.canActionTask(report, session?.accountID ?? CONST.DEFAULT_NUMBER_ID)}
text={translate(ReportUtils.isCompletedTaskReport(report) ? 'task.markAsIncomplete' : 'task.markAsComplete')}
onPress={Session.checkIfActionIsAllowed(() => {
// If we're already navigating to these task editing pages, early return not to mark as completed, otherwise we would have not found page.
Expand All @@ -55,8 +50,4 @@ function TaskHeaderActionButton({report, session}: TaskHeaderActionButtonProps)

TaskHeaderActionButton.displayName = 'TaskHeaderActionButton';

export default withOnyx<TaskHeaderActionButtonProps, TaskHeaderActionButtonOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
})(TaskHeaderActionButton);
export default TaskHeaderActionButton;
55 changes: 38 additions & 17 deletions src/libs/actions/Task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Onyx.connect({
key: ONYXKEYS.SESSION,
callback: (value) => {
currentUserEmail = value?.email ?? '';
currentUserAccountID = value?.accountID ?? -1;
currentUserAccountID = value?.accountID ?? CONST.DEFAULT_NUMBER_ID;
},
});

Expand Down Expand Up @@ -365,8 +365,13 @@ function getOutstandingChildTask(taskReport: OnyxEntry<OnyxTypes.Report>) {
/**
* Complete a task
*/
function completeTask(taskReport: OnyxEntry<OnyxTypes.Report>) {
const taskReportID = taskReport?.reportID ?? '-1';
function completeTask(taskReport: OnyxEntry<OnyxTypes.Report>, reportIDFromAction?: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function completeTask(taskReport: OnyxEntry<OnyxTypes.Report>, reportIDFromAction?: string) {
function completeTask(taskReport: OnyxEntry<OnyxTypes.Report>) {

Do we have a use case that requires using this parameter? If the change comes from a previous update, I think we can consider remove this prop (the same with the reopenTask function)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the Concierge chat is opened for the first time, we don't have access to the task report details. Therefore, we need to retrieve the assignee and author IDs from the report action and pass them to canActionTask and canModifyTask. This ensures that the user can complete the task from the task preview.

same, we need to pass the taskID to complete/incomplete task.

const taskReportID = taskReport?.reportID ?? reportIDFromAction;

if (!taskReportID) {
return;
}

const message = `marked as complete`;
const completedTaskReportAction = ReportUtils.buildOptimisticTaskReportAction(taskReportID, CONST.REPORT.ACTIONS.TYPE.TASK_COMPLETED, message);
const parentReport = getParentReport(taskReport);
Expand Down Expand Up @@ -450,8 +455,11 @@ function completeTask(taskReport: OnyxEntry<OnyxTypes.Report>) {
/**
* Reopen a closed task
*/
function reopenTask(taskReport: OnyxEntry<OnyxTypes.Report>) {
const taskReportID = taskReport?.reportID ?? '-1';
function reopenTask(taskReport: OnyxEntry<OnyxTypes.Report>, reportIDFromAction?: string) {
const taskReportID = taskReport?.reportID ?? reportIDFromAction;
if (!taskReportID) {
return;
}
const message = `marked as incomplete`;
const reopenedTaskReportAction = ReportUtils.buildOptimisticTaskReportAction(taskReportID, CONST.REPORT.ACTIONS.TYPE.TASK_REOPENED, message);
const parentReport = getParentReport(taskReport);
Expand Down Expand Up @@ -608,7 +616,7 @@ function editTask(report: OnyxTypes.Report, {title, description}: OnyxTypes.Task

function editTaskAssignee(report: OnyxTypes.Report, sessionAccountID: number, assigneeEmail: string, assigneeAccountID: number | null = 0, assigneeChatReport?: OnyxEntry<OnyxTypes.Report>) {
// Create the EditedReportAction on the task
const editTaskReportAction = ReportUtils.buildOptimisticChangedTaskAssigneeReportAction(assigneeAccountID ?? 0);
const editTaskReportAction = ReportUtils.buildOptimisticChangedTaskAssigneeReportAction(assigneeAccountID ?? CONST.DEFAULT_NUMBER_ID);
const reportName = report.reportName?.trim();

let assigneeChatReportOnyxData;
Expand Down Expand Up @@ -1214,9 +1222,14 @@ 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, except auditor
* Check if you're allowed to modify the task - only the author can modify the task
*/
function canModifyTask(taskReport: OnyxEntry<OnyxTypes.Report>, sessionAccountID: number): boolean {
function canModifyTask(taskReport: OnyxEntry<OnyxTypes.Report>, sessionAccountID: number, taskOwnerAccountID?: number): boolean {
const ownerAccountID = getTaskOwnerAccountID(taskReport) ?? taskOwnerAccountID;
if (ownerAccountID !== sessionAccountID) {
return false;
}

if (ReportUtils.isCanceledTaskReport(taskReport)) {
return false;
}
Expand All @@ -1227,22 +1240,30 @@ function canModifyTask(taskReport: OnyxEntry<OnyxTypes.Report>, sessionAccountID
return false;
}

if (sessionAccountID === getTaskOwnerAccountID(taskReport) || sessionAccountID === getTaskAssigneeAccountID(taskReport)) {
if (sessionAccountID === ownerAccountID) {
return true;
}

if (!ReportUtils.canWriteInReport(taskReport) || ReportUtils.isAuditor(taskReport)) {
return false;
}

return !isEmptyObject(taskReport) && ReportUtils.isAllowedToComment(taskReport);
return false;
}

/**
* Check if you can change the status of the task (mark complete or incomplete). Only the task owner and task assignee can do this.
*/
function canActionTask(taskReport: OnyxEntry<OnyxTypes.Report>, sessionAccountID: number): boolean {
return sessionAccountID === getTaskOwnerAccountID(taskReport) || sessionAccountID === getTaskAssigneeAccountID(taskReport);
function canActionTask(taskReport: OnyxEntry<OnyxTypes.Report>, sessionAccountID: number, taskOwnerAccountID?: number, taskAssigneeAccountID?: number): boolean {
if (ReportUtils.isCanceledTaskReport(taskReport)) {
return false;
}

const parentReport = getParentReport(taskReport);
const reportNameValuePairs = ReportUtils.getReportNameValuePairs(parentReport?.reportID);
if (ReportUtils.isArchivedRoom(parentReport, reportNameValuePairs)) {
return false;
}

const ownerAccountID = getTaskOwnerAccountID(taskReport) ?? taskOwnerAccountID;
const assigneeAccountID = getTaskAssigneeAccountID(taskReport) ?? taskAssigneeAccountID;
return sessionAccountID === ownerAccountID || sessionAccountID === assigneeAccountID;
}

function clearTaskErrors(reportID: string) {
Expand Down Expand Up @@ -1286,9 +1307,9 @@ export {
getTaskAssigneeAccountID,
clearTaskErrors,
canModifyTask,
canActionTask,
setNewOptimisticAssignee,
getNavigationUrlOnTaskDelete,
canActionTask,
};

export type {PolicyValue, Assignee, ShareDestination};
Loading