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

[PAID] [CVP] [$250] Always show primary workspace at the top when submitting an expense #46683

Closed
6 tasks done
zsgreenwald opened this issue Aug 1, 2024 · 45 comments
Closed
6 tasks done
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@zsgreenwald
Copy link
Contributor

zsgreenwald commented Aug 1, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: v9.0.14-6
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers): everyone
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL: n/a
Issue reported by: Zach Greenwald
Slack conversation: https://expensify.slack.com/archives/C03U7DCU4/p1722529376302179

Action Performed:

  1. Log into Expensify, and make sure you are already tied to a workspace
  2. Select Global Create button
  3. Select Submit expense
  4. Either scan a receipt if you have one, or select Manual
  5. Then, under Recents underneath the search bar you'll see recents

Expected Result:

In step 5 above, under Recents, the first option should show your Workspace name and that option should be static

Actual Result:

In step 5 above, only my recent chats showed up.

Workaround:

n/a

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

IMG_DB902F377E35-1

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c2dd3d58e5ea6e4b
  • Upwork Job ID: 1820986743163806073
  • Last Price Increase: 2024-08-07
  • Automatic offers:
    • allgandalf | Contributor | 103521986
    • nkdengineer | Contributor | 103523822
Issue OwnerCurrent Issue Owner: @strepanier03
@zsgreenwald zsgreenwald added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 1, 2024
Copy link

melvin-bot bot commented Aug 1, 2024

