Skip to content
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

Filter collection items with a string contains instead of equal #808

Closed
jr01 opened this issue Jul 14, 2021 · 8 comments
Closed

Filter collection items with a string contains instead of equal #808

jr01 opened this issue Jul 14, 2021 · 8 comments

Comments

@jr01
Copy link
Collaborator

jr01 commented Jul 14, 2021

I'm submitting a Feature request

Motivation / Use Case

Hi!

My oData backend returns a field that holds a comma delimited string of values: [{ roles: 'User,Sales,SuperUser' }].

I'm presenting this in the grid with a formatter that displays each value as a (bootstrap) badge 📛. Nice!

Ultimately I want to be able to search with a (custom) multiselect FilterControl on any combination of the values.

Now at first I started with a single select and because searching for User shouldn't return SuperUser I added a , to the query field and each queried value and use a 'contains':

	queryFieldFilter: 'concat(roles, \',\')',
	filter: {
          collection: [
            { value: 'User,', label: 'User' },
            { value: 'Sales,', label: 'Sales' },
            { value: 'SuperUser,', label: 'SuperUser' },
          ],
          operator: OperatorType.contains,
          ...

This works and $filter=contains(concat(roles, ','), 'User,') is send to the oData backend. 😀

The next step is to get the multiselect working and I noticed OperatorType.inContains which sounds like it fits my need, but IN_CONTAINS isn't implemented: https://github.com/ghiscoding/Angular-Slickgrid/blob/master/src/app/modules/angular-slickgrid/services/grid-odata.service.ts#L352 😢

Now I'm not completely sure what the purpose of IN_CONTAINS is and I'm wondering if it can be used and if I can implement it roughly like:

	...
	else if (operator === 'IN_CONTAINS') {
		for (let j = 0, lnj = searchTerms.length; j < lnj; j++) {
			const s = odataVersion >= 4 ? `contains(${fieldName}, ${searchTerms[j]})` : `substringof(${searchTerms[j]}, ${fieldName})`;
			tmpSearchTerms.push(s);
		}
		searchBy = tmpSearchTerms.join(' or ');
	...

or if there would be a different direction/solution perhaps? 😄

What do you think?

Expected Behavior

IN_CONTAINS can be used in oData and other backends.

Other Information

@ghiscoding
Copy link
Owner

ghiscoding commented Jul 14, 2021

The IN_CONTAINS was added when creating the Example 27 and yes it's only available for local grids not grids with a backend service. In the Example 27, it's for this column shown below where the cell text is a CSV string and that is what the IN_CONTAINS is for, it will search in that CSV string (in fact it only works with CSV string), the Example is also using useFormatterOuputToFilter

id: 'languageName', field: 'languages.name', name: 'Names', width: 60,
formatter: Formatters.arrayObjectToCsv, columnGroup: 'Language',
params: { propertyNames: ['name'], useFormatterOuputToFilter: true },
filterable: true,
// this Filter is a bit more tricky than others since the values are an array of objects
// what we can do is use the Formatter to search from the CSV string coming from the Formatter (with "useFormatterOuputToFilter: true")
// we also need to use the Operator IN_CONTAINS
filter: {
model: Filters.multipleSelect,
collectionAsync: this.getLanguages(),
operator: OperatorType.inContains,
collectionOptions: {
addBlankEntry: true,
// the data is not at the root of the array, so we must tell the Select Filter where to pull the data
collectionInsideObjectProperty: 'data.languages'
},

image

For this kind of filtering, I typically use a Select Filter (single/multiple) and build the collection with all the available values and that would work with a backend service as well. Are there any reasons why you can't do that or are the values too dynamic and unknown to do that?

The code for the IN_CONTAINS is a simple split and includes text search. It might be possible to add this to the OData Service but I'm very close to release the next major version and any new feature should really go into Slickgrid-Universal monorepo which will be used by Angular-Slickgrid 3.x, that would be this Slickgrid-Universal OData package (all backend services will become opt-in to make smaller builds, so you won't have GraphQL in your future build even when you don't use it like today). At the end of the day, I'm always open to contributions (especially yours on the backend side) and if you want to add that in the backend services then I'd be ok with that but it might become only available in the next major version (it's too much work for 1 person, that is me, to support multiple versions once a new major is out) and just so you know all the contributions you've made in the past were all ported to Slickgrid-Universal as well. I'm anxious to have only 1 common place (Slickgrid-Universal) to add features instead of 3 different libs that I support with the exact same code (Angular-Slickgrid, Aurelia-Slickgrid, Slickgrid-Universal), Slickgrid-Universal will fix that and I'm expecting to ship probably before the end of this week :)

For reference here's the next breaking PR #803 and the announcement of it in the last release

@jr01
Copy link
Collaborator Author

jr01 commented Jul 15, 2021

For this kind of filtering, I typically use a Select Filter (single/multiple) and build the collection with all the available values and that would work with a backend service as well. Are there any reasons why you can't do that or are the values too dynamic and unknown to do that?

Well, that multiple select only works if the backend field contains a single value.

For example: [ { id: 1, role: 'SuperUser' }, { id: 2, role: 'User' }] and when you multiselect both User and SuperUser and use OperatorType.in the oData query would be $filter=(role eq 'SuperUser' or role eq 'User') and that indeed works well and returns both users.

But in my case a user can have multiple roles and the roles field is a plain string field which holds comma delimited values (CSV). So like [ { id: 1, roles: 'SuperUser' }, { id: 2, role: 'User,Sales' }] where the 2nd user has both User and Sales roles. Now when you multiselect User and SuperUser and use the OperatorType.in the oData query is $filter=(roles eq 'SuperUser' or roles eq 'User') which doesn't return the 2nd user.

I'm very close to release the next major version

Cool!

I'm anxious to have only 1 common place (Slickgrid-Universal) to add features instead of 3 different libs that I support with the exact same code

I understand and agree we should not fix this in the current Angular-Slickgrid, also because I found a workaround:

export class CustomGridODataService extends GridOdataService {
  updateFilters(columnFilters: ColumnFilters | CurrentFilter[], isUpdatedByPresetOrDynamically?: boolean): void {
    const replacers: { searchValue: string; replaceValue: string}[] = [];
    if (!Array.isArray(columnFilters)) {
      for (const key in columnFilters) {
        if ({}.hasOwnProperty.call(columnFilters, key)) {
          const columnFilter = columnFilters[key];
          if (columnFilter.operator === OperatorType.inContains) {
            columnFilter.operator = OperatorType.in;

            const delimiter = columnFilter.columnDef?.params?.inContainsDelimiter ?? ',';
            const searchField = columnFilter.columnDef.queryFieldFilter ?? columnFilter.columnDef.queryField ?? columnFilter.columnDef.field;

            for (const searchTerm of columnFilter.searchTerms) {
              // Example with , as delimiter and searching for a role 'User'. Imagine the backend has [{ roles: 'SuperUser,User,UserEditor' }]
              //
              // - searchValue: roles eq 'User'
              // - replaceValue: contains(concat(',', concat(roles, ',')), ',User,')
              //
              // On the backend this translates to SQL: ... WHERE CHARINDEX(',Sales,', ',' + Roles + ',') > 0
              replacers.push({
                searchValue: `${searchField} eq '${searchTerm}'`,
                replaceValue: `contains(concat('${delimiter}', concat(${searchField}, '${delimiter}')), '${delimiter}${searchTerm}${delimiter}')`
              });
            }
          }
        }
      }
    }

    // Monkey patch the 'updateOptions' method.
    const oldUpdateOptions = this.odataService.updateOptions;
    this.odataService.updateOptions = (options: Partial<OdataOption>): void => {
      if (typeof options.filter === 'string') {
        for (const replacer of replacers) {
          options.filter = options.filter.replace(replacer.searchValue, replacer.replaceValue);
        }
      }

      oldUpdateOptions.apply(this.odataService, [options]);
    };

    try {
      super.updateFilters(columnFilters, isUpdatedByPresetOrDynamically);
    } finally {
      this.odataService.updateOptions = oldUpdateOptions;
    }
  }
}

Not the nicest, but it's good enough 😄

For the GridOdataService in Slickgrid-universal I can imagine:

  1. support IN_CONTAINS with my contains/concat approach ^^^ - even though it might be specific to my situation... 🥺
  2. make it possible to override parts of the GridOdataService behavior, so if someone needs some 'custom' oData behavior it easier to do.

I can work on that, but will be after summer vacation...

@ghiscoding
Copy link
Owner

ghiscoding commented Jul 15, 2021

I can work on that, but will be after summer vacation...

Nice, I'm going in vacation the week after that and that is why I'm pushing hard to release the new major version before then 😉

For the GridOdataService in Slickgrid-universal I can imagine:

  1. support IN_CONTAINS with my contains/concat approach ^^^ - even though it might be specific to my situation... 🥺

if it's something that could also be used by others, feel free to push new code and/or new function(s) on the Slickgrid-Universal repo, like I said earlier the Service is exactly the same as the one from Angular-Slickgrid (which the exception that it's not a standalone package)

  1. make it possible to override parts of the GridOdataService behavior, so if someone needs some 'custom' oData behavior it easier to do.

There's only a few private properties that I could switch to protected but apart from that I guess you can already extends the Service (which is what you did now isn't it)?

