Skip to content
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

Odata Query encoding #463

Closed
meta-akshita-agrawal opened this issue May 20, 2020 · 7 comments · Fixed by #471
Closed

Odata Query encoding #463

meta-akshita-agrawal opened this issue May 20, 2020 · 7 comments · Fixed by #471

Comments

@meta-akshita-agrawal
Copy link

I'm submitting a Bug report

Your Environment

Software Version(s)
Angular 9.0
Angular-Slickgrid 2.17
TypeScript 3.7

Describe the Bug

Hi Author,

Thanks for making such a great library :). I am reporting this issue for backendservice oData query
So the current behaviour is that when a multiselect value contains a value having special symbol(/, @, #, etc.) it encodes the string and then oData query is built.
This works properly when a single value is selected but when user selects multiple values encoding does not work properly. I replicated the issue in your code too.
https://github.com/ghiscoding/Angular-Slickgrid/blob/master/src/app/examples/grid-odata.component.ts

Also, note that single quotes and double quotes are not encoded if present in the value

Steps to Reproduce

Expected Behavior

  • Change singleselect(male/female) to multiselect in column definitions
  • Change value of male label from male to ma/le in collection
  • Now select male and female both from dropdown

Current Behavior

Possible Solution

Code Sample

{ id: 'gender', name: 'Gender', field: 'gender', filterable: true, filter: { model: Filters.multipleSelect, collection: [{ value: '', label: '' }, { value: 'ma/le"', label: 'male' }, { value: 'female', label: 'female' }] } }

@ghiscoding
Copy link
Owner

Sorry but I don't really want to do any changes for this, if I encode the value then the users (you) will have to decode it. I don't think this is ideal and I think you should encode/decode yourself.

@meta-akshita-agrawal
Copy link
Author

meta-akshita-agrawal commented May 20, 2020

Umm.. Sorry if I was unable to make myself clear, but the issue is it encodes the string if I only select male
$inlinecount=allpages&$top=20&$orderby=Name asc&$filter=(Gender eq 'ma%2Fle')
and if I select both male and female:
$inlinecount=allpages&$top=20&$orderby=Name asc&$filter=(Gender eq 'ma/le' or Gender eq 'female')
It doesn't.

I have no problem in decoding/encoding but the oData query produced is not consistent.
How will I know when its already encoded and when it's not.

Hope you are understanding my problem.

@ghiscoding
Copy link
Owner

ghiscoding commented May 20, 2020

Hmm ok I see, can you try to troubleshoot the problem and fix it? I'd be happy to have help on this since we don't use OData anymore.

By looking at the code, I see that I dealt with single search by escaping single quote and encoding on this line but I don't remember how it's dealing with multiple search values. The project is already setup with debugging, if you could just clone and contribute to a fix that would be really great. Thanks

I wish you had find your issue before I release a new version though (first in 2 months). Oh well..

@meta-akshita-agrawal
Copy link
Author

I can see the issue is on this line.
I guess this works only when searchTerms has length 1. In this case its ['ma/le', 'female']

@ghiscoding
Copy link
Owner

No I don't think so, that line is only to check if there's an Operator that was typed in the filter. It's most probably around this line but like I said earlier, I wish that you troubleshoot with your use case then contribute by providing a Pull Request. Thanks

@meta-akshita-agrawal
Copy link
Author

Sorry I tagged the wrong line.
This seems correct

let fieldSearchValue = (Array.isArray(searchTerms) && searchTerms.length === 1) ? searchTerms[0] : '';

@ghiscoding
Copy link
Owner

This is now fixed in latest version 2.18.x. I'd be happy to have more help next time around, thanks.

Please upvote ⭐ if you haven't already.
Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants