Escape comma #25
Escape comma #25
Conversation
Related fields sort filter
|
Getting the timeout failure again... |
|
@jamesdixon Do you have time to look at this? |
|
@alechirsch unfortunately, i do not. Right in the middle of beginning the beta phase for our product. I'll take a look when I can! |
|
@alechirsch i'd be more than happy to add you as a contributor if you'd be interested |
|
@jamesdixon I am interested, I have never been a contributor for an open source package before though. Would that give me publishing privileges to npm as well? |
|
@alechirsch initially, it would just give you the ability to manage issues, merge PRs, etc. |
|
@jamesdixon Sounds good to me |
|
@jamesdixon Any updates for this? |
|
@alechirsch sorry for the delay. Added you as a collaborator |
|
@jamesdixon are you good with this PR? |
|
@alechirsch are you talking about the use case for something like
? |
|
This is the use case where you have a comma in the data itself, for example if you had a row with the data "city, state", what you send across is "city\\\\,state" |
|
Ok, thanks. Would you mind adding a test for this specific case? |
|
@jamesdixon Sure thing, I have been very busy recently. Not sure when I will get it in. |
|
@jamesdixon Finally got around to adding a test for this. |
| value = value.toString().indexOf(',') !== -1 ? value.split(',') : value; | ||
| qb.whereIn(key, value); | ||
|
|
||
| if (_isNull(value)){ |
There was a problem hiding this comment.
This is the fix for filtering on null
…e select clause with the value
|
@jamesdixon Should I go ahead and merge this? Would you be able to release to npm? |
|
@alechirsch my apologies for the delayed reply. Things have been very busy for me and it's honestly been difficult to divert attention to my open source projects. That said, feel free to merge this and the other PRs. I'd recommend we label this as a beta given that there have been a lot of changes. Once you've merged, I can publish to NPM and start testing in my own project. Thanks for all of your hard work! James |
|
@jamesdixon Great, I agree this should be kept labeled as beta. |
|
@alechirsch would you mind updating the changelog and close out any bugs that these PRs resolved? I can publish to NPM once that's done. Thanks! |
|
@jamesdixon I closed related issues. The change log does not have anything for beta1, was there a separate place where you put that or was it not added. |
|
@alechirsch thanks. It probably wasn't added, but I wouldn't worry about it at this point 👍 |
I had a use case where I needed to filter on items that included a comma. I added some logic to support escaping the comma character.