Triggered auto assignment to @strepanier03 (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.

@ShridharGoel
Copy link
Contributor

ShridharGoel commented Aug 1, 2024

Proposal

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

Always show primary workspace at the top when submitting an expense.

What is the root cause of that problem?

We don't have any logic to show workspaces on top while submitting an expense.

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

While creating the options list, we create the recent reports list and add it. We can update the logic to show workspaces on top.

Update this logic to the below:

if (action === CONST.IOU.ACTION.CREATE && reportOption.isPolicyExpenseChat) {
    recentReportOptions.unshift(reportOption);
} else if (action === CONST.IOU.ACTION.CATEGORIZE) {
    const policyCategories = allPolicyCategories?.[`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${reportOption.policyID}`] ?? {};
    if (getEnabledCategoriesCount(policyCategories) !== 0) {
        recentReportOptions.push(reportOption);
    }
} else {
    recentReportOptions.push(reportOption);
}

The above is one example, we might need to include some more items based on discussion.

@FitseTLT
Copy link
Contributor

FitseTLT commented Aug 1, 2024

Proposal

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

Always show primary workspace at the top when submitting an expense

What is the root cause of that problem?

New Feature Request

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

We already have ordereOptions function here

/**
* Options need to be sorted in the specific order
* @param options - list of options to be sorted
* @param searchValue - search string
* @returns a sorted list of options
*/
function orderOptions(options: ReportUtils.OptionData[], searchValue: string | undefined, {preferChatroomsOverThreads = false} = {}) {
return lodashOrderBy(
options,
[
(option) => {
if (option.isSelfDM) {
return 0;
}

We need to add an new config like shouldPrioritizeWorkspaceChat for the ordering along with preferChatroomsOverThreads to give an option to order policy expense chats on top then we can return low number if shouldPrioritizeWorkspaceChat is true and option.isPolicyExpenseChat in the iteratee of the lodashOrderBy.
Then we can use the new config in OptionsListUtils.filterOptions. we can pass the param shouldPrioritizeWorkspaceChat as true when the iou type is submit here and inside OptionsListUtils.filterOptions we order accordingly the recentReports before returning the options result here and here.

We might need to add the action type into our condition may be to order only when it create type.

What alternative solutions did you explore? (Optional)

@nkdengineer
Copy link
Contributor

nkdengineer commented Aug 2, 2024

Proposal

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

In step 5 above, only my recent chats showed up.

What is the root cause of that problem?

We don't prioritize the workspace chat option on the participant page of the money request flow

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

  1. In getOptions function, we have a config param sortByReportTypeInSearch here and if it's true, we will sort the recent report option here. So we should add an extra param sortByReportTypeInSearch with the default value is false in getFilteredOptions function and pass this param to getOptions function here

const optionList = OptionsListUtils.getFilteredOptions(

Then we pass this param as true here

const optionList = OptionsListUtils.getFilteredOptions(

  1. Add another config preferPolicyExpenseChat in orderOptions here and then if it's true, prioritize the WS chat by returning the minimum value for the policy expense option
if (option.isPolicyExpenseChat && preferPolicyExpenseChat) {
    return 0;
}

function orderOptions(options: ReportUtils.OptionData[], searchValue: string | undefined, {preferChatroomsOverThreads = false} = {}) {

  1. Then pass preferPolicyExpenseChat as !!actions here. We can do this because the action here is the iouAction and it only exists when we get the option in the money request flow.
recentReportOptions = orderOptions(recentReportOptions, searchValue, {preferChatroomsOverThreads: true, preferPolicyExpenseChat: !!action});

recentReportOptions = orderOptions(recentReportOptions, searchValue, {preferChatroomsOverThreads: true});

We also need to update this condition here to always sort the recent report in money request flow even if the searchValue is empty.

if (sortByReportTypeInSearch && (searchValue !== '' || !!action)) {

if (sortByReportTypeInSearch && searchValue !== '') {

What alternative solutions did you explore? (Optional)

Copy link
Contributor

github-actions bot commented Aug 2, 2024

true

@nkdengineer
Copy link
Contributor

Proposal updated to fix the typo.

Copy link
Contributor

github-actions bot commented Aug 2, 2024

true

@melvin-bot melvin-bot bot added the Overdue label Aug 5, 2024
@strepanier03
Copy link
Contributor

I agree we should have the workspace be a default at the top if you're submitting an expense and part of one.

@melvin-bot melvin-bot bot removed the Overdue label Aug 7, 2024
@strepanier03 strepanier03 added the External Added to denote the issue can be worked on by a contributor label Aug 7, 2024
Copy link

melvin-bot bot commented Aug 7, 2024

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

@melvin-bot melvin-bot bot changed the title Always show primary workspace at the top when submitting an expense [$250] Always show primary workspace at the top when submitting an expense Aug 7, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 7, 2024
Copy link

melvin-bot bot commented Aug 7, 2024

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

@JmillsExpensify
Copy link

@trjExpensify should we just start by prioritizing the primary workspace?

@trjExpensify
Copy link
Contributor

We could, I'd like to get @mountiny's take though. This would be an alt proposal to Update Recents in Participants page to reflect the type of action taken, which was aiming to build the recents list based on the action in question.

@mountiny
Copy link
Contributor

mountiny commented Aug 8, 2024

This would be an alt proposal to #34227, which was aiming to build the recents list based on the action in question.

That one got really convoluted, and ideally, it would require a backend solution—something similar to the quick action NVP but for various types of flows.

I think this is a bit simpler and we can see if it will be fine for now. Eventually, I guess we cannot avoid using some kind of NVP to store the recent target reports for the individual flows

@JmillsExpensify
Copy link

Cool, I think optimizing for the primary policy would cover the mainline case. If it's easy, I say we go with it and get a quick win.

@mountiny
Copy link
Contributor

mountiny commented Aug 9, 2024

This one seems to be much smaller scope and easier for sure. Given it will cover the mainline case I think we should do this

@trjExpensify
Copy link
Contributor

Cool, then let's do this then for now. 👍

@melvin-bot melvin-bot bot added the Overdue label Aug 9, 2024
@trjExpensify trjExpensify moved this from Polish to Release 2.5: SuiteWorld (Sept 9th) in [#whatsnext] #wave-collect Aug 9, 2024
@trjExpensify trjExpensify changed the title [$250] Always show primary workspace at the top when submitting an expense [CVP] [$250] Always show primary workspace at the top when submitting an expense Aug 9, 2024
@nkdengineer
Copy link
Contributor

@allgandalf The PR is here.

@allgandalf
Copy link
Contributor

♻️ on it

@mountiny
Copy link
Contributor

I have made a backend change that should ensure the policy expense chat update is queued when the active policy is changed in NewDot that should fix the issue @trjExpensify saw during his testing https://github.com/Expensify/Auth/pull/12062

@trjExpensify
Copy link
Contributor

Dope, thanks for that!

@allgandalf
Copy link
Contributor

PR is in staging 🙇

@mountiny mountiny added Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review Weekly KSv2 labels Aug 27, 2024
@melvin-bot melvin-bot bot added the Overdue label Aug 27, 2024
@mountiny mountiny changed the title [CVP] [$250] Always show primary workspace at the top when submitting an expense [HOLD for payment 2024-08-27] [CVP] [$250] Always show primary workspace at the top when submitting an expense Aug 27, 2024
@mountiny
Copy link
Contributor

$250 to @allgandalf and to @nkdengineer please @strepanier03

I assume we should add a regression test. @nkdengineer @allgandalf can you please propose one with steps that will verify for sure this logic works in the submit expense flow? Thanks

@allgandalf
Copy link
Contributor

Regression Test Proposal

Precondition: User should have a primary workspace configured on OD.

  1. Login to an existing account on ND having a few workspaces and DM's.
  2. Go to FAB > Submit expense.
  3. Enter a valid amount and go to the participant page

Verify that the first option on the Participants page is the primary workspace chat

Do we agree 👍 or 👎

@strepanier03
Copy link
Contributor

Payment summary

@melvin-bot melvin-bot bot removed the Overdue label Aug 27, 2024
@strepanier03
Copy link
Contributor

Offers sent and reg test created. Will check in this afternoon to take next steps.

@strepanier03
Copy link
Contributor

Contract paid and closed for @allgandalf. I'll check again in the morning for @nkdengineer's contract.

@strepanier03
Copy link
Contributor

Will check again later today.

@melvin-bot melvin-bot bot added Overdue Daily KSv2 and removed Daily KSv2 Overdue labels Aug 30, 2024
@strepanier03
Copy link
Contributor

@nkdengineer - friendly bump on accepting the offer so I can pay it out and close this GH.

@nkdengineer
Copy link
Contributor

@strepanier03 Sorry, I accepted

@strepanier03
Copy link
Contributor

No worries, thanks for doing that. I'll finish up now and close out.

@strepanier03
Copy link
Contributor

Paid out the last contract and closed it. We're all set here.

Thanks for all the hard work everyone!

@github-project-automation github-project-automation bot moved this from Release 2.5: SuiteWorld (Sept 9th) to Done in [#whatsnext] #wave-collect Sep 4, 2024
@strepanier03 strepanier03 changed the title [HOLD for payment 2024-08-27] [CVP] [$250] Always show primary workspace at the top when submitting an expense [PAID] [CVP] [$250] Always show primary workspace at the top when submitting an expense Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Status: Done
Development

No branches or pull requests

10 participants