search

alwint3r / sequelize-datatable-node

MIT License
11 stars 12 forks source link

Add column search, regex search #12

Closed egisz closed 4 years ago

egisz commented 4 years ago

Hi,

I have added:

@alwint3r, could you publish to npmjs, so that we could include to our projectsr (or grant me permissions)? I would like to avoid publishing my own npm as duplicate.

alwint3r commented 4 years ago

First of all, this is great! This should solve the compatibility issue with newer versions of sequelize. However, i can not test this using PostgreSQL database due to PostgreSQL driver installation failure. Probably it's because of my messed-up OS installation. Other than that, all tests are passed.

Before merging this PR and publish the new version to npm, maybe we should state that node 4 is no longer supported in the README.md since there are async functions all over the place. Since we're dropping support for older version of node, should we bump the major version instead of minor? Although there's no major change in the API. I'd like to hear your thought on it.

Thanks a lot!

egisz commented 4 years ago

Hi, It's good that you mentioned tests, I double checked and fixed one bug. I missed that datatables search.regex parameter is passed as string with value true or false. I expected boolean :) Also fixed test with case insensitive search, it should fail on mysql
I have tested on PostgreSQL, all tests now passed.

image

Before merging this PR and publish the new version to npm, maybe we should state that node 4 is no longer supported in the README.md since there are async functions all over the place.

Good point, updated README. in last commit.

Since we're dropping support for older version of node, should we bump the major version instead of minor? Although there's no major change in the API. I'd like to hear your thought on it.

You are right, I was very confused regarding versions. In npmjs there is another published package https://www.npmjs.com/package/sequelize-datatable and it is already claiming version 2.0.0. So now it would be wise to set it higher version as this is more fresh and it has more features. This is why I thought of version 2.1.0. But we could easily name it 3.0.0 as well. I just want to avoid mess in npmjs, so that developers could easily find needed module.