search

cognitect-labs / vase

Data driven microservices
Eclipse Public License 1.0
374 stars 40 forks source link

Query action params #47

Closed milt closed 7 years ago

milt commented 7 years ago

Provides a fix for #46

The use of mapcat in ...actions/query-action-exprs works fine for a single parameter, but causes multiple parameters to be lumped into a single invocation of ...actions/coerce-arg-val. Using mapv and removing the enclosing vector fixes this.

A test for this specific behaviour was also added to the actions tests.

mtnygard commented 7 years ago

Thanks for this. I'm currently adding test cases to exercise more of the literals. Once I get those in place, I'll pull this in as the fix. Holding off on the merge until then.

milt commented 7 years ago

Cool! BTW, there should be a signed contributor agreement for Yetanalytics already, but let me know if you don't see it.

mtnygard commented 7 years ago

@milt I'll ask @ohpauleez to check on the CA.

In the meantime, I wanted to improve our testability with query & transact actions, so I ended up going a different direction with the tests. At that point, it was easier to just fix the issue directly on master than to merge this PR.

milt commented 7 years ago

I agree, those tests are way better. Thanks!