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

Fix Task shared in admin-only room can be marked done in chat, but not in the task report #28268

Merged
merged 5 commits into from
Oct 2, 2023
Merged

Fix Task shared in admin-only room can be marked done in chat, but not in the task report #28268

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4a17c1b
Centralize disable updating task status checkboxes
hoangzinh Sep 26, 2023
d2b8d08
Add default props of current user personal details
hoangzinh Sep 28, 2023
15a30eb
Use flag canModifyTask for more readable
hoangzinh Sep 28, 2023
46d88bb
Merge branch 'main' into df/27402
hoangzinh Oct 1, 2023
839e52b
Empty-Commit
hoangzinh Oct 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
6 changes: 5 additions & 1 deletion src/components/ReportActionItem/TaskPreview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import compose from '../../libs/compose';
import styles from '../../styles/styles';
import ONYXKEYS from '../../ONYXKEYS';
import withLocalize, {withLocalizePropTypes} from '../withLocalize';
import withCurrentUserPersonalDetails, {withCurrentUserPersonalDetailsPropTypes} from '../withCurrentUserPersonalDetails';
import Icon from '../Icon';
import CONST from '../../CONST';
import * as Expensicons from '../Icon/Expensicons';
Expand Down Expand Up @@ -51,6 +52,8 @@ const propTypes = {
}),

...withLocalizePropTypes,

...withCurrentUserPersonalDetailsPropTypes,
Copy link
Contributor

Choose a reason for hiding this comment

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

We also need to use withCurrentUserPersonalDetailsDefaultProps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in commit d2b8d08

};

const defaultProps = {
Expand Down Expand Up @@ -91,7 +94,7 @@ function TaskPreview(props) {
style={[styles.mr2]}
containerStyle={[styles.taskCheckbox]}
isChecked={isTaskCompleted}
disabled={ReportUtils.isCanceledTaskReport(props.taskReport)}
disabled={!Task.canModifyTask(props.taskReport, props.currentUserPersonalDetails.accountID)}
onPress={Session.checkIfActionIsAllowed(() => {
if (isTaskCompleted) {
Task.reopenTask(props.taskReport, taskTitle);
Expand All @@ -118,6 +121,7 @@ TaskPreview.displayName = 'TaskPreview';

export default compose(
withLocalize,
withCurrentUserPersonalDetails,
withOnyx({
taskReport: {
key: ({taskReportID}) => `${ONYXKEYS.COLLECTION.REPORT}${taskReportID}`,
Expand Down
5 changes: 2 additions & 3 deletions src/components/ReportActionItem/TaskView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,8 @@ function TaskView(props) {
const taskTitle = convertToLTR(props.report.reportName || '');
const isCompleted = ReportUtils.isCompletedTaskReport(props.report);
const isOpen = ReportUtils.isOpenTaskReport(props.report);
const isCanceled = ReportUtils.isCanceledTaskReport(props.report);
const canModifyTask = Task.canModifyTask(props.report, props.currentUserPersonalDetails.accountID);
const disableState = !canModifyTask || isCanceled;
const disableState = !canModifyTask;
const isDisableInteractive = !canModifyTask || !isOpen;
return (
<View>
Expand Down Expand Up @@ -102,7 +101,7 @@ function TaskView(props) {
containerBorderRadius={8}
caretSize={16}
accessibilityLabel={taskTitle || props.translate('task.task')}
disabled={isCanceled || !canModifyTask}
disabled={disableState}
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 it's more readable directly using Task.canModifyTask(props.report, props.currentUserPersonalDetails.accountID). Similar to what we did for other components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in commit 15a30eb. I think we should use the existing flag canModifyTask instead of recalling the function again.

/>
<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 @@ -34,7 +34,7 @@ function TaskHeaderActionButton(props) {
<View style={[styles.flexRow, styles.alignItemsCenter, styles.justifyContentEnd]}>
<Button
success
isDisabled={ReportUtils.isCanceledTaskReport(props.report) || !Task.canModifyTask(props.report, props.session.accountID)}
isDisabled={!Task.canModifyTask(props.report, props.session.accountID)}
medium
text={props.translate(ReportUtils.isCompletedTaskReport(props.report) ? 'task.markAsIncomplete' : 'task.markAsDone')}
onPress={() =>
Expand Down
4 changes: 4 additions & 0 deletions src/libs/actions/Task.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,10 @@ function getTaskOwnerAccountID(taskReport) {
* @returns {Boolean}
*/
function canModifyTask(taskReport, sessionAccountID) {
if (ReportUtils.isCanceledTaskReport(taskReport)) {
return false;
}

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