Add Guides and AMs to the #admins Chat Without Making Them Policy Admin

[davidcardoza]

Background

• Project Tracking Issue
• To allow Guides and AMs to be added to the #admins chat without being added as an admin member to the workspace

Tasks:

Identify Flows for Adding Guides to Policies:

• Identify and list all instances where a Guide or AM is added as a policy admin.
• Document these workflows for reference and modification.
• Instead of adding Guides as policy admins, modify these workflows to invite Guides and AMs to the #admins chat room.
• Ensure this change is reflected in all relevant parts of the system where Guides and AMs are currently being added as policy admins.

Update Auth Logic:

• Review the existing logic in Auth that restricts non-admins from leaving comments.
• Implement a modification to this logic to allow exceptions for Expensify or Teamdot email addresses, enabling Guides and AMs to be a member of #admins rooms without being an admin on the workspace

Non-engineering tasks

• [ ] Test with Guides and AMs that they are no longer added as an admin when they are assigned a new customer or lead.
• [ ] Announce the change in #guides-team

[melvin-bot[bot]]

Triggered auto assignment to @MonilBhavsar (AutoAssignerNewDotQuality)

[davidcardoza]

Here's a possible plan

Step 1

• [x] Update Auth SetAssignedGuide to add guide to the admins channel for any policies we add them to.
• [x] Also make it so they can chat on the #admins room.
• [x] Confirm in NewDot that they can also do this and nothing prevents it there.

Step 2

Stop making them policy admin (which all happens in Web-Expensify)...

BulkDirectAssignGuides.php

• [x] This needs to be updated so that we look at the policy NVP $.assignedGuide data instead of checking for policy admin
• [x] This line adding them to the policy can be removed

BulkAssignGuides.php

• [x] Looks like duplicated logic that does something similar to BulkDirectAssignGuides - update so that we look at the policy NVP
• [x] Don't add them to the policy here

Policy Utils.php

• [x] Remove this here to stop making them an admin

UserAPI.php

• [x] Remove this spot where we add a guide to all of a user's policies

guideReassignment.php

• [x] Eh there is some place where we do a wacky thing and add the guide to all of a user's policies. I am fairly confused about why we do this... there's a comment that says:

// If the user doesn't already have an assigned guide, we can just add the new guide to all // possible policies. Otherwise, its possible the user has policies that the Assigned Guide is // not a part of. Attempting to remove them from these policies will cause an error, so we don't // want to remove the old guide, or assign a new one as there may not be a guide for a reason.

Which is extremely vague. But I think we can remove this whole block of code. When we call SetAssignedGuide in Auth without a policyID then they $.assignedGuide data is added for all the policies.

• [ ] GuidesPoliciesCleanup job also needs cleanup as it removes guides from policies. We need to remove guides from #admins room

[MonilBhavsar]

I have got drafts up and testing the flow

[MonilBhavsar]

Regarding guideReassignment script, we need clarification where it needs to be handled. In this issue or the other issue https://github.com/Expensify/Expensify/issues/402099#issuecomment-2165017503

[MonilBhavsar]

An interesting issue encountered.

This logic to find a assignable guide is - get the guide who hasn't been shared the policy lately. Now that we're no longer adding guides as an admin/sharing policy with guides. We need to update that logic to find least recently added guide.

When we just add guide to the #admins room, we create an entry in the sharedReports table, but we don't have a timestamp column there. https://github.com/Expensify/Auth/blob/61117d6bb2f0afeeec35109f1317b92d6867d70b/auth/command/SelectAssignedGuide.cpp#L60-L67

I am having couple of questions in my mind -

1. With which data point we'll be replacing the query with. I was thinking about storing timestamp in assignedGuide data. And it also arises question 2.
2. How do we backfill the assigned guide data since it's already stored in the database. Are we going to temporarily break the logic to assign guides evenly among the customers?

I'll think more about this tomorrow

[MonilBhavsar]

One thought is how about we forget about timestamps and assign the guide that has lowest assignments, or is present in the lowest admins room. Said other way, we try to keep number of deals/assignments with all guides equal?

For example: If guideA is member of 3 admin rooms and guideB is member of 5 admin rooms. Next time, we want to pick a guide, we pick A and they become member of 4 admin rooms. Next time, again we pick A and they become member of 5 admin rooms.

Now, since A and B are in equal rooms, we pick any one randomly. Let's say we picked A, and now A is a member of 6 admin rooms. And next time, we want to pick a guide, we pick B and they become member of 6 admin rooms.

This will help us to equally divide the guides, i think

[MonilBhavsar]

