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.

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

"Expensify chat" is shown in contact list for "start chat" #44750

Closed
1 of 6 tasks
m-natarajan opened this issue Jul 2, 2024 · 7 comments
Closed
1 of 6 tasks

"Expensify chat" is shown in contact list for "start chat" #44750

m-natarajan opened this issue Jul 2, 2024 · 7 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@m-natarajan
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.3-6
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @shawnborton
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1719906408139169

Action Performed:

Scenario1:

  1. Go to staging.new.expensify.com and login
  2. Click FAB > Start chat
  3. Observe the contact list
    Scenario 2:
    Create a group with "concierge" and "Expensify"

Expected Result:

  • "Expensify" should not display in the contact list as can't initiate a chat
  • Shouldn't be able to create a group for the above reason

Actual Result:

  • "Expensify" appears in the contact list
  • In gmail domain able to initiate the chat
  • Able to create a group

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

CleanShot 2024-07-02 at 09 48 46@2x
CleanShot 2024-07-02 at 09 47 52@2x
CleanShot 2024-07-02 at 09 47 02@2x
image (40)
Snip - (3) New Expensify - Google Chrome (3)
Snip - (3) New Expensify - Google Chrome (2)

View all open jobs on GitHub

@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 2, 2024
Copy link

melvin-bot bot commented Jul 2, 2024

Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@Krishna2323
Copy link
Contributor

Krishna2323 commented Jul 2, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

"Expensify chat" is shown in contact list for "start chat"

What is the root cause of that problem?

The excludedGroupEmails are only passed when isGroupChat is true and we don't pass isGroupChat as true when using useOptions.

function NewChatPage({isGroupChat}: NewChatPageProps) {
const {translate} = useLocalize();
const {isOffline} = useNetwork();
const {isSmallScreenWidth} = useWindowDimensions();
const styles = useThemeStyles();
const personalData = useCurrentUserPersonalDetails();
const {insets} = useStyledSafeAreaInsets();
const [isSearchingForReports] = useOnyx(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, {initWithStoredValues: false});
const selectionListRef = useRef<SelectionListHandle>(null);
const {headerMessage, searchTerm, debouncedSearchTerm, setSearchTerm, selectedOptions, setSelectedOptions, recentReports, personalDetails, userToInvite, areOptionsInitialized} =
useOptions({
isGroupChat,
});

isGroupChat ? excludedGroupEmails : [],

What changes do you think we should make in order to solve the problem?

Pass isGroupChat as true or make the default value as true.

useOptions({
isGroupChat,
});

What alternative solutions did you explore? (Optional)

I believe we don't need isGroupChat prop in NewChatPage component, we can remove the prop and directly pass isGroupChat as true when using useOptions.

@neonbhai
Copy link
Contributor

neonbhai commented Jul 2, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

"Expensify chat" is shown in contact list for "start chat"

What is the root cause of that problem?

We use the isGroupChat here as a prop, but we don't ever set it.

component={NewChatPage}

This needs to be variable on the page, and may have been missed over the refactors.

What changes do you think we should make in order to solve the problem?

We should have isGroupChat as a variable in NewChatPage calculated as:

const isGroupChat = selectedOptions.length > 0;

This will be used here and here to carry out the group chat logic.

This will mean that we will remove this from prop definition here and here

@bernhardoj
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Expensify notification chat is able to be added to a group.

What is the root cause of that problem?

In #41290, we allow the Expensify notification chat to be listed in the list (specifically this change).

But we forgot to cover the case where the user is able to add the Expensify notification to a group.

The isGroupChat prop is a very old prop added first in #5138. At that time, we had a new chat and a new group page separated. New group page uses new chat page and pass down the isGroupChat.

What changes do you think we should make in order to solve the problem?

To keep both behavior of #41290 and #5138, we can hide the "Add to group" button when it's the excluded account.

const itemRightSideComponent = useCallback(
(item: ListItem & OptionsListUtils.Option) => {
if (item.isSelfDM) {
return null;
}

if (item.isSelfDM || excludedGroupEmails.includes(item.login ?? '')) {
    return null;
}

Then, consider removing the unused isGroupChat prop.

@shawnborton
Copy link
Contributor

I actually think I'm just going to close this, as we did decide that Expensify should still remain in your contacts list.

And I can't reproduce being able to chat with the Expensify contact using a Gmail account login.

@bernhardoj
Copy link
Contributor

@shawnborton I think we can take this chance to not allow Expensify to be added to a group, so we can prevent this issue too.

@shawnborton
Copy link
Contributor

Well I'm not actually sure if we want to prevent that or not - it seems like we want to just treat this contact like a normal contact? cc @danielrvidal in case you have thoughts there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2
Projects
None yet
Development

No branches or pull requests

6 participants