-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[HOLD for payment 2024-06-03] [QBO] Not all default settings & accounts are selected when establishing the connection to QBO #41046
Comments
Current assignee @trjExpensify is eligible for the Bug assigner, not assigning anyone new. |
I think here is the OldDot code choosing the default settings: https://github.com/Expensify/Web-Expensify/blob/52310ff233e1bca38dd0c3238336646c1c66e640/site/deprecated/lib_policy_quickbooksOnline.js#L370-L409 |
Created draft PR for the integration server to select the default values during the first import: https://github.com/Expensify/Integration-Server/pull/7882 |
@trjExpensify, @s77rt Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Whoops! We need to assign Aldo here, he's actively working on PRs to address these. :) |
For choosing the defaults, what I did is:
The configuration at this point is what I used as a reference for the default in the QBO flow from NewDot. Do you think that is correct? |
^^ working theory on the above is that this PR will fix them: #41155 |
Haven't investigated this again. no updates |
@trjExpensify, @s77rt, @aldo-expensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
What's the plan? Wait for the other PRs to hit staging and re-test? |
Can we retest this and summarize what is left to fix? I think some of the things mentioned in this issue must have been fixed, right? |
@trjExpensify @s77rt @aldo-expensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
I checked on this, and the value of |
The default value is set to I'll make a PR to fix this. |
Once this PR hits production, I gonna test on staging to check if all default values are set correctly and then if it tests well, I'll close this issue |
Yep, sounds good. Would be good to know how close we are! |
the PR wasn't been deployed yet because of the freeze. I think the fire issue has been fixed, and once deployed, I'll test this 👍 |
#42102 is deployed to staging. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.75-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-06-03. 🎊 For reference, here are some details about the assignees on this issue:
|
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:
|
tested well for me test.1.mov |
We keep this issue open to handle the payment for @s77tr for this App PR. |
|
Payment Summary
BugZero Checklist (@trjExpensify)
|
Payment summary as follows:
Offer sent! |
@trjExpensify Accepted! Thanks! |
Paid. Closing! |
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: v1.4.66-1
Reproducible in staging?: Y
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @trjExpensify
Slack conversation: https://expensify.slack.com/archives/C036QM0SLJK/p1714060188157459?thread_ts=1714057353.559839&cid=C036QM0SLJK & https://expensify.slack.com/archives/C036QM0SLJK/p1714091531288649?thread_ts=1713799261.297609&cid=C036QM0SLJK
Action Performed:
Expected Result:
Export page
Preferred exporter
=[policyOwner]
Date
=Export date
Export out of pocket expenses as
=Export as
=Vendor bill
Accounts payable
=[first applicable account in the list is selected]
Note: This select list appears to be completely blank right now, let's fix thatExport invoices as
=[first applicable account in the list is selected]
Export company cards as
=Export as
=Credit card
Account
=[first applicable account in the list is selected]
"Export out of pocket expenses as" alt settings
Export as
=Journal entry
Accounts payable
=[first applicable account in the list is selected]
Export as
=Check
Accounts payable
=[first applicable account in the list is selected]
"Export company cards as" alt settings
Export as
=Debit card
Account
=[first applicable account in the list is selected]
Export as
=Vendor bill
Accounts payable
=[first applicable account in the list is selected]
Default vendor
(if enabled, disabled by default)Vendor
=[first applicable account in the list is selected]
Sync reimbursed reports
(setting should be toggled on by default) =QuickBooks bill payment account
=[first applicable account in the list is selected]
QuickBooks invoice collections account
=[first applicable account in the list is selected]
Credit card
toVendor bill
, make sure the value in the push input for the associated account selection is also updated along with the push label change. I.eExport as
=Credit card
Account
=Amex Card Account
Export as
=Vendor bill
Accounts payable
=[first applicable account in the vendor list is selected]
Actual Result:
Preferred exporter
list looks to be selected by default on the export settings pageCredit Card
) the first value in the sub-account that's revealed is not selectedWorkaround:
Yes, they can select values and change values.
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
2024-04-26_01-51-56.mp4
View all open jobs on GitHub
CC: @hayata-suenaga @aldo-expensify @s77rt @teneeto @narefyev91 @zanyrenney
Issue Owner
Current Issue Owner: @trjExpensifyThe text was updated successfully, but these errors were encountered: