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

[HOLD #27510][$500] Dev: Web - Selecting a workspace selects other (all) workspaces as well #27588

Closed
1 of 6 tasks
kbecciv opened this issue Sep 16, 2023 · 14 comments
Closed
1 of 6 tasks
Assignees
Labels
External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Sep 16, 2023

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


Action Performed:

  1. Start money request flow
  2. Ensure you have multiple workspaces created
  3. Upon selecting one workspace(for split)
  4. Observe all workspaces are selected

Expected Result:

Only selected workspace should be selected

Actual Result:

All workspaces are selected

Workaround:

Unknown

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: Dev 1.3.70-5
Reproducible in staging?: no
Reproducible in production?: no
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
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-09-15.at.6.02.36.PM.mov

Expensify/Expensify Issue URL:
Issue reported by: @BhuvaneshPatil
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694781139958309

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01dd4d5419ef8224cd
  • Upwork Job ID: 1703111319192711168
  • Last Price Increase: 2023-09-16
@kbecciv kbecciv added the External Added to denote the issue can be worked on by a contributor label Sep 16, 2023
@melvin-bot melvin-bot bot changed the title Dev: Web - Selecting a workspace selects other (all) workspaces as well [$500] Dev: Web - Selecting a workspace selects other (all) workspaces as well Sep 16, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 16, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01dd4d5419ef8224cd

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 16, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 16, 2023

Triggered auto assignment to @mallenexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot melvin-bot bot added the Daily KSv2 label Sep 16, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 16, 2023

Triggered auto assignment to Contributor-plus team member for initial proposal review - @robertKozik (External)

@kbecciv
Copy link
Author

kbecciv commented Sep 16, 2023

Proposal by: @BhuvaneshPatil
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694781139958309

Proposal

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

Selecting a workspace selects other (all) workspaces as well

What is the root cause of that problem?

While calculating isSelected here -

const isSelected = _.some(selectedOptions, (option) => {
if (option.accountID === item.accountID) {
return true;
}
if (_.isEmpty(option.name)) {
return false;
}
return option.name === item.searchText;
});

We check for accountID only, as workspace chats/reports have accountID 0.
We are showing other workspaces as selected as well

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

if (option.accountID && option.accountID === item.accountID) return true
if (option.reportID && option.reportID === item.reportID) return true

What alternative solutions did you explore? (Optional)

@akinwale
Copy link
Contributor

This is a duplicate of #27510.

@BhuvaneshPatil
Copy link
Contributor

Not a dupe, that's about showing workspace details and this one is about multiple workspaces being selected. Both have different RCA and solutions as well

@saranshbalyan-1234
Copy link
Contributor

Proposal

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

Selecting a workspace selects other (all) workspaces as well

What is the root cause of that problem?

Upon searching i found that value of isSelected is only based on accountID, as all workspace have accountId as 0 its causing the issue. Which results in selecting all the workspaces.

[App/src/components/OptionsList/BaseOptionsList.js]

 const isSelected = _.some(selectedOptions, (option) => { 
     if (option.accountID === item.accountID) { 
         return true; 
     } 
  
     if (_.isEmpty(option.name)) { 
         return false; 
     } 
  
     return option.name === item.searchText; 
 }); 

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

Instead of searching with accountID we need to search with reportID only as reportID is different for everything.

We have to pass reportID here

[App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsSelector.js]
Line: 169

newSelectedOptions = [...participants, {accountID: option.accountID, login: option.login, selected: true, reportID: option.reportID}];

and need to change this to
[App/src/components/OptionsList/BaseOptionsList.js]

const isSelected = _.some(selectedOptions, (option) => { 
     if (option.reportID === item.reportID) { 
         return true; 
     } 
     if (_.isEmpty(option.name)) { 
         return false; 
     } 
     return option.name === item.searchText; 
 }); 

What alternative solutions did you explore? (Optional)

N/A

@melvin-bot melvin-bot bot added the Overdue label Sep 19, 2023
@mallenexpensify
Copy link
Contributor

@BhuvaneshPatil, do you know why, when you click split with a workspace, you get this blank account that shows up? I noticed in your screenvid and it shows for me too
image

@melvin-bot melvin-bot bot removed the Overdue label Sep 19, 2023
@BhuvaneshPatil
Copy link
Contributor

@mallenexpensify This is known issue - #27510

@mallenexpensify
Copy link
Contributor

Thanks @BhuvaneshPatil , I was unable to reproduce, going to put this on hold pending

@mallenexpensify mallenexpensify added Weekly KSv2 and removed Help Wanted Apply this label when an issue is open to proposals by contributors Daily KSv2 labels Sep 19, 2023
@mallenexpensify mallenexpensify changed the title [$500] Dev: Web - Selecting a workspace selects other (all) workspaces as well [HOLD #27510][$500] Dev: Web - Selecting a workspace selects other (all) workspaces as well Sep 19, 2023
@BhuvaneshPatil
Copy link
Contributor

BhuvaneshPatil commented Sep 19, 2023

@mallenexpensify I think I mis-understood with what you have said.

To reproduce this issue we need more than 1 workspace.

When we select a workspace, notice is shows check icon for all the workspaces

Can you try this one?

@BhuvaneshPatil
Copy link
Contributor

@mallenexpensify This is known issue - #27510

I was mentioning about the workspace detail not showing in selected participant, that is the issue linked.
This one is about, multiple workspace showing check icon when we select on workspace

@robertKozik
Copy link
Contributor

@BhuvaneshPatil I believe selected proposal in #27510 will address this issue as well

@melvin-bot melvin-bot bot added the Overdue label Sep 28, 2023
@mallenexpensify
Copy link
Contributor

Above closed and I'm unable to reproduce this so I'm closing
@BhuvaneshPatil and/or @robertKozik , if you're able to repro, please comment.

2023-10-04_10-28-07.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

6 participants