Skip to content

Commit

Permalink
Merge pull request #24019 from Thanos30/fix/task-share-destination-fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
thienlnam authored Aug 3, 2023
2 parents b08bf66 + 18daf91 commit 245b81d
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 49 deletions.
25 changes: 21 additions & 4 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,7 @@ function getOptions(
includeThreads = false,
includeTasks = false,
includeMoneyRequests = false,
excludeUnknownUsers = false,
},
) {
if (!isPersonalDetailsReady(personalDetails)) {
Expand Down Expand Up @@ -796,7 +797,8 @@ function getOptions(
((Str.isValidEmail(searchValue) && !Str.isDomainEmail(searchValue) && !Str.endsWith(searchValue, CONST.SMS.DOMAIN)) ||
(parsedPhoneNumber.possible && Str.isValidPhone(LoginUtils.getPhoneNumberWithoutSpecialChars(parsedPhoneNumber.number.input)))) &&
!_.find(loginOptionsToExclude, (loginOptionToExclude) => loginOptionToExclude.login === addSMSDomainIfPhoneNumber(searchValue).toLowerCase()) &&
(searchValue !== CONST.EMAIL.CHRONOS || Permissions.canUseChronos(betas))
(searchValue !== CONST.EMAIL.CHRONOS || Permissions.canUseChronos(betas)) &&
!excludeUnknownUsers
) {
// Generates an optimistic account ID for new users not yet saved in Onyx
const optimisticAccountID = UserUtils.generateAccountID(searchValue);
Expand Down Expand Up @@ -966,17 +968,32 @@ function getNewChatOptions(reports, personalDetails, betas = [], searchValue = '
*
*/

function getShareDestinationOptions(reports, personalDetails, betas = [], searchValue = '', selectedOptions = [], excludeLogins = [], includeOwnedWorkspaceChats = true) {
function getShareDestinationOptions(
reports,
personalDetails,
betas = [],
searchValue = '',
selectedOptions = [],
excludeLogins = [],
includeOwnedWorkspaceChats = true,
excludeUnknownUsers = true,
) {
return getOptions(reports, personalDetails, {
betas,
searchInputValue: searchValue.trim(),
selectedOptions,
maxRecentReportsToShow: 5,
maxRecentReportsToShow: 0, // Unlimited
includeRecentReports: true,
includeMultipleParticipantReports: true,
includePersonalDetails: true,
includePersonalDetails: false,
showChatPreviewLine: true,
forcePolicyNamePreview: true,
includeThreads: true,
includeMoneyRequests: true,
includeTasks: true,
excludeLogins,
includeOwnedWorkspaceChats,
excludeUnknownUsers,
});
}

Expand Down
11 changes: 11 additions & 0 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,16 @@ function isConciergeChatReport(report) {
return lodashGet(report, 'participantAccountIDs', []).length === 1 && Number(report.participantAccountIDs[0]) === CONST.ACCOUNT_ID.CONCIERGE && !isChatThread(report);
}

/**
* Returns true if this report has only one participant and it's an Expensify account.
* @param {Object} report
* @returns {Boolean}
*/
function isExpensifyOnlyParticipantInReport(report) {
const reportParticipants = _.without(lodashGet(report, 'participantAccountIDs', []), currentUserAccountID);
return lodashGet(report, 'participantAccountIDs', []).length === 1 && _.some(reportParticipants, (accountID) => _.contains(CONST.EXPENSIFY_ACCOUNT_IDS, accountID));
}

/**
* Returns true if there are any Expensify accounts (i.e. with domain 'expensify.com') in the set of accountIDs
* by cross-referencing the accountIDs with personalDetails.
Expand Down Expand Up @@ -2862,6 +2872,7 @@ export {
getPolicyName,
getPolicyType,
isArchivedRoom,
isExpensifyOnlyParticipantInReport,
isPolicyExpenseChatAdmin,
isPolicyAdmin,
isPublicRoom,
Expand Down
9 changes: 3 additions & 6 deletions src/pages/home/report/ReportActionCompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -450,12 +450,9 @@ class ReportActionCompose extends React.Component {
* @param {Array} reportParticipants
* @returns {Boolean}
*/
getTaskOption(reportParticipants) {
getTaskOption() {
// We only prevent the task option from showing if it's a DM and the other user is an Expensify default email
if (
!Permissions.canUseTasks(this.props.betas) ||
(lodashGet(this.props.report, 'participantAccountIDs', []).length === 1 && _.some(reportParticipants, (accountID) => _.contains(CONST.EXPENSIFY_ACCOUNT_IDS, accountID)))
) {
if (!Permissions.canUseTasks(this.props.betas) || ReportUtils.isExpensifyOnlyParticipantInReport(this.props.report)) {
return [];
}

Expand Down Expand Up @@ -1116,7 +1113,7 @@ class ReportActionCompose extends React.Component {
anchorAlignment={{horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT, vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM}}
menuItems={[
...this.getMoneyRequestOptions(reportParticipants),
...this.getTaskOption(reportParticipants),
...this.getTaskOption(),
{
icon: Expensicons.Paperclip,
text: this.props.translate('reportActionCompose.addAttachment'),
Expand Down
43 changes: 9 additions & 34 deletions src/pages/tasks/TaskShareDestinationSelectorModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,35 +45,27 @@ function TaskShareDestinationSelectorModal(props) {
const [searchValue, setSearchValue] = useState('');
const [headerMessage, setHeaderMessage] = useState('');
const [filteredRecentReports, setFilteredRecentReports] = useState([]);
const [filteredPersonalDetails, setFilteredPersonalDetails] = useState([]);
const [filteredUserToInvite, setFilteredUserToInvite] = useState(null);

const filteredReports = useMemo(() => {
const reports = {};
_.keys(props.reports).forEach((reportKey) => {
if (!ReportUtils.isAllowedToComment(props.reports[reportKey])) {
if (
!ReportUtils.isAllowedToComment(props.reports[reportKey]) ||
ReportUtils.isArchivedRoom(props.reports[reportKey]) ||
ReportUtils.isExpensifyOnlyParticipantInReport(props.reports[reportKey])
) {
return;
}
reports[reportKey] = props.reports[reportKey];
});
return reports;
}, [props.reports]);
const updateOptions = useCallback(() => {
const {recentReports, personalDetails, userToInvite} = OptionsListUtils.getShareDestinationOptions(
filteredReports,
props.personalDetails,
props.betas,
searchValue.trim(),
[],
CONST.EXPENSIFY_EMAILS,
true,
);

setHeaderMessage(OptionsListUtils.getHeaderMessage(recentReports?.length + personalDetails?.length !== 0, Boolean(userToInvite), searchValue));

setFilteredUserToInvite(userToInvite);
const {recentReports} = OptionsListUtils.getShareDestinationOptions(filteredReports, props.personalDetails, props.betas, searchValue.trim(), [], CONST.EXPENSIFY_EMAILS, true);

setHeaderMessage(OptionsListUtils.getHeaderMessage(recentReports?.length !== 0, false, searchValue));

setFilteredRecentReports(recentReports);
setFilteredPersonalDetails(personalDetails);
}, [props, searchValue, filteredReports]);

useEffect(() => {
Expand Down Expand Up @@ -101,23 +93,6 @@ function TaskShareDestinationSelectorModal(props) {
indexOffset += filteredRecentReports?.length;
}

if (filteredPersonalDetails?.length > 0) {
sections.push({
data: filteredPersonalDetails,
shouldShow: true,
indexOffset,
});
indexOffset += filteredRecentReports?.length;
}

if (filteredUserToInvite) {
sections.push({
data: [filteredUserToInvite],
shouldShow: true,
indexOffset,
});
}

return sections;
};

Expand Down
9 changes: 4 additions & 5 deletions tests/unit/OptionsListUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -570,9 +570,8 @@ describe('OptionsListUtils', () => {
// When we pass an empty search value
let results = OptionsListUtils.getShareDestinationOptions(REPORTS, PERSONAL_DETAILS, [], '');

// Then we should expect 5 recent reports to show because we're grabbing DM chats and group chats
// because we've limited the number of recent reports to 5
expect(results.recentReports.length).toBe(5);
// Then we should expect all the recent reports to show but exclude the archived rooms
expect(results.recentReports.length).toBe(_.size(REPORTS) - 1);

// When we pass a search value that doesn't match the group chat name
results = OptionsListUtils.getShareDestinationOptions(REPORTS, PERSONAL_DETAILS, [], 'mutants');
Expand All @@ -590,8 +589,8 @@ describe('OptionsListUtils', () => {
results = OptionsListUtils.getShareDestinationOptions(REPORTS_WITH_WORKSPACE_ROOMS, PERSONAL_DETAILS, [], '');

// Then we should expect the DMS, the group chats and the workspace room to show
// We should expect 5 recent reports to show because we've limited the number of recent reports to 5
expect(results.recentReports.length).toBe(5);
// We should expect all the recent reports to show, excluding the archived rooms
expect(results.recentReports.length).toBe(_.size(REPORTS_WITH_WORKSPACE_ROOMS) - 1);

// When we search for a workspace room
results = OptionsListUtils.getShareDestinationOptions(REPORTS_WITH_WORKSPACE_ROOMS, PERSONAL_DETAILS, [], 'Avengers Room');
Expand Down

0 comments on commit 245b81d

Please sign in to comment.