Better handling of filters based on column type

[rmallah]

the SQL operator for column filtering is now based on the type of the property specified in collection.

[rmallah]

Please help with the test failures , what is the proper way to run the tests in development environment before creating any PR?

[mohawk2]

Observations:

• you haven't added tests for this new behaviour
• also no docs
• I don't understand why the deep indentation (but maybe the owner is Ok with it)

The test failures may partly be because of JSON boolean or other coercion issues, cf this:

        #   Failed test at t/api.t line 534.
        #     Structures begin differing at:
        #          $got->{contact} = 1
        #     $expected->{contact} = '1'

As you said on IRC, you can run the tests with one of:

• perl Makefile.PL && make test
• prove -lr t

Another problem is that currently the Yancy::Backend::Test isn't very SQL-aware, which means you adding SQL things to the API controller will not work out well. Another direction I believe the owner wants to go is to add a MongoDB backend, which is also not SQL. Therefore probably you'd want to find a more expressive way to require truth and/or an exact integer value. If so, you'd probably use JSON values.

[rmallah]

thanks for the comments. Currently all existing backends are SQL based. I suppose even the existing 'like' operator would not work with nosql databases.

I am new to yancy / openapi . What would be a better way to handle the original problem? which is i needed to filter on a integer column of an sql based table. (the db is postgresql btw).

[preaction]

My current idea for future support of MongoDB is that the MongoDB backend will need to understand the SQL::Abstract WHERE clause. That puts the burden off of us for now when adding general features: Features added to the editor are problems for the MongoDB backend to figure out how to support (or for us to figure out a way to have degraded support in the editor for backends that don't understand things).

Yancy::Backend::Test is very much the same way: Support for SQL::Abstract WHERE clause options is added as the editor/controllers need it.

Tests/docs are important though: Tests of the filters from the API controller are the minimum amount of tests we'd need to merge this.

It may also benefit you to add tests to t/lib/Local/Test.pm sub test_backend to ensure that all of the SQL::Abstract WHERE clause options you're using are well-supported by all the backends. If one backend needs translation (I don't think \'IS TRUE' is supported by MySQL for example), we can do that in the backend module's list method. If there's no other way to implement a query other than raw SQL with a scalar reference, that's how we can do that compatibly across databases.

[rmallah]

its possible to change \'IS TRUE' to https://metacpan.org/pod/SQL::Abstract#Unary-operators:-bool

But prior to that i am facing problem in writing tests where the bool fields are being used in query params.

eg: below test fails

      $t->get_ok( $api_path . '/people?contact=1' )
          ->status_is( 200 );

which message: Unknown match ref type: true at <path/to>/lib/git/Yancy/t/lib/Yancy/Backend/Test.pm line 62.

[preaction]

Right. You'll need to add support for true/false to t/lib/Yancy/Backend/Test.pm. That's the "Test" backend, which is more accurately an in-memory mock backend used for standard testing (because it's fast and requires no external modules).

[preaction]

Hey, there've been a few changes to some of the files you've been working on in this ticket, so there are some conflicts that will need to be resolved. Did you still have time to work on this? Did you want some help getting this merged in, or perhaps did you want someone else to finish up the last little bits?

[mohawk2]

To echo what @preaction said, I'll also be happy to take this on and finish it up. Also feel free to come on the IRC channel!

[rmallah]

Hi I would like to revisit this issue after sometime. I would love to see /yancy/api much powerful than what it is today as it is great to have such tools for rapid app development.

However i am not able to devote the time and attention it demands at this moment. I am proceeding to close this PR and shall re-submit it later based on the current changes which has been done.

Thanks to everyone involved.