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

QBO - Changing to other option does not save for the first time #49372

Closed
5 of 6 tasks
izarutskaya opened this issue Sep 18, 2024 · 44 comments
Closed
5 of 6 tasks

QBO - Changing to other option does not save for the first time #49372

izarutskaya opened this issue Sep 18, 2024 · 44 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review Weekly KSv2

Comments

@izarutskaya
Copy link

izarutskaya commented Sep 18, 2024

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: v9.0.37-0
Reproducible in staging?: Y
Reproducible in production?: N
Found when validation PR: #48277
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Prerequisite: Your workspace needs to connect with QBO.

  1. Go to Workspace -> Accounting
  2. Go to Export -> Export invoice to
  3. Choose another account

Expected Result:

Chosen option saves

Actual Result:

When changing option for the first time Chosen option returns back to the initial option.

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6607200_1726624177582.Screen_Recording_2024-09-18_at_4.28.11_at_night.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @aldo-expensify
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. DeployBlocker Indicates it should block deploying the API labels Sep 18, 2024
Copy link

melvin-bot bot commented Sep 18, 2024

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

Copy link

melvin-bot bot commented Sep 18, 2024

Triggered auto assignment to @grgia (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Sep 18, 2024
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@izarutskaya
Copy link
Author

I added 2 labels because I am not sure what I need to use. The behavior with Use Staging Server disabled is different when running in production and staging with Use Staging Server enabled. Check my video please

Recording.2826.mp4

@grgia
Copy link
Contributor

grgia commented Sep 18, 2024

Potentially related to #48277

@hoangzinh
Copy link
Contributor

@dangrous can you double check if the API UpdateQuickbooksOnlineReceivableAccount is deployed to Prod? Thank you

@dangrous
Copy link
Contributor

Hm yeah it's on prod... And I just double checked the new code from your PR, it's effectively no different from the old code. @aldo-expensify do you think this might be the sync issue we're dealing with separately? Like the initial sync hasn't yet completed?

@dangrous
Copy link
Contributor

yeah i think this is the sync issue - I just tried on my own staging account, the first time i switched the setting (right after connecting) it switched back, but then i just tried it again after waiting a few seconds, and it worked.

@izarutskaya can you confirm if changing the setting - after waiting at least 10 seconds or so after connecting QBO - does work as intended? If so, I think we can close this and handle the syncing separately.

@zfurtak
Copy link
Contributor

zfurtak commented Sep 18, 2024

@dangrous if you need any frontend work for it, I can help 😊

@grgia
Copy link
Contributor

grgia commented Sep 18, 2024

@dangrous the bug is specific to the first time

@grgia
Copy link
Contributor

grgia commented Sep 18, 2024

testing again now

@grgia
Copy link
Contributor

grgia commented Sep 18, 2024

image I made a new workspace and waited 10 min but no avail the 1st time Seems the funky update is coming from this `getMissingONYXUpdates` call @dangrous

@grgia
Copy link
Contributor

grgia commented Sep 18, 2024

Right after I call

UpdateQuickbooksOnlineReceivableAccount

{
    "jsonCode": 200,
    "requestID": "8c528932cdf5bd9d-LHR",
    "onyxData": [
        {
            "key": "policy_96C3808269C266A8",
            "onyxMethod": "merge",
            "value": {
                "connections": {
                    "quickbooksOnline": {
                        "config": {
                            "receivableAccount": {
                                "currency": "USD",
                                "glCode": "",
                                "id": "1150040001",
                                "name": "Testing New Dot"
                            }
                        }
                    }
                }
            }
        }
    ],
    "previousUpdateID": 1920887604,
    "lastUpdateID": 1920893835
}

I see a call to GetMissingOnyxMessages

        {
            "key": "policy_96C3808269C266A8",
            "onyxMethod": "merge",
            "value": {
                "connections": {
                    "quickbooksOnline": {
                        "config": {
                            "receivableAccount": {
                                "currency": "USD",
                                "glCode": "",
                                "id": "53",
                                "name": "Accounts Receivable"
                            }
                        }
                    }
                }
            }
        },

So after I refresh it's persisted the original call- technically it has saved

@grgia grgia removed the DeployBlockerCash This issue or pull request should block deployment label Sep 18, 2024
@grgia
Copy link
Contributor

grgia commented Sep 18, 2024

Demoting as an app blocker since it's an erroneous onyx update

@grgia grgia added Daily KSv2 and removed Hourly KSv2 Daily KSv2 labels Sep 18, 2024
@mananjadhav
Copy link
Collaborator

I haven't checked it yet. I can try to take a look from FE side.

@aldo-expensify
Copy link
Contributor

Now I have my dev env fully working again. From testing I can see that the IS code call several times SavePolicy during import and during sync. Each call to SavePolicy will save some onyx updates to the database that are later processed by the job www-prod/SendFetchableOnyxUpdates that pushes them to the clients.

Considering this, I guess it is expected that the onyx updates will arrived in any order since:

  • The jobs www-prod/SendFetchableOnyxUpdates are not executed in order, and
  • The pushed events not necessarily arrive in order because of the network (specially if they come from different jobs).

This makes it very likely that some calls to GetMissingOnyxUpdates will happen because of the bad order.

@aldo-expensify
Copy link
Contributor

No updates, I've been focusing on this other QBO quality issue: #49371 which has a more clear solution.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 19, 2024
@aldo-expensify
Copy link
Contributor

Holding on #49371 before continuing in case the fix there helps with this too.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 21, 2024
@aldo-expensify
Copy link
Contributor

aldo-expensify commented Oct 25, 2024

Still waiting for #51110 to be reviewed...

@melvin-bot melvin-bot bot removed the Overdue label Oct 25, 2024
Copy link

melvin-bot bot commented Oct 28, 2024

@JmillsExpensify, @aldo-expensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Oct 28, 2024
@aldo-expensify
Copy link
Contributor

#51110 was merged, I'll get back on testing this asap

@melvin-bot melvin-bot bot removed the Overdue label Oct 29, 2024
@muttmuure
Copy link
Contributor

Great!

@melvin-bot melvin-bot bot added the Overdue label Oct 31, 2024
Copy link

melvin-bot bot commented Nov 1, 2024

@JmillsExpensify, @aldo-expensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@aldo-expensify
Copy link
Contributor

aldo-expensify commented Nov 2, 2024

So I have been working today on this, and I have manage to reproduce it a few times, but is hard... most of the time you don't have enough time to go to the setting and change it. I have tried with slow connections and it doesn't make it easier.

I'll be also OOO next week, so considering that and the above, I'll lower the priority to Weekly because I don't think this can happen easily for a real user.

Having said that, I think I have an idea of how to fix this in the Integration-Server. The current behaviour is:

  1. We import the data from QBO
  2. Call the command Policy_Connection_Set to store it
  3. Pick default configs using the data read in step 1
  4. Call the command Policy_Connection_Set to store the default configs

I want to change this to:

  1. We import the data from QBO
  2. Pick default configs using the data read in step 1
  3. Call the command Policy_Connection_Set to store the imported data and the default configs

If we do a single Policy_Connection_Set call, it won't be possible that the frontend receives the imported data in a separate update from the one containing the default configs.

@melvin-bot melvin-bot bot removed the Overdue label Nov 2, 2024
@aldo-expensify
Copy link
Contributor

@aldo-expensify aldo-expensify added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Nov 2, 2024
@aldo-expensify
Copy link
Contributor

Closing this, the backend PR was deployed last week: https://github.com/Expensify/Integration-Server/pull/8184

@trjExpensify
Copy link
Contributor

Thanks for sticking with this one, Aldo.

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. Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review Weekly KSv2
Projects
Status: Release 2.5: SuiteWorld (Sept 9th)
Development

No branches or pull requests

10 participants