[$500] [Simple AA in NewDot] Reports with expenses on Hold by Approver A are being transferred to Approver B when approvers change

garrettmknight 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: N/A Reproducible in staging?: N/A Reproducible in production?: N/A Expensify/Expensify Issue URL: Issue reported by: @garrettmknight Slack conversation: https://expensify.slack.com/archives/C06ML6X0W9L/p1724850669484329?thread_ts=1724791781.089579&cid=C06ML6X0W9L

Action Performed:

Create a Control workspace in OldDot
Invite Submitter, Approver A, Approver B - all as employees
Set Submitter's submitsTo to Approver A
As Submitter, submits a report with two expenses to Approver A
As Approver A, hold one expense and approve the other.
As Admin, change Submitter's submits to Approver B
As Approver A, approve the unheld expense.

Expected Result:

No error, held expense is added to new report awaiting Approver A's approval in the processing state

Actual Result:

Error so Aprprover A can't approver just the unheld expnese. The held expense and report is now assigned to Approver B. Also, Approver B doesn't have GBR or RBR on the report + can't access in NewDot.

Workaround:

Not really - Approver B can only see the report by directly linking to it.

Platforms:

All

Screenshots/Videos

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021836334441203620774
Upwork Job ID: 1836334441203620774
Last Price Increase: 2024-10-03
Automatic offers:

allgandalf | Reviewer | 104270672

Issue Owner

Current Issue Owner: @garrettmknight

melvin-bot[bot] commented 3 months ago

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.

garrettmknight commented 3 months ago

WIP, I'm actually seeing the report is rejected back to submitter after step 5.

garrettmknight commented 2 months ago

Ok, retested and the rejected expense is still following an unexpected pattern, but I think I know why:

When an approver holds an expense and approves the remaining expenses, we create a new report for the held expense.
When hold was introduced, we only had Instant Submit. Now that we have Delay submission, I think we're just defaulting the new report to whatever the Delay submission setting says.
That means when an approver holds and expense and approves the others, the new report we're creating is created in a draft state, like any other new report would be created in on the delay submission setting.

This feels unexpected because this is effectively rejecting the expense back to the submitter, requiring they submit again in order to have the expense approved.

I think the fix would be to create the new report in the exact state that the original report was in:

Processing, not draft so we don't require resubmission.
Awaiting the current approver's approval - this ensures that if something's held later in an advanced workflow that we don't go all the way back to the initial approver.

garrettmknight commented 2 months ago

Confirming expected behavior here

garrettmknight commented 2 months ago

Expected behavior doesn't really exist so we're defining it.

garrettmknight commented 2 months ago

Expected behavior is outlined, and OP is updated. Opening it up.

robertjchen commented 2 months ago

Thanks! It looks like there's a minor frontend component in updating the existing logic to doing this optimistically as well, we can see if someone at the agency can help out with this part.

In the meantime, I'll update the logic on the backend.

garrettmknight commented 2 months ago

@robertjchen any reason not to use the External label? Feels like that might go a bit faster than getting someone from an agency.

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

hraheel8519 commented 2 months ago

Hey, Here is the technical explanation:

Step 1: Identifying the Root Cause

Investigate the reported issue thoroughly in the linked GitHub issue.
Review the existing React Native codebase, looking specifically at how the current platform-specific implementations differ.
Use debugging tools (such as React Developer Tools and Chrome DevTools) to reproduce the issue on various platforms.
Check for platform discrepancies or unhandled edge cases in component rendering and state management.

Step 2: Plan for Cross-Platform Consistency

Given that React Native’s goal is to unify front-end development, I will aim to isolate the platform-specific logic and refactor the code to create shared components.
Implement Platform module from React Native, which allows platform-specific code to be conditionally included.
Ensure consistent styling across platforms using StyleSheet.create, which helps maintain platform-specific UI.

Step 3: Code Refactor

Refactor or create a shared component that resolves the issue and works seamlessly across all platforms (iOS, Android, Web).
Update state management if needed, ensuring it doesn’t rely on platform-specific behaviors.

melvin-bot[bot] commented 2 months ago

📣 @hraheel8519! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

Make sure you've read and understood the contributing guidelines.
Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:
```
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
```

melvin-bot[bot] commented 2 months ago

Upwork job price has been updated to $500

allgandalf commented 2 months ago

Waiting for Proposals 👍

1subodhpathak commented 2 months ago

Hi! could you please upload a screen recording @garrettmknight @allgandalf

melvin-bot[bot] commented 2 months ago

@garrettmknight, @robertjchen, @allgandalf Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

allgandalf commented 2 months ago

This is waiting for proposals melvin

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? 💸

allgandalf commented 2 months ago

Still waiting for proposals

melvin-bot[bot] commented 1 month ago

@garrettmknight, @robertjchen, @allgandalf Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

allgandalf commented 1 month ago

waiting for proposals

hraheel8519 commented 1 month ago

Can I send you a proposal on upwork?

garrettmknight commented 1 month ago

@hraheel8519 we only accept proposals via GH. You'll need to submit a more detailed proposal following our contribution guidelines for us to consider it.

allgandalf commented 1 month ago

@garrettmknight what are the next steps here? @dylanexpensify posted last week too in open source channel ? should we raise the bounty, ask CS or SWM, or other ?

Guccio163 commented 1 month ago

Hi @garrettmknight, I'm Wiktor Gut from SWM agency, please assign me this issue per @DylanDylann's request

allgandalf commented 1 month ago

