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

The match filter does not work for string arrays when the value is coercible to another primitive type #1

Closed
pfurini opened this issue May 4, 2016 · 6 comments

Comments

@pfurini
Copy link

pfurini commented May 4, 2016

Given the following thinky model:

const EntryCategory = thinky.createModel('entry_category', {
  id: type.string(),
  extCategories: [type.string().allowNull(false).min(1).max(50).alphanum()]
}, {
  enforce_extra: 'strict'
});

and the following resource:

const Category = SoftDeleteResource.extend({
  options: {
    queryset: Model.filter({}),
    filtering: {
      extCategories: 1
    },
    allowed: {
      list: { get: true, post: true },
      detail: { get: true, put: true, patch: true, delete: true }
    }
  },
  fields: {
    id: {type: 'char', readonly: true},
    extCategories: {type: 'array', default: () => []}
   }
}

If you try to apply a filter like extCategories__match=123 it does not work as expected, because in the filter implementation:

  , 'match': {
    value: function (r, field, term) {
      return toField(r, field).contains(term);
    }
  }

term is a number and not a string, so the ReQl query does not return any result even if there are documents containing "123" in extCategories array.
The only elegant way (IMHO) to resolve this issue is to add an array type option to node-tastypie (see https://github.com/esatterwhite/node-tastypie/issues/14), and to force casting the filter value before applying the filter.

@esatterwhite
Copy link
Member

@Nexbit things in an array can get complex and not guaranteed to be homogeneous. I think you might really want a custom field.

You can use actual field instances in the resource definition

Resource.extend({
    all_ints: new IntArrayField({default:1})
});

You could also register a field with the built in fields

Object.defineProperty(tastypie.fields, 'iarray',{
    get: function(){
        return IntArrayField;
    }
});

then it will work like before

Resource.extend({
    all_ints: {type:'iarray', default:1}
});

I was also planning on breaking the reql filters out into separate modules so you could tweak them or define new ones as well. Which would also be a good option. Thats what I ended up doing with the bookshelf implementation.

@esatterwhite
Copy link
Member

esatterwhite commented May 4, 2016

@Nexbit You could also do per field hydration to clean data on a particular field

Resource.extend({
   hydrate_extCategories: function( bundle ){
       bundle.object.extCategories = bundle.object.extCategories.map((val)=>{
              return ~~val;
       });

       return bundle;
   }
});

@esatterwhite
Copy link
Member

@Nexbit You could also make a special filter type for the use case

   'smatch': {
    value: function (r, field, term) {
      return toField(r, field).contains( "" + term );
    }
  }

@pfurini
Copy link
Author

pfurini commented May 4, 2016

Yep, I had changed the predefined match filter like you did above, but now I've added the smatch filter and restored the match one, I like the solution.
This and other fixes are in my fork of your repo, if you want to take a look. There's also the fixed implementation for the tastypie's put_detail and patch_detail methods. (I'll make them a PR ASAP)
P.

@pfurini
Copy link
Author

pfurini commented May 4, 2016

Oh I forgot to mention that the fork is updated for tastypie 3.

@esatterwhite
Copy link
Member

@Nexbit Cool.
Appreciate all the input / feedback!

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

No branches or pull requests

2 participants