-
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 2024-07-24] Filter out payroll and bill reports from the client #45104
Comments
Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane ( |
Current assignee @puneetlath is eligible for the NewFeature assigner, not assigning anyone new. |
|
Triggered auto assignment to Design team member for new feature review - @dubielzyk-expensify ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Filter out payroll reports What is the root cause of that problem?New feature What changes do you think we should make in order to solve the problem?In the shouldReportBeInOptionList function, we need to include a condition to filter reports that has
What alternative solutions did you explore? (Optional) |
ProposalPlease re-state the problem that we are trying to solve in this issue.Filter out payroll reports from the client. What is the root cause of that problem?New change. What changes do you think we should make in order to solve the problem?We need to update
We'll add the isPaycheck method in ReportUtils:
We'll add the isBill method in ReportUtils:
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Filter out payroll reports from the client What is the root cause of that problem?This is a new feature What changes do you think we should make in order to solve the problem?
Line 5307 in c973e62
OPTIONAL: We can also create a beta for the paycheck report or bill report as we do for the default room Lines 5309 to 5311 in c973e62
What alternative solutions did you explore? (Optional)NA |
FYI, I updated the OP to include reports of |
ProposalUpdated to include bill check. |
Updated proposal #45104 (comment) |
Taking over as C+ (https://expensify.slack.com/archives/C02NK2DQWUX/p1720624712448849) |
@cretadn22's proposal looks good to me. we can address refactors on the PR stage (include reports of 🎀👀🎀 C+ reviewed |
Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.7-8 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 2024-07-24. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@cretadn22 bump. Any thoughts on this? |
In your case Lines 100 to 105 in c973e62
I think hasErrorsOtherThanFailedReceipt is true, the report is included in reportsToDisplay. And, we also skip ReportUtils.shouldReportBeInOptionList checking |
The PR will be up soon |
Payment Summary
BugZero Checklist (@puneetlath)
|
@rayane-djouah @cretadn22 can you link me your Upwork profiles so that I can send you an offer? |
No regression tests are needed as we included automated tests |
@rayane-djouah offer: https://www.upwork.com/nx/wm/offer/103254979 Please ping me when you've accepted. |
@puneetlath - Accepted, Thanks! |
Ok @rayane-djouah has been paid. Just waiting on the Upwork profile for @cretadn22 now. |
@cretadn22 sent you an offer here: https://www.upwork.com/nx/wm/offer/103263505 Please ping me on this issue when you've accepted. |
@puneetlath I accepted your offer |
Ok all paid now. Thanks everyone! |
If a report has the report type
type: paycheck
ortype: bill
we should filter it out in the NewDot inbox. It shouldn't show up in the LHN even if it has unread messages or error messages. It just shouldn't ever show up there.You won't be able to test this with real data as reports of
type:paycheck
isn't currently a publicly accessible feature.Issue Owner
Current Issue Owner: @puneetlathThe text was updated successfully, but these errors were encountered: