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.57k stars 2.91k forks source link

[$250] Move onboarding tasks into the #admins room for admins #51443

Open zsgreenwald opened 4 weeks ago

zsgreenwald commented 4 weeks ago

Problem

By placing onboarding steps in a Concierge DM, it limits the assigned onboarding specialist's ability to collaborate with the customer directly.

Solution

When a user selects Manage my team's expenses as their onboarding intent, let's update the assigned setup specialists' message to include onboarding Tasks, and post them in the #admins room. See below:

image

1) Since the assigned onboarding specialist's message will get sent first, we'll provide onboarding steps in that same message so it doesn't seem disjointed. 2) Let's update the assigned onboarding specialist's message to:

Hey there :wave: I'm your dedicated setup specialist. I look forward to helping you explore and configure Expensify. You can chat with me here anytime if you have any questions, or book a call with me directly at your convenience. I've shared some onboarding steps to help you get started below:

  • [ ] task 1
  • [ ] task 2
  • [ ] ....

3) Let's keep the same GIF from the Concierge message and use it in the #admins post 4) Let's move the Start your free trial tooltip over to the #admins row in LHN 5) Let's keep the welcome message from Concierge, but not have it be pinned in LHN

Issue Checklist

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021851283917795612465
  • Upwork Job ID: 1851283917795612465
  • Last Price Increase: 2024-11-05
  • Automatic offers:
    • c3024 | Contributor | 104646441
Issue OwnerCurrent Issue Owner: @yuwenmemon
melvin-bot[bot] commented 3 weeks ago

Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

zsgreenwald commented 3 weeks ago

Updated the mock in the OP

melvin-bot[bot] commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks 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 πŸ“–

melvin-bot[bot] commented 2 weeks ago

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

yuwenmemon commented 2 weeks ago

I'll be handling the internal end here.

melvin-bot[bot] commented 2 weeks ago

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

yuwenmemon commented 2 weeks ago

Discussing this at a planning level here: https://expensify.slack.com/archives/C07HPDRELLD/p1730844663049119?thread_ts=1730777288.450589&cid=C07HPDRELLD

Curious for @deetergp @MonilBhavsar and @francoisl 's take on things as they have worked on this feature in the past.

NJ-2020 commented 1 week ago

Edited by proposal-police: This proposal was edited at 2024-11-10 01:20:34 UTC.

Proposal

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

Move onboarding tasks into the #admins room for admins

What is the root cause of that problem?

New feature

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

if the user select Manage my team's expenses

const onboardingIntroductionMessage = 'introductionMessage' in data && data.introductionMessage ? data.introductionMessage : CONST.ONBOARDING_INTRODUCTION;
const introductionComment = ReportUtils.buildOptimisticAddCommentReportAction(
    typeof onboardingIntroductionMessage === 'function' ? onboardingIntroductionMessage({bookCallLink: `${environmentURL}/`}) : onboardingIntroductionMessage,
    undefined,
    actorAccountID,
);

[onboardingChoices.MANAGE_TEAM]: {
    introductionMessage: ({bookCallLink}) =>
        `Hey there πŸ‘‹ I'm your dedicated setup specialist. I look forward to helping you explore and configure Expensify. You can chat with me here anytime if you have any questions, or [book a call](${bookCallLink}) with me directly at your convenience. I've shared some onboarding steps to help you get started below:`,
    message: 'Here are some important tasks to help get your team’s expenses under control.',

Meaning if we sign in with new account then go offline mode and complete the onboarding flow, the target chat report id for the nvp_onboarding value is undefined https://github.com/Expensify/App/blob/b8210689939b584ac7f371a721c738ff4d18febe/src/libs/ReportUtils.ts#L8248-L8255 and if we simply check if it'sΒ #admin room then we will simply return true then it will show for all admins room https://github.com/Expensify/App/blob/b8210689939b584ac7f371a721c738ff4d18febe/src/libs/ReportUtils.ts#L8254 In that case we can have a new prop inside the report value called isUsedForOnboarding which will be used to display the Free trial button, and pass this prop as true when completing the onboarding flow and also this require BE changes also

The rest of the changes can be found in this test branch

Result

https://github.com/user-attachments/assets/200792ee-e602-4e30-b7a7-07fc453fa386

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 1 week ago

@yuwenmemon, @c3024 Huh... This is 4 days overdue. Who can take care of this?

yuwenmemon commented 1 week ago

My apologies, this is not open to proposals! @c3024 is going to be working on the front-end portion of this.

Removing the label. It wasn't supposed to be there - sorry about that.

yuwenmemon commented 1 week ago

Back-end portions for this are in - but @c3024 we should talk about how the Guide messaging logic will work.

c3024 commented 1 week ago

@yuwenmemon

Backend seems to reject posting the messages and tasks to #admins room from frontend with CompleteGuidedSetup.

https://github.com/user-attachments/assets/4d550245-2769-45dc-909d-72488dda59f6

Screenshot 2024-11-15 at 8 51 30β€―AM

They get posted in the #admins room fine offline.

It was working well before.

yuwenmemon commented 1 week ago

@c3024 What test account are you using?

c3024 commented 1 week ago

@yuwenmemon

I can repro this on all gmail IDs. These are two emails on which I just reproduced it.

expensifytest729+adminroomtasks3@gmail.com

https://github.com/user-attachments/assets/91541f7e-079a-4b94-97c9-420e4fcd4e5e

expensifytest729+adminroomtasks4@gmail.com

expensifytest729+adminroomtasks5@gmail.com

https://github.com/user-attachments/assets/3948faa6-fcfe-4d55-9693-2371bcd7e3c7

yuwenmemon commented 6 days ago

Hmmmm... I wasn't able to reproduce it when checking out your branch. Odd.

yuwenmemon commented 6 days ago

@c3024 From the logs I see that you're trying to create a task from an accountID that doesn't seem to exist πŸ€” I would guess it has something to do with you still trying to use the hardcoded QA Account Guide in your code here: https://github.com/Expensify/App/pull/52314/files#diff-ce6bb391d0654bf99a360da7d25fd73b85bb120211c8b4c2432ef7c7d8c708f3R3496

8e2cb634288c4af8-HYD    db1.sjc 2024-11-15 05:06:24 589 expensifytest729+adminroomtasks4@gmail.com  Trying to create action (reportID 1653526330306475, requestingAccountID: 500068347245, actionAccountID: 500068347245, action: ADDCOMMENT, reportActionID: 8212635028226681013)
8e2cb634288c4af8-HYD    db1.sjc 2024-11-15 05:06:24 589 expensifytest729+adminroomtasks4@gmail.com  Account writing to chat/task report without permission.
8e2cb634288c4af8-HYD    db1.sjc 2024-11-15 05:06:24 590 expensifytest729+adminroomtasks4@gmail.com  User does not have access to this report (requestingAccountID: 500068347245, reportID: 1653526330306475)

As we discussed, the backend is handling the logic for which account will be sending the tasks. You should not be sending it.

Also, once https://github.com/Expensify/Auth/pull/13080 is deployed you will only need to send the tasks. The Guide intro message will be sent from the back-end.

c3024 commented 6 days ago

@yuwenmemon

Oh, it seemed to work well before.

But, an account ID is required for the sender to create the report actions for tasks in the report. If backend rejects sending an optimistic account ID from front-end for qa.guide, I think CreateWorkspace called from onboarding intent might need to include qa.guide as a participant in #admins. Otherwise, I think there will only be the user in the #admins and the task sender should be the user themself.

yuwenmemon commented 6 days ago

@c3024, the QA Guide account is merely a test account. It's not going to be the only guide assigned to each user in production. We have different guide accounts that will already be added as participants part of CreateWorkspace. This logic is already in place.

What you need to do is pull the guide that is assigned to the user as a part of CreateWorkspace and have that account be the account creating the tasks. Does that make sense?

c3024 commented 6 days ago

@yuwenmemon

CreateWorkspace does not seem to be returning the personal detail of the guide.

https://github.com/user-attachments/assets/f4474ee7-d0de-4e4c-b952-9c4a7de5f570

The guide's message in admins room comes a bit later and then the guide's details are available.

https://github.com/user-attachments/assets/a58ca637-cdc0-4815-997c-d2c16a69740f

Screenshot 2024-11-15 at 10 50 14β€―PM

We have different guide accounts that will already be added as participants part of CreateWorkspace

Is this already on staging server? I do not see the guide as a participant for the workspace or admins room. Can you please help me with what I am missing? Thanks!

yuwenmemon commented 6 days ago

CreateWorkspace does not seem to be returning the personal detail of the guide.

Yes, that will happen when https://github.com/Expensify/Auth/pull/13080 is deployed. What is changing is that the guide message used to be sent via a delayed job, so the message from the guide came a bit later in the flow. Now, the guide message will immediately be in the #admins room upon the workspace's creation.

However, that doesn't seem to be necessary for the tasks from the guide to come through successfully when I test on your branch. Have you verified that the hardcoded QA email/account in your PR is not causing your error?

c3024 commented 4 days ago

Could you check for this email why backend is rejecting the report action creation in the admins room?

expensifytest729+202411181126@gmail.com

I changed the actor for the tasks to currentUserAccountID so there should be no problem with backend accepting the report actions because current user is already as a member of the admins room. I tried with actor as Concierge too with this email expensifytest729+202411181107@gmail.com but backend rejects for that too.

yuwenmemon commented 3 days ago

I see the same error you recieved last week

8e45b67af9b6f3bd-BOM    db1.sjc 2024-11-18 05:55:39 569 expensifytest729+202411181126@gmail.com Account writing to chat/task report without permission.
8e45b67af9b6f3bd-BOM    db1.sjc 2024-11-18 05:55:39 569 expensifytest729+202411181126@gmail.com User does not have access to this report (requestingAccountID: 500068347245, reportID: 293350425599161)
8e45b67af9b6f3bd-BOM    db1.sjc 2024-11-18 05:55:39 569 expensifytest729+202411181126@gmail.com Throwing exception with message: '404 Report not found' from auth/lib/Report.cpp:772

In fact, it's the same non-existent accountID from before: 500068347245

Can you relay what accountID you are expecting that to correspond to locally?

yuwenmemon commented 3 days ago

The reportID there is the #admins room of policy 273496EC1141128F

jjcoffee commented 2 days ago

@yuwenmemon Please can you assign me here as C+ for the PR review? Thanks!