App icon indicating copy to clipboard operation
App copied to clipboard

[HOLD for payment 2024-12-17] [$250] Search - When plenty of expenses are created already, haven't created expenses message shown

Open IuliiaHerets opened this issue 1 year ago β€’ 41 comments

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: V9. 0.51-1 Reproducible in staging?: Y Reproducible in production?: Y Issue reported by: Applause Internal Team

Action Performed:

  1. Go to https://staging.new.expensify.com/home
  2. Log in account which has more expenses
  3. Tap search at bottom
  4. Note in expenses section, lots of expenses created are displayed
  5. Tap on search icon on top
  6. Search any random text "yin"
  7. Tap on the search term shown in dropdown
  8. Note user navigated to page showing haven't created any expenses

Expected Result:

When plenty of expenses are created already, haven't created expenses message must not be displayed.

Actual Result:

When plenty of expenses are created already, haven't created expenses message is displayed.

Workaround:

Unknown

Platforms:

  • [x] Android: Standalone
  • [x] Android: HybridApp
  • [x] Android: mWeb Chrome
  • [ ] iOS: Standalone
  • [ ] iOS: HybridApp
  • [ ] iOS: mWeb Safari
  • [x] MacOS: Chrome / Safari
  • [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/8f4095d8-61f4-4fab-affa-a628f5a107f6

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021848389047150206359
  • Upwork Job ID: 1848389047150206359
  • Last Price Increase: 2024-10-28
  • Automatic offers:
    • FitseTLT | Contributor | 104700929
Issue OwnerCurrent Issue Owner: @isabelastisser

IuliiaHerets avatar Oct 21 '24 10:10 IuliiaHerets

Triggered auto assignment to @isabelastisser (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] avatar Oct 21 '24 10:10 melvin-bot[bot]

@isabelastisser 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

IuliiaHerets avatar Oct 21 '24 10:10 IuliiaHerets

Edited by proposal-police: This proposal was edited at 2024-10-21 10:54:45 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

When plenty of expenses are created already, haven't created expenses message shown

What is the root cause of that problem?

https://github.com/Expensify/App/blob/4575341e6c472682663e149e4a8d6b350fce0154/src/components/Search/index.tsx#L280 We only check if data.length===0 here, and data here represents searchResult so it would trigger following code for and show EmptySearchView for both case -

  1. when there are no expenses.(Expected result)
  2. when search data returned from server is empty.(Needs Improvement)

What changes do you think we should make in order to solve the problem?

We should show EmptySearchView only when search query is empty and expanses are empty as well. When search query is not empty (ie. case 2) but search results returned from server is empty.

  • we could create a new component to show user for NoMatchingResults instead of EmptySearchView.
  • NoMatchingResults component would have design similar to EmptySearchView

More About The `NoMatchingResults` component...

  • Should have an appropriate illustration (chosen by design team)
  • Title could be - "No Matching Results" or "No Matching Results To Show" or something similar.
  • Description - "There are no matching results to show . Use green button below to edit search search filters." or "Use green button to edit search filters or take a tour of Expensify to learn more" or something similar.
  • Add button - Reset Filters or Reset - to reset search query.
  • Add button - Search Filters - which will redirect to search filters for editing search query.
  • Optionally add - Take a Tour button.

[!NOTE] This is general outline, Particular details should be discussed with Design team.

Optionally we should also do the same for other types - Chat, Invoices, Trips.(ie. show variation of NoMatchingResult when query is not empty but search result returned from server is empty).

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.

ChavdaSachin avatar Oct 21 '24 10:10 ChavdaSachin

Edited by proposal-police: This proposal was edited at 2024-10-24 15:45:05 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Search - When plenty of expenses are created already, haven't created expenses message shown

What is the root cause of that problem?

For Expense, we always show "You haven't created any expenses" on the empty state regardless the search filter and expense status https://github.com/Expensify/App/blob/f806997ff987e5b1ba8724a0428950fe8dc2aa16/src/pages/Search/EmptySearchView.tsx#L116-L131 https://github.com/Expensify/App/blob/b2fc5c9627821ed4647d7fa656ba9d2b0a5d6d58/src/languages/en.ts#L4292-L4293

What changes do you think we should make in order to solve the problem?

Pass queryJSON to EmptySearchView https://github.com/Expensify/App/blob/642fd97e9a078495176e01b3ac17d01262bf1fa4/src/components/Search/index.tsx#L301

Inside case CONST.SEARCH.DATA_TYPES.EXPENSE: create new variable shouldShowNoExpenseCreated and set this value to true if no filter and the status is "all"

const shouldShowNoExpenseCreated = isEmptyObject(queryJSON.filters) && queryJSON.status === CONST.SEARCH.STATUS.EXPENSE.ALL;

and only change the title based on shouldShowNoExpenseCreated, and keep the subtitle and the buttons the same for all conditions on Expense type since I think it is needed

title: shouldShowNoExpenseCreated ? translate('search.searchResults.emptyExpenseResults.title') : translate('search.searchResults.emptyResults.title'),

Additionally, I think we should create new copy for this, like "No expenses to display"

https://github.com/Expensify/App/blob/642fd97e9a078495176e01b3ac17d01262bf1fa4/src/pages/Search/EmptySearchView.tsx#L116-L131

If we want to change all(the title, subtitle, and remove the buttons) we can change to the following

if (type === CONST.SEARCH.DATA_TYPES.TRIP) {
            return {
                headerMedia: LottieAnimations.TripsEmptyState,
                headerStyles: StyleUtils.getBackgroundColorStyle(theme.travelBG),
                headerContentStyles: StyleUtils.getWidthAndHeightStyle(375, 240),
                title: translate('travel.title'),
                titleStyles: { ...styles.textAlignLeft },
                subtitle: subtitleComponent,
                buttons: [
                    {
                        buttonText: translate('search.searchResults.emptyTripResults.buttonText'),
                        buttonAction: () => TripsResevationUtils.bookATrip(translate, setCtaErrorMessage, ctaErrorMessage),
                        success: true,
                    },
                ],
            };
        } else if (type === CONST.SEARCH.DATA_TYPES.EXPENSE && isEmptyObject(queryJSON.filters) && queryJSON.status === CONST.SEARCH.STATUS.EXPENSE.ALL) {
            return {
                headerMedia: LottieAnimations.GenericEmptyState,
                headerStyles: [StyleUtils.getBackgroundColorStyle(theme.emptyFolderBG)],
                title: translate('search.searchResults.emptyExpenseResults.title'),
                subtitle: translate('search.searchResults.emptyExpenseResults.subtitle'),
                buttons: [
                    {
                        buttonText: translate('emptySearchView.takeATour'),
                        buttonAction: () => Link.openExternalLink(navatticLink),
                    },
                    {
                        buttonText: translate('iou.createExpense'),
                        buttonAction: () => interceptAnonymousUser(() => IOU.startMoneyRequest(CONST.IOU.TYPE.CREATE, ReportUtils.generateReportID())),
                        success: true,
                    },
                ],
                headerContentStyles: styles.emptyStateFolderWebStyles,
            };
        } else {
            return {
                headerMedia: LottieAnimations.GenericEmptyState,
                headerStyles: [StyleUtils.getBackgroundColorStyle(theme.emptyFolderBG)],
                title: translate('search.searchResults.emptyResults.title'),
                subtitle: translate('search.searchResults.emptyResults.subtitle'),
                headerContentStyles: styles.emptyStateFolderWebStyles,
            };
        }

What alternative solutions did you explore? (Optional)

Create new function on ReportUtils to check if there is any expense on ONYXKEYS.COLLECTION.REPORT

function hasAnyExpense(): boolean {
    const allReports = ReportConnection.getAllReports();

    if (!allReports) {
        return false;
    }

    return Object.values(allReports).some((report) => isExpenseReport(report));
}

Or use allTransactions instead and create the function on TransactionUtils

Call that function here and only show "You haven't created any expenses yet" if hasAnyExpense is false, show "Nothing to show" if hasAnyExpense true

nyomanjyotisa avatar Oct 21 '24 11:10 nyomanjyotisa

Edited by proposal-police: This proposal was edited at 2024-10-29 15:23:08 UTC. Note for @mollfpr please take a note to the timestamps of the other proposals as when I proposed a complete proposal including the condition to use isEmptyObject(queryJSON.filters) as well as how we are going to update EmptySearchView component, the other proposal were lacking both vital parts of the proposal but were updated later.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Search - When plenty of expenses are created already, haven't created expenses message shown

What is the root cause of that problem?

We currently display haven't created message along with buttons in EmptySearchView when the search data type is CONST.SEARCH.DATA_TYPES.EXPENSE https://github.com/Expensify/App/blob/b2fc5c9627821ed4647d7fa656ba9d2b0a5d6d58/src/pages/Search/EmptySearchView.tsx#L116-L129 so even if there are expenses we display the empty search view whenever the result data is empty https://github.com/Expensify/App/blob/c3dde46202d2f48934ef7aa617d69c8b4c0b5ff6/src/components/Search/index.tsx#L305 without checking there is filter applied or not

What changes do you think we should make in order to solve the problem?

We should check for no data created yet by additionally checking if queryJSON.filters are empty (we can also optionally determine by the emptiness of transaction) https://github.com/Expensify/App/blob/b2fc5c9627821ed4647d7fa656ba9d2b0a5d6d58/src/components/Search/index.tsx#L280

  const shouldShowEmptyState = !isDataLoaded || data.length === 0;
    const noDataCreated = shouldShowEmptyState && isEmptyObject(queryJSON.filters);

pass default_empty(we will create a const for it) type to EmptySearchView for the non noDataCreated case https://github.com/Expensify/App/blob/b2fc5c9627821ed4647d7fa656ba9d2b0a5d6d58/src/components/Search/index.tsx#L287

                <EmptySearchView type={!noDataCreated && type === CONST.SEARCH.DATA_TYPES.EXPENSE ? `default_empty` : type} />

so that the common Nothing to show message can be displayed but if we want a more specific message for the case of empty search results for expenses we can have new copies and display them for the case of type expenses here https://github.com/Expensify/App/blob/b2fc5c9627821ed4647d7fa656ba9d2b0a5d6d58/src/pages/Search/EmptySearchView.tsx#L139-L140

We can solve similar problem for TRIP too

What alternative solutions did you explore? (Optional)

We can also optionally make the noDataCreated prop to EmptySearchView and display different copies (title and subtitle) for non noDataCreated case for expenses (and also for trip case if needed)

FitseTLT avatar Oct 21 '24 13:10 FitseTLT

Job added to Upwork: https://www.upwork.com/jobs/~021848389047150206359

melvin-bot[bot] avatar Oct 21 '24 15:10 melvin-bot[bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mollfpr (External)

melvin-bot[bot] avatar Oct 21 '24 15:10 melvin-bot[bot]

@isabelastisser Could you help with what is expected to be shown on the screen? Maybe we should get the design team to help.

mollfpr avatar Oct 23 '24 15:10 mollfpr

@shawnborton Can you give us an idea in to what is expected, in terms of design, here? as asked ^

FitseTLT avatar Oct 24 '24 14:10 FitseTLT

The expected behavior is that we should use the "nothing to show" message for the empty state when expenses do actually exist but none are being returned for the current query or the current canned search (like Expenses > Outstanding)

shawnborton avatar Oct 24 '24 15:10 shawnborton

@isabelastisser thoughts on adding in the problem from this issue here as well? I think the two issues are pretty closely related, so we can solve them all at the same time just from this issue (which was opened first).

shawnborton avatar Oct 24 '24 15:10 shawnborton

Note for @mollfpr : Based on the expected result https://github.com/Expensify/App/issues/51168#issuecomment-2435651135 I had already proposed a full solution (also including the case for TRIP) and I am seeing a update notified here which is basically the same as mine please take a :eyes: on the recent changes and timestamps in https://github.com/Expensify/App/issues/51168#issuecomment-2426382687 Thx

FitseTLT avatar Oct 24 '24 15:10 FitseTLT

@shawnborton, I agree that we can combine the issues if the PR here can fix both. I closed the other one.

isabelastisser avatar Oct 25 '24 14:10 isabelastisser

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

melvin-bot[bot] avatar Oct 28 '24 16:10 melvin-bot[bot]

@mollfpr, @isabelastisser Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] avatar Oct 28 '24 18:10 melvin-bot[bot]

