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

[$250] 'Recents' contacts when submitting expense does not update #48608

Closed
6 tasks done
m-natarajan opened this issue Sep 4, 2024 · 22 comments
Closed
6 tasks done

[$250] 'Recents' contacts when submitting expense does not update #48608

m-natarajan opened this issue Sep 4, 2024 · 22 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Sep 4, 2024

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: n/a
Reproducible in staging?: needs reproduction
Reproducible in production?: needs reproduction
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: @flodnv
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1724228186903849

Action Performed:

  1. Submit an expenses 3 days in a row to the same workspace
  2. Right after the 3rd time, go to submit another expense
  3. Observe that the workspace is still not in 'Recents' (although another workspace is there that I don't even use on NewDot)

Expected Result:

The workspace where I always submit to should be in 'Recents' when using it every day

Actual Result:

I don't see the workspace in 'Recents', I have to type it out every day

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021833488575321005184
  • Upwork Job ID: 1833488575321005184
  • Last Price Increase: 2024-09-17
  • Automatic offers:
    • c3024 | Contributor | 104052304
Issue OwnerCurrent Issue Owner: @Ollyws
@m-natarajan m-natarajan added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

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

@MelvinBot
Copy link

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

@twisterdotcom
Copy link
Contributor

@flodnv are these steps right?

Submit an expenses 3 days in a row to the same workspace
Right after the 3rd time, go to submit another expense

Does it have to be 3 expenses over 3 days?

@flodnv
Copy link
Contributor

flodnv commented Sep 6, 2024

No, maybe it's just 1. I used 3 in my example to point out that it's obvious it should be there.

@melvin-bot melvin-bot bot added the Overdue label Sep 9, 2024
@twisterdotcom

This comment was marked as outdated.

@melvin-bot melvin-bot bot removed the Overdue label Sep 10, 2024
@twisterdotcom
Copy link
Contributor

twisterdotcom commented Sep 10, 2024

Put an iOS vid above, but doing a web one now for commentary, this is a real bug that I think is recreatable just by:

  1. Be invited to a Workspace in a normal active Expensify account (that has recent chats)
  2. Submit at expense on that workspace
  3. Submit an expense from the "+" fab
  4. In the "recents" which appear, the workspace doesn't appear at all

We should absolutely during the "Submit expense" flow especially be prioritising places you have submitted expenses before and Workspace chats overall, over ALL normal chats where no expenses have been shared whatsoever.

48608.mp4

@twisterdotcom twisterdotcom added External Added to denote the issue can be worked on by a contributor and removed Needs Reproduction Reproducible steps needed labels Sep 10, 2024
@melvin-bot melvin-bot bot changed the title 'Recents' contacts when submitting expense does not update [$250] 'Recents' contacts when submitting expense does not update Sep 10, 2024
Copy link

melvin-bot bot commented Sep 10, 2024

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

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

melvin-bot bot commented Sep 10, 2024

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

Copy link

melvin-bot bot commented Sep 13, 2024

@twisterdotcom, @Ollyws Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Sep 13, 2024
@twisterdotcom
Copy link
Contributor

Asking for some proposals here: https://expensify.slack.com/archives/C01GTK53T8Q/p1726487090872509

@melvin-bot melvin-bot bot removed the Overdue label Sep 16, 2024
@c3024
Copy link
Contributor

c3024 commented Sep 17, 2024

Edited by proposal-police: This proposal was edited at 2024-09-17 03:13:38 UTC.

Proposal

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

Recent reports do not show the workspace chat even though an expense was submitted to it recently.

What is the root cause of that problem?

We give the highest priority only to the active policy here in ordering

function orderOptions(options: ReportUtils.OptionData[], searchValue: string | undefined, {preferChatroomsOverThreads = false, preferPolicyExpenseChat = false} = {}) {
return lodashOrderBy(
options,
[
(option) => {
if (option.isPolicyExpenseChat && preferPolicyExpenseChat && option.policyID === activePolicyID) {
return 0;
}

For all other policies, even though there were some expenses recently submitted to them they were given a lower priority than DMs here
if (!option.login) {
return 2;
}
if (option.login.toLowerCase() !== searchValue?.toLowerCase()) {
return 1;
}

So, only the user's active policy is at the top but after that DMs with others are prioritized over non-active policies.

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

Based on this comment by @twisterdotcom

We should absolutely during the "Submit expense" flow especially be prioritising places you have submitted expenses before and Workspace chats overall, over ALL normal chats where no expenses have been shared whatsoever.

I think the order should be like this for Submit Expense flow

  1. Active policy at the top
  2. Sort reports by the time of last money request made
  3. Other workspace chats (where no requests are not made yet)
  4. Others in the order of last activity on the report

To achieve this order, we can add a field lastMoneyRequestTime to reportOption when there is an action in the request like this

if (!!action) {
                const iouReportID = reportOption.iouReportID;
                if (iouReportID) {
                    const iouReportActions = allSortedReportActions[iouReportID] ?? [];
                    const lastIOUAction = iouReportActions.find((iouAction) => iouAction.actionName === "IOU");
                    if (lastIOUAction) {
                        reportOption.lastMoneyRequestTime = lastIOUAction.lastModified;
                    }


            }

here


and we can pass a parameter to orderOptions such as preferRecentExpenseReports here

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

like this

recentReportOptions = orderOptions(recentReportOptions, searchValue, {preferChatroomsOverThreads: true, preferPolicyExpenseChat: !!action, preferRecentExpenseReports: !!action});

and the sorting in orderOptions can be changed to

function orderOptions(options: ReportUtils.OptionData[], searchValue: string | undefined, {preferChatroomsOverThreads = false, preferPolicyExpenseChat = false, preferRecentExpenseReports = false} = {}) {
    return lodashOrderBy(
        options,
        [
            (option) => {
                if (option.isPolicyExpenseChat && preferPolicyExpenseChat && option.policyID === activePolicyID) {
                    return 0;
                }
                if (option.isSelfDM) {
                    return 0;
                }
                if (preferRecentExpenseReports && !!option?.lastMoneyRequestTime) {
                    return 1;
                   // We can have a bit different logic like this then we do not need two `desc` `desc` breakers. We can just check for `policyExpenseChat` with just on `desc` tie breaker
                   // We are reducing the `lastMoneyRequestTime` to a number between 0 and 1 with min and max times as -+1 years
                    // const oneYearInMs = 365 * 24 * 60 * 60 * 1000; // Approximate milliseconds in one year
                    // const today = new Date();
                    // const minTime = new Date(today).getTime() - oneYearInMs;
                    // const maxTime = new Date(today).getTime() + oneYearInMs;
                    // const lastMoneyRequestTimestamp = (new Date(option.lastMoneyRequestTime).getTime() - minTime)/(maxTime - minTime);
                    // return 1 - lastMoneyRequestTimestamp;
                }
                if (preferRecentExpenseReports && option.isPolicyExpenseChat) {
                    return 1;
                }
                if (preferChatroomsOverThreads && option.isThread) {
                    return 4;
                }
                if (!!option.isChatRoom || option.isArchivedRoom) {
                    return 3;
                }
                if (!option.login) {
                    return 2;
                }
                if (option.login.toLowerCase() !== searchValue?.toLowerCase()) {
                    return 1;
                }

                // When option.login is an exact match with the search value, returning 0 puts it at the top of the option list
                return 0;
            },
            preferRecentExpenseReports ? (option) => new Date(option?.lastMoneyRequestTime ?? '1970-01-01').getTime() : 0,
            preferRecentExpenseReports ? (option) => option?.isPolicyExpenseChat : 0,
        ],
        ['asc', 'desc', 'desc'],
    );
}

We can similarly pass the action value to filterOptions from MoneyRequestPariticipantSelector

const newOptions = OptionsListUtils.filterOptions(defaultOptions, debouncedSearchTerm, {
           // other params
            preferPolicyExpenseChat: isPaidGroupPolicy,
            action,
        });

and use that action value to check for preferRecentExpenseReports and pass it to orderOptions in filterOptions.

What alternative solutions did you explore? (Optional)

@Ollyws
Copy link
Contributor

Ollyws commented Sep 17, 2024

@c3024's proposal LGTM but we should get some confirmation on the correct order.
🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Sep 17, 2024

Triggered auto assignment to @MariaHCD, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

Copy link

melvin-bot bot commented Sep 17, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Sep 18, 2024

@twisterdotcom @Ollyws @MariaHCD this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@MariaHCD
Copy link
Contributor

The order that @c3024 suggests sounds good to me:

I think the order should be like this for Submit Expense flow

  1. Active policy at the top
  2. Sort reports by the time of last money request made
  3. Other workspace chats (where no requests are not made yet)
  4. Others in the order of last activity on the report

Does this sound good to you, @twisterdotcom ?

@melvin-bot melvin-bot bot added the Overdue label Sep 19, 2024
@twisterdotcom
Copy link
Contributor

Yeah, this seems logical. I see an argument for switching 2 and 3, but on balance I agree prioritising places you have submitted expenses before is better.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 20, 2024
Copy link

melvin-bot bot commented Sep 20, 2024

📣 @c3024 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@c3024
Copy link
Contributor

c3024 commented Oct 8, 2024

PR was deployed to production on 1-Oct. Payment is due today!

cc: @twisterdotcom

@twisterdotcom
Copy link
Contributor

Payment Summary:

@Ollyws
Copy link
Contributor

Ollyws commented Oct 10, 2024

Requested in ND.

@garrettmknight
Copy link
Contributor

$250 approved for @Ollyws

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. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Status: No status
Development

No branches or pull requests

8 participants