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.48k stars 2.83k forks source link

[$250] Accounting tab appears then disappears after selecting an integration during onboarding #50637

Open lanitochka17 opened 5 days ago

lanitochka17 commented 5 days 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.48-0 Reproducible in staging?: Y Reproducible in production?: N/A Unable to check If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): sdjsdudsiuhi@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Sign up with a new Gmail account
  3. Select Manage my team's expenses on onboarding modal
  4. Select employee amount > Next
  5. Choose any integration > Next
  6. Complete the onborading
  7. Go to workspace settings

Expected Result:

Accounting tab will not appear and disappear

Actual Result:

Accounting tab appears then disappears

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/ffcd907c-b369-4e3e-8b96-e855e540cf9b

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021844716640793633414
  • Upwork Job ID: 1844716640793633414
  • Last Price Increase: 2024-10-11
Issue OwnerCurrent Issue Owner: @allroundexperts
melvin-bot[bot] commented 5 days ago

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

melvin-bot[bot] commented 5 days ago

💬 A slack conversation has been started in #expensify-open-source

github-actions[bot] commented 5 days 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.
nkuoch commented 5 days ago

I don't think this needs to be a blocker, so removing the labels, but yes let's find someone to work on it!

melvin-bot[bot] commented 5 days ago

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

melvin-bot[bot] commented 5 days ago

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

NJ-2020 commented 5 days ago

Proposal

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

Accounting tab appears then disappears after selecting an integration during onboarding

What is the root cause of that problem?

Right here when we complete the onboarding we set the areConnectionsEnabled to true: https://github.com/Expensify/App/blob/abdfb7233e040a61d1e4433e202a8b644c7455f4/src/libs/actions/Report.ts#L3647-L3677

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

We should remove this code https://github.com/Expensify/App/blob/abdfb7233e040a61d1e4433e202a8b644c7455f4/src/libs/actions/Report.ts#L3647-L3677

What alternative solutions did you explore? (Optional)

twilight294 commented 5 days ago

Edited by proposal-police: This proposal was edited at 2024-10-11 14:52:46 UTC.

Proposal

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

Accounting tab appears then disappears after selecting an integration during onboarding

What is the root cause of that problem?

We only set the areConnectionsEnabled value optimistically to true here, but then when we call the API, we only sent areConnectionsEnabled and not the policyID, so the BE won't update areConnectionsEnabled for that policy and when we get response from the api, the accounting is disabled.

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

In CompleteGuidedSetupParams pass onboardingPolicyID as well, this makes sure we are referencing the correct policyID.:

https://github.com/Expensify/App/blob/abdfb7233e040a61d1e4433e202a8b644c7455f4/src/libs/actions/Report.ts#L3716

Note: BE changes would also be needed to update the areConnectionsEnabled value of the policy.

We do the same thing in enablePolicyConnections here

What alternative solutions did you explore? (Optional)

s77rt commented 4 days ago

Oh we forgot to send the policy id cc @nkdengineer. Please raise a PR for this since it's a regression

@marcaaron regarding https://github.com/Expensify/App/pull/49161#discussion_r1765675105, what's the param that we should use to send the policy id over?

s77rt commented 4 days ago

@allroundexperts I'd like to handle this being a "regression" from https://github.com/Expensify/App/pull/49161

nkdengineer commented 4 days ago

@marcaaron regarding https://github.com/Expensify/App/pull/49161#discussion_r1765675105, what's the param that we should use to send the policy id over?

@s77rt I will raise the PR when we confirm this. Or we can do this in the second PR of the new feature when the task translation is confirmed.

marcaaron commented 4 days ago

Should be policyID.

marcaaron commented 4 days ago

Or we can do this in the second PR of the new feature when the task translation is confirmed.

That sounds fine to me! Let's make sure to add that QA step.

marcaaron commented 4 days ago

@nkuoch I will take this over as it's related to something I missed when working with these guys.

twilight294 commented 4 days ago

Hey this had Help Wanted Label attached and I posted a solution here which will be used in the PR, I guess contributors are compensated in such case right @marcaaron ?

marcaaron commented 4 days ago

Hey @twilight294 I'd suggest bringing that up in Slack or shooting an email over to contributors@expensify.com. Typically, when something is a regression it's the responsibility of the person who implemented the PR. I appreciate the help and can provide this document to you review. And will also say that there are always exceptions. But I can't personally provide any guarantees right now.

twilight294 commented 4 days ago

Thanks for responding, I will write a mail to contributors@expensify.com to discuss more on this 😄

s77rt commented 2 days ago

Not overdue. Will be fixed in next PR