-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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-22] [$500] Expense - Submit button appears for archived workspace chat if delayed submission is enabled #49169
Comments
Triggered auto assignment to @johncschuster ( |
@johncschuster 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 |
Edited by proposal-police: This proposal was edited at 2024-09-13 12:03:09 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Submit button appears for archived workspace chat if delayed submission is en bled What is the root cause of that problem?we dont disable the submit button if the workspce is archived here App/src/components/MoneyReportHeader.tsx Line 126 in eb5a569
What changes do you think we should make in order to solve the problem?we should add another condition that the workspace is not deleted above using the or it could be added to the isOpenExpenseReport directly (to follow the same concept as in the isOpenTaskReport function) : return isExpenseReport(report) && !isArchivedRoomWithID(report?.reportID) && report?.stateNum === CONST.REPORT.STATE_NUM.OPEN && report?.statusNum === CONST.REPORT.STATUS_NUM.OPEN; POCScreen.Recording.2024-09-13.at.14.21.13.movWhat alternative solutions did you explore? (Optional) |
Edited by proposal-police: This proposal was edited at 2024-10-17 10:27:58 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Expense - Submit button appears for archived workspace chat if delayed submission is enabled What is the root cause of that problem?
Then we make
3. For Pay button we are already hiding the button if the workspace is archived Here we check if workspace is achieved Line 6985 in 987ff1b
And we make canIOUBePaid false if the workspace is archivedApp/src/components/ReportActionItem/ReportPreview.tsx Lines 325 to 332 in 987ff1b
What changes do you think we should make in order to solve the problem?
And here
For approve and pay button we don't have to make any change since we already hiding the buttons when the workspace is archived.
Currently the composer with all the submit, track expense option is closed when workspace chat is archived What alternative solutions did you explore? (Optional) |
Note A kind reminder for C+: The fist section of the first proposal's permalink edit that points to shouldShowSubmitButton comes after my proposal. Prior to that it pointed to shouldDisableApproveButton. |
Hi @abzokhattab. Writing few details takes time and make a difference in proposal submission time, and there seems to be a little difference between what you have suggested at first which is disabling the button (we dont disable the submit button ) by pointing a permalink to shouldDisableApproveButton, and what you suggested after the edit. That was why I raised the question. I have no ill intention towards you in saying that though🙏. Thanks! |
ProposalPlease re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
All of these conditions can be handled in: Line 6693 in 00bc167
const shouldShowSubmitButton = isDraft && reimbursableSpend !== 0 && !hasAllPendingRTERViolations && ReportUtils.canUserPerformWriteAction(moneyRequestReport); and
const shouldShowSubmitButton = isOpenExpenseReport && reimbursableSpend !== 0 && !showRTERViolationMessage && ReportUtils.canUserPerformWriteAction(iouReport); What alternative solutions did you explore? (Optional) |
@johncschuster Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Job added to Upwork: https://www.upwork.com/jobs/~021836147581047135142 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Ollyws ( |
@etCoderDysto's proposal LGTM. |
Triggered auto assignment to @srikarparsi, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
Before edit, it seems that you were suggesting disabling the button as I have commented here. |
Hm, wait. Why is the expense report being archived in the OP video, @srikarparsi? 🤔 We should have stopped doing that, shouldn't we? |
As for how to handle this case of an I think to handle this properly and holistically for all report states and setup scenarios, we're going to need better planning and probably the inclusion of the feature to let people manually move a report to a different workspace. |
Hmm, I think we wanted to stop
Yeah I think I agree |
Okay, I can see why we kept doing that for now because of otherwise having the ability to add expenses to the report, but we want to continue to allow people to chat on an expense report to discuss what to do about outstanding reports - and more importantly, we need to stop the actions of submit/approve/pay which all will fail - so I don't think we should archive them. Thinking through how we should handle this broadly and consistently: When a workspace is deleted
*Q: If the submitter chooses their How does that sound? Let's align, and then I can create mocks for the relevant parts above. CC: @JmillsExpensify |
Will reimbursed expenses not be archived as well? I feel like this distinction would be kind of weird, since you would no longer be able to chat on chat threads or task threads but would be able to continue chatting on expense threads after a workspace is deleted. I'm also not sure if allowing users to chat on these expense threads is useful since they have to be moved to a different report to be paid anyway (where they can also chat). But I agree with everything else! |
Right, if the expense report isn't archived neither are the expenses on them. People retain the ability to comment on expense reports after a workspace is deleted today on OldDot because ultimately it can be moved to another workspace to progress to resolution, and for example, if the workspace was deleted and they still need to be processed to paid, the conversation can be had. So the suggestion is to keep that behaviour intact. |
There is a pending BE pr. As soon as it is merged we will have a final test and record snapshots 👍 |
That's great! Thanks, @FitseTLT! |
@FitseTLT and @Ollyws, I will be OOO starting December 23 and will be returning January 6th. A handful of folks on the BZ team will be online for a few days in between the 25th and the 1st, but we'll be operating with a skeleton crew. I will be issuing my payments when I return on January 6th. If the PR linked passes the regression threshold before I return and you would like the payment issued sooner, please post this issue in #expensify-open-source and someone on the team will jump in. Thank you! |
Hey everyone! I'm back from break and will continue to push this forward 👋 |
Ok! It looks like the BE PR from above was merged into Auth about two weeks ago. That means we're waiting on tests and snapshots, right @FitseTLT? |
Yep correct on C+ review stage |
Great! I'll leave it to you then. Thanks, @FitseTLT! |
Looks like this #52183 is almost finished, which, I think is our only blocker. |
#52183 has been deployed to staging 🥳 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.85-4 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-22. 🎊 For reference, here are some details about the assignees on this issue:
|
@Ollyws @johncschuster @Ollyws 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 Proposal Template
Regression Test ProposalPrecondition:Test:Do we agree 👍 or 👎 |
@Ollyws can you complete the BZ Checklist above? |
BugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
Regression Test ProposalPrecondition:Test:
Do we agree 👍 or 👎 |
Requested in ND. |
Thanks, @Ollyws! We're good to close this up! |
$500 approved for @Ollyws |
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.33-4
Reproducible in staging?: Y
**Reproducible in production?:**Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
Submit button doesn't show on the archived workspace chat on the expense
Actual Result:
Submit button shows on both the employee's and admin's archived workspace chat and clicking on the button results in an error
Additional Expected Behavior:
Submit
,Approve
orPay
action buttons.Submit expense
&Track expense
options should be hidden from the expense report’s create menu (i.e the menu where Add attachment etc lives).Workaround:
Unknown
Platforms:
Screenshots/Videos
Bug6602198_1726224396008.bandicam_2024-09-13_13-35-07-112.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @johncschusterThe text was updated successfully, but these errors were encountered: