[HOLD for payment 2024-12-25] [$125] Copilot - Selection checkmark is displayed an access level option that is not verified yet

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.57-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Go Settings > Security > Add copilot
2. Select a user > Select 'Limited' for access level > Add copilot > Verify code
3. Click on the added copilot > Change access level > Select 'Full'
4. Click on RHP back button > Select 'Full' access again
5. Click on the added copilot again

Expected Result:

On Step 4, checkmark should be displayed on 'Limited' option, and user should be navigated to magic code page on clicking 'Full' option since the access level is Limited as we see it in step 5

Actual Result:

On step 4, checkmark is displayed on 'Full' option, and RHP is closed on clicking 'Full' option

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6654478_1730737964207.Screen_Recording_2024-11-04_at_7.21.42_in_the_evening.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021856451982020785346
• Upwork Job ID: 1856451982020785346
• Last Price Increase: 2024-12-05
• Automatic offers:
• nyomanjyotisa | Contributor | 105213634

Issue Owner

Current Issue Owner: @alitoshmatov

[melvin-bot]

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

[daledah]

Edited by proposal-police: This proposal was edited at 2024-11-04 17:31:49 UTC.

Proposal

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

On step 4, checkmark is displayed on 'Full' option, and RHP is closed on clicking 'Full' option

What is the root cause of that problem?

App/src/pages/settings/Security/AddDelegate/UpdateDelegateRole/UpdateDelegateRolePage.tsx

Line 70 in fcb4a5b

if (option.isSelected) {

We always dismiss modal if users click on selected option

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

we need to change the condition to

    const [account] = useOnyx(ONYXKEYS.ACCOUNT);
    const delegates = account?.delegatedAccess?.delegates ?? [];
    const currentRealRole = delegates.find((delegate) => delegate.email === login)?.role;

.......

               onSelectRow={(option) => {
                    if (option.isSelected && currentRealRole === option.value) {
                        Navigation.dismissModal();
                        return;
                    }

                    setCurrentRole(option.value);

                    if (currentRealRole !== option.value) {
                        requestValidationCode();
                        Navigation.navigate(ROUTES.SETTINGS_UPDATE_DELEGATE_ROLE_MAGIC_CODE.getRoute(login, option.value));
                    }
                }}

[nyomanjyotisa]

Edited by proposal-police: This proposal was edited at 2024-11-04 17:35:42 UTC.

Proposal

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

Copilot - Selection checkmark is displayed an access level option that is not verified yet

What is the root cause of that problem?

We set the selected value as the current role, although it is not saved yet

App/src/pages/settings/Security/AddDelegate/UpdateDelegateRole/UpdateDelegateRolePage.tsx

Line 76 in fcb4a5b

setCurrentRole(option.value);

And we set isSelected on the role options base on currentRole

App/src/pages/settings/Security/AddDelegate/UpdateDelegateRole/UpdateDelegateRolePage.tsx

Line 32 in fcb4a5b

isSelected: role === currentRole,

And we dismiss the modal if the selected role is currently has isSelected = true

App/src/pages/settings/Security/AddDelegate/UpdateDelegateRole/UpdateDelegateRolePage.tsx

Lines 70 to 73 in fcb4a5b

	if (option.isSelected) {
	Navigation.dismissModal();
	return;
	}

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

Remove this code and remove useState from currentRole since it not used anymore

const currentRole = route.params.currentRole;

What alternative solutions did you explore? (Optional)

[Nodebrute]

Proposal

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

Selection checkmark is displayed an access level option that is not verified yet

What is the root cause of that problem?

We are passing the shouldUpdateFocusedIndex. This will update the index when user will select the "other option"

App/src/pages/settings/Security/AddDelegate/UpdateDelegateRole/UpdateDelegateRolePage.tsx

Line 55 in fcb4a5b

shouldUpdateFocusedIndex

Also here we already set the current role to selected option

App/src/pages/settings/Security/AddDelegate/UpdateDelegateRole/UpdateDelegateRolePage.tsx

Line 76 in fcb4a5b

setCurrentRole(option.value);

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

Remove shouldUpdateFocusedIndex

App/src/pages/settings/Security/AddDelegate/UpdateDelegateRole/UpdateDelegateRolePage.tsx

Line 55 in fcb4a5b

shouldUpdateFocusedIndex

Let's remove this setCurrentRole

App/src/pages/settings/Security/AddDelegate/UpdateDelegateRole/UpdateDelegateRolePage.tsx

Line 76 in fcb4a5b

setCurrentRole(option.value);

and let's change the currentRole value to route.params.currentRole

What alternative solutions did you explore? (Optional)

[melvin-bot]

@CortneyOfstad 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

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

[melvin-bot]

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

[CortneyOfstad]

@alitoshmatov we have three proposals for review. Can you please take a look at your earliest convenience? Thanks!

[alitoshmatov]

@daledah Thank you for your proposal, please provide some explanations next time. Your code changes is not solving the issue completely. Checkmark is still being updated to Full option

[alitoshmatov]

@nyomanjyotisa Thank you for your proposal, Your RCA is correct and solution is working

[alitoshmatov]

@Nodebrute Thank you for your proposal, your RCA is correct and solution is working. But it is almost the same as @nyomanjyotisa 's proposal except shouldUpdateFocusedIndex part

[alitoshmatov]

I think we should go with @nyomanjyotisa 's proposal which was the first one to solve the issue correctly.

C+ reviewed 🎀 👀 🎀

[melvin-bot]

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

[melvin-bot]

@youssef-lr @CortneyOfstad @alitoshmatov this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[Nodebrute]

@alitoshmatov shouldUpdateFocusedIndex is an important part of the solution. With the selected proposal, we encounter an issue where the checkmark appears on one item while a different item is highlighted. I believe the chosen solution should at least be complete and not cause any regressions.

cc: @youssef-lr

[melvin-bot]

Upwork job price has been updated to $125

[melvin-bot]

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

[CortneyOfstad]

just dropping an update that the PR is actively being reviewed — great movement so far!

[CortneyOfstad]

@nyomanjyotisa and @alitoshmatov, I will be OOO starting this afternoon (December 20) and will be returning January 6th. A handful of folks on the BZ team will be online for a few days in between the 25th and the 1st, but we'll be operating with a skeleton crew. I will be issuing my payments when I return on January 6th. If you would like the payment issued sooner, please post this issue in #expensify-open-source and someone on the team will jump in.

Thank you!

[alitoshmatov]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: https://github.com/Expensify/App/pull/47984/files#r1899186243

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: No discussion needed

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

No regression test needed

[alitoshmatov]

Looks like automation failed here, completed the checklist. Only handling payment is left

[melvin-bot]

@youssef-lr, @CortneyOfstad, @alitoshmatov, @nyomanjyotisa Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@youssef-lr, @CortneyOfstad, @alitoshmatov, @nyomanjyotisa 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[alitoshmatov]

Requesting $125 in ND. Hopefully @CortneyOfstad could complete payment review

[CortneyOfstad]

Payment Summary

@nyomanjyotisa — paid $125 via Upwork
@alitoshmatov — to be paid $125 via NewDot

Regression Test

Not needed based on checklist here.

[JmillsExpensify]

$125 approved for @alitoshmatov