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.56k stars 2.9k forks source link

[HOLD #43783][$250] Accounting - Submit button flickers in and out for non-admin submitter immediately after clicking Submit #45624

Closed lanitochka17 closed 6 days ago

lanitochka17 commented 4 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: 9.0.8-0 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Issue found when executing PR https://github.com/Expensify/App/pull/44170

Action Performed:

Pre-requisite:

Expected Result:

Right after a non-admin submitter clicks submit, the Submit button should disappear and no other button should show up.

Actual Result:

After a non-admin submits the expense, a Pay button shows up where the Submit button was before.

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/a11a832f-89ee-4b3d-94f7-6728d83897d8

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~011ac532d9ba102642
  • Upwork Job ID: 1813660172360646886
  • Last Price Increase: 2024-08-07
melvin-bot[bot] commented 4 months ago

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

github-actions[bot] commented 4 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.
francoisl commented 4 months ago

Going to look at that PR https://github.com/Expensify/App/pull/44170 to see if anything stands out.

In the meantime, /cc @kosmydel @hungvu193 if you have any idea

francoisl commented 4 months ago

The issue seems to be that in the rerender right after calling IOU.submitReport(), the money request's statusNum is still 0, and so shouldShowPayButton here is set to false (because isOpenExpenseReport here is true)

image

We correctly update the optimistic data of the IOU here in submitReport() though, so I can't tell why the statusNum is wrong in the memo 😕

melvin-bot[bot] commented 4 months ago

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

melvin-bot[bot] commented 4 months ago

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

francoisl commented 4 months ago

In the meantime, here's a draft revert if we can't figure out a fix.

kosmydel commented 4 months ago

This problem must have been present before, but instead of displaying the "Export" button, no button was shown at all.

const shouldShowExportIntegrationButton = !shouldShowPayButton && !shouldShowSubmitButton && connectedIntegration && !!policy;

As we can see, the Pay button has higher priority, and if it is displayed, the export integration button must be hidden.

trjExpensify commented 4 months ago

Okay, I did two tests on production to corroborate that theory a bit, @kosmydel.

Workflows config:

Result: The pay button appears immediately after clicking Submit.

https://github.com/user-attachments/assets/bb5b5d21-1154-4601-8a46-12eb5b5238e5

Workflows config:

Result: There's a brief flash of the Submit button after clicking submit, and then no button.

https://github.com/user-attachments/assets/b9545d30-4d8e-4e97-a714-00ef52b11b7e

melvin-bot[bot] commented 4 months ago

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

mountiny commented 4 months ago

We are going to demote this one as it is specific to the admin submitting on a policy with a specific setup so it is not very common case, additionally, it can be fixed by revisiting the report.

Looking for a proposal to fix this flicker of the incorrect button

lakchote commented 4 months ago

With this PR that fixes the Export option showing wrongfully for workspace members (it should only be for admins), there's still a glitch.

Now, here's what happens:

Pay button -> Submit button briefly showing -> Pay button

https://github.com/user-attachments/assets/868d6428-c276-477f-a037-a6cd2d2916d4

mountiny commented 4 months ago

Nice, thanks for testing again. This just supports the theory the optimistic data is somewhat incorrect probably for this specific submit and close setting

melvin-bot[bot] commented 3 months ago

@francoisl, @sakluger, @hoangzinh Eep! 4 days overdue now. Issues have feelings too...

sakluger commented 3 months ago

Still looking for proposals.

melvin-bot[bot] commented 3 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 3 months ago

@francoisl, @sakluger, @hoangzinh Huh... This is 4 days overdue. Who can take care of this?

sakluger commented 3 months ago

Still no new proposals.

melvin-bot[bot] commented 3 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

hoangzinh commented 3 months ago

awaiting for proposals

melvin-bot[bot] commented 3 months ago

@francoisl @sakluger @hoangzinh 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!

sakluger commented 3 months ago

No new proposals.

melvin-bot[bot] commented 3 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

hoangzinh commented 3 months ago

Bump again on Slack https://expensify.slack.com/archives/C01GTK53T8Q/p1723047317165469

war-in commented 3 months ago

Hi 👋 I'm Marcin from Software Mansion, an expert agency, and I started investigating this issue.

I found out that after clicking the submit button SubmitReport API call is performed and it returns correct stateNum and statusNum values (both different than 0).

But immediately after this call, there is another one GetMissingOnyxMessages and it sets statusNum and stateNum to 0 again (which is wrong after a successful submit call).

From another issue, I know that GetMissingOnycMessages is used when there are some gaps detected in the onyx update. Does anyone have some context and maybe ideas on what is happening here?

war-in commented 3 months ago

What's even more interesting is that it's not always reproducible. It happens often after creating a new workspace but not always 🤷

trjExpensify commented 3 months ago

Trying to think who would have the most context on this flow, @mountiny probably for manual Submit from the drafts project?

trjExpensify commented 3 months ago

Also, @sakluger, I think the OP and title of this issue are outdated. The export to Pay problem is solved, it's not the Pay/Submit scenarios when re-reading the history.

Non-admin submitter gets a brief flicker of the submit button: https://github.com/Expensify/App/issues/45624#issuecomment-2236098279 Admin submitter gets a brief flicker of pay button, submit button, pay button again: https://github.com/Expensify/App/issues/45624#issuecomment-2236273958

Can you update to reflect the current state of the problems left to solve? Thanks!

trjExpensify commented 3 months ago

Ah, I think we can close this and progress the Approve and Submit button flickers here. The investigation is pretty advanced there.

hoangzinh commented 3 months ago

I'm unsure if they have the same root cause, but based on reproducing steps it looks similar. Yep I think we can close.

mountiny commented 3 months ago

It seems like some race condition, and later, we sent the missed onyx updates, which reverted the state of the report.

What is confusing is that its not reliably reproducible with drafts/ submit flow based on the comments so it does not seem like an issue in the Submit flow purely

hoangzinh commented 3 months ago

Yep, agree with @mountiny, it seems related to missing Onyx updates flow.

hoangzinh commented 3 months ago

Should we put this issue on hold for https://github.com/Expensify/App/issues/43783?

francoisl commented 3 months ago

Sounds good. Hopefully the fix for https://github.com/Expensify/App/issues/43783 will also fix this, let's revisit when it's sorted out.

sakluger commented 3 months ago

Sorry guys, catching up on this one. Thanks for putting this on hold for https://github.com/Expensify/App/issues/43783!

Regarding the correct expected behavior vs actual behavior, a non-admin submitter should see no button after clicking submit, right? Or is there also still an issue here for admin submitters?

trjExpensify commented 3 months ago

Correct that non-admin submitter case what happens is that it flickers. I.e the submit button disappears, comes back briefly, goes away again.

sakluger commented 3 months ago

I'm going to move this to weekly while it's on hold.

sakluger commented 2 months ago

Still on hold

hoangzinh commented 2 months ago

Still on hold

sakluger commented 2 months ago

Still on hold for https://github.com/Expensify/App/issues/43783

francoisl commented 1 month ago

still on hold for the above ^

francoisl commented 1 month ago

Still on hold

francoisl commented 1 month ago

Still on hold for the above

sakluger commented 3 weeks ago

Still on hold.

sakluger commented 2 weeks ago

Looks like the hold issue (https://github.com/Expensify/App/issues/43783) has been closed because it's no longer reproduceable, and a new, similar issue is being worked on here: https://github.com/Expensify/App/issues/43783#issuecomment-2434525091. @francoisl what do you think we should do for this one? Should we get a retest?

hoangzinh commented 2 weeks ago

I prefer a retest.

francoisl commented 2 weeks ago

Yeah let's get a retest please, I can't repro anymore on my end.

https://github.com/user-attachments/assets/a655dc7a-da04-44b3-9056-d50298090290

sakluger commented 6 days ago

Let's close since Francois can't reproduce.