-
-
Notifications
You must be signed in to change notification settings - Fork 323
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
pull common SQL adapters and helpers into reusable functions #595
Conversation
@verdverm You are doing an amazing job here. I am doing something a bit differently in #524. That is generating sql queries with Quick intro to my approach:
With the help of Branch is currently work in progress and many things still do not yet fully work as intended. I am officially on vacation an really promised my wife I will not do any work over the holidays. If any of this interest you we can discuss it further, but I will not really be responsive, maybe twice day at most. After the holidays I will continue on this and I can already see many things you done, that can be useful to me in what I am doing. |
I was definitely thinking about how the query path will probably be needed in the |
@verdverm Looks interesting! A bit hard to wrap my head around all the code. Will take a closer look, when I have some time. |
I added a HOC Table based on react-table that integrates with the paging crud helper in my madd sci PR |
I have a hard time wrapping my head around it too! |
import { decamelize } from 'humps'; | ||
import { has } from 'lodash'; | ||
|
||
export function currentFilter(queryBuilder, filter) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this function is specific to user
module. Why is it here, then?
|
||
uses a more advanced filtering input and processing. | ||
|
||
See https://github.com/sysgears/apollo-universal-starter-kit/issues/569 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it okay to send user to the PR in the docs?
I've upgraded this system a bunch on my mad-sci branch. Lots of other helpers for other things too. They are starting to become better together. Check the latest commit(s) over there |
Wouldn't it make sense to pull all those upgrades into smaller PR? mad-sci branch is f*****g huge and kinda impossible to do a PR or make sense out of it. |
c53e4c9
to
3fa038e
Compare
22cc234
to
06fd4b4
Compare
@verdverm Are you going to take into account the points I have raised in this PR to finish it and merge? See my comments under lines of your code above |
I cut a new PR on this one too. #650 |
…functions