I didn't use OData in a few years but I know you are a big user of it and there are also a lot of other developers using it (probably more than the GraphQL Service). I'm always open to your contributions and they always bring enhancements, so feel free to contribute some more in the future. I won't be able to test these enhancements myself because I don't have access to an OData Server anymore but I can still review the changes.

@ghiscoding
Copy link
Owner

hey what's the try/finally? I've never seen this syntax, is that like a finally from a Promise and you can omit the catch?

@jr01
Copy link
Collaborator Author

jr01 commented Jul 15, 2021

The try statement consists of a try-block, which contains one or more statements. {} must always be used, even for single statements. At least one catch-block, or a finally-block, must be present. This gives us three forms for the try statement:
try...catch
try...finally
try...catch...finally

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/try...catch

@ghiscoding
Copy link
Owner

ghiscoding commented Jul 20, 2021

Some updates

  1. Angular-Slickgrid next major 3.x version is out, see Release 🚀
  2. also before releasing the new major version, I did process a PR to go through all Services (not just OData) in Slickgrid-Universal and changed every private properties I found to protected, so that should address this comment you mentioned earlier and pretty much everything is now extendable (editors, filters, services).

make it possible to override parts of the GridOdataService behavior, so if someone needs some 'custom' oData behavior it easier to do.

@ghiscoding
Copy link
Owner

@jr01 if there's no update on this, I think I'll turn this into a "Discussion" instead of an "Issue" since I think it's pretty much that anyway...

@jr01
Copy link
Collaborator Author

jr01 commented Aug 19, 2021

@ghiscoding I'll migrate to 3.x first and will then create an issue or PR on the new repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants