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

Show primary workspace chat first when creating expense #47437

Merged
merged 6 commits into from
Aug 15, 2024
Merged

Show primary workspace chat first when creating expense #47437

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
02ef09a
Show primary workspace chat first when creating exepnse
nkdengineer Aug 14, 2024
7f3878b
rename primaryPolicyID
nkdengineer Aug 14, 2024
caffcd1
revert change name
nkdengineer Aug 14, 2024
3cf5578
merge main
nkdengineer Aug 15, 2024
38d3d8e
rename variable
nkdengineer Aug 15, 2024
8836661
only display the active policy expense chat at the top
nkdengineer Aug 15, 2024
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
34 changes: 27 additions & 7 deletions src/libs/OptionsListUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ type PreviewConfig = {showChatPreviewLine?: boolean; forcePolicyNamePreview?: bo
type FilterOptionsConfig = Pick<
GetOptionsConfig,
'sortByReportTypeInSearch' | 'canInviteUser' | 'betas' | 'selectedOptions' | 'excludeUnknownUsers' | 'excludeLogins' | 'maxRecentReportsToShow'
> & {preferChatroomsOverThreads?: boolean; includeChatRoomsByParticipants?: boolean};
> & {preferChatroomsOverThreads?: boolean; includeChatRoomsByParticipants?: boolean; preferPolicyExpenseChat?: boolean};

type HasText = {
text?: string;
Expand Down Expand Up @@ -359,6 +359,12 @@ Onyx.connect({
callback: (value) => (allReportsDraft = value),
});

let primaryPolicyID: OnyxEntry<string>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we call it activePolicyID as thats how the nvp key is called?

This comment was marked as outdated.

Sign in to view

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mountiny I updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

NAB, but i feel primaryPolicyID is right over here, activePolicyID is the policyID we are on currently

Copy link
Contributor

Choose a reason for hiding this comment

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

@nkdengineer @allgandalf I realize this is not ideal, but we already call the variables from the NVP_ACTIVE_POLICY_ID as activePolicyID elsewhere so I think we should just keep using that instead of using this term

Onyx.connect({
key: ONYXKEYS.NVP_ACTIVE_POLICY_ID,
callback: (value) => (primaryPolicyID = value),
});

/**
* Get the report or draft report given a reportID
*/
Expand Down Expand Up @@ -1617,11 +1623,19 @@ function createOptionFromReport(report: Report, personalDetails: OnyxEntry<Perso
* @param searchValue - search string
* @returns a sorted list of options
*/
function orderOptions(options: ReportUtils.OptionData[], searchValue: string | undefined, {preferChatroomsOverThreads = false} = {}) {
function orderOptions(options: ReportUtils.OptionData[], searchValue: string | undefined, {preferChatroomsOverThreads = false, preferPolicyExpenseChat = false} = {}) {
return lodashOrderBy(
options,
[
(option) => {
if (option.isPolicyExpenseChat && preferPolicyExpenseChat && option.policyID === primaryPolicyID) {
return 0;
}

if (option.isPolicyExpenseChat && preferPolicyExpenseChat) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mountiny I have a question here: Do we also prefer re-order the other policy expense chat that isn't the active policy ID when we re-order the active policy expense chat.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I implemented this logic. If we don't want to do that, I can remove this logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mountiny What do you think about my comment above?

return 1;
}

if (option.isSelfDM) {
return 0;
}
Expand Down Expand Up @@ -2060,11 +2074,14 @@ function getOptions(
}

// If we are prioritizing 1:1 chats in search, do it only once we started searching
if (sortByReportTypeInSearch && searchValue !== '') {
if (sortByReportTypeInSearch && (searchValue !== '' || !!action)) {
// When sortByReportTypeInSearch is true, recentReports will be returned with all the reports including personalDetailsOptions in the correct Order.
recentReportOptions.push(...personalDetailsOptions);
personalDetailsOptions = [];
recentReportOptions = orderOptions(recentReportOptions, searchValue, {preferChatroomsOverThreads: true});
// If we're in money request flow, we only order the recent report option.
if (!action) {
recentReportOptions.push(...personalDetailsOptions);
personalDetailsOptions = [];
}
recentReportOptions = orderOptions(recentReportOptions, searchValue, {preferChatroomsOverThreads: true, preferPolicyExpenseChat: !!action});
}

return {
Expand Down Expand Up @@ -2173,6 +2190,7 @@ function getFilteredOptions(
recentlyUsedPolicyReportFieldOptions: string[] = [],
includeInvoiceRooms = false,
action: IOUAction | undefined = undefined,
sortByReportTypeInSearch = false,
) {
return getOptions(
{reports, personalDetails},
Expand Down Expand Up @@ -2201,6 +2219,7 @@ function getFilteredOptions(
recentlyUsedPolicyReportFieldOptions,
includeInvoiceRooms,
action,
sortByReportTypeInSearch,
},
);
}
Expand Down Expand Up @@ -2426,6 +2445,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt
excludeLogins = [],
preferChatroomsOverThreads = false,
includeChatRoomsByParticipants = false,
preferPolicyExpenseChat = false,
} = config ?? {};
if (searchInputValue.trim() === '' && maxRecentReportsToShow > 0) {
return {...options, recentReports: options.recentReports.slice(0, maxRecentReportsToShow)};
Expand Down Expand Up @@ -2539,7 +2559,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt

return {
personalDetails,
recentReports: orderOptions(recentReports, searchValue, {preferChatroomsOverThreads}),
recentReports: orderOptions(recentReports, searchValue, {preferChatroomsOverThreads, preferPolicyExpenseChat}),
userToInvite,
currentUserOption: matchResults.currentUserOption,
categoryOptions: [],
Expand Down
9 changes: 8 additions & 1 deletion src/pages/iou/request/MoneyRequestParticipantsSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ import useDismissedReferralBanners from '@hooks/useDismissedReferralBanners';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import usePermissions from '@hooks/usePermissions';
import usePolicy from '@hooks/usePolicy';
import useScreenWrapperTranstionStatus from '@hooks/useScreenWrapperTransitionStatus';
import useThemeStyles from '@hooks/useThemeStyles';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import Navigation from '@libs/Navigation/Navigation';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
import * as ReportUtils from '@libs/ReportUtils';
import * as SubscriptionUtils from '@libs/SubscriptionUtils';
import * as Policy from '@userActions/Policy/Policy';
Expand Down Expand Up @@ -63,6 +65,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
const {didScreenTransitionEnd} = useScreenWrapperTranstionStatus();
const [betas] = useOnyx(ONYXKEYS.BETAS);
const [activePolicyID] = useOnyx(ONYXKEYS.NVP_ACTIVE_POLICY_ID);
const policy = usePolicy(activePolicyID);
const [isSearchingForReports] = useOnyx(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, {initWithStoredValues: false});
const {options, areOptionsInitialized} = useOptionsList({
shouldInitialize: didScreenTransitionEnd,
Expand All @@ -71,6 +74,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
const cleanSearchTerm = useMemo(() => debouncedSearchTerm.trim().toLowerCase(), [debouncedSearchTerm]);
const offlineMessage: string = isOffline ? `${translate('common.youAppearToBeOffline')} ${translate('search.resultsAreLimited')}` : '';

const isPaidGroupPolicy = useMemo(() => PolicyUtils.isPaidGroupPolicy(policy), [policy]);
const isIOUSplit = iouType === CONST.IOU.TYPE.SPLIT;
const isCategorizeOrShareAction = [CONST.IOU.ACTION.CATEGORIZE, CONST.IOU.ACTION.SHARE].some((option) => option === action);

Expand Down Expand Up @@ -126,6 +130,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
undefined,
iouType === CONST.IOU.TYPE.INVOICE,
action,
isPaidGroupPolicy,
);

return optionList;
Expand All @@ -141,6 +146,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
options.personalDetails,
options.reports,
participants,
isPaidGroupPolicy,
]);

const chatOptions = useMemo(() => {
Expand All @@ -162,9 +168,10 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
selectedOptions: participants as Participant[],
excludeLogins: CONST.EXPENSIFY_EMAILS,
maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW,
preferPolicyExpenseChat: isPaidGroupPolicy,
});
return newOptions;
}, [areOptionsInitialized, betas, defaultOptions, debouncedSearchTerm, participants]);
}, [areOptionsInitialized, betas, defaultOptions, debouncedSearchTerm, participants, isPaidGroupPolicy]);

/**
* Returns the sections needed for the OptionsSelector
Expand Down
Loading