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

hide expensify from new chat page #50937

Merged
merged 7 commits into from
Nov 6, 2024
Merged

hide expensify from new chat page #50937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5866325
hide expensify from new chat page
Nodebrute Oct 16, 2024
039f0d6
fix type error
Nodebrute Oct 16, 2024
0fd9a41
fix lint error
Nodebrute Oct 16, 2024
0c50840
Merge branch 'Expensify:main' into hideexpensify
Nodebrute Oct 23, 2024
1a6950b
remove is group chat
Nodebrute Oct 23, 2024
694ecd7
Merge branch 'Expensify:main' into hideexpensify
Nodebrute Oct 29, 2024
6916c06
remove unused var
Nodebrute Oct 29, 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
4 changes: 0 additions & 4 deletions src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2910,10 +2910,6 @@ const CONST = {
get RESTRICTED_ACCOUNT_IDS() {
return [this.ACCOUNT_ID.NOTIFICATIONS];
},
// Account IDs that can't be added as a group member
get NON_ADDABLE_ACCOUNT_IDS() {
return [this.ACCOUNT_ID.NOTIFICATIONS, this.ACCOUNT_ID.CHRONOS];
},

// Auth limit is 60k for the column but we store edits and other metadata along the html so let's use a lower limit to accommodate for it.
MAX_COMMENT_LENGTH: 10000,
Expand Down
2 changes: 1 addition & 1 deletion src/libs/OptionsListUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1893,7 +1893,7 @@ function getOptions(
allPersonalDetailsOptions = lodashOrderBy(allPersonalDetailsOptions, [(personalDetail) => personalDetail.text?.toLowerCase()], 'asc');
}

const optionsToExclude: Option[] = [];
const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}];

// If we're including selected options from the search results, we only want to exclude them if the search input is empty
// This is because on certain pages, we show the selected options at the top when the search input is empty
Expand Down
22 changes: 7 additions & 15 deletions src/pages/NewChatPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,9 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {SelectedParticipant} from '@src/types/onyx/NewGroupChatDraft';

type NewChatPageProps = {
isGroupChat?: boolean;
};
const excludedGroupEmails: string[] = CONST.EXPENSIFY_EMAILS.filter((value) => value !== CONST.EMAIL.CONCIERGE);

const excludedGroupEmails = CONST.EXPENSIFY_EMAILS.filter((value) => value !== CONST.EMAIL.CONCIERGE);

function useOptions({isGroupChat}: NewChatPageProps) {
function useOptions() {
const [searchTerm, debouncedSearchTerm, setSearchTerm] = useDebouncedState('');
const [selectedOptions, setSelectedOptions] = useState<Array<ListItem & OptionData>>([]);
const [betas] = useOnyx(ONYXKEYS.BETAS);
Expand All @@ -57,22 +53,20 @@ function useOptions({isGroupChat}: NewChatPageProps) {
personalDetails: listOptions.personalDetails ?? [],
betas: betas ?? [],
selectedOptions,
excludeLogins: isGroupChat ? excludedGroupEmails : [],
maxRecentReportsToShow: 0,
includeSelfDM: true,
});
return filteredOptions;
}, [betas, isGroupChat, listOptions.personalDetails, listOptions.reports, selectedOptions]);
}, [betas, listOptions.personalDetails, listOptions.reports, selectedOptions]);

const options = useMemo(() => {
const filteredOptions = OptionsListUtils.filterOptions(defaultOptions, debouncedSearchTerm, {
selectedOptions,
excludeLogins: isGroupChat ? excludedGroupEmails : [],
maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW,
});

return filteredOptions;
}, [debouncedSearchTerm, defaultOptions, isGroupChat, selectedOptions]);
}, [debouncedSearchTerm, defaultOptions, selectedOptions]);
const cleanSearchTerm = useMemo(() => debouncedSearchTerm.trim().toLowerCase(), [debouncedSearchTerm]);
const headerMessage = useMemo(() => {
return OptionsListUtils.getHeaderMessage(
Expand Down Expand Up @@ -129,7 +123,7 @@ function useOptions({isGroupChat}: NewChatPageProps) {
};
}

function NewChatPage({isGroupChat}: NewChatPageProps) {
function NewChatPage() {
const {translate} = useLocalize();
const {isOffline} = useNetwork();
// We need to use isSmallScreenWidth instead of shouldUseNarrowLayout to show offline indicator on small screen only
Expand All @@ -142,9 +136,7 @@ function NewChatPage({isGroupChat}: NewChatPageProps) {
const selectionListRef = useRef<SelectionListHandle>(null);

const {headerMessage, searchTerm, debouncedSearchTerm, setSearchTerm, selectedOptions, setSelectedOptions, recentReports, personalDetails, userToInvite, areOptionsInitialized} =
useOptions({
isGroupChat,
});
useOptions();

const [sections, firstKeyForList] = useMemo(() => {
const sectionsList: OptionsListUtils.CategorySection[] = [];
Expand Down Expand Up @@ -217,7 +209,7 @@ function NewChatPage({isGroupChat}: NewChatPageProps) {

const itemRightSideComponent = useCallback(
(item: ListItem & OptionsListUtils.Option, isFocused?: boolean) => {
if (!!item.isSelfDM || (item.accountID && CONST.NON_ADDABLE_ACCOUNT_IDS.includes(item.accountID))) {
if (!!item.isSelfDM || (item.login && excludedGroupEmails.includes(item.login))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add more context about using login instead of accountID? Is there any comparison between them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the excludedGroupEmails contains the logins of all Expensify accounts. That's why I'm using login here. Previously, we were using NON_ADDABLE_ACCOUNT_IDS, which had the account IDs for CHRONOS and NOTIFICATIONS. However, excludedGroupEmails also includes the logins for these two accounts.

Copy link
Contributor

Choose a reason for hiding this comment

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

More context about using login instead of accountID is accountID can be optimisticAccountID in userToInvite util API respond with the full user data

App/src/libs/OptionsListUtils.ts

Lines 1661 to 1664 in 652d2ff

[optimisticAccountID]: {
accountID: optimisticAccountID,
login: searchValue,
},

And this can cause this issue

  1. Login with new account (to confirm there is no local data)
  2. Click fab > start chat
  3. Search for chronos@expensify.com for the first time

You will see add to group is displayed temporary, and you can add it to group util API respond with the full user data.

issue.mp4

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 we need to remove NON_ADDABLE_ACCOUNT_IDS because it is not used in anywhere now

return null;
}
/**
Expand Down
Loading