search

futil-js / contexture-core

Contexture DSL Processor
MIT License
21 stars 1 forks source link

feature/64 - pass full search to type filters, but... #65

Closed doug-patterson closed 3 years ago

doug-patterson commented 3 years ago

Fixes smartprocure/contexture#64

Description

decrapifier commented 3 years ago
Fails
:no_entry_sign: The version was not updated. Please update the version.
:no_entry_sign: The changelog has not been updated. Please update the changelog.
:no_entry_sign: This PR has failing tests. Please alleviate the errors and commit them
should handle savedSearch Failed running search for savedSearch (savedSearch) at filter: processGroup is not a function
should handle subquery Failed running search for subquery (subquery) at filter: getProvider is not a function
should handle subquery Failed running search for subquery (subquery) at filter: getProvider is not a function
should handle subquery by saved search id Failed running search for subquery (subquery) at filter: getProvider is not a function
:no_entry_sign: Please assign someone to merge this PR, and optionally include people who should review.
Warnings
:warning: The README has not been updated. Please update the README.
:warning: You should add at least 1 more reviewer to the PR
Messages
:book: Could not find any browser results.

Generated by :no_entry_sign: dangerJS against 81c092d52cbfda9a3aee7a52814e66fb2345f7ed

doug-patterson commented 3 years ago

@daedalus28

I was wanting to pass a getSearch fn down to the type filter functions when I realized that the code was already more or less set up to pass the search as an argument directly -- see changes here.

However.... this change is going to break everything since search isn't the last argument. That seems really not fun to deal with.

Suggestion: if we've never been on the true side of the ternary, really, then we can just start passing search as the last argument at the line indicated.

Let me know your thoughts on how I should proceed when you're able -- thanks!

daedalus28 commented 3 years ago

@daedalus28

I was wanting to pass a getSearch fn down to the type filter functions when I realized that the code was already more or less set up to pass the search as an argument directly -- see changes here.

However.... this change is going to break everything since search isn't the last argument. That seems really not fun to deal with.

Suggestion: if we've never been on the true side of the ternary, really, then we can just start passing search as the last argument at the line indicated.

Let me know your thoughts on how I should proceed when you're able -- thanks!

Both sides of that ternary are used - when search is present, it's for things like result. It's something I'm not happy with on the API and is mentioned as something to fix in smartprocure/monorepo-test#4