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.51k stars 2.86k forks source link

[QBO] Error message appears when change any of the configuration on Advanced tab #41092

Closed kbecciv closed 6 months ago

kbecciv commented 6 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: v1.4.66-3 Reproducible in staging?: y Reproducible in production?: n Issue found when executing PR: https://github.com/Expensify/App/pull/41002 Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to URL https://staging.new.expensify.com/
  2. Login with any Expensify account
  3. Create a workspace, enable the Accounting feature on the More Features page, and connect to Quickbooks Online on the Accounting page.
  4. Click on the Advanced tab and change any of the configurations (toggle on/off several times)

Expected Result:

No error message appears

Actual Result:

Error message appears

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/93399543/1e5e1c87-6d20-4fba-a744-4bb693726c9e

View all open jobs on GitHub

melvin-bot[bot] commented 6 months ago

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

github-actions[bot] commented 6 months 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:

  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.
kbecciv commented 6 months ago

We think that this bug might be related to #wave-collect - Release 1

mountiny commented 6 months ago

I believe some of these errors are known, the backend is not fully supporting this. cc @hayata-suenaga @aldo-expensify @trjExpensify @teneeto @narefyev91

mountiny commented 6 months ago

Its behind beta so not a blocker

rlinoz commented 6 months ago

FWIW the error is 400 Current value and value are using different types, expected both to be JSON

trjExpensify commented 6 months ago

I was just looking at that as well when testing to try and figure it out.

Network tab:

"400 Current value and value are using different types, expected both to be JSON"
onyxData
requestID
"87a7d5a0ba774164-LHR"

Here are the logs from a recent attempt.

Processing 'UpdatePolicyConnectionConfiguration' for 'expensify.com' from '104.28.154.112' ~~ command: 'UpdatePolicyConnectionConfiguration' policyID: '9D04CF0F37B62AC9' connectionName: 'quickbooksOnline' settingName: 'autoCreateVendor' settingValue: 'true' appversion: '1.4.66-2' pusherSocketID: '706150.195755' authToken: '<REDACTED>' referer: 'ecash' platform: 'web' api_setCookie: 'false' email: 'tom+41079v3@trj.chat' isFromDevEnv: 'false' partnerName: 'expensify.com' browserGUID: '662bd19a1b35e' HTTP_CF_BOT_SCORE: '99' HTTP_CF_JA3_HASH: '28f8b76bbfbc4a3e5733a62d625cd973'
[OOPS!] API Threw: Auth UpdatePolicyConnectionConfiguration returned an error ExpException - 83187a0d1e744a12a5bd7f3d584d169d ~~ message: '400 Current value and value are using different types, expected both to be JSON' exceptionMessage: 'Auth UpdatePolicyConnectionConfiguration returned an error' exceptionFile: '/git/releases/expensify.com/b62fe01/lib/Auth.php' exceptionLine: '125' exceptionCode: '400'

Comes from here in Auth.

    JSON::Value currentValueJSON(JSON::NIL);
    try {
        currentValueJSON = JSON::Value::parse(currentSettingValue);
    } catch (...) {
        // We're catching this so the comand doesn't fail, we just want to try to parse it and see if it's the same type
    }
    if (currentValueJSON.type() != requestValueJSON.type()) {
        STHROW("400 Current value and value are using different types, expected both to be JSON");
    }

Maybe something to do with this is wrong, or that settingValue=true which seems like a boolean and not a string maybe?

I also have another example here for settingName exportCompanyCardAccount, but that doesn't have a settingValue in the logs at all 🤔

Processing 'UpdatePolicyConnectionConfiguration' for 'expensify.com' from '104.28.154.112' ~~ command: 'UpdatePolicyConnectionConfiguration' policyID: '9D04CF0F37B62AC9' connectionName: 'quickbooksOnline' settingName: 'exportCompanyCardAccount' appversion: '1.4.66-2' pusherSocketID: '706150.195755' authToken: '<REDACTED>' referer: 'ecash' platform: 'web' api_setCookie: 'false' email: 'tom+41079v3@trj.chat' isFromDevEnv: 'false' partnerName: 'expensify.com' browserGUID: '662bd9f1e8eab' HTTP_CF_BOT_SCORE: '99' HTTP_CF_JA3_HASH: '28f8b76bbfbc4a3e5733a62d625cd973'
PolicyAPI::updatePolicyConnectionConfiguration(): Argument #5 ($settingValue) must be of type string, null given, called in /git/releases/expensify.com/b62fe01/api.php on line 2159"
requestID: "87a809c5d8a64164-LHR"
hayata-suenaga commented 6 months ago

maybe there is something when we're storing the boolean? the type of value received and the value read from the database isn't matching for some reason in this line

we can also log the type if that helps with debugging. What do you thing @aldo-expensify ?

trjExpensify commented 6 months ago

Another one:

image

Choosing an Accounts Payable account to export as is throwing the invalid setting name this time.

{jsonCode: 400, message: "400 Invalid setting name", onyxData: [], requestID: "87a853ed2b824164-LHR"}

Logs

Processing 'UpdatePolicyConnectionConfiguration' for 'expensify.com' from '104.28.154.112' ~~ command: 'UpdatePolicyConnectionConfiguration' policyID: '9D04CF0F37B62AC9' connectionName: 'quickbooksOnline' settingName: 'exportAccountPayable' settingValue: 'Accounts Payable (A/P) - NEW' appversion: '1.4.66-2' pusherSocketID: '706130.231519' authToken: '<REDACTED>' referer: 'ecash' platform: 'web' api_setCookie: 'false' email: 'tom+41079v3@trj.chat' isFromDevEnv: 'false' partnerName: 'expensify.com' browserGUID: '662be5cf02869' HTTP_CF_BOT_SCORE: '99' HTTP_CF_JA3_HASH: '28f8b76bbfbc4a3e5733a62d625cd973'

So that one is down to the settingName exportAccountPayable missing from here which we need to add.

trjExpensify commented 6 months ago

P.S - @narefyev91 PR here that you can test an adhoc build of here has cleared up a lot of the below type of error for me, as the account is selected when the export type is selected.

PolicyAPI::updatePolicyConnectionConfiguration(): Argument #5 ($settingValue) must be of type string, null given, called in /git/releases/expensify.com/b62fe01/api.php on line 2159"

Mainly want errors remain for me on that build are:

  1. That invalid setting name error for exportAccountPayable for company card export
  2. Toggling off default vendor shows an error
  3. Automatically create entities on the Advanced page shows an error when toggled on
melvin-bot[bot] commented 6 months ago

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

hayata-suenaga commented 6 months ago

Choosing an Accounts Payable account to export as is throwing the invalid setting name this time.

The error is coming from the following lines in the Auth code:

https://github.com/Expensify/Auth/blob/12244a365ca86635b327d6782a1dc66588a5f9f2/auth/command/UpdatePolicyConnectionConfiguration.cpp#L47-L49

The issue might be solved by this PR which was already merged

-> Yes! the missing keys were added in this PR around these lines of code

aldo-expensify commented 6 months ago

I don't see new errors for the missing settings after the deploy: https://www.expensify.com/_devportal/tools/logSearch/#query=blob%3A%20%22400%20Invalid%20setting%20name%22%20AND%20timestamp%3A%5B2024-04-25T00%3A00%20TO%202024-04-30T23%3A59%5D&index=_all

Please reopen if we find new missing settings