Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hybrid - Android - Copilot - User logged as copilot is able to switch to Expensify Classic. #53812

Closed
1 of 8 tasks
IuliiaHerets opened this issue Dec 10, 2024 · 11 comments
Closed
1 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Dec 10, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.73-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both
Issue was found when executing this PR: #52103
Email or phone of affected tester (no customers): ibellicotest+83@gmail.com
Issue reported by: Applause Internal Team

Action Performed:

  1. Open the Expensify app.
  2. Log in with an account that is a Copilot.
  3. Tap on "Settings"
  4. Scroll down and tap on "Switch to Expensify Classic"
  5. Verify that a "Not so fast" message is displayed and that you are not able to switch to Expensify Classic.

Expected Result:

User logged as a copilot, shouldn´t be able to switch to Expensify Classic.

Actual Result:

User logged as a copilot is able to switch to Expensify Classic and the "Not so fast" message is not displayed.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6689771_1733816663955.Copilot_Access.mp4

View all open jobs on GitHub

@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 10, 2024
Copy link

melvin-bot bot commented Dec 10, 2024

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

@ChavdaSachin
Copy link
Contributor

@IuliiaHerets I think this issue should be present on android/ios hybrid app and not on any of the standalone app.

@ChavdaSachin
Copy link
Contributor

@sonialiap it's coming from my pr,
Issue should be limited to the Hybrid apps.
And adding following code block here should solve the issue

                                  if (isActingAsDelegate) {
                                      setIsNoDelegateAccessMenuVisible(true);
                                      return;
                                  }

Since I am external contributor, I don't have access to hybrid app and I could not test it, and for the same reason the case was not handled in my pr.

@c3024
Copy link
Contributor

c3024 commented Dec 10, 2024

I wonder why it is failing on standalone app. This should not fail on standalone app, unless standalone app too returns true for NativeModules.HybridAppModule.

@ChavdaSachin
Copy link
Contributor

I checked android-standalone, but it is working as expected.
My guess is that tester might have marked standalone by mistake.

@IuliiaHerets
Copy link
Author

@ChavdaSachin I asked the tester to recheck. I'll be back soon with an answer

@IuliiaHerets
Copy link
Author

@ChavdaSachin Yes, you are right. Issue happens only on the hybrid app. Sorry for confusing

@IuliiaHerets IuliiaHerets changed the title Android - Copilot - User logged as copilot is able to switch to Expensify Classic. Hybrid - Android - Copilot - User logged as copilot is able to switch to Expensify Classic. Dec 10, 2024
@sonialiap sonialiap added Internal Requires API changes or must be handled by Expensify staff Hot Pick Ready for an engineer to pick up and run with labels Dec 11, 2024
@melvin-bot melvin-bot bot added the Overdue label Dec 12, 2024
@sonialiap
Copy link
Contributor

posted in slack for an internal engineer

@melvin-bot melvin-bot bot removed the Overdue label Dec 12, 2024
@Julesssss Julesssss assigned Julesssss and unassigned Julesssss Dec 12, 2024
@Julesssss
Copy link
Contributor

It should be a simple HybridApp change, but sorry, I can't commit to this. Ignore my brief assignment

@sonialiap
Copy link
Contributor

In a slack discussion it was agreed that we're ultimately moving towards allowing Copilot to take all actions so this is maybe not yet expected, but not a bug that we need to fix. Since it's not necessary to fix, closing

@dangrous
Copy link
Contributor

As discussed in the issue, we think this is probably fine - once https://github.com/Expensify/Mobile-Expensify/pull/13289 is merged we should be able to switch back and forth between old and new dot as a copilot. The only reason to try to fix this, I think, is if we think it's a dealbreaker to be able to switch to old dot and not back, for a limited time. We won't be able to switch back until the PR I linked is merged.

Thoughts?

If so, we can make the change @ChavdaSachin mentioned here, happy to act as the engineer on this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff
Projects
Development

No branches or pull requests

6 participants