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

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

Closed
6 tasks done
lanitochka17 opened this issue Jul 17, 2024 · 48 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Jul 17, 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.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 #44170

Action Performed:

Pre-requisite:

  • Have a collect workspace
  • Enable delayed submission for the workspace
  • Create either a QBO or Xero connection
  1. Navigate to staging.new.expensify.com
  2. Sign in as the admin of the workspace
  3. Create an expense in the workspace chat
  4. Navigate to the combined expense report page and click on submit
  5. Wait for a few seconds and observe the header button
  6. Navigate back to the workspace chat page
  7. Navigate back to the combined expense report page

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?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6544931_1721208467307.bandicam_2024-07-17_12-18-06-082.mp4

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
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Jul 17, 2024
Copy link

melvin-bot bot commented Jul 17, 2024

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

Copy link
Contributor

👋 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 francoisl removed the DeployBlocker Indicates it should block deploying the API label Jul 17, 2024
@francoisl
Copy link
Contributor

francoisl commented Jul 17, 2024

Going to look at that PR #44170 to see if anything stands out.

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

@francoisl
Copy link
Contributor

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 😕

@francoisl francoisl added the External Added to denote the issue can be worked on by a contributor label Jul 17, 2024
@melvin-bot melvin-bot bot changed the title Accounting - Export button is shown first instead of showing Pay button for delayed submission [$250] Accounting - Export button is shown first instead of showing Pay button for delayed submission Jul 17, 2024
Copy link

melvin-bot bot commented Jul 17, 2024

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

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

melvin-bot bot commented Jul 17, 2024

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

@francoisl
Copy link
Contributor

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

@kosmydel
Copy link
Contributor

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
Copy link
Contributor

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

Workflows config:

  • Delayed submission: Enabled
  • Approvals: Disabled
  • Payments: Enabled

Result: The pay button appears immediately after clicking Submit.

2024-07-18_10-41-39.mp4

Workflows config:

  • Delayed submission: Enabled
  • Approvals: Disabled
  • Payments: Disabled

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

2024-07-18_10-44-43.mp4

@mountiny mountiny added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Jul 18, 2024
Copy link

melvin-bot bot commented Jul 18, 2024

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
Copy link
Contributor

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
Copy link
Contributor

lakchote commented Jul 18, 2024

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

Screen.Recording.2024-07-18.at.13.26.37.mov

@mountiny
Copy link
Contributor

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 melvin-bot bot removed the Overdue label Sep 3, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 11, 2024
@sakluger
Copy link
Contributor

Still on hold for #43783

@melvin-bot melvin-bot bot removed the Overdue label Sep 11, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 20, 2024
@francoisl
Copy link
Contributor

still on hold for the above ^

@melvin-bot melvin-bot bot removed the Overdue label Sep 24, 2024
@melvin-bot melvin-bot bot added the Overdue label Oct 2, 2024
@francoisl

This comment was marked as outdated.

@melvin-bot melvin-bot bot removed the Overdue label Oct 2, 2024
@melvin-bot melvin-bot bot added the Overdue label Oct 11, 2024
@francoisl
Copy link
Contributor

Still on hold for the above

@sakluger
Copy link
Contributor

Still on hold.

@melvin-bot melvin-bot bot removed the Overdue label Oct 21, 2024
@melvin-bot melvin-bot bot added the Overdue label Oct 29, 2024
@sakluger
Copy link
Contributor

sakluger commented Oct 31, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Oct 31, 2024
@hoangzinh
Copy link
Contributor

I prefer a retest.

@francoisl
Copy link
Contributor

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

Screen.Recording.2024-11-01.at.2.42.48.PM.mov

@melvin-bot melvin-bot bot added the Overdue label Nov 11, 2024
@sakluger
Copy link
Contributor

Let's close since Francois can't reproduce.

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. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
Status: Release 2.5: SuiteWorld (Sept 9th)
Development

No branches or pull requests

9 participants