If you import a product and the price is zero, the price gets dropped. Here's the patch to fix it.

CommentFileSizeAuthor
#21 commerce_feeds-Issue-1183006-21.patch1.69 KBgeek-merlin
#15 commerce_feeds-1183006-15.patch664 bytesiMiksu
#11 commerce_feeds-1183006-11.patch649 bytesiMiksu
#8 commerce_feeds-1183006-8.patch516 bytesiMiksu
commerce_feeds.zero_price_gets_tossed.patch461 bytesrfay
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra commented
Status: Needs review » Fixed

Good catch, commited.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

iMiksu’s picture

iMiksu
Location Finland
CreditAttribution: iMiksu commented
Version: 7.x-1.x-dev » 7.x-1.1

After this patch I'm getting error:
SQLSTATE[HY000]: General error: 1366 Incorrect integer value: '' for column 'commerce_price_amount' at row 1

multiple times and in the end I get:
SQLSTATE[21S01]: Insert value list does not match column list: 1136 Column count doesn't match value count at row 1

I was using XML file which contained an XML-node having an attribute being the price. In some cases there were not price attribute defined at all which probably caused my errors...

I'm not currently that familiar with Feeds API yet so I won't post a patch yet. Feel free to post a patch which handles my case too :)

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented
Status: Closed (fixed) » Needs work

It sounds like you have an empty input value in your price field?

I do kind of think this would be better as

if (!isset($price)) {
$price = 0;
}

When I made the patch I was just trying to respect existing code, but I think it would be better to set the price to zero.

iMiksu’s picture

iMiksu
Location Finland
CreditAttribution: iMiksu commented
Status: Needs work » Needs review
FileSize
commerce_feeds-1183006-8.patch516 bytes

Apparently in my case I was getting empty strings into the mapper function which wasn't meeting the conditions of !isset().

Because of that the value was passed as empty string which leaded PDO complaining about the incorrect integer value.

To fix the issue (and still being handling intended 0 values) I added extra check for skipping empty strings values too. This patch shouldn't drop 0 values, but I haven't tested it. Feel free to test :)

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented
Status: Needs review » Needs work

I disagree. I think an empty value should be set to zero unless there's some reason not to. @iMiksu, you can solve your own problem by just having a nonempty field.

If somebody can say why a product should not get a price of 0 on import if no other price is set, please say so.

iMiksu’s picture

iMiksu
Location Finland
CreditAttribution: iMiksu commented

According to your first message, I understood that the price of zero needs to be mapped and other empty values not. For me it sounds good to have default value of zero whatever happens.

How about checking if the value is not numeric, it would be replaced by zero?

iMiksu’s picture

iMiksu
Location Finland
CreditAttribution: iMiksu commented
FileSize
commerce_feeds-1183006-11.patch649 bytes
rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented 
+        $field[LANGUAGE_NONE][$i]['amount'] = is_numeric($v) ? $v : 0;

I think it has to be (!empty($v) && is_numeric($v)) or you'll have a warning.

Also, I think it's Drupal convention to use more meaningful variable names, so probably $value is better.

iMiksu’s picture

iMiksu
Location Finland
CreditAttribution: iMiksu commented

True, that would be better. I think this is RTBC.

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

Did you mean to attach a patch?

iMiksu’s picture

iMiksu
Location Finland
CreditAttribution: iMiksu commented
Status: Needs work » Needs review
FileSize
commerce_feeds-1183006-15.patch664 bytes

Yes. Would be great to have it tested.

My import works good with these changes.

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented
Version: 7.x-1.1 » 7.x-1.x-dev
Status: Needs review » Reviewed & tested by the community

Well, I didn't actually test it but it looks perfect to me.

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra commented
Status: Reviewed & tested by the community » Fixed

Ok, let's commit it then, thanks Randy and iMiksu!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin commented
Issue summary: View changes
Status: Closed (fixed) » Needs work

I ran into this and have an answer to #9 from rfay:
> If somebody can say why a product should not get a price of 0 on import if no other price is set, please say so.

Yep here: I have a "special price" field which may or may not have a value (is not required).

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin commented

Also, looking at the code, it looks like it will break for multiple value price fields:
* have 3 values in price field
* importer brings 2 values
result: 2 values imported, 3rd value as before.

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin commented
Status: Needs work » Needs review
FileSize
commerce_feeds-Issue-1183006-21.patch1.69 KB

After applying this patch
* ordinary price is imported to price field
* zero pricce is imported as zero
* empty price is imported as empty field

AdamGerthel’s picture

AdamGerthel CreditAttribution: AdamGerthel at Odd Hill commented

I was having the same problem as Axel (using a secondary "on sale" price field). Patch in #21 works very well.

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at Clever Systems, MachbarMacher, DruConsult commented
Status: Needs review » Reviewed & tested by the community

so RTBC.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Assigned: Unassigned » mglaman
Status: Reviewed & tested by the community » Needs work

This doesn't apply anymore with recent commit. Thanks for patch axel.rutz, I'll re-roll.

  • mglaman committed 0a34112 on 7.x-1.x authored by axel.rutz 
    Issue #1183006 by iMiksu, rfay, axel.rutz, mglaman: Price of zero gets...
mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Status: Needs work » Fixed

Thanks, everyone!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.