-
Notifications
You must be signed in to change notification settings - Fork 3k
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 for payment 2025-01-13] [$250] After paying expense and canceling payment don't scroll down to comments #53077
Comments
Triggered auto assignment to @kadiealexander ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.After paying expense and canceling payment don't scroll down to the comments What is the root cause of that problem?We do not call What changes do you think we should make in order to solve the problem?Add Line 7309 in e08c53a
|
ProposalPlease re-state the problem that we are trying to solve in this issue.After paying expense and canceling payment don't scroll down What is the root cause of that problem?We are missing Line 7464 in 51e25af
Lines 7397 to 7406 in 51e25af
What changes do you think we should make in order to solve the problem?We should add
What alternative solutions did you explore? (Optional) |
I can help take this issue as C+ since I have the context on it @kadiealexander |
Job added to Upwork: https://www.upwork.com/jobs/~021861289275630369185 |
Current assignee @dukenv0307 is eligible for the External assigner, not assigning anyone new. |
ProposalPlease re-state the problem that we are trying to solve in this issue.After paying expense and canceling payment don't scroll down to comments What is the root cause of that problem?We already have a code that will scroll to bottom whenever the last action is changed on the report here App/src/pages/home/report/ReportActionsList.tsx Lines 374 to 379 in 084a711
but in our case hasNewesReportAction will be false
hasNewesReportAction will ensure that the last action does exist in our report action list but in our case although we added the payed and cancelled payment report actions optimistically we forgot to update lastVisibleActionCreated with the last action created timestampLines 6615 to 6625 in 084a711
Lines 7385 to 7395 in 084a711
What changes do you think we should make in order to solve the problem?We will need to set the lastVisibleActionCreated field correctly Line 7394 in 084a711
Line 6625 in 084a711
and for cancel payment we need to revert previous value on failureData here Line 7435 in 084a711
but for pay we are already doing it here Lines 6673 to 6675 in 084a711
POC:2024-11-26.17-14-42.mp4What alternative solutions did you explore? (Optional) |
@dukenv0307 bump! |
I'm reviewing. |
Thanks for all your proposals @mkzie2 Your solution doesn't fix @FitseTLT Your solution is not correct to me Here's the PR to do that: 1d4aef1 And it's purpose is to handle deleting message. I also applied your solution and it didn't work web-resize.mp4@nkdengineer's proposal looks good to me, we used the same approaches in many places. 🎀👀🎀 C+ reviewed |
Triggered auto assignment to @jasperhuangg, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
This issue has not been updated in over 15 days. @jasperhuangg, @kadiealexander, @dukenv0307, @nkdengineer eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.80-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2025-01-13. 🎊 For reference, here are some details about the assignees on this issue:
|
@dukenv0307 @kadiealexander @dukenv0307 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
BugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
Regression Test ProposalTest:
Do we agree 👍 or 👎 |
Payments due:
|
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.66-3
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dukenv0307
Slack conversation (hyperlinked to channel name): ts_external_expensify_bugs
Action Performed:
Expected Result:
After paying expense and canceling payment scroll down to the comments
Actual Result:
After paying expense and canceling payment don't scroll down to the comments
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
bug.mp4
Recording.794.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @Issue Owner
Current Issue Owner: @kadiealexanderThe text was updated successfully, but these errors were encountered: