product_picker.js sku lookup actually works

[jrochkind]

This is used in the backend UI for picking products for
Spree::Promotion::Rules::Product, might be used other places too.

There was code in there that looked like someone was trying to
let the query match a product name or sku. But it totally didn't work,
didn't do anything, was wrong in like two or three different ways. (useless ransack attribute name for sku; m has to be inside the q; value of m has to be lowercased or not OR).

Took me a couple hours to figure out what was going on, so I figured
I'd share the fix. Sorry, don't know how to test this, looks like
there aren't any tests for this kind of JS yet?

[jhawthorn]

Nice catch. Looks good to me 👍

For testing this we would normally use a feature test. There is a select2_search helper that can be used for this. IMO that isn't necessary here for this to get merged (but would always be appreciated).

[jrochkind]

Thanks! Last time I looked the CI was failing although I couldn't figure out why; unrelated?

I also wonder if the sku lookup in this area should be exact match only, not partial match -- or they're going to be flooded with imprecise results after sku lookup actually works.

I suggest I add that before merging (I've done that in my local fix in my local app).

[cbrunsdon]

I'm also good with this, thanks @jrochkind, looks like that would be pretty terrible to debug 👍

[jhawthorn]

How about using sku_start? IMO that's the best compromise between wildcard on both ends and exact match.

[mamhoff]

This solution only looks for master variant SKUs. 👍 for `sku_start.

[jrochkind]

Okay on sku_start.

Ah, true on the master variant only thing, that's a problem. Not sure how to solve that, I'll see if I can figure it out.

Keep in mind that at present, sku lookup simply does not work at all, it's not looking up skus.

[jrochkind]

Okay, here's my stab at it.

Haven't really tested this. It probably needs automated tests, but I'm not sure I'm going to be able to do it.

[mamhoff]

You could test it with a feature spec that runs with JS enabled. Create two products, one with variants, and then check for the presence of the right variants in the select input.

[jrochkind]

I'm not sure how to start here regarding specs. Can anyone suggest an existing feature spec I can model this off of?

Or is there anyone that wants to help by writing a feature spec?

Or do any committers want to @jhawthorn that a feature spec may not be required for a merge? (Although I realize things have turned out slightly more complicated since his comment to that effect).

I suspect the product_picker.js currently has no specs because it's kind of painful to write.

[jhawthorn]

Thanks @jrochkind

I've created #1171 which adds some feature specs to test the fixed behaviour.