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

Show green dot for uncompleted tasks assigned to current user #20808

Merged
merged 13 commits into from
Jun 15, 2023
Merged

Show green dot for uncompleted tasks assigned to current user #20808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f12b390
Show green dot for uncompleted tasks assigned to current user
youssef-lr Jun 15, 2023
4f22290
Apply suggestions from code review
youssef-lr Jun 15, 2023
0421d53
Add JSDocs
youssef-lr Jun 15, 2023
d09672d
Merge branch 'main' into youssef_greenDot_uncompleted_tasks
youssef-lr Jun 15, 2023
44db560
Fix bug when assigning tasks
youssef-lr Jun 15, 2023
7cef0a7
Prettier
youssef-lr Jun 15, 2023
90f06a0
Use the new isTaskCompleted
youssef-lr Jun 15, 2023
76de934
Fix assignee not displayed in task header
youssef-lr Jun 15, 2023
ddb17f8
Cleanup
youssef-lr Jun 15, 2023
31594b7
Make check more reliable
youssef-lr Jun 15, 2023
5597e7f
Fix bug when trying to edit a task and assign it to self
youssef-lr Jun 15, 2023
ccd79e8
Make check more reliable
youssef-lr Jun 15, 2023
286c623
Fix condition
youssef-lr Jun 15, 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/LHNOptionsList/OptionRowLHN.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,11 @@ function OptionRowLHN(props) {
const focusedBackgroundColor = styles.sidebarLinkActive.backgroundColor;

const hasBrickError = optionItem.brickRoadIndicator === CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR;
const shouldShowGreenDotIndicator = !hasBrickError && (optionItem.isUnreadWithMention || (optionItem.hasOutstandingIOU && !optionItem.isIOUReportOwner));
const shouldShowGreenDotIndicator =
!hasBrickError &&
(optionItem.isUnreadWithMention ||
(optionItem.hasOutstandingIOU && !optionItem.isIOUReportOwner) ||
(optionItem.isTaskReport && optionItem.isTaskAssignee && !optionItem.isTaskCompleted));
Comment on lines +87 to +90
Copy link
Contributor

Choose a reason for hiding this comment

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

We forgot to take into account the task cancelled state. This later caused #27805


/**
* Show the ReportActionContextMenu modal popover.
Expand Down
5 changes: 2 additions & 3 deletions src/components/TaskHeader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React, {useEffect} from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import lodashGet from 'lodash/get';
import _ from 'underscore';
import reportPropTypes from '../pages/reportPropTypes';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import * as ReportUtils from '../libs/ReportUtils';
Expand Down Expand Up @@ -39,7 +38,7 @@ function TaskHeader(props) {
const assigneeName = ReportUtils.getDisplayNameForParticipant(props.report.managerID);
const assigneeAvatar = UserUtils.getAvatar(lodashGet(props.personalDetails, [props.report.managerID, 'avatar']), props.report.managerID);
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;
const isCompleted = ReportUtils.isTaskCompleted(props.report);

useEffect(() => {
TaskUtils.setTaskReport(props.report);
Expand All @@ -60,7 +59,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) && (
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
{props.report.managerID && props.report.managerID > 0 && (
<>
<Avatar
source={assigneeAvatar}
Expand Down
22 changes: 22 additions & 0 deletions src/libs/ReportUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,26 @@ function isTaskReport(report) {
return lodashGet(report, 'type') === CONST.REPORT.TYPE.TASK;
}

/**
* Checks if a task is completed
*
* @param {Object} report
* @returns {Boolean}
*/
function isTaskCompleted(report) {
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
return lodashGet(report, 'stateNum') === CONST.REPORT.STATE_NUM.SUBMITTED && lodashGet(report, 'statusNum') === CONST.REPORT.STATUS.APPROVED;
}

/**
* Checks if the current user is assigned to the task report
*
* @param {Object} report
* @returns {Boolean}
*/
function isTaskAssignee(report) {
return lodashGet(report, 'managerEmail') === currentUserEmail;
}

/**
* Checks if a report is an IOU or expense report.
*
Expand Down Expand Up @@ -2296,6 +2316,8 @@ export {
isExpenseReport,
isIOUReport,
isTaskReport,
isTaskCompleted,
isTaskAssignee,
isMoneyRequestReport,
chatIncludesChronos,
getNewMarkerReportActionID,
Expand Down
6 changes: 5 additions & 1 deletion src/libs/SidebarUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ function getOrderedReportIDs(reportIDFromRoute) {
return;
}

if (ReportUtils.isTaskReport(report) && report.stateNum !== CONST.REPORT.STATE.OPEN && report.statusNum !== CONST.REPORT.STATUS.OPEN) {
if (ReportUtils.isTaskReport(report) && ReportUtils.isTaskCompleted(report)) {
archivedReports.push(report);
return;
}
Expand Down Expand Up @@ -266,6 +266,10 @@ function getOptionData(reportID) {
result.isThread = ReportUtils.isThread(report);
result.isChatRoom = ReportUtils.isChatRoom(report);
result.isTaskReport = ReportUtils.isTaskReport(report);
if (result.isTaskReport) {
result.isTaskCompleted = ReportUtils.isTaskCompleted(report);
result.isTaskAssignee = ReportUtils.isTaskAssignee(report);
Comment on lines +270 to +271
Copy link
Contributor

Choose a reason for hiding this comment

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

do you think we should add defaults values for these above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We only grab these if the report is a task, so I think they'll either be true or false.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi ✋ Coming from #22149

We are using the managerEmail to check the task assignee and show the green button if it's isTaskAssignee. Still, in the sidebar links selector, we didn't specify the property managerEmail to the Onyx selector function, making the sidebar not re-rendering on the managerEmail change.

}
result.isArchivedRoom = ReportUtils.isArchivedRoom(report);
result.isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report);
result.isMoneyRequestReport = ReportUtils.isMoneyRequestReport(report);
Expand Down
5 changes: 3 additions & 2 deletions src/libs/actions/Task.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,9 @@ function editTaskAndNavigate(report, ownerEmail, ownerAccountID, {title, descrip
// If we make a change to the assignee, we want to add a comment to the assignee's chat
let optimisticAssigneeAddComment;
let assigneeChatReportID;
if (assigneeAccountID && assigneeAccountID !== report.managerID) {
if (assigneeAccountID && assigneeAccountID !== report.managerID && assigneeAccountID !== ownerAccountID) {
assigneeChatReportID = ReportUtils.getChatByParticipants([assigneeAccountID]).reportID;

if (assigneeChatReportID !== report.parentReportID.toString()) {
optimisticAssigneeAddComment = ReportUtils.buildOptimisticTaskCommentReportAction(
report.reportID,
Expand Down Expand Up @@ -493,7 +494,7 @@ function setAssigneeValue(assignee, assigneeAccountID, shareDestination, isCurre
}

// This is only needed for creation of a new task and so it should only be stored locally
Onyx.merge(ONYXKEYS.TASK, {assignee, newAssigneeAccountID});
Onyx.merge(ONYXKEYS.TASK, {assignee, assigneeAccountID: newAssigneeAccountID});
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down