Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.9k forks source link

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

Closed m-natarajan closed 1 month ago

m-natarajan commented 2 months ago

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?

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
melvin-bot[bot] commented 2 months ago

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 commented 2 months ago

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

twisterdotcom commented 2 months ago

@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 commented 2 months ago

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

twisterdotcom commented 2 months ago

https://github.com/user-attachments/assets/2cb0882b-475b-49f8-b36d-bf9e135e4e2d

twisterdotcom commented 2 months ago

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.

https://github.com/user-attachments/assets/c2dea30d-6d83-4caa-a351-84544c3a12d4

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

twisterdotcom commented 2 months ago

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

c3024 commented 1 month ago

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 https://github.com/Expensify/App/blob/4b41dd187dc63ab01af824f106f1ea1bb189523d/src/libs/OptionsListUtils.ts#L1576-L1583 For all other policies, even though there were some expenses recently submitted to them they were given a lower priority than DMs here https://github.com/Expensify/App/blob/4b41dd187dc63ab01af824f106f1ea1bb189523d/src/libs/OptionsListUtils.ts#L1594-L1599 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 https://github.com/Expensify/App/blob/2af5a95b658f6426d411f1fa6ea5377507c65dbe/src/libs/OptionsListUtils.ts#L1983 and we can pass a parameter to orderOptions such as preferRecentExpenseReports here

https://github.com/Expensify/App/blob/2af5a95b658f6426d411f1fa6ea5377507c65dbe/src/libs/OptionsListUtils.ts#L2032

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 commented 1 month ago

@c3024's proposal LGTM but we should get some confirmation on the correct order. πŸŽ€πŸ‘€πŸŽ€ C+ reviewed

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

melvin-bot[bot] commented 1 month ago

@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 commented 1 month ago

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 ?

twisterdotcom commented 1 month ago

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[bot] commented 1 month ago

πŸ“£ @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 commented 1 month ago

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

cc: @twisterdotcom

twisterdotcom commented 1 month ago

Payment Summary:

Ollyws commented 1 month ago

Requested in ND.

garrettmknight commented 1 month ago

$250 approved for @Ollyws