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

izarutskaya commented 1 month 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: v9.0.37-0 Reproducible in staging?: Y Reproducible in production?: N Found when validation PR: https://github.com/Expensify/App/pull/48277 Email or phone of affected tester (no customers): dave0123seife@gmail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

Prerequisite: Your workspace needs to connect with QBO.

Go to Workspace -> Accounting
Go to Export -> Export invoice to
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?

[x] Android: Native
[x] Android: mWeb Chrome
[ ] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/e1e4a21c-cba3-47ac-841b-2c83e7495d4f

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @aldo-expensify

melvin-bot[bot] commented 1 month ago

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.

melvin-bot[bot] commented 1 month ago

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

github-actions[bot] commented 1 month ago

:wave: 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:

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

izarutskaya commented 1 month ago

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

https://github.com/user-attachments/assets/de976eb3-e323-4eb3-b80e-3045c3dbc866

grgia commented 1 month ago

Potentially related to https://github.com/Expensify/App/pull/48277

hoangzinh commented 4 weeks ago

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

dangrous commented 4 weeks ago

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 commented 4 weeks ago

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 commented 4 weeks ago

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

grgia commented 4 weeks ago

@dangrous the bug is specific to the first time

grgia commented 4 weeks ago

testing again now

grgia commented 4 weeks ago

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 commented 4 weeks ago

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 commented 4 weeks ago

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

grgia commented 4 weeks ago

Triaged but I'm already working on a wave-collect issue, unassigning

melvin-bot[bot] commented 4 weeks ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

aldo-expensify commented 4 weeks ago

I'll try to fix this

trjExpensify commented 3 weeks ago

Why did Melv put this to Monthly? Changing it back.

aldo-expensify commented 2 weeks ago

I'm having problems reproducing this in either dev or staging:

https://github.com/user-attachments/assets/7eabf7a6-170c-40dc-9dec-9012c19c90ab

From reading the thread above, it sounds like there may be some race condition where not all the updates containing the initial connection have arrived and we change the setting. This results in an old onyx update changing the setting back. Would be nice to be able to reproduce this more consistently.

I have problems with the pusher updates in dev, they are very flakey or they simply don't arrive at all.. which is making it though for me to debug/reproduce.

aldo-expensify commented 2 weeks ago

As @grgia mentioned, there is a call to GetMissingOnyxMessages (two really in my case) that happen after the connection is stablished without me taking any extra action:

Maybe I can investigate what is triggering these GetMissingOnyxMessages requests and try to prevent that.

melvin-bot[bot] commented 2 weeks ago

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

aldo-expensify commented 2 weeks ago

Was OOO yesterday, I'll try to get back to this on Wednesday or Thursday

melvin-bot[bot] commented 2 weeks ago

@JmillsExpensify @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!

aldo-expensify commented 1 week ago

haven't have time yet 😬 , I'll try again next week.

aldo-expensify commented 1 week ago

Didn't have time today.

melvin-bot[bot] commented 6 days ago

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

aldo-expensify commented 6 days ago

I'm getting back to this after lunch

aldo-expensify commented 6 days ago

So.. when I was preparing to work on this, I destroyed my VM by mistake and now I have been struggling with certificates for hours and I still can't get past it. I'll continue on this on Tuesday.

mananjadhav commented 2 days ago

Does this need help?

melvin-bot[bot] commented 2 days ago

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

aldo-expensify commented 2 days ago

Does this need help?

~Not for now~ maybe, I plan to get back to this today. Do you think there is room for improvement in the frontend? Currently I don't know where is the problem really

mananjadhav commented 2 days ago

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

aldo-expensify commented 2 days ago

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.

Expensify / App