-
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 #55680][$250] Reports - Top bar does not disappear after deleting all the invoices in Outstanding tab #54731
Comments
Triggered auto assignment to @anmurali ( |
user Your proposal will be dismissed because you did not follow the proposal template. |
mharootian Your proposal will be dismissed because you did not follow the proposal template. |
ProposalPlease re-state the problem that we are trying to solve in this issue.The Header does not disappear and the Select all checkbox stays checked even after deleting all the outstanding invoices. What is the root cause of that problem?The code doesn't check the type of report when creating sections. This means reports of other types (like chat) are included instead of just invoices. Lines 367 to 381 in 8b56d93
What changes do you think we should make in order to solve the problem?We should check the type of each report. If the type is not the one currently selected (in this case, invoices), we should skip that item:
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?What alternative solutions did you explore? (Optional)Screen.Recording.2025-01-02.at.2.19.02.AM.mov |
ProposalPlease re-state the problem that we are trying to solve in this issue.Reports - Top bar does not disappear after deleting all the invoices in Outstanding tab What is the root cause of that problem?We are checking if App/src/components/Search/index.tsx Line 353 in 6e78dd4
This happens because App/src/components/Search/index.tsx Lines 235 to 240 in 6e78dd4
What changes do you think we should make in order to solve the problem?Let's use App/src/components/Search/index.tsx Line 353 in 6e78dd4
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?What alternative solutions did you explore? (Optional)Create a function called
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Top bar does not disappear after deleting all the invoices in Outstanding tab What is the root cause of that problem?When we create a new invoice, the value will be added to the After an invoice is deleted from the search, as described in this ticket, one Because the backend only updates the So, when we retrieve data from the App/src/components/Search/index.tsx Line 232 in 4eb86b4
Because this report does not have a transactions value, the UI will not render due to this code. App/src/components/SelectionList/Search/ReportListItem.tsx Lines 84 to 86 in 273a4be
The reason the empty search view is not displayed is that App/src/components/Search/index.tsx Line 343 in 4eb86b4
And when we change tabs or a report loads, the What changes do you think we should make in order to solve the problem?To resolve this issue, we need to check the data. If the search data type is update to: function getSections(type: SearchDataTypes, status: SearchStatus, data: OnyxTypes.SearchResults['data'], metadata: OnyxTypes.SearchResults['search']) {
if (type === CONST.SEARCH.DATA_TYPES.CHAT) {
return getReportActionsSections(data);
}
if (status === CONST.SEARCH.STATUS.EXPENSE.ALL) {
return getTransactionsSections(data, metadata);
}
return getReportSections(data, metadata, type); // update this line
} Line 360 in eaebfdf
function getReportSections(data: OnyxTypes.SearchResults['data'], metadata: OnyxTypes.SearchResults['search'], type): ReportListItemType[] {
...
for (const key in data) {
if (isReportEntry(key)) {
const reportItem = {...data[key]};
const reportKey = `${ONYXKEYS.COLLECTION.REPORT}${reportItem.reportID}`;
const transactions = reportIDToTransactions[reportKey]?.transactions ?? [];
const isIOUReport = reportItem.type === CONST.REPORT.TYPE.IOU;
if (type === 'invoice' && reportItem.type === CONST.REPORT.TYPE.CHAT && transactions.length === 0) { // add this line
continue; // add this line
} // add this line
reportIDToTransactions[reportKey] = {
...reportItem,
action: getAction(data, key),
keyForList: reportItem.reportID,
from: data.personalDetailsList?.[reportItem.accountID ?? CONST.DEFAULT_NUMBER_ID],
to: reportItem.managerID ? data.personalDetailsList?.[reportItem.managerID] : emptyPersonalDetails,
transactions,
reportName: isIOUReport ? getIOUReportName(data, reportItem) : reportItem.reportName,
};
} ...
}
} What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?We added a new test for this case: if the type is invoice, the reportType is chat, and transactions is an empty array, it should not be included in the data What alternative solutions did you explore? (Optional)Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. |
@anmurali Eep! 4 days overdue now. Issues have feelings too... |
Job added to Upwork: https://www.upwork.com/jobs/~021876819538768498068 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav ( |
Reviewing the proposals. |
@mananjadhav, @anmurali Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Triggered auto assignment to @dangrous, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@mananjadhav Could you review my proposal? |
@huult Yes I reviewed, but I still need to verify it. |
I verified the proposals on the local account @dangrous. I still feel we should use |
That sounds good to me. Seems like exactly what that should be used for! |
It seems there's an unrelated bug with the bulk delete option not removing items from the UI. I've reported it in the bugs channel https://expensify.slack.com/archives/C049HHMV9SM/p1737617937376769 |
Ah, is that something we have to wait for? Like, can we still test this one, or not? Alternately, can we find and implement a fix here too? |
We cannot test until that's fixed.
Yes we can do that. I think @Tony-MK tried to do that, I haven't checked it yet. |
Morning! How are things looking on this one? Do we think we can tackle that other bug as part of this as well? Or should I hold this issue on that one? |
I took a look at the bug again and failed to reproduce it now. It seems that it's fixed so I believe we can proceed. |
Great! |
Took a look at the issue, the bug seems to be on the backend. I'll try to review this today. |
The PR is reviewed. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
The fix for this issue was reverted. |
Thanks @cristipaval! Let's look into another solution for this one... |
@dangrous Considering the linked issue is related to the API call taking time and then not refreshing the UI, I was wondering if we should put this current issue on hold and check if waiting for the API call fixes this one too? |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.92-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-02-07. 🎊 For reference, here are some details about the assignees on this issue:
|
@mananjadhav @anmurali @mananjadhav 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] |
Fix was reverted, so we'll need to give this another pass, and hold payment |
Hi! Yeah I think that makes sense, I'll update the title and once that is done we can look again |
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.80-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
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): [email protected]
Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
After deleting all the invoices in Outstanding tab, empty state will show
Actual Result:
After deleting all the invoices in Outstanding tab, the top bar remains and the checkbox is still checked
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6704835_1735756831542.20250102_023649.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @anmuraliThe text was updated successfully, but these errors were encountered: