contentful / contentful_model

A lightweight wrapper around the Contentful api gem, to make it behave more like ActiveRecord
MIT License
44 stars 42 forks source link

category field params breaking for category with commas #126

Closed FuzzWorley closed 5 years ago

FuzzWorley commented 5 years ago

Category field params are not working as expected for one of our categories: "Design, Photography, & Fashion". My theory is that it has something to do with the commas, as it's our only category (of 8) that isn't working, and the only category that has commas in it.

Using find_by, and passing a string to the category field, the query works as expected:

cats = Contentful::Article.find_by(production: true, category: "Design, Photography, & Fashion").limit(1000).load.to_a
cats.count
 => 31 

But if I pass it as an argument in an array, it won't return any results for that category:

cats = Contentful::Article.find_by(production: true, category: ["Design, Photography, & Fashion", "Sports & Games"]).limit(1000).load.to_a
cats.count
 => 11 
cats.map(&:category).uniq
 => ["Sports & Games"] 

Using .params instead of find_by is even worse off:

no_design = Contentful::Article.find_by(production: true).limit(1000).params({'fields.category[nin]' => "Design, Photography, & Fashion"}).load.to_a

no_design.map(&:category).uniq
 => ["Film & TV", "Culinary Arts", "Music & Entertainment", "Politics & Society", "Design, Photography, & Fashion", "Sports & Games", "Writing", "Science & Technology"] 

"Design, Photography, & Fashion" should have been excluded but clearly was not.

design = Contentful::Article.find_by(production: true).limit(1000).params({'fields.category[in]' => "Design, Photography, & Fashion"}).load.to_a
 => [] 

We have 31 articles in this category, as you can see in the first code block, but nothing is returned.

Please fix ASAP. This is breaking our application. Let me know if there is any more data or information that would be helpful for your debugging. Thanks!

dlitvakb commented 5 years ago

Hey @FuzzWorley,

Your issue is happening because it's treating each of the comma separated values as a different element on the array (this is how arrays are interpreted when sent as query parameters in an HTTP request), in order to get this to work for your case when the comma is part of what you're searching for, you need to replace the , character to the HTML URL Encoded Entity sequence that replaces the comma, which is %2C.

For reference, here you have a complete list of all HTML URL Encoded entities: https://www.w3schools.com/tags/ref_urlencode.asp

Hope this helps,

Cheers

FuzzWorley commented 5 years ago

Hey @dlitvakb ,

That makes sense, thanks for the info! Still having trouble after encoding.

CGI::escape(article_category)
=> "Design%2C+Photography%2C+%26+Fashion"

Contentful::Article.all.params({"fields.category[in]"=>"Design%2C+Photography%2C+%26+Fashion"}).load.total
=> 0 #where there should be 31 entries.

This string matches the one that I get from the w3schools generator, and I've also tried multiple encoding libraries because that one didn't work: CGI.escapeHTML CGI::escape URI.escape

And none of them seem to work. It appears URL encoding breaks this query in general:

(byebug) Contentful::Article.all.params({"fields.category[in]"=>"Culinary Arts"}).load.total
164
(byebug) Contentful::Article.all.params({"fields.category[in]"=>"Culinary+Arts"}).load.total
0
(byebug) Contentful::Article.all.params({"fields.category[in]"=>"Culinary%20Arts"}).load.total
0

It only appears to work when not encoded. Again thanks for your help!

P.S. Using a different query where the encoded string is passed in an array doesn't work either:

2.5.3 :373 > resp = Contentful::Article.find_by(production: true, category: ["Design%2C+Photography%2C+%26+Fashion" , "Sports & Games"]).limit(1000).load.to_a

2.5.3 :374 > resp.map(&:category).uniq
 => ["Sports & Games"] 
dlitvakb commented 5 years ago

Thanks for trying this out, I think that the issue is that your %2C is getting actually transformed to %252C (basically the % character itself getting URL encoded.

I'll take a look into what possibly be done to avoid this issue.

Cheers

FuzzWorley commented 5 years ago

Cool, I appreciate your time on this as it's quite important to our business. I'm not convinced that %2C being transformed is the issue though based on the fact that even when I encode something that returns no percentages, the query still breaks.

Contentful::Article.all.params({"fields.category[in]"=>"Culinary Arts"}).load.total
 => 164

CGI::escape("Culinary Arts")
 => "Culinary+Arts" 

Contentful::Article.all.params({"fields.category[in]"=>"Culinary+Arts"}).load.total
 => 0
FuzzWorley commented 5 years ago

Hey @dlitvakb

Any update on this issue?

dlitvakb commented 5 years ago

Not yet, sadly hadn't got dedicated time to look into this until today.

Will check and come back to you as soon as I find something.

Cheers

dlitvakb commented 5 years ago

Hey @FuzzWorley,

I've looked into the SDK - it is sending the proper percent encoded values (so you don't need to do it yourself). This means that there is a bug in our backend service. I will forward this to the corresponding team and give them the appropriate context.

Cheers

dlitvakb commented 5 years ago

Looks like our backend treats %2C and , as the same character for in queries, therefore it will not work for your particular use case. If possible, you can replace it to using eq instead, and it will be working properly.

eq also checks for single values within text arrays, therefore should be equivalent for looking if a tag is within a list of tags.

This is sadly a technical limitation of our backend, hopefully you can use the workaround.

Cheers

FuzzWorley commented 5 years ago

Thanks @dlitvakb! This appears to be fixing the issue for now.