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

QBD - Admin is not selected as default Preferred exporter after connecting to QBD #52134

Open
1 of 8 tasks
lanitochka17 opened this issue Nov 6, 2024 · 13 comments
Open
1 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Nov 6, 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.58-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: https://expensify.testrail.io/index.php?/tests/view/5184323
Email or phone of affected tester (no customers): applausetester+dqbd7@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

Precondition: Have a workspace connected to QBD

  1. Navigate to Workspace > Accounting
  2. Click on Export
  3. Click Preferred exporter

Expected Result:

Admin is selected as default Preferred exporter after connecting to QBD

Actual Result:

Admin(owner) is not selected as default Preferred exporter after connecting to QBD

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
Bug6656407_1730886082792.Screen_Recording_2024-11-06_at_12.33.31_in_the_afternoon.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021854584654634877527
  • Upwork Job ID: 1854584654634877527
  • Last Price Increase: 2024-11-07
Issue OwnerCurrent Issue Owner: @hoangzinh
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 6, 2024
Copy link

melvin-bot bot commented Nov 6, 2024

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

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Nov 6, 2024

Edited by proposal-police: This proposal was edited at 2024-11-06 20:19:47 UTC.

Proposal

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

QBD - Admin is not selected as default Preferred exporter after connecting to QBD

What is the root cause of that problem?

The main problem is that by default exporter is equal to an empty string

And since in order to use the owner as a spare value exporter must be undefined or null (specifics of ?? operator)
We will never be able to use owner value

title: qbdConfig?.export?.exporter ?? policyOwner,

isSelected: (currentExporter ?? policy?.owner) === exporter.email,

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

To fix this bug we can replace ?? with ||
So that we can use owner if exporter is empty

And I suppose we need to check QBO just in case
Since the code for QBO and QBD are very similar to make sure there is no similar bug there

What alternative solutions did you explore? (Optional)

NA

Additional
I found an additional bug:

  1. If we add an user and assign them as admins
  2. Then we will select them as an Preferred exporter
  3. Then removed from admins
  4. It will still be selected as a preferred exporter

To fix this we can do an additional check for the current exporter and check if it is in the list of admins using the getAdminEmployees function
And in case of its absence we can again use owner as the current exporter

Or make a more complex solution and in case of connected integration and removal of admins from workspace we can make additional requests to save a preferred exporter as owner

And we will also need to check the QBO (and possibly other integrations)

@garrettmknight garrettmknight added the External Added to denote the issue can be worked on by a contributor label Nov 7, 2024
Copy link

melvin-bot bot commented Nov 7, 2024

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

@melvin-bot melvin-bot bot changed the title QBD - Admin is not selected as default Preferred exporter after connecting to QBD [$250] QBD - Admin is not selected as default Preferred exporter after connecting to QBD Nov 7, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 7, 2024
Copy link

melvin-bot bot commented Nov 7, 2024

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

@hoangzinh
Copy link
Contributor

Hi @lakchote, I am concerned about whether it's a BE issue (like we should set qbdConfig?.export?.exporter to policyOwner in the BE after connecting to QBD).
Basically, this issue can be fixed in FE as @ZhenjaHorbach's proposed here, however will it cause any inconsistency behavior between FE and BE? FE infers exporter as policyOwner if it's an empty string, but do we have same logic in BE?

@lakchote
Copy link
Contributor

@hoangzinh I don't think there is a problem at all. And I do agree that if there ever was, we should instead fix it in the backend and not the frontend.

QBD integration backend logic hasn't changed regarding the preferred exporter and it works fine on OldDot.

cc @francoisl for your input.

@melvin-bot melvin-bot bot added the Overdue label Nov 11, 2024
@lakchote
Copy link
Contributor

Not overdue, see my comment above.

@hoangzinh
Copy link
Contributor

Oh thanks for your hint @lakchote. I just compared it with OD. In OD if the "exporter" is empty, it will show the policy Owner as a default exporter. We can apply this logic in FE ND 👀
Screenshot 2024-11-11 at 16 56 26

@melvin-bot melvin-bot bot removed the Overdue label Nov 11, 2024
@lakchote
Copy link
Contributor

@hoangzinh thanks for testing it out in OldDot. I had trouble connecting to QBD this morning (Web connector screen stuck).

Let's do @ZhenjaHorbach's solution then?

It wouldn't be eligible for payment though, as @ZhenjaHorbach you were the one in charge of handling the feature in #50297 and it could be considered as a regression from OldDot.

@hoangzinh
Copy link
Contributor

Let's do @ZhenjaHorbach's solution then?

Yep, I think so.

@ZhenjaHorbach
Copy link
Contributor

I will create a PR soon

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Nov 11, 2024
Copy link

melvin-bot bot commented Nov 12, 2024

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

@hoangzinh
Copy link
Contributor

hey @aldo-expensify. We already have @lakchote as an internal engineer here.

@lakchote lakchote assigned lakchote and unassigned aldo-expensify Nov 12, 2024
@lakchote lakchote changed the title [$250] QBD - Admin is not selected as default Preferred exporter after connecting to QBD QBD - Admin is not selected as default Preferred exporter after connecting to QBD Nov 12, 2024
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. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Reviewing Has a PR in review Weekly KSv2
Projects
Status: Bugs and Follow Up Issues
Development

No branches or pull requests

6 participants