sorry @DylanDylann it must be @dylanexpensify they meant 🫣

allgandalf commented 1 month ago

@Guccio163 let me know when you can have the PR ready, lets get this done 🚀

Guccio163 commented 1 month ago

Oh yes of course, I meant Dylan Courtney; I just picked it today and am reading into this, but I hope I'll get it done quickly 🚀

allgandalf commented 1 month ago

Oh yes of course, I meant Dylan Courtney;

@DylanDylann and @dylanexpensify are both cool guys, you might want to know them both 😉 .

I just picked it today and am reading into this, but I hope I'll get it done quickly

No pressure, lets get it over the fence 🚀

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? 💸

Guccio163 commented 1 month ago

Hi, I'm coming with a question that occurred to me when inviting members to reproduce the issue; I invited 3 members to the new workspace and can see all of them, but they doesn't show up as members of this workspace's main chat:

https://github.com/user-attachments/assets/62d989aa-c2ab-44a0-b9f1-7563e5391407

Each one of the members can see themselves and me as one:

https://github.com/user-attachments/assets/1c6c6027-9ef9-49eb-b1c2-3d613781ad22

Should it work like that? It got me confused because in my common sense all of them should be able to read f.ex. expenses made to this workspace. It isn't a front-end bug though, because Workspace's room members are extracted from room's report.participants and Workspace's members are from policy.employeeList, I don't quite get it though. Can you please shed some light on this for me?

Guccio163 commented 1 month ago

Hi, I'm coming with a second question; when user submits an expense to the workspace it just shows in the workspace's chat:

And when expense's approver receives it, the second chat is created solely for this expense and it isn't visible in the main chat:

Why do these two situations differ? I hope that understanding it better can direct us towards the solution since the bug contains not showing report to the original approver.

allgandalf commented 1 month ago

Asked on slack for help @Guccio163 , someone should answer soon 😄

Guccio163 commented 1 month ago

Additionally, I (as an approver) can't hold an expense right away, I have to open an expense first:

https://github.com/user-attachments/assets/141b69f7-9d13-4d86-9662-671d4761b656

Is it by design that approver has to open it first?

Beamanator commented 1 month ago

Oof i highly doubt it, that looks like a 🐛 - @robertjchen can you confirm?

melvin-bot[bot] commented 1 month ago

📣 @allgandalf 🎉 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

Guccio163 commented 1 month ago

Hi, as I'm experimenting a bit with possible expense submissions, couple of not-so-obvious occurrences happened to me; is this the most recent Advanced Approvals design doc? I'd love to check them by myself and gather all these bugs in one place 🔎🪲 fyi I'll be OOO on Monday but I can't wait to patching them on tuesday

melvin-bot[bot] commented 1 month ago

@garrettmknight, @robertjchen, @Guccio163, @allgandalf Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

allgandalf commented 1 month ago

not overdue @Guccio163 will be back tomorrow

Guccio163 commented 1 month ago

Hi @allgandalf, could you please share a doc/figma for submitting expenses with me? I'm trying to judge whether what I'm encountering is a bug; I also requested a Advanced Approvals figma too, so hopefully I'll be back to speed when it's granted 🙏

allgandalf commented 1 month ago

Hi @allgandalf, could you please share a doc/figma for submitting expenses with me? I'm trying to judge whether what I'm encountering is a bug; I also requested a Advanced Approvals figma too, so hopefully I'll be back to speed when it's granted 🙏

I will post up in control channel on slack to give you access it should be granted soon, are you in #wave-control slack?

Update: Asked on slack for help https://expensify.slack.com/archives/C06ML6X0W9L/p1728375629091899?thread_ts=1728339111.738029&cid=C06ML6X0W9L

Guccio163 commented 1 month ago

Yes I am thank you, I'll watch closely on any updates 👀

Guccio163 commented 1 month ago

Hi @allgandalf, today I pulled new main and stumbled over this wall when trying to add an expense to workspace:

Do we have an Expensify-friendly burner card to input as a developer? If no, should I go back to older main to postpone that problem?

allgandalf commented 1 month ago

not really, for every new account we have 7 day trial period, we can extend your trail for the email address, please provide your email address here and @mallenexpensify can add it to the extended trail list.

Also to not block your progress, can you create a new account by adding + in your existing email? that way you get a 7 day free trail for that new account

Guccio163 commented 1 month ago

Oh okay, I'm currently using 3 '+' accounts for the purposes of this issue, but I guess I'll have to make couple more once they extend 7-days lifespan. Please add my main email: wiktor.gut@swmansion.com to the extended accounts list and thanks for the help 😄

mallenexpensify commented 1 month ago

Successfully updated the trial for wiktor.gut@swmansion.com to 2025-01-07 23:59:59 UTC!

@Guccio163 , you should be set! If anyone needs free trials extended for other emails, drop 'em in here. (caveat, they can't have a balance due on them)

Guccio163 commented 1 month ago

Hi @mallenexpensify, I'm just checking; Is there a cooldown time after which trial should extend? I'm still getting the same Add payment card wall 🚧

allgandalf commented 1 month ago

Hi @mallenexpensify, I'm just checking; Is there a cooldown time after which trial should extend? I'm still getting the same Add payment card wall 🚧

Do you get the same when you create a new workspace :

Guccio163 commented 1 month ago

Submitting expense to a newly created workspace doesn't work either 💔

allgandalf commented 1 month ago

just maybe, try relogin/cache reset ? if none work then lets wait for @mallenexpensify to re-check on supportal

Expensify / App