Components: [Table] MatTableDataSource does not execute filter predicate if filter evaluates to false

[narthur157]

Bug, feature request, or proposal:

Bug

What is the expected behavior?

Filter text should not be required for using a filter when a custom filterPredicate is supplied

What is the current behavior?

Filter text must be truthy in order to use a filterPredicate which does not require filter text

What are the steps to reproduce?

Make a filterPredicate which does not depend on the filter value

What is the use-case or motivation for changing an existing behavior?

Filter predicates which do not require the filter text (for example, checking that a value in the data is past the current date)

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Current

Is there anything else we should know?

https://github.com/angular/material2/blob/master/src/lib/table/table-data-source.ts#L213
!this.filter ? data : data.filter(obj => this.filterPredicate(obj, this.filter));
Instead of !this.filter ? check if a filterPredicate has been supplied

[hudzenko]

We have a problem with that too, but a little bit different. We want to filter by couple of fields. We have made custom predicate, to provide an object with couple of fields for filtering. But filter should be a string and we have building error.
So our solution can be only parsing object to json for using it as filter param.

[narthur157]

Might be better to just filter the data and not use the built in filtering functionality

[Eddygn]

Somewhat related,
when making a custom predicate, if it's not dependent on the filter string but instead on external flags and the row data, there is no way to trigger the filter itself except to pass some new string to the filter text, even if it's meaningless. So for now 'stringifying' those flags looks like the best option even if the string is not used.
I think it will be great if there is a method to call to filter, bypassing the filter change detection, or triggering it manually.

[RomainSanchez]

@Eddygn

when making a custom predicate, if it's not dependent on the filter string but instead on external flags and the row data, there is no way to trigger the filter itself except to pass some new string to the filter text, even if it's meaningless. So for now 'stringifying' those flags looks like the best option even if the string is not used.
I think it will be great if there is a method to call to filter, bypassing the filter change detection, or triggering it manually.

i worked around it this way (ugly)

const currentFilter = this.tableDataSource.filter;

this.tableDataSource.filter = '------';
this.tableDataSource.filter = currentFilter;

[FARHANE]

@Eddygn

when making a custom predicate, if it's not dependent on the filter string but instead on external flags and the row data, there is no way to trigger the filter itself except to pass some new string to the filter text, even if it's meaningless. So for now 'stringifying' those flags looks like the best option even if the string is not used.
I think it will be great if there is a method to call to filter, bypassing the filter change detection, or triggering it manually.

i worked around it this way (ugly)

const currentFilter = this.tableDataSource.filter;

this.tableDataSource.filter = '------';
this.tableDataSource.filter = currentFilter;

Not worked for me

[avin-shum]

If changing the handling for the falsy values (e.g. empty string) is not feasible. How about introducing a method to trigger filtering explicitly, no matter filter is changed or not?

[s4m0r4m4]

+1 to this. I have a situation where I am doing a dual-filter on a table. The table shows log entries, which have a bunch of string & number fields, as well as a enum field for the log level type. The user can enter a text filter in an input and can select a minimum level to display.

I am setting the datasource filter to the value of the text box:
this.m_cTableDataSource.filter = sFilterValue;

and then also overriding the filter predicate to check that the logs have at least the minimum specified log level:

// Setup the filter to handle the minimum log level
this.m_cTableDataSource.filterPredicate =
    (data: AIMLogEntryModel, sFilterVal: string): boolean => {

        // Filter first to make sure it meets minimum log level criteria
        const bLevelOk: boolean = MyLoggingUtils.passesLevelFilter(data, this.m_eMessageTypeFilter);

        let bValueMatch: boolean = false;
        if (bLevelOk) {

            // Search through all log attributes to look for a match
            Object.values(data).forEach((oVal) => {
                if (String(oVal).toLowerCase().includes(sFilterVal.toLowerCase())) {
                    bValueMatch = true;
                }
            });

        }

        return bLevelOk && bValueMatch;

    };

This works fine if the user enters any text in the filter box, which then triggers the filterPredicate to run. But if there is no text in the filter value, then the filterPredicate does not run at all, which is not desired. It took me about an hour of debugging and searching to realize that the filterPredicate does not run if the filter value is set to a falsey value.

You could go 3 ways with this:
1) update the docs to clarify this situation (it took me about an hour of debug and search time to narrow down to and figure out the behavior of the Table Data Source filter functionality) and suggest that users implement their own filter scheme if they need this to work (or perhaps prefix the search filter and then remove the prefix in filterPredicate)
2) update the code HERE so the filterPredicate function is always run if it is overriden by the user, regardless of filter value. Just move the null/'' check inside the default filter function, that way a user can easily override it when they override filterPredicate. Would be a breaking change.

this.filteredData =  this.filterData(data) // always, no null check here
...
filterData(data){
   if (this.filter == null || this.filter === '') {
       return data;
    } else {
        // Existing filter code
    }
}

3) provide a configuration option to the DataSource that configures whether it behaves like 1 or 2 to give the user the most flexibility.

I'd like 2 or 3, but can live with any of these.

[s4m0r4m4]

Hi @annieyw , thanks for tagging this issue. I like your PR and think it's a good improvement. Maybe I'm reading the PR wrong, but it does not seem to completely address this issue. You changed the null check from:
!this.filter

to:
(this.filter == null || this.filter === '')

If a custom filterPredicate is provided that relies on more than the filter string (or does not rely on the filter string at all), the filterPredicate will still not be run even after your PR. As I mentioned in my comment above, I see 3 different paths forward and would be happy to discuss further! I would even be willing to contribute a PR.

[annieyw]

@crisbeto Can you take another look at this?

[crisbeto]

@s4m0r4m4 I think that your second proposal is reasonable (moving the null check inside the predicate), but I suspect that doing so at this point will be a breaking change for some apps. Also it's worth noting this part of the docs:

**Note:** This class is meant to be a simple data source to help you get started. As such
it isn't equipped to handle some more advanced cases like robust i18n support or server-side
interactions. If your app needs to support more advanced use cases, consider implementing your
own `DataSource`.

Since we can't realistically handle all of the different use cases out there, my fix in #19094 tried to provide a reasonable default.

[s4m0r4m4]

Yes I agree this would be a breaking change. You make a reasonable point (about it being a simple data source, not necessarily intended for complex setups). As per option 1 above, I'd be ok if you wanted to leave it that way and just clarify in the docs how the filterPredicate behaves, though I do think that the behavior in Option 2 feels more intuitive to me personally.

[angular-automatic-lock-bot[bot]]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{_This action has been performed automatically by a bot._}