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
2.99k stars 2.5k forks source link

[Guided Setup Stage 2] [$250] Make the GBR on tasks at highest context and assign onboarding tasks to users #41619

Open anmurali opened 2 weeks ago

anmurali commented 2 weeks ago

We are currently deliberately not assigning the onboarding tasks to users because the GBRs on the tasks are at lowest context. I.e. each task is shown in the LHN with a GBR and it looks very noisy and potentially annoying. But we're losing out by doing this because after the first read, the user can ignore the chat with their onboarding tasks, or worse forget where it was originally and kinda lose motivation.

So, let's

  1. if the user that the task is assigned to is in the parent chat where the task is shared, then the GBR is at that parent chat level
  2. If the user that the task is assigned to is NOT in the parent chat where the task is shared, then the GBR is at the task level
Issue OwnerCurrent Issue Owner: @Santhosh-Sellavel
cretadn22 commented 1 week ago

Proposal

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

  1. Make the GBR on tasks at highest context
    • if the user that the task is assigned to is in the parent chat where the task is shared, then the GBR is at that parent chat level
    • If the user that the task is assigned to is NOT in the parent chat where the task is shared, then the GBR is at the task level
  2. Assign all the onboarding tasks created for a user to said user

What is the root cause of that problem?

This is not a bug, it requires to refactor the old logic. Currently, we use this logic to determined the Task report with GBR

https://github.com/Expensify/App/blob/ab1e924bf0d4f928a9006f5bdd6b9da60346b1df/src/libs/ReportUtils.ts#L2247

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

  1. Make the GBR on tasks at highest context

Initially, we must eliminate the outdated logic https://github.com/Expensify/App/blob/ab1e924bf0d4f928a9006f5bdd6b9da60346b1df/src/libs/ReportUtils.ts#L2247

followed by segregating the task report with GBR into a distinct variable

const assignedTaskReports: Array<OnyxEntry<Report>> = [];
if (requiresAttentionFromCurrentUserToAssignedTask(report, reportAction)) {
        assignedTaskReports.push(report);
} else {
    ...... <old logic>
}

The requiresAttentionFromCurrentUserToAssignedTask function will be designed as follows:

  1. Assign all the onboarding tasks created for a user to said user

When generating onboarding tasks, it's necessary to assign taskReport.managerID = currentUserID

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

Auto-assigning issues to engineers is no longer supported. If you think this issue should receive engineering attention, please raise it in #whatsnext.

allroundexperts commented 1 week ago

Taking over!

Santhosh-Sellavel commented 1 week ago

Unassigning due to low bandwidth, thanks @allroundexperts!

allroundexperts commented 1 week ago

@cretadn22 Do you have any branch with these changes so I can test?

cretadn22 commented 1 week ago

@allroundexperts Are you inquiring about the process of coding it? Certainly, I'll submit a pull request for my solution. Before I do, could you share your thoughts on the approach I've taken?

Thanks

allroundexperts commented 1 week ago

@cretadn22 I need to test your approach a little. Did you test that as well? If so, can you provide a link to your branch?

cretadn22 commented 6 days ago

@allroundexperts This is my draft implementation: https://github.com/Expensify/App/pull/42025

cretadn22 commented 6 days ago

@allroundexperts In this point:

Assign all the onboarding tasks created for a user to said user

we require some changes from BE to finish

allroundexperts commented 6 days ago

Thanks @cretadn22. I will review this today.

allroundexperts commented 4 days ago

@cretadn22 I reviewed the draft implementation and its looking fine to me. Can you elaborate in more detail what exact changes you need from the backend?

cretadn22 commented 4 days ago

@allroundexperts More detail here: https://github.com/Expensify/App/pull/42025#discussion_r1600813641

trjExpensify commented 3 days ago

Thoughts on that, @allroundexperts?

allroundexperts commented 2 days ago

Thoughts on that, @allroundexperts?

Will update today.

puneetlath commented 2 days ago

@allroundexperts I would think that we want the back-end to set hasOutstandingChildRequest on these reports, just like we do for expense reports, IOUs etc. Otherwise if you're in focus mode, the app won't have enough info to know whether to show the GBR.

allroundexperts commented 2 days ago

@allroundexperts I would think that we want the back-end to set hasOutstandingChildRequest on these reports, just like we do for expense reports, IOUs etc. Otherwise if you're in focus mode, the app won't have enough info to know whether to show the GBR.

Given this, @cretadn22 shouldn't you update your proposal? I think that will make things more easier?

allroundexperts commented 1 day ago

Any update here @cretadn22?