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

Task Share Destination Fixes #24019

Merged
merged 17 commits into from
Aug 3, 2023
Merged

Task Share Destination Fixes #24019

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
a262192
Task Share Destination Fixes
Thanos30 Aug 1, 2023
8c98c9a
Apply options discussed in issue
Thanos30 Aug 1, 2023
b670ed5
remove not needed parameter
Thanos30 Aug 1, 2023
c3591e5
Update test to match new functionality
Thanos30 Aug 1, 2023
603b305
prettify code and correct test to exclude archived rooms
Thanos30 Aug 1, 2023
3dd5301
add fic on workspace reports
Thanos30 Aug 1, 2023
157f56d
make name more obvious
Thanos30 Aug 2, 2023
b778073
Merge remote-tracking branch 'upstream/main' into fix/task-share-dest…
Thanos30 Aug 2, 2023
e938e74
exclude Archived Rooms
Thanos30 Aug 3, 2023
1308dfd
exclude expensify accounts
Thanos30 Aug 3, 2023
a21355e
Use common function to determine if we can share a task in a report
Thanos30 Aug 3, 2023
c703f7d
prettify
Thanos30 Aug 3, 2023
6bac6dc
fix name accuracy
Thanos30 Aug 3, 2023
de39deb
remove unneeded parameter
Thanos30 Aug 3, 2023
b9cc192
update function's docs and name
Thanos30 Aug 3, 2023
ce779cc
remove unneeded space
Thanos30 Aug 3, 2023
18daf91
remove space
Thanos30 Aug 3, 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
25 changes: 21 additions & 4 deletions src/libs/OptionsListUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,7 @@ function getOptions(
includeThreads = false,
includeTasks = false,
includeMoneyRequests = false,
disallowNewChats = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use exclude word to be consistent with others.
I suggest excludeUnknownUsers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, changed it to your suggestion. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the quick update. Also please pull main

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @situchan , done!

I pulled main before I made my PR, seems like a lot of stuff was merged afterwards

},
) {
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)) &&
!disallowNewChats
) {
// 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,
disallowNewChats = 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,
disallowNewChats,
});
}

Expand Down
37 changes: 4 additions & 33 deletions src/pages/tasks/TaskShareDestinationSelectorModal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ 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 = {};
Expand All @@ -59,21 +57,11 @@ function TaskShareDestinationSelectorModal(props) {
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 +89,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
View file
Open in desktop
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