[$750] [MEDIUM] Update Recents in Participants page to reflect the type of action taken

[mountiny]

Problem

Coming from this discussion.

When taking some action like requesting money or assigning a task, the participants page does not take the type of the action into consideration when creating the list of Recents. We list out the people/ workspace chats we have interacted with the latest even if we have for example never requested money from them.

 Solution

Update the Global create participants page Recents list to showrecents based on the type of action user is taking.

So if they are taking:

• creating scan request
• creating distance request
• creating manual request
• creating task
• creating split bill

for each of these, try to pick the recents such we list first the chats where the user has most recently done such action. If no data to do this optimization is available, default to the current behaviour

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b6cc63ba2f30aad7
• Upwork Job ID: 1749580420206743552
• Last Price Increase: 2024-02-05
• Automatic offers:
• rojiphil | Contributor | 28122004

[melvin-bot[bot]]

Triggered auto assignment to @muttmuure (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[mountiny]

In terms of implementation, we do not have report actions on openApp, but we should have most transactions ready, you can look at the transaction data to see if its scan or manual request and what report they are tied to

[shawnborton]

cc @trjExpensify - we were chatting about this one today

[muttmuure]

@mountiny do we need to get this into a Wave or VIP?

[trjExpensify]

It's on the wave6 project board already, but yeah.. needs progressing. Vit can maybe confirm if it needs to stay internal or not.

[mountiny]

I think we can try this Externally. We would need to look through the trasactions and then identified the reports which we requested that given type of action latest from

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[abzokhattab]

if i understand you correctly you want to prioritize the specified actions (show them on top) in the recent reports list where we should display only the first action of each type if found then display the rest of the recent list ? @mountiny

[mountiny]

Not sure from your message, flow Global Crete:

• I choose Manual request
• I choose scan and take picture
• on the participant page the recents should list the latest in choronological order chats I have requested money with scan request.

If there was no scan request before, just keep the current logic

If there would be only one or less than current recents, just show that number of chats (so there could be only 1 in recents) this is up for discussion though

Same goes for other types. For tasks this might be hard to do for requests it should be possible as we should have all the transactions locally

[rojiphil]

Proposal

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

We list out the people/ workspace chats we have interacted with the latest even if we have for example never requested money from them. The same problem can be extended to tasks as well. We want to solve this problem by displaying the recent list in the participant page such that recent reports based on the type of action taken by the user are displayed.

What is the root cause of that problem?

This is a new feature.

The scope is as follows: 1) Show recent reports based on the following type of actions chosen by user a) Scan Request creation b) Distance request creation c) Manual request creation d) Split bill creation e) Task creation 2) For each of the above, try picking recents for the specific type of action. If no such results are available, show the recents based on current behaviour which is the default one.

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

For money request action involving various types of requests, the participant list is displayed through IOURequestStepParticipants. This page makes use of MoneyRequestParticipantsSelector here.

So, the first thing to do is to determine the type of money request action by relying on iouRequestType and number of participants like this here:

const actionType = iouType === CONST.IOU.TYPE.REQUEST && participants.length > 0 ? CONST.IOU.TYPE.SPLIT : iouRequestType;

Further, we can pass actionType to getFilteredOptions here.

To implement this, we can add a helper method in TransactionUtils called getTransactionsByRequestType which will help us to get the transactions based on the action type. This helper method implementation can be like this:

function getTransactionsByRequestType(iouRequestType?: string): Array<OnyxEntry<Transaction>> {
    return Object.values(allTransactions ?? {})
        .filter((transaction): transaction is Transaction => transaction != null && (iouRequestType === CONST.IOU.TYPE.SPLIT ? isSplitRequest(transaction) : getRequestType(transaction) ===  iouRequestType))
        .sort((transactionA, transactionB) => {
            const transactionATime = new Date(transactionA?.created);
            const transactionBTime = new Date(transactionB?.created);
            return transactionATime.valueOf() - transactionBTime.valueOf();
        })
        .reverse();
}

Also, since there is no helper method for identifying split request, we can add a helper for this by checking against comment.source like this:

function isSplitRequest(transaction: Transaction): boolean {
    return Boolean(transaction?.comment?.source === CONST.IOU.TYPE.SPLIT);
}