One of the case where the above solution does not work better would be the case where a new guide joins the team and since they would have 0 assignments, they would be added to all rooms for a long time, till they have assignments equal to that of the team.

[puneetlath]

What would happen if we went pure round robin? Or even just selected randomly.

[davidcardoza]

I think we should move to a pure round-robin system, as it seems the easiest. The current logic was designed to ensure a new lead who creates a workspace is assigned to a Guide who is online at that time to provide immediate support. However, our Guides are currently managing hundreds of leads, making it impossible for them to respond immediately. Switching to a pure round-robin system will simplify our process and ensure even distribution of leads.

[MonilBhavsar]

I think it was random before we implemented this logic, but I think it didn't work well especially because as Doza mentioned our goal was to connect with the guide who would respond asap(queue0).

I am thinking how can we achieve round robin with minimal changes and current data storage

[MonilBhavsar]

For round robin, I'm thinking to store, last assigned index in the nameValuePairs table, and next time we want to pick a guide we increment that index and choose a guide. Index is derived from current_index ?? 0 % total_num_of guides

For example:

1. We query guides, sorted by accountID? and then store them in a vector. {g1, g2. g3, g4, g5}
2. We query lastAssignedIndex from nameValuePairs table(default to 0, it will be needed for first time).
3. We decide which guide to choose by lastAssignedIndex % totalNumberOfGuides. In this case 0 % 5 which gives 0 and guide g1.
4. We increment the index and update the nameValuePairs table. The value in DB will now be 1.

In next cycle,

We choose guide 2 because 1 % 5 = 1 and so on...

[puneetlath]

That sounds good to me.

[davidcardoza]

I like that, let's go with your plan @MonilBhavsar

[MonilBhavsar]

Working on the PR's Aim to get Auth PR ready for review today

[MonilBhavsar]

Noticed this case. I think it's a feature and not a bug and we improved the guide selection logic. Curious for your thoughts @puneetlath @davidcardoza

Let's say there are two guides - g1 and g2 And an admin a1 guide g1 creates a policy p1 for a1 and they are automatically assigned guide for p1 (not picked through round robin).

Currently, Next time, when a new user creates a new policy, we assign g2 because g1 was recently shared a policy.

With new round robin update, Next time, when a new user creates a new policy, we assign g1 because g1 not recently picked by round robin algorithm.

[MonilBhavsar]

Oh and also reassigning guide using supportal tool, also kind of breaks round robin as we pick a guide manually. I think that's okay since the tool is not used frequently and only when current guide is offboarded

[puneetlath]

I feel like both of those scenarios are fine, but I'll defer to @davidcardoza

[davidcardoza]

With new round robin update, Next time, when a new user creates a new policy, we assign g1 because g1 not recently picked by round robin algorithm.

I think this logic is actually preferred. Guides have been vocal about receiving assignments for leads that other Guides are already managing. Ideally, When a user creates multiple Workspace they are permanently assigned the same Guide

Oh and also reassigning guide using supportal tool, also kind of breaks round robin as we pick a guide manually. I think that's okay since the tool is not used frequently and only when current guide is offboarded

The tool is used pretty frequently, but the manual assignment logic the tool provides is completely fine.

[MonilBhavsar]

I think this logic is actually preferred. Guides have been vocal about receiving assignments for leads that other Guides are already managing

Nice! It should not happen ideally because we check current assigned first before picking a new guide. But if there is ongoing issue, would be good to make a new issue to investigate and fix it.

The tool is used pretty frequently, but the manual assignment logic the tool provides is completely fine.

👍

[davidcardoza]

@MonilBhavsar Per our conversation in Slack, we agreed that we'll also need to update the wonTimestamp tool to remove Guides from admins room moving forward. Do you want me to create a separate GH for that?

[MonilBhavsar]

Yes, that will be great. How about tackling this in steps -

1. We deploy the PR's in this issue to stop sharing policies with guides
2. We run the manual script to unshare guide's policies
3. We update the wontimestamp tool to remove the guides from #admins room instead of policy since at this time, no guide would be in the customer's policy.

cc @puneetlath

[davidcardoza]

That plan sounds good to me. Do we need to add a step to update Guide reassignment and bulk reassignment tool too?

[MonilBhavsar]

It will be updated in this issue, so that's step 1

[MonilBhavsar]

Update: Very close to get Auth PR in review. I noticed automated tests for assigning guides SelectAssignedGuide are failing for a timezone case. I'm working on to update the round robin logic and fix the test

[MonilBhavsar]

Okay, i figured it out. I have to change the implementation to queue based approach. I'm working on it and testing it.

