Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.59k stars 2.93k forks source link

[$250] QBO-If "Export company card expenses as" is set to "Vendor bill", Locations toggle is locked #50112

Closed lanitochka17 closed 1 month ago

lanitochka17 commented 2 months ago

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.43-1 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5027294 Issue reported by: Applause - Internal Team

Action Performed:

  1. Launch app
  2. Go to workspace settings
  3. Connect QBO
  4. Tap export
  5. Set export company card expense to "vendor bills"
  6. Set export out of pocket expense to "journal entry"
  7. Navite to import
  8. Tap locations -- import

Expected Result:

If "Export company card expenses as" is set to "Vendor bill", the Locations toggle must not be locked

Actual Result:

If "Export company card expenses as" is set to "Vendor bill", the Locations toggle is locked

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/66d9415a-b335-434d-aa29-c0c683ce039e

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021843661642289072627
  • Upwork Job ID: 1843661642289072627
  • Last Price Increase: 2024-10-15
Issue OwnerCurrent Issue Owner: @shubham1206agra
melvin-bot[bot] commented 2 months ago

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.

lanitochka17 commented 2 months ago

@sonialiap FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

mkzie2 commented 2 months ago

Proposal

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

If "Export company card expenses as" is set to "Vendor bill", the Locations toggle is locked

What is the root cause of that problem?

https://github.com/Expensify/App/blob/f475642a182c37e95a9d271119d4ca6290d8da11/src/pages/workspace/accounting/qbo/import/QuickbooksLocationsPage.tsx#L25-L27

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

    const canImportLocation =
        qboConfig?.reimbursableExpensesExportDestination === CONST.QUICKBOOKS_REIMBURSABLE_ACCOUNT_TYPE.JOURNAL_ENTRY &&
        qboConfig?.nonReimbursableExpensesExportDestination === CONST.QUICKBOOKS_NON_REIMBURSABLE_EXPORT_ACCOUNT_TYPE.VENDOR_BILL;

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

allgandalf commented 1 month ago

Edited by proposal-police: This proposal was edited at 2024-10-08 14:55:26 UTC.

Proposal

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

Location toggle is locked even when it is allowed on the BE.

What is the root cause of that problem?

We block the location toggle for vendor bills :

https://github.com/Expensify/App/blob/f475642a182c37e95a9d271119d4ca6290d8da11/src/pages/workspace/accounting/qbo/import/QuickbooksLocationsPage.tsx#L25-L27

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

If the expected result is to allow the toggle then remove the second condition:


 const canImportLocation = 
     qboConfig?.reimbursableExpensesExportDestination === CONST.QUICKBOOKS_REIMBURSABLE_ACCOUNT_TYPE.JOURNAL_ENTRY 

What alternative solutions did you explore? (Optional)

allgandalf commented 1 month ago

[!NOTE] I checked the API response when we set it to vendor bill and the BE accepts this case:

Screenshot 2024-10-08 at 8 22 22 PM
shubham1206agra commented 1 month ago

@allgandalf Can you check the condition on OldDot too?

allgandalf commented 1 month ago

yeah i too had a double guess here, let me check

allgandalf commented 1 month ago

Do you know where we check imports on OD?

Where's import?

Screenshot 2024-10-08 at 8 38 15 PM
allgandalf commented 1 month ago

Not allowed in OD:

Screenshot 2024-10-08 at 8 52 39 PM
shubham1206agra commented 1 month ago

@sonialiap Which behavior is correct here?

hungvu193 commented 1 month ago

This seems not an issue according to this docs.

cc @hayata-suenaga since I think you implemented this feature (https://github.com/Expensify/App/pull/41638).

melvin-bot[bot] commented 1 month ago

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

shubham1206agra commented 1 month ago

We are waiting for @hayata-suenaga to respond.

paultsimura commented 1 month ago

@hayata-suenaga doesn't work at Expensify anymore. I doubt you'll get a response for this 🤔

garrettmknight commented 1 month ago

@zanyrenney might be able to help out!

JmillsExpensify commented 1 month ago

Pretty sure that vendor bills support locations, so would love to hear from @zanyrenney on which is right.

allgandalf commented 1 month ago

@JmillsExpensify noting here that BE ALLOWS the toggle, so we are only restricting it on the FE. So i guess you're right here:

image

zanyrenney commented 1 month ago

Hi! This is working as expected and is not a bug for QBO on Collect Workspaces.

Vendor Bills only support importing Locations as a Report Field on QBO. Since we did not build report fields for Collect (we said this was a control feature), Locations on Vendor Bills should be locked.

Pretty sure that vendor bills support locations, so would love to hear from @zanyrenney on which is right.

They do but only as a report field cc @JmillsExpensify

I could have sworn someone was taking on Report Fields as a project for QBO/Xero. I saw a pre-design about it in Control.

zanyrenney commented 1 month ago

Let me get a bit more info from the doc and have a look in Slack for that pre-design.

Hope the answer above helps @shubham1206agra @hungvu193 !

zanyrenney commented 1 month ago

2024-10-15_15-03-20

From the Design Doc!

zanyrenney commented 1 month ago

Here is the pre-design on Report Fields support for QBO / Xero that I thought would build this added "Control" functionality atop this QBO project.

Happy to help get this added if not though! cc @dylanexpensify

melvin-bot[bot] commented 1 month ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

zanyrenney commented 1 month ago

We can close this cc @sonialiap

melvin-bot[bot] commented 1 month ago

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