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

Jump to bottom

Update Task Permissions #23971

Merged
merged 4 commits into from
Aug 2, 2023
Merged

Update Task Permissions #23971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
07cfe14
update canModifyTask
thienlnam Aug 1, 2023
d9af096
Update all the places that use it
thienlnam Aug 1, 2023
8fddbd7
remove policy admin check
thienlnam Aug 1, 2023
48d0d39
Merge branch 'main' into jack-removePermissionBlocks
thienlnam Aug 1, 2023
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
8 changes: 3 additions & 5 deletions src/components/ReportActionItem/TaskView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import * as ReportUtils from '../../libs/ReportUtils';
import * as OptionsListUtils from '../../libs/OptionsListUtils';
import * as StyleUtils from '../../styles/StyleUtils';
import * as Task from '../../libs/actions/Task';
import * as PolicyUtils from '../../libs/PolicyUtils';
import CONST from '../../CONST';
import Checkbox from '../Checkbox';
import convertToLTR from '../../libs/convertToLTR';
Expand Down Expand Up @@ -48,9 +47,8 @@ function TaskView(props) {
const isCompleted = ReportUtils.isCompletedTaskReport(props.report);
const isOpen = ReportUtils.isOpenTaskReport(props.report);
const isCanceled = ReportUtils.isCanceledTaskReport(props.report);
const policy = ReportUtils.getPolicy(props.report.policyID);
const canEdit = PolicyUtils.isPolicyAdmin(policy) || Task.isTaskAssigneeOrTaskOwner(props.report, props.currentUserPersonalDetails.accountID);
const disableState = !canEdit || !isOpen;
const canModifyTask = Task.canModifyTask(props.report, props.currentUserPersonalDetails.accountID);
const disableState = !canModifyTask || !isOpen;
return (
<View>
<Hoverable>
Expand Down Expand Up @@ -80,7 +78,7 @@ function TaskView(props) {
containerBorderRadius={8}
caretSize={16}
accessibilityLabel={taskTitle || props.translate('task.task')}
disabled={isCanceled || !canEdit}
disabled={isCanceled || !canModifyTask}
/>
<View style={[styles.flexRow, styles.flex1]}>
<Text
Expand Down
2 changes: 1 addition & 1 deletion src/components/TaskHeaderActionButton.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function TaskHeaderActionButton(props) {
<View style={[styles.flexRow, styles.alignItemsCenter, styles.justifyContentEnd]}>
<Button
success
isDisabled={ReportUtils.isCanceledTaskReport(props.report) || !Task.isTaskAssigneeOrTaskOwner(props.report, props.session.accountID)}
isDisabled={ReportUtils.isCanceledTaskReport(props.report) || !Task.canModifyTask(props.report, props.session.accountID)}
medium
text={props.translate(ReportUtils.isCompletedTaskReport(props.report) ? 'task.markAsIncomplete' : 'task.markAsDone')}
onPress={() =>
Expand Down
17 changes: 12 additions & 5 deletions src/libs/actions/Task.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,14 +719,21 @@ function getTaskOwnerAccountID(taskReport) {
}

/**
* Check if current user is either task assignee or task owner
*
* Check if you're allowed to modify the task - anyone that has write access to the report can modify the task
* @param {Object} taskReport
* @param {Number} sessionAccountID
* @returns {Boolean}
*/
function isTaskAssigneeOrTaskOwner(taskReport, sessionAccountID) {
return sessionAccountID === getTaskOwnerAccountID(taskReport) || sessionAccountID === getTaskAssigneeAccountID(taskReport);
function canModifyTask(taskReport, sessionAccountID) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't call this a regression per se, but we updated this to restrict task permissions in rooms in #31863

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

// 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
const parentReport = ReportUtils.getParentReport(taskReport);
return ReportUtils.isAllowedToComment(parentReport);
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from #46782, there's a bit of a missed condition here when a user A is initially assigned to a task (so they have write access), and then the task is assigned to someone else (user A still has write access, but they shouldn't be able to complete the task).

}

export {
Expand All @@ -747,5 +754,5 @@ export {
cancelTask,
dismissModalAndClearOutTaskInfo,
getTaskAssigneeAccountID,
isTaskAssigneeOrTaskOwner,
canModifyTask,
};
8 changes: 4 additions & 4 deletions src/pages/home/HeaderView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ function HeaderView(props) {
const shouldShowCallButton = (isConcierge && guideCalendarLink) || (!isAutomatedExpensifyAccount && !isTaskReport);
const threeDotMenuItems = [];
if (isTaskReport) {
const isTaskAssigneeOrTaskOwner = Task.isTaskAssigneeOrTaskOwner(props.report, props.session.accountID);
if (ReportUtils.isOpenTaskReport(props.report) && isTaskAssigneeOrTaskOwner) {
const canModifyTask = Task.canModifyTask(props.report, props.session.accountID);
if (ReportUtils.isOpenTaskReport(props.report) && canModifyTask) {
threeDotMenuItems.push({
icon: Expensicons.Checkmark,
text: props.translate('task.markAsDone'),
Expand All @@ -110,7 +110,7 @@ function HeaderView(props) {
}

// Task is marked as completed
if (ReportUtils.isCompletedTaskReport(props.report) && isTaskAssigneeOrTaskOwner) {
if (ReportUtils.isCompletedTaskReport(props.report) && canModifyTask) {
threeDotMenuItems.push({
icon: Expensicons.Checkmark,
text: props.translate('task.markAsIncomplete'),
Expand All @@ -119,7 +119,7 @@ function HeaderView(props) {
}

// Task is not closed
if (props.report.stateNum !== CONST.REPORT.STATE_NUM.SUBMITTED && props.report.statusNum !== CONST.REPORT.STATUS.CLOSED && isTaskAssigneeOrTaskOwner) {
if (props.report.stateNum !== CONST.REPORT.STATE_NUM.SUBMITTED && props.report.statusNum !== CONST.REPORT.STATUS.CLOSED && canModifyTask) {
threeDotMenuItems.push({
icon: Expensicons.Trashcan,
text: props.translate('common.cancel'),
Expand Down
Loading