Now, we can retrieve the recent transactions based on the action type like this here:

    const isMoneyRequestActionType = actionType === CONST.IOU.REQUEST_TYPE.MANUAL || actionType === CONST.IOU.REQUEST_TYPE.SCAN || actionType === CONST.IOU.REQUEST_TYPE.DISTANCE || actionType === CONST.IOU.TYPE.SPLIT;
        let recentTransactions = [];
        if (isMoneyRequestActionType) {
            recentTransactions = TransactionUtils.getTransactionsByRequestType(actionType );
        }

And, then, populate a new array recentReportOptionsByAction based on action type like this here:

        if (isMoneyRequestActionType && recentTransactions.some((transaction) => transaction.reportID === String(reportOption.iouReportID))) {
            recentReportOptionsByAction.push(reportOption);
        }

        // Stop adding options to the recentReports by action array when we reach the maxRecentReportsToShow value
        if (recentReportOptionsByAction.length > 0 && recentReportOptionsByAction.length === maxRecentReportsToShow) {
            break;
        }

Similarly for tasks, we can pass CONST.REPORT.TYPE.TASK as actionType in TaskAssigneeSelectorModal here. Further, we can push the reportOption for tasks to recentReportOptionsByAction like this:

            if (actionType == CONST.REPORT.TYPE.TASK && reportOption.isTaskReport) {
                recentReportOptionsByAction.push(reportOption);
            }

Finally, when available, we can display recent reports based on recentReportOptionsByAction or fallback to the existing recentReportOptions here like this:

recentReports: recentReportOptionsByAction.length > 0 ? recentReportOptionsByAction : recentReportOptions,

What alternative solutions did you explore? (Optional)

[mountiny]

cc @shawnborton would you want to assign this one to yourself to oversee the correct logic implementation?

[shawnborton]

I would prefer that someone from BZ or the project team oversees it, but happy to help test and guide implementation.

[mountiny]

@eVoloshchak can you review the proposal above?

[eVoloshchak]

@rojiphil's proposal looks good to me!

iouRequestType?: string

We can use iouRequestType?: ValueOf<typeof CONST.IOU.REQUEST_TYPE>

🎀👀🎀 C+ reviewed!

[melvin-bot[bot]]

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

[mountiny]

Melvin, what a prankster

[melvin-bot[bot]]

📣 @rojiphil 🎉 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 📖

[mountiny]

const transactionATime = new Date(transactionA?.created); const transactionBTime = new Date(transactionB?.created); return transactionATime.valueOf() - transactionBTime.valueOf();

@rojiphil could you try to compare the times as strings? Unsure how easy that would be if they would not have the same format but converting to date is not efficient so if we can avoid it, that would be great

[rojiphil]

Thank you for the assignment here. I will start work on this today and provide an update over the weekend.

[eVoloshchak]

Not overdue, PR is being drafted by @rojiphil

[rojiphil]

Update: Will share PR for review by tomorrow this time.

[mountiny]

PR is up as a draft

[rojiphil]

@mountiny @eVoloshchak Do you think we can consider an increase in the bounty here? The reason why I think so is that I notice quite a lot of thoughtful work involved due to the sheer number of test cases involved here.

[melvin-bot[bot]]

Upwork job price has been updated to $750

[mountiny]

Increased to 750, noting that a regression still cuts the price in half though so please be thorough as possible in testing. Thank you

[melvin-bot[bot]]

This issue has not been updated in over 15 days. @rojiphil, @eVoloshchak, @mountiny, @muttmuure eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[melvin-bot[bot]]

@rojiphil, @eVoloshchak, @mountiny, @muttmuure, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

[eVoloshchak]

This should be open, the PR is still in review (recent changes broke the logic a bit)

[trjExpensify]

Agreed!

[muttmuure]

Still waiting on linked PR to be merged

[muttmuure]

PR still open

[mountiny]

I am still torn on this implementation. It seems to be bringing really complex changes to the App to try to deduce the recent target accounts for various actions, but its not reliable (as we might not have the report actions available after signing in) and the changes are hard to follow.

I am now more inclined towards a solution with backend change similar to the quick action button if we really want this feature. this would simplify the logic and code.

