[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] [Guided Setup Stage 2] "Meet your specialist" setup task not using proper Guide's calendar link

[francoisl]

Version Number: N/A (v1.4.70-0)
Reproducible in staging?: Yes
Reproducible in production?: Yes
Email or phone of affected tester (no customers): Any new account
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @francoisl
Slack conversation:

Action Performed:

1. Create a new account
2. During the onboarding flow, pick the option "manage my team's expenses"
3. After completing the onboarding modal steps, open the task "Meet your setup specialist"
4. Look at the "schedule a call" link

Expected Result:
The link should point to the assigned guide's calendly link.

As discussed in this thread, we need to remove the guide link from the message if not present at the time of flow completion.

If the guide link is present in NewDot in time, no changes to the current flow.

If not, the message will omit the link and as the task is created in the backend, we need to recreate the message correctly with the link. This is the internal task in this case

Actual Result:

The link points to the fallback new.expensify.com value, despite having a value in Onyx

App/src/libs/actions/Report.ts

Line 3055 in c5a1f96

guideCalendarLink: guideCalendarLink ?? CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL,

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Issue Owner

Current Issue Owner: @Christinadobrzyn

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018008e7f232e1b5aa
• Upwork Job ID: 1788750602254168064
• Last Price Increase: 2024-05-10

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (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.

[melvin-bot]

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

[melvin-bot]

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

[trjExpensify]

No idea why Melvin did that, will look at it here.

I've tried to restore Franc's wonderful words in the OP!

[ShridharGoel]

Proposal

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

"Meet your specialist" setup task not using proper Guide's calendar link

What is the root cause of that problem?

guideCalendarLink will be undefined if the task's data got created before we get the guidesCalendarLink from backend.

App/src/libs/actions/Report.ts

Lines 167 to 173 in 7636b01

	let guideCalendarLink: string | undefined;
	Onyx.connect({
	key: ONYXKEYS.ACCOUNT,
	callback: (value) => {
	guideCalendarLink = value?.guideCalendarLink ?? undefined;
	},
	});

App/src/libs/actions/Report.ts

Lines 3051 to 3057 in 7636b01

	const taskDescription =
	typeof task.description === 'function'
	? task.description({
	adminsRoomLink: `${CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL}${ROUTES.REPORT_WITH_ID.getRoute(adminsChatReportID ?? '')}`,
	guideCalendarLink: guideCalendarLink ?? CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL,
	})
	: task.description;

This will take the link only if it is already available, else new.expensify.com will be used.

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

If guideCalendarLink is undefined while creating the task description, we should add it as an empty string while creating the optimistic data.

Then, backend can check if the link for "schedule a call" is empty in the description. If yes, then backend will add the guide's calendar link and send it in the task description.

App side change will be to change the fallback to '':

typeof task.description === 'function'
                ? task.description({
                      adminsRoomLink: `${CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL}${ROUTES.REPORT_WITH_ID.getRoute(adminsChatReportID ?? '')}`,
                      guideCalendarLink: guideCalendarLink ?? '',
                  })
                : task.description;

[Christinadobrzyn]

@rayane-djouah can you check out the above proposal when you have a moment? TY!

[rayane-djouah]

Thank you for the proposal @ShridharGoel, I agree that this is a backend bug.

The bug cause is that we build task description data in completeOnboarding function before the CreateWorkspace API request is finished, which returns the guideCalendarLink in the response.

But, changing the fallback to '' will break the link markdown. I think we should keep using NEW_EXPENSIFY_URL as a fallback and BE should handle that.

I think that the backend should return the updated task description with the guide calendar link in completeOnboarding response.

cc @mountiny

App/src/pages/OnboardingWork/BaseOnboardingWork.tsx

Lines 46 to 57 in 0f39e43

	const {adminsChatReportID} = Policy.createWorkspace(undefined, true, work);
	Report.completeOnboarding(
	onboardingPurposeSelected,
	CONST.ONBOARDING_MESSAGES[onboardingPurposeSelected],
	{
	login: currentUserPersonalDetails.login ?? '',
	firstName: currentUserPersonalDetails.firstName ?? '',
	lastName: currentUserPersonalDetails.lastName ?? '',
	},
	adminsChatReportID,
	);

App/src/libs/actions/Report.ts

Lines 3094 to 3101 in 0f39e43

	const tasksData = data.tasks.map((task, index) => {
	const taskDescription =
	typeof task.description === 'function'
	? task.description({
	adminsRoomLink: `${CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL}${ROUTES.REPORT_WITH_ID.getRoute(adminsChatReportID ?? '')}`,
	guideCalendarLink: guideCalendarLink ?? CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL,
	})
	: task.description;

[ShridharGoel]

Then should we update the fallback to concierge chat? Concierge can provide the specialist link if user sends a message.

[trjExpensify]

Discussing next steps in this thread.

[melvin-bot]

@Christinadobrzyn, @rayane-djouah Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[Christinadobrzyn]

Just checking in here @rayane-djouah do you have everything you need to plan the next steps for this? Do you need anything from the team on what this should look like/how this should behave?

[rayane-djouah]

This bug may be fixed as part of this: #41589. I will retest today.

[Christinadobrzyn]

Awesome thank you @rayane-djouah - I'll add a HOLD for #41589 while we retest

[Christinadobrzyn]

looks like this isn't fixed

2024-05-09_13-47-13.mp4

Monitoring - #41589

[rayane-djouah]

The bug didn't get fixed by #41589 and it is still reproducible. I think we should make this issue internal as it requires backend changes

[melvin-bot]

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

[melvin-bot]

Current assignee @rayane-djouah is eligible for the Internal assigner, not assigning anyone new.

[melvin-bot]

@Christinadobrzyn, @rayane-djouah Huh... This is 4 days overdue. Who can take care of this?

[Christinadobrzyn]

This is on @mountiny's radar so we'll keep this as is for now. Let me know if you'd prefer I seek another volunteer @mountiny!

[rayane-djouah]

unassigning myself as this is internal

[Christinadobrzyn]

Ah good reminder @rayane-djouah - don't we need a C+ on this?

[mountiny]

PR is in its final stretch now

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.81-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Do not add the guide calendar link optimistically #43236

If no regressions arise, payment will be issued on 2024-06-18. 🎊

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mountiny] The PR that introduced the bug has been identified. Link to the PR:
• [@mountiny] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mountiny] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@mountiny] Determine if we should create a regression test for this bug.
• [@mountiny] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@Christinadobrzyn] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[Christinadobrzyn]

Just a heads up - I'm going to be ooo until June 24th.

@mountiny Are there any contributors that need to be paid for this? Doesn't look like it?

[mountiny]

Yeah I think we can close this now

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.82-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Do not add the guide calendar link optimistically #43236

If no regressions arise, payment will be issued on 2024-06-20. 🎊

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mountiny] The PR that introduced the bug has been identified. Link to the PR:
• [@mountiny] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mountiny] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@mountiny] Determine if we should create a regression test for this bug.
• [@mountiny] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@Christinadobrzyn] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: