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.59k stars 2.92k forks source link

[$250] Expense report-Submit & approve button not disappear until page refreshed #48463

Open izarutskaya opened 3 months ago

izarutskaya commented 3 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.28.0 Reproducible in staging?: Y Reproducible in production?: N Found when validation PR: https://github.com/Expensify/App/pull/48362 Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

Precondition: log into OD as an admin, create a control policy, enable workspace chats, select "Advanced Approvals", invite 3 members (non-admin): 2 approvers and 1 submitter, set the approval flow as submitter submits to approver 1

  1. Log into ND as submitter
  2. Navigate to the workspace chat
  3. Submit one manual expense
  4. Open the report and click on the "Submit" button
  5. As admin, change the the submitter's submitsTo to approver 2 in OD
  6. Log into ND as approver 1
  7. Opened the submitted report
  8. Click on the "Approve" button

Expected Result:

The 'Submit" button disappears immediately for the submitter (step 4) and the "Approve" button disappears for the approver (step 8)

Actual Result:

The "Submit" and "Approve" button do not disappear until the page is refreshed or it is clicked on the second time (the "Approve" button)

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/fbe312ce-13f0-4ca2-b847-90377f45606f

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01efec68070b933fc3
  • Upwork Job ID: 1831071506567798044
  • Last Price Increase: 2024-10-29
  • Automatic offers:
    • nkdengineer | Contributor | 104649054
Issue OwnerCurrent Issue Owner:
melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

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

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

We think this issue might be related to the #wave-control

izarutskaya commented 3 months ago

Production

https://github.com/user-attachments/assets/22cd9b0c-8fcf-4a91-a157-6a32110a047f

https://github.com/user-attachments/assets/226b3376-227d-489b-b719-69b05e0ddb4e

srikarparsi commented 3 months ago

Looked into this and I think this might be a problem with manually submitting reports. I verified this by:

  1. Creating a workspace
  2. Setting the submit frequency to manual
  3. Creating an expense
  4. Submitting the expense
  5. Seeing the submit button re-appear

(First part of the video is from staging and the second is from production)

https://github.com/user-attachments/assets/5f94e465-37f8-4c63-8a2c-e0edc2ef4563

srikarparsi commented 3 months ago

I don't think this is a backend bug because the SubmitReport response is the same on staging and production.

srikarparsi commented 3 months ago

I think this could be a possible culprit

situchan commented 3 months ago

I think this could be a possible culprit

No, that PR didn't hit staging yet

srikarparsi commented 3 months ago

Ah you're right thanks

srikarparsi commented 3 months ago

Hm, I'm not able to reproduce this in the latest dev.

https://github.com/user-attachments/assets/0396d84c-ea8b-419d-ada7-756d4b8aba57

srikarparsi commented 3 months ago

Still debugging on dev but going to add the external label to see if we can speed things up

melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

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

srikarparsi commented 3 months ago

Hmm, actually this might be backend. After the SubmitReport call, we do a GetMissingOnyxUpdates which is setting the status of the report back to 0 which is re-rendering the submit button. For some reason, this only happens in staging and not production:

image
srikarparsi commented 3 months ago

Seeing some flickering behavior on production as well occasionally:

https://github.com/user-attachments/assets/be7404be-37e0-494e-b88a-6a703d57338a

srikarparsi commented 3 months ago

Yeah actually I'm able to reproduce this on production pretty reliably:

https://github.com/user-attachments/assets/46ba12dd-5a7f-4599-8611-c33c5b216457

I think this is a larger problem with the SettlementButton. Since this is occurring on production as well, I think we can demote this from a DeployBlocker, what do you think @roryabraham?

roryabraham commented 3 months ago

Yep, agreed that since it's reproducible on prod we can demote this

nkdengineer commented 3 months ago

Edited by proposal-police: This proposal was edited at 2024-09-04 06:17:59 UTC.

Proposal

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

The "Submit" and "Approve" button do not disappear until the page is refreshed or it is clicked on the second time (the "Approve" button)

What is the root cause of that problem?

The "Submit" and "Approve" button do not disappear until the page is refreshed or it is clicked on the second time (the "Approve" button)

https://github.com/Expensify/App/blob/3e5da7810fdc48ce391db3ed3aa7dab86a59c490/src/libs/actions/IOU.ts#L6815-L6823

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

const isControlOnAdvancedApprovalMode = PolicyUtils.isControlOnAdvancedApprovalMode(policy);
if (isControlOnAdvancedApprovalMode) {
    const ownerAccountID = iouReport?.ownerAccountID ?? '-1';
    const employee = policy?.employeeList?.[ownerAccountID];
    return employee?.submitsTo === currentUserEmail && !isOpenExpenseReport && !isApproved && !iouSettled && !isArchivedReport;
}

What alternative solutions did you explore? (Optional)

eh2077 commented 2 months ago

@nkdengineer Thanks for your proposal.

The submit button still appears because after SubmitReport API is complete the GetMissingOnyxMessage API is called that returns the data of the expense report with stateNum and statusNum as 0.

Can you elaborate on this part? What's the current behaviour of the API that causes the bug? And what the expected behaviour should be for the API? So, it'll be easier for the internal engineering team to look into the issue.

nkdengineer commented 2 months ago

Can you elaborate on this part? What's the current behaviour of the API that causes the bug? And what the expected behaviour should be for the API?

The SubmitReport API returns the correct data with stateNum and statusNum as 1. After that GetMissingOnyxMessage API is called that returns stateNum and statusNum as 0 which causing the first bug.

melvin-bot[bot] commented 2 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

eh2077 commented 2 months ago

I agreed with @nkdengineer 's investigation. Let's go with their proposal.

@srikarparsi Please help with the backend part and all yours!

๐ŸŽ€๐Ÿ‘€๐ŸŽ€ C+ reviewed

melvin-bot[bot] commented 2 months ago

Current assignee @srikarparsi is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

srikarparsi commented 2 months ago

Hey @slafortune! My K2 is pretty full right now and since this is a backend issue I don't think I'll have time for it this week or next week. Do you think you could check if this fits into a wave/vip so that someone with bandwidth can maybe work on it? This is the SO for that I believe.

nkdengineer commented 2 months ago

@slafortune We also need the front-end change in this issue as I mentioned in my proposal. We can do it separately since the front-end bug doesn't require the BE change.

trjExpensify commented 2 months ago

Precondition: log into OD as an admin, create a control policy,

Moving to the #wave-control project.

eh2077 commented 2 months ago

Not overdue, we're waiting for the backend fix

melvin-bot[bot] commented 2 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 2 months ago

@slafortune @eh2077 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!

eh2077 commented 2 months ago

Not overdue, we're waiting for the backend fix

Not overdue. Same^

melvin-bot[bot] commented 2 months ago

@slafortune, @eh2077 Eep! 4 days overdue now. Issues have feelings too...

eh2077 commented 2 months ago

Not overdue, we're waiting for the backend fix

Not overdue. Same^

Same^

melvin-bot[bot] commented 2 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 2 months ago

@slafortune, @eh2077 Whoops! This issue is 2 days overdue. Let's get this updated quick!

eh2077 commented 2 months ago

Not overdue, we're waiting for the backend fix

Not overdue. Same^

Same^

Same^

@slafortune Can you help to ask an internal engineer to pick up this? Or maybe we can demote it to weekly

melvin-bot[bot] commented 2 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 2 months ago

@slafortune @eh2077 this issue is now 4 weeks old, please consider:

Thanks!

melvin-bot[bot] commented 1 month ago

@slafortune, @eh2077 Whoops! This issue is 2 days overdue. Let's get this updated quick!

eh2077 commented 1 month ago

@slafortune Can you ask an internal engineer to hot pick this one for the backend fix?

melvin-bot[bot] commented 1 month 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 1 month ago

@slafortune, @eh2077 Whoops! This issue is 2 days overdue. Let's get this updated quick!

eh2077 commented 1 month ago

Not overdue, we're still waiting for hot pick

melvin-bot[bot] commented 1 month ago

@slafortune, @eh2077 Whoops! This issue is 2 days overdue. Let's get this updated quick!

eh2077 commented 1 month ago

Bumped in slack https://expensify.slack.com/archives/C01GTK53T8Q/p1728660271578649

nkdengineer commented 1 month ago

@eh2077 Another thing, we have two problems in this issue and the frontend bug doesn't depend on the BE change then can you add a C+ review comment here again then we can move forward with the frontend bug.

melvin-bot[bot] commented 1 month ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

eh2077 commented 1 month ago

@eh2077 Another thing, we have two problems in this issue and the frontend bug doesn't depend on the BE change then can you add a C+ review comment here again then we can move forward with the frontend bug.

@nkdengineer Thanks for your comment. Can you check if the frontend bug is still reproducible? If so, then I think we can move forward with the frontend bug and handle the backend issue separately

slafortune commented 1 month ago

Since the GH does have the Hotpick tag, it is seen for an engineer to pick up. Once you've tested if this is reproducible, I can post it to a room to have the status updated to something appropriate ๐Ÿ‘

mvtglobally commented 1 month ago

Issue not reproducible during KI retests. (First week)