Whenever user takes some action from the global create flows, we would update an NVP with the target reportIDs, it would basically be same as the QAB NVP, but we would keep rolling history of the 5 recent reports for any given flow.

@rojiphil @eVoloshchak you have been battling with the implementation but it seems like there has been numerous small issues with your solution and the code is hard to follow, what is your opinion?

[rojiphil]

I am now more inclined towards a solution with backend change similar to the quick action button if we really want this feature. this would simplify the logic and code.

Whenever user takes some action from the global create flows, we would update an NVP with the target reportIDs, it would basically be same as the QAB NVP, but we would keep rolling history of the 5 recent reports for any given flow.

Agree. And also I see a significant performance benefit as we get the recent list directly from the Onyx entries.

[rojiphil]

Thinking of this a little further, this is what I could I arrive at:

1. We can take the advantage of quick actions here which would map to the mentioned flow:

       REQUEST_MANUAL==> MANUAL
       REQUEST_SCAN==> SCAN
       REQUEST_DISTANCE==> DISTANCE
       SPLIT_MANUAL==> SPLIT   
       SPLIT_SCAN==> SPLIT 
       SPLIT_DISTANCE==> SPLIT 
       TRACK_MANUAL==> MANUAL  
       TRACK_SCAN==> SCAN  
       TRACK_DISTANCE==> DISTANCE  
       ASSIGN_TASK==> TASK 
       SEND_MONEY==> MANUAL

2. The following Onyx entries will be added:

   np_recent_manual_requests
   nvp_recent_scan_requests
   nvp_recent_distance_requests
   nvp_recent_split_requests
   nap_recent_tasks

   Each of these will hold the flow specific array of reportIDs with rolling history of the 5 recent reports

3. For every quick action that reaches BE, the BE will push the report ID in the flow specific nvp_recent_* list and send/push the updated entries to FE.

4. The FE will also optimistically update the flow specific nvp_recent_* Onyx entries by pushing the reportID.

5. While displaying recent list, the FE will use the flow specific Onyx entries. If not present, the default one will be displayed as shown currently.

6. We also need to remove the redundant reportID from the recent list in certain cases like leaving a group chat. Also, BE need to update the recent list in such cases and push the updated entries back to FE. Also, there would be cases where a report is not present in Onyx but is present in recent list. Such reports would also be not shown in recent list. So we might end up with less than 5 entries in recent list on removal of such reports. Hope this is fine.

@mountiny Is this what we are aiming for? Or are there any changes required?

[mountiny]

I think we would need to hold internal discussion about this, currently focusing on higher priority tasks though

[muttmuure]

No update

[muttmuure]

Not overdue

[mountiny]

This task showed up to be quite of a rabbit hole. A proper solution would require a backend changes too something similar along the lines of QAB but we did not really prioritise this. Instead we are trying out this change https://github.com/Expensify/App/issues/46683 and lets see if it will be enough from the UX perspective.

I think we could close this issue out for focus @trjExpensify what do you think? We can always come back to this if we want to prioritize creating a proper fix for this. We would of course pay some reward for the effort

[trjExpensify]

Yep, works for me!

[rojiphil]

We would of course pay some reward for the effort

@mountiny Thanks for mentioning this. I also think this deserves to be rewarded as quite a lot of effort went into it from my side. Would $500 sound fair here instead of the actual price of $750?

Reasonings:

1. The PR itself needed a lot of work to cover about 7 different test cases as mentioned in the OP here
2. Quite a lot of review cycles occurred here which included verification via Timing tests apart from regular re-reviews that spanned for almost about 2 months.

[mountiny]

@rojiphil Yes I think thats valid. I would go for 50% of the planned amount so $375, how does that sound? @eVoloshchak

[eVoloshchak]

@mountiny, yeah, a 50% payout seems fair to me

[rojiphil]

I would go for 50% of the planned amount so $375, how does that sound?

Thanks @mountiny. Maybe we can make this issue daily to close on this sooner. cc @muttmuure

[muttmuure]

Handling

[muttmuure]

$375 - @rojiphil (paid) $375 - @eVoloshchak

[muttmuure]

@eVoloshchak please submit your request in New Expensify!

[JmillsExpensify]

$375 approved for @eVoloshchak