-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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][$250] Send invoices - Date notification not copied when invoice date is changed #43571
Comments
Triggered auto assignment to @strepanier03 ( |
@strepanier03 FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
@strepanier03 Huh... This is 4 days overdue. Who can take care of this? |
Testing now. |
Confirmed reproducible on mac Chrome. |
Job added to Upwork: https://www.upwork.com/jobs/~0111e39bcf78303fbe |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Attempting to copy the notification fails What is the root cause of that problem?When creating an invoice, in Line 3560 in 340f221
createdIOUReportActionID and reportActionID (IOU action id) are not sent to back-end to use as real report action ids.
So the back-end is creating new IOU action id and send back to front-end. This makes the Invoice has 2 IOUs, so the Because of that, the Next, the So the copying doesn't work because What changes do you think we should make in order to solve the problem?When creating an invoice, in Line 3560 in 340f221
createdIOUReportActionID and reportActionID (IOU action id) should be sent to back-end.
Those can be returned from the Line 1852 in 340f221
The back-end should be updated to make sure to support those params, and use them as the real action ids. If not mistaken, at the moment it's supporting What alternative solutions did you explore? (Optional)In Line 6042 in 340f221
reportActions after applying getContinuousReportActionChain so that it will get the correct chain of report actions. This is same method as of ReportScreen
|
@strepanier03, @getusha Eep! 4 days overdue now. Issues have feelings too... |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@strepanier03 Reassigning this due to low bandwidth @c3024 will takeover |
@strepanier03 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! |
Need a little more time to finalize @MelvinBot |
📣 @c3024 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
Not overdue @MelvinBot |
@strepanier03 @cristipaval @c3024 this issue is now 4 weeks old, please consider:
Thanks! |
@c3024 @cristipaval If we want to handle the similar errors in Front End based on this message, we could remove the optimistic created report actions (and optimistic IOU report action) after the API request completed. Similar to my proposal in other issue. |
Hey @c3024, Could you please take a look at this issue?, looks like these two bugs will be fixed by the same changes proposed here and here, wdyt? |
@rayane-djouah yes, you are right. Both issues will be fixed by the same changes. |
@tsa321 Removing the optimistic report actions in success data does not look a correct pattern to me. Is there anywhere in the repo that we do this? |
Ok, so we are holding on to this issue for #43797. Correct? |
Oh, it looks like that issue was already assigned. I thought that issue was held and this was moving. 😆 Yes, this issue can be held. PR for #43797 should fix this. |
Still held |
We're actively working on the issue that this one is held on |
The PR fixing the issue that this one is held on has been merged. Waiting for it to get deployed. |
This issue can be retested in staging. |
Testing now. |
This is no longer reproducible. When following the testing steps on staging it's working as expected. |
I think we can close this out now since it's no longer needed. Please reopen if you disagree. |
Held on #43797
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: 1.4.82-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: N/A
Email or phone of affected tester (no customers): abebemiherat@gmail.com
Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
The date notification should be copied when the invoice date is changed
Actual Result:
After sending the invoice, changing the date triggers a notification. However, attempting to copy the notification fails
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6510653_1718188876505.Screen_Recording_2024-06-12_at_3.39.34_AM.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @getushaThe text was updated successfully, but these errors were encountered: