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

[$250] QBO - Lag in connecting to QBO integration to appear #44225

Closed
1 of 6 tasks
m-natarajan opened this issue Jun 23, 2024 · 40 comments
Closed
1 of 6 tasks

[$250] QBO - Lag in connecting to QBO integration to appear #44225

m-natarajan opened this issue Jun 23, 2024 · 40 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@m-natarajan
Copy link

m-natarajan commented Jun 23, 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: 9.0.1-0
Reproducible in staging?: y
Reproducible in production?: y
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: @jamesdeanexpensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1718998564033989

Action Performed:

  1. Go to workspace
  2. Enable Accounting
  3. Connect to QBO

Expected Result:

After connected to QBO integration settings should appear (Export import and advanced)

Actual Result:

User has to switch to another menu item and come back to appear or it lags to appear

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

2024-06-21_12-34-18.mp4
Recording.239.mp4
2024-06-21_12-38-43.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~010007783bfe9633b1
  • Upwork Job ID: 1806492121279205749
  • Last Price Increase: 2024-07-05
  • Automatic offers:
    • brunovjk | Reviewer | 103032600
    • nkdengineer | Contributor | 103032602
Issue OwnerCurrent Issue Owner: @brunovjk
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 23, 2024
Copy link

melvin-bot bot commented Jun 23, 2024

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

@m-natarajan m-natarajan changed the title QBO - Lag in connecting to QBO integration to appear QBO/Xero - Lag in connecting to QBO integration to appear Jun 23, 2024
@m-natarajan m-natarajan changed the title QBO/Xero - Lag in connecting to QBO integration to appear QBO - Lag in connecting to QBO integration to appear Jun 23, 2024
@mallenexpensify
Copy link
Contributor

@s77rt , I see you've worked on the QBO project, any thoughts here?

@s77rt
Copy link
Contributor

s77rt commented Jun 25, 2024

This seems to be pusher related. cc @aldo-expensify since you fixed this previously #40377

@aldo-expensify
Copy link
Contributor

This is know, there is this slack conversation about it: https://expensify.slack.com/archives/C036QM0SLJK/p1718817925842049

We don't have a proposed solution yet.

@mallenexpensify
Copy link
Contributor

@aldo-expensify , can it be external?

@aldo-expensify
Copy link
Contributor

Don't know really, I'm not sure if it possible to solve this with a purely front end solution or if we need stuff done in the backend.

@aldo-expensify
Copy link
Contributor

Maybe we can add External and see if someone has a purely frontend proposal that can work?

@mallenexpensify mallenexpensify added the External Added to denote the issue can be worked on by a contributor label Jun 28, 2024
@melvin-bot melvin-bot bot changed the title QBO - Lag in connecting to QBO integration to appear [$250] QBO - Lag in connecting to QBO integration to appear Jun 28, 2024
Copy link

melvin-bot bot commented Jun 28, 2024

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

@melvin-bot melvin-bot bot added Overdue Help Wanted Apply this label when an issue is open to proposals by contributors labels Jun 28, 2024
Copy link

melvin-bot bot commented Jun 28, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Jun 28, 2024
@mallenexpensify
Copy link
Contributor

Maybe we can add External and see if someone has a purely frontend proposal that can work?

I like it! @brunovjk , do you think it can be external!!?

@nkdengineer
Copy link
Contributor

nkdengineer commented Jun 28, 2024

Proposal

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

User has to switch to another menu item and come back to appear or it lags to appear

What is the root cause of that problem?

When checking isSyncInProgress here

We only check that connectionSyncProgress.stageInProgress !== CONST.POLICY.CONNECTIONS.SYNC_STAGE_NAME.JOB_DONE, we don't check if the connection data is already available in the app.

So there's a few seconds between "Job done" Pusher update and connection data Pusher update, and the loading indicator does not show and connection data also doesn't show up during this period.

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

When checking isSyncInProgress, if the stageInProgress is JOB_DONE, but the connection data for that specific connection is not there yet, isSyncInProgress will still be true

It will become false only after both stageInProgress becomes JOB_DONE and connection data becomes available.

We can update this line to something like

(connectionSyncProgress.stageInProgress !== CONST.POLICY.CONNECTIONS.SYNC_STAGE_NAME.JOB_DONE || !policy.connections?.[connectionSyncProgress.connectionName]) &&

As now there could be a brief period where stageInProgress is JOB_DONE, and we need to show the description under the connection item accordingly, indicating it's waiting for the connection data. Here add an item

case 'jobDone':
     return 'Waiting for imported data to load';

The copy can be decided by design team.

What alternative solutions did you explore? (Optional)

There could be cases where isSyncInProgress should only check JOB_DONE and not policy.connections data as I suggested. In this case we can keep isSyncInProgress logic as is, and introduce a new variable isSyncInProgressOrConnectionDataUnavailable that has the suggested logic, and use that new variable where we show the UI (like for loading indicator and connection data list)

In the condition we may not have to check [connectionSyncProgress.connectionName] individually but just check policy.connections empty or not. But the main solution is more precise.

@nkdengineer
Copy link
Contributor

Proposal updated to add an alternative solution

@nkdengineer
Copy link
Contributor

nkdengineer commented Jun 28, 2024

Proposal updated to clarify the change on connection description

@brunovjk
Copy link
Contributor

brunovjk commented Jun 28, 2024

Maybe we can add External and see if someone has a purely frontend proposal that can work?

I like it! @brunovjk , do you think it can be external!!?

@mallenexpensify I believe so. We need to update how we indicate the status to the user after connecting. I tested the main solution from @nkdengineer' proposal, and it looks good to me. I tested and the update in isSyncInProgress appears to solve the issue well. We just need to confirm the copy for the warning indicating that it's waiting for the connection data, right?

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Jun 28, 2024

Triggered auto assignment to @deetergp, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@jamesdeanexpensify
Copy link
Contributor

@deetergp I also just found the Slack thread where this was discussed recently, might have some helpful info for you!

Copy link

melvin-bot bot commented Jul 1, 2024

@deetergp, @mallenexpensify, @brunovjk Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Jul 7, 2024

@deetergp @LLPeckham @mallenexpensify @brunovjk this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Jul 7, 2024
@jamesdeanexpensify
Copy link
Contributor

jamesdeanexpensify commented Jul 8, 2024

We could also do something fun like Almost there... on this last loading step - but don't let that suggestion be a blocker if nothing can be changed further!

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 8, 2024
Copy link

melvin-bot bot commented Jul 8, 2024

📣 @brunovjk 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Jul 8, 2024

📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot removed the Overdue label Jul 8, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Jul 9, 2024
@nkdengineer
Copy link
Contributor

@brunovjk this PR is ready for review

@brunovjk
Copy link
Contributor

PR reached production 5 days ago, it looks like the automation didn't work here. @mallenexpensify can you help me, so I can fill out the checklist? Thank you.

@mallenexpensify
Copy link
Contributor

Contributor: @nkdengineer paid $250 via Upwork
Contributor+: @brunovjk paid $250 via Upwork

@brunovjk plz complete the BZ checklist below.

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:

  • [@] The PR that introduced the bug has been identified. Link to the PR:
  • [@] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@] Determine if we should create a regression test for this bug.
  • [@] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@bz] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@mallenexpensify mallenexpensify added Daily KSv2 Weekly KSv2 and removed Weekly KSv2 Reviewing Has a PR in review labels Jul 22, 2024
@brunovjk
Copy link
Contributor

  • [@brunovjk] The PR that introduced the bug has been identified. Link to the PR: N/A
  • [@brunovjk] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
  • [@brunovjk] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
  • [@brunovjk] Determine if we should create a regression test for this bug. Yes
  • [@brunovjk] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

  1. Go to a Workspace.
  2. Enable Accounting.
  3. Connect with an accounting provider.
  4. Verify that the UI updates with status messages throughout the process.
  5. Ensure that integration settings (Export, Import, Advanced) appear immediately after connection.

Do we agree 👍 or 👎

@LLPeckham LLPeckham removed their assignment Jul 25, 2024
@brunovjk
Copy link
Contributor

Do you think we can close this issue @mallenexpensify ? Thank you :D

Copy link

melvin-bot bot commented Jul 29, 2024

@deetergp, @mallenexpensify, @brunovjk, @nkdengineer Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Jul 29, 2024
@deetergp deetergp removed the Weekly KSv2 label Jul 29, 2024
@mallenexpensify
Copy link
Contributor

Test case created, closing. Thanks all!

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. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Status: Done
Development

No branches or pull requests

9 participants