Query_filter updateFilter() issue


(Jacob Brandt) #1

Let's take a look at the updateFilter

queryFilter.updateFilter = function (filter) {
  const mergedFilter = _.assign({}, filter.source, filter.model);
  mergedFilter.meta.alias = filter.alias;
  //If the filter type is changed we want to discard the old type
  //when merging changes back in
  const filterTypeReplaced = filter.model[filter.type] !== mergedFilter[filter.type];
  if (filterTypeReplaced) {
    delete mergedFilter[filter.type];
  }

  return angular.copy(mergedFilter, filter.source);
};

To me the comment seems like it wants to make sure if you are changing the filter type from the source then it will remove it from the mergedFilter. However, look at how it's deleting from mergedFilter. It will only remove a property from it that matches the passed in filter's type. This seems wrong when I see filter.type and think that it identifies the filter's type when in reality it looks like it represents the source filter's type. The only way I have been able to update filters is to incorrectly set the type value of the passed in filter to the property I want removed from the merge with the filter source. So if I have a filter of geo_polygon and want to update it to geo_shape I have to set the filter type of the passed in value to geo_polygon (i.e. The type of the original source and not the type it will end up being). Does this seem completely confusing to anyone else? It's like the type property of the passed in filter is suppose to represent the source filter type. It should be named as such if that is the case.


(Felix Stürmer) #2

Hi @brandtj,

I have to agree that this function is very hard to make sense of - if that's even possible. In addition to having the flaws in the internal logic that you pointed out, it seems to be written specifically to work with the JSON editing functionality of the filter bar, which is no beauty itself.

The whole query_filter interface leaks abstractions left and right and provides little guarantees about anything. As such it does not provide significant value to external consumers. And don't get me started on the way it uses root scope watches... :rolling_eyes:

Anyway, if you want to modify existing filters from outside of the filter bar (e.g. from plugin code), just modifying the filter object by yourself seem no worse than using that function. Sorry that you had to fight with that code and thank you for bringing it to our attention. I will add that to my todo list.


(system) #3

This topic was automatically closed 28 days after the last reply. New replies are no longer allowed.