@ChavdaSachin I think the root cause can be clearer instead of the one-liner. So please update your proposal.

We should add an additional check, show EmptySearchView only id queryJSON is non empty.

This solution seems outdated, we actually expected to show Nothing to show instead show a blank page.

Optionally we could create a new component to show user for noMatchingResults instead of EmptySearchView when query is not empty and search data is empty.

Could you elaborate what's the component will show here?

mollfpr avatar Oct 29 '24 15:10 mollfpr

For Expense, we always show "You haven't created any expenses" on the empty state regardless of the filter and status

@nyomanjyotisa What status do you mean here?

Your solution seems outdated, could you update your proposal and choose a solution that potentially solves the issue?

mollfpr avatar Oct 29 '24 15:10 mollfpr

We currently display haven't created message along with buttons as empty search view for expenses

@FitseTLT The RCA can be written better to explain what's the issue and how the app handling it right now.

we can also optionally determine by the emptiness of transaction

From your solution, you mentioned using the emptiness of transaction, but it's optional, why is that and what is the difference if we didn't add that?

mollfpr avatar Oct 29 '24 15:10 mollfpr

@FitseTLT The RCA can be written better to explain what's the issue and how the app handling it right now.

@mollfpr Updated RCA to make it clear

From your solution, you mentioned using the emptiness of transaction, but it's optional, why is that and what is the difference if we didn't add that?

No basic difference, just giving an alternative πŸ‘

FitseTLT avatar Oct 29 '24 15:10 FitseTLT

Updated Proposal

@ChavdaSachin I think the root cause can be clearer instead of the one-liner. So please update your proposal.

Updated RCA, explained in depth.

Could you elaborate what's the component will show here?

Added more Information about the new component I proposed.

cc. @mollfpr

ChavdaSachin avatar Oct 29 '24 19:10 ChavdaSachin

@nyomanjyotisa What status do you mean here?

I mean the expense status image image

Your solution seems outdated, could you update your proposal and choose a solution that potentially solves the issue?

Removed the outdated solution on my main solution, and moved the alternative solution to main solution

Proposal updated

nyomanjyotisa avatar Oct 30 '24 12:10 nyomanjyotisa

Thank you all for the proposals!

Summarizing the RCA: So our current component to show the empty search modal is EmptySearchView and the contents value is determined by the type that passed as a prop which can be expense, invoice, trip, and chat. As answered here https://github.com/Expensify/App/issues/51168#issuecomment-2435649773 we do want to show Nothing to show similar to what shows for other than expense and trip (by mean it's default content). So the solution should show default content for expense type when there's no result when using the filter.

@ChavdaSachin, I appreciate the suggestion to model the content of the empty search modal, but I think we're still good with the current component and need a little tweak to fix the issue.

@nyomanjyotisa I think we will overdo it in your solution to achieve the same result as @FitseTLT. The first part of your solution suggests passing queryJSON to the component and changing the title for type expense and with only all status. I don't agree with passing the whole object to the component where it only uses a small value of it, and the check for the status seems unnecessary. Maybe next time try to suggest more readable props and correspond with the mean of the props for example isFilterEmpty etc. The other suggestion I would give you is to try to write a solution with more clear achievement, and don't overcomplicate it with optional things. If you think the way to go is to create a new copy, then put it as your solution, and if you are not sure about the expected or have a better idea, then try to discuss it with the person concerned in this case the design team.

@FitseTLT I think you have the straight solution and minimum changes to solve the issue, and as I outlined above we just need to return the default value for the content. So I think we can go with your proposal.

πŸŽ€ πŸ‘€ πŸŽ€ C+ reviewed!

mollfpr avatar Nov 01 '24 17:11 mollfpr

Triggered auto assignment to @dangrous, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

melvin-bot[bot] avatar Nov 01 '24 17:11 melvin-bot[bot]

πŸ“£ @FitseTLT πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

melvin-bot[bot] avatar Nov 01 '24 20:11 melvin-bot[bot]

@mollfpr I think the status check is necessary to solve this issue, since we closed that issue to fix it here based on this

nyomanjyotisa avatar Nov 01 '24 23:11 nyomanjyotisa

@dangrous @mollfpr @isabelastisser @FitseTLT 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!

melvin-bot[bot] avatar Nov 04 '24 18:11 melvin-bot[bot]

Not overdue Will raise PR shortly!

FitseTLT avatar Nov 04 '24 18:11 FitseTLT

Thanks!

dangrous avatar Nov 04 '24 20:11 dangrous

how are we looking on this one?

dangrous avatar Nov 19 '24 15:11 dangrous