reverbdotcom / reverb-magento

Magento 1.x plugin for syncing with Reverb
Other
7 stars 10 forks source link

Mapping to top level categories doesn't work correctly #164

Closed skwp closed 8 years ago

skwp commented 8 years ago

When a magento user maps his category to a top level category like "electric guitars", we send this to reverb:

"categories":["electric-guitars"],"product_type":"electric-guitars"

This is strictly speaking not correct and the API rejects it. The categories field should only contain subcategories. If no subcategories are provided, we should only send the product type.

I will attempt to fix this on the API side so it's more permissive, but it should also be patched in magento

skwp commented 8 years ago

I think there's a deeper issue. This customer is showing that his category is mapped to a subcategory, yet what we're receiving on reverb is only the toplevel "electric-guitars" category. Please see screenshots

The problem might be that the more specific "Solid Body" category is not "winning" over the more generic toplevel electric-guitars category. What we're sending to reverb is:

{categories: [nil, "electric-guitars"], product_type: "electric-guitars"}

What we should be sending: {categories: ["solid-body"], product_type: "electric-guitars"}

jac_291-6031-503categories_png reverbcatmappings_png

skwp commented 8 years ago

@dunagan5887 if you're feeling up to it, could use your eyes on this

dunagan5887 commented 8 years ago

Not sure why "nil" is being sent, that's definitely not right. As for "Solid Body" not taking precedence over "Electric Guitars" I hadn't gotten around to establishing precedence for deeper subcategories yet, that seems to be one of the fixes here. As for the "nil" value I would need a database dump of their site to figure that one out, potentially their codebase too

dunagan5887 commented 8 years ago

I'm seeing how the "nil" happened now, I'll address that. Something to consider: In terms of taking the "deepest" category, should I take the deepest Magento category, or the deepest Reverb category? For instance:

Magento category structure: Guitars > Acoustic Guitars > Left-handed Reverb category mappings: Acoustic Guitars > Left-handed

Magento Acoustic Guitars mapped to Left-handed Magento Left-handed mapped to Guitars

Obviously this is a very contrived example, but it details the different between Magento nested level and Reverb nested level. Should the deepest Reverb category be used?

dunagan5887 commented 8 years ago

Also, you originally said that only one category should be passed in ideally, correct?

skwp commented 8 years ago

There should be up to two categories passed and they should be the deepest reverb categories. A reverb product_type should never be passed into the categories field. So if they only map to "electric-guitars" then categories would be blank. If they map to electric-guitars and hollow-body then electric-guitars is the product_type and hollow-body is in categories.

skwp commented 8 years ago

This issue was fixed incorrectly, please see comments on https://github.com/reverbdotcom/reverb-magento/pull/166

skwp commented 8 years ago

Reverted again (1bb9bf6)

I think there are a number of bugs here:

  1. the product_type ends up in the categories field
  2. according to the code, the product type is read off of the "deepest reverb category". There may not always be one. Sometimes you may only have a toplevel category
  3. reading the code from ReverbSync/Model/Category/Reverb.php, I believe there is also a bug there in that for the top level categories, the "slug" is actually "product_type_slug"

I apologize for our confusing API here...let's talk through it in person

skwp commented 8 years ago

Here is how I think it's best spelled out

  1. If the user maps to a toplevel category (which has only "slug" and no "product_type_slug"), then the "slug" becomes the "product_type" in the request to reverb and "categories" is blank.
  2. If the user maps to a subcategory (which has "slug" and "product_type_slug"), then "slug" goes into "categories" and "product_type_slug" goes into "product_type" in the request to reverb. Subcategories are identified by the fact that they are in the "subcategories" field in the reverb json and that they have both "slug" and "product_type_slug". They also have a " > " in their title, but that's not a reliable way to check for them being subcategories.
  3. If the user has mapped a product to both a subcategory and a toplevel category, the subcategory information wins.
  4. If there are two subcategories, both should be sent in the "categories" field as long as they have the same product_type_slug. If someone for some reason mapped their product into multiple subcategories with different parent product types, we should just take the first one. (For example someone could map their guitar into Hollow Body Guitars and Microphones, that would be wrong...). If this is too complicated, then simply allow only a single subcategory (pick one of the list that is in magento) and ignore any others.
skwp commented 8 years ago

basically if someone maps something to "Pro-Audio" and "Microphones" then I expect "pro-audios" in the product_type field and "microphones" in the categories array

dunagan5887 commented 8 years ago

Before I create a PR for this update, please verify that the following below are the desired functionality:

Product mapped to Pro-Audio: "categories":[],"product_type":"pro-audios"

Product mapped to Pro-Audio > Microphones: "categories":["microphones"],"product_type":"pro-audios"

Product mapped to Pro-Audio and Pro-Audio > Microphones: "categories":["microphones"],"product_type":"pro-audios"

Product mapped to Pro-Audio, Pro-Audio > Microphones and Pro-Audio > Monitors: "categories":["monitors","microphones"],"product_type":"pro-audios"

Product mapped to Pro-Audio and Parts: "categories":[],"product_type":"pro-audios"

Product mapped to Pro-Audio, Parts and Pro-Audio > Monitors: "categories":["monitors"],"product_type":"pro-audios"

Product mapped to Pro-Audio, Parts and Parts > Necks: "categories":["necks"],"product_type":"parts"

Product mapped to Pro-Audio, Parts, Pro-Audio > Monitors and Parts > Necks: "categories":["monitors"],"product_type":"pro-audios" ^ The order of precedence here is currently very arbitrary; it is the reverse order in which the Reverb categories where loaded from the complete categories JSON (due to mysql primary key loading and PHP usort() execution)

skwp commented 8 years ago

Those examples look good. Order of precedence in the case of conflicts doesn't really matter so whatever is easier.