[puneetlath]

Sounds good to me, thanks.

[MonilBhavsar]

Update: The queue based approach(circular queue) also had some limitations in even distribution of leads among guides. The issue was everytime, we have the sorted list of eligible guides. And even if we make a queue of those guides, and assign them to the user, it didn't work for some cases and distribution was very much uneven.

To fix that we had to persist the queue somewhere. But I though storing the list of accountID's is not a good idea. We also had to sync it when a new guide is added or removed. So I went with the approach that we most commonly use - store the lastAssigned time for each guide in NVP and get the one who hasn't been assigned since a while. The NVP value is updated when a guide is assigned. This fixed the issues and worked for all cases.

...And with that Auth PR is ready for review https://github.com/Expensify/Auth/pull/11246 Aiming to get Web-E also out for review today, but it will be held on Auth PR

[MonilBhavsar]

I also need to handle this before merging Auth PR https://github.com/Expensify/Auth/pull/11246#issuecomment-2182860467

[davidcardoza]

store the lastAssigned time for each guide in NVP and get the one who hasn't been assigned since a while. The NVP value is updated when a guide is assigned. This fixed the issues and worked for all cases.

Just to confirm for my own knowledge, does the new lastAssigned NVP update every time a new lead is assigned to the Guide?

[MonilBhavsar]

Yes, that's correct. We initialise it once when guide is onboarded and then it updates everytime a new lead is assigned

[davidcardoza]

Cool, that should make a pretty good even distribution of leads between Guides.

[MonilBhavsar]

PR being reviewed and actively being worked on

[MonilBhavsar]

Created this issue to run a CQ to initialize expensify_guideLastAssignedTime NVP in guides account https://github.com/Expensify/Expensify/issues/407440

[MonilBhavsar]

There are many PR's and issues around. So I'm listing it here

Deployment plan

1. [x] 1. Complete this issue to initialize NVP in guides account https://github.com/Expensify/Expensify/issues/407440
2. [x] 2. Merge this PR to set NVP in guides account in fixAccount, if not set https://github.com/Expensify/Web-Expensify/pull/42476
3. [x] 3. Merge and CP this PR to production to unblock the auth PR below https://github.com/Expensify/Web-Expensify/pull/42466
4. [x] 4. Merge this approved Auth PR https://github.com/Expensify/Auth/pull/11246
5. [x] 5. Merge this Web-E PR https://github.com/Expensify/Web-Expensify/pull/42353
6. [x] 6. Make a PR to update GuidesPoliciesCleanup job. I'm working on it.

[MonilBhavsar]

@puneetlath @davidcardoza after 5 above is deployed to production. We should no longer be sharing policies with guides and I think that's the right time to run the manual script once again to unshare policies from guides.

[puneetlath]

Sounds great!

[MonilBhavsar]

@davidcardoza there is this one workflow with calendly webhook, where user schedules the appointment with calendly and we create a policy for them, assign SDR to it and add SDR as an admin to that policy.

https://github.com/Expensify/Web-Expensify/blob/b82d61878cb7d0e7f29f4c99a53a51f731e8dd5d/lib/Calendly/API.php#L250-L297

Do we need to stop sharing policies with SDR's? I'm not completely sure as SDR's are different from guides as per this https://stackoverflowteams.com/c/expensify/questions/16067

[muttmuure]

We don't use SDRs anymore

[muttmuure]

I think we can just remove that workflow entirely

[davidcardoza]

Yeah, the SDR program has been sunset for about a year now, that webhook workflow can be removed. We didn't do it at the time of sunsetting the program because it was going to require a technical resource to remove it.

[MonilBhavsar]

Thank you both for your thoughts. I'm not going to update that webhook code to add SDR's to admins room. Can remove that code later to cleanup.

It would be great to update this SO https://stackoverflowteams.com/c/expensify/questions/16067 that SDR's are deprecated

[MonilBhavsar]

Auth PR is deployed https://github.com/Expensify/Auth/pull/11246 And Web-E PR is review. I hope we can deploy it today(Thursday PST time), so we can have it on staging over the weekend and monitor it

[MonilBhavsar]

Addressed reviews on PR. I'm going to be ooo until 8th of July. I will try my best to address comments on this issue/PR

[muttmuure]

Do we need someone to keep up with this while you're out?

[cead22]

We got the PR merged!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@MonilBhavsar Still overdue 6 days?! Let's take care of this!

[melvin-bot[bot]]

@MonilBhavsar Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[MonilBhavsar]

PR's for GuidesPoliciesCleanup (last task) are up!

[melvin-bot[bot]]

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