datashift_spree icon indicating copy to clipboard operation
datashift_spree copied to clipboard

Add variant_sku and price

Open savrand opened this issue 10 years ago • 15 comments

I have added to csv fields as it is indicated https://github.com/autotelik/datashift_spree/wiki/Spree-Import-Export-Tips#variants variant_sku . before saving
https://github.com/autotelik/datashift/blob/master/lib/loaders/loader_base.rb#L346 I output @load_object.variants and it shows me variants with filled sku as in csv but after saving they are changed to standard ones.

savrand avatar Aug 21 '14 12:08 savrand

Ok this is a bug in datashift_spree products loader

See method add _ options _ variants

It sets the sku automatically on the variant regardless, need to add a check and only auto generate the sku if not present in the spreadsheet

Sent from Yahoo Mail on Android

autotelik avatar Aug 21 '14 13:08 autotelik

Hmmm maybe im talking nonsense... seems actually as long as variant_sku column appear AFTER variants column this should not matter as the auto sku should be over written

autotelik avatar Aug 21 '14 14:08 autotelik

I cannot change at moment but I believe bug in line 136 - 138 of product loader

else super end

The call to super will not work in this case as the column name is actually 'sku' not 'variant_sku' so specific code needs adding here to assign the sku to the Variant

autotelik avatar Aug 21 '14 14:08 autotelik

something like .....

if(@load_object.variants.size > 1) @load_object.variants.each_with_index {|v, i| v.sku = "#{current_value}_#{i}" } else @load_object.variants.first.sku = current_value.to_s end

@load_object.save

autotelik avatar Aug 21 '14 14:08 autotelik

This is still an issue.

I don't completely understand all the code so I'm not sure where to stand but from what I gather, the variant_sku are correctly assigned but not saved.

When the loop is done, @load_object.save is called, which attemps to save the master product only, so it seems like the Variant SKUs are getting replaced locally but the @load_object affects only the master and the changes are not being persisted.

Since they use the same code, I guess it affects variant_price and count_on_hand equally.

Any suggestions?

fredericboivin avatar Mar 03 '15 19:03 fredericboivin

Some changes to fix count_on_hand for Spree 2 have literally just been checked in.

Sorry bit hard to follow the trail ..can you specify exactly which fields are not being persisted as you expect .. On Variants ?

sku, price and count_on_hand ? For what version of Spree ? And using master branch from github (not gem) ?

thanks tom

autotelik avatar Mar 03 '15 20:03 autotelik

I'm using the latest version of everything :

Datashift v0.16.0@d561aa (from github) Datashift_spree v.0.6.0@d6e271 (from github)

Spree 2.4.3

And I'm trying to assign specific SKUs to variant by using variant_sku, "sku1|sku2|sku3"

I've found the issue though :

SKU are not updated because the children (Spree::Variant) are updated locally through the parent (Spree::Product) and Spree::Product does not accepts_nested_attributes for Spree::Variant so the changes on the children are not persisted when saving the parent (@load_object.save)

So it's possible to replace @load_object.save by

@load_object.variants.each do |i| i.save end

Or we could use v.update_attributes( sku: values[i].to_s") instead.

I don't know the project well enough to know the best way to fix it, what are your thoughts?

(English is not my mother tongue so sorry if it's not exactly clear)

I'm referring to this part of the code

elsif(current_method_detail.operator?('variant_sku') && current_value)
      if(@load_object.variants.size > 0)

        if(current_value.to_s.include?(Delimiters::multi_assoc_delim))

          # Check if we processed Option Types and assign  per option
          values = current_value.to_s.split(Delimiters::multi_assoc_delim)

          if(@load_object.variants.size == values.size)
            @load_object.variants.each_with_index {|v, i| v.sku = values[i].to_s}
            @load_object.variants.each do |i|
              i.save
            end
            @load_object.save
          else
            puts "WARNING: SKU entries did not match number of Variants - None Set"
          end

        end
      else
        super
      end

fredericboivin avatar Mar 03 '15 21:03 fredericboivin

Hum... I've not changed any code regarding 'variant_sku' method and I got my custom variant sku's saved...

redglory avatar Mar 04 '15 16:03 redglory

I will post a more detailed use case later today with a CSV sample, dependencies version, log entries from Datashift and resulting SQL.

fredericboivin avatar Mar 04 '15 16:03 fredericboivin

Sample CSV

SKU Name ShippingCategory Price Option Types Option Types Option Types variant_sku
ILJR0247 TSHIRT PROMO DEFAUT 100 color:Q; size:M color:Q; size:G color:A; size:TP 10583856

Resultings SKU :

| ILJR0247_1 | ILJR0247_2 | ILJR0247_3

Variant_SKU is processed without problem but values aren't overwritten. From the logs, it's clear that after looping over @load_object. variants, the SKUs are replaced locally but the changes aren't persisted when the Product is saved.

If you look at a relevant part of the log, you see that no UPDATE is being called on any variant.

http://pastebin.com/uLLc8NTC

Could you post a sample of your working CSV ? Also, which version of Rails/Spree are you running?

I might be doing something wrong but still, if I save the Product.variants explicitly in the 'variant_sku' block, it works

I'm curious about your thoughts on this

fredericboivin avatar Mar 05 '15 22:03 fredericboivin

redglory kindly commited some fixes all around this yesterday (https://github.com/autotelik/datashift_spree/commit/936b96ef04994b1676ad9d5f602981edbef4d0f0)

Have you tried pulling this latest from master ? if this does not fix your problem just let me know and I'll try and go through your examples/logs.

autotelik avatar Mar 06 '15 13:03 autotelik

@load_object.variants.each_with_index {|v, i| v.sku = values[i].to_s } @load_object.save

I assume save on Parent product peculates down to associations but given what you see perhaps not

I don't have time to dev/test this right now, so if you're desperate ..try changing

Line 154 .... @load_object.variants.each_with_index {|v, i| v.sku = values[i].to_s }

To save the Variant itself, something like :

@load_object.variants.each_with_index do |v, i| v.sku = values[i].to_s begin v.save! rescue => x logger.error "Warning SKU not set on Variant #{v.inspect} #{e.message}" end

cheers tom

autotelik avatar Mar 06 '15 13:03 autotelik

I will try again with the master branch and get back to you

Cheers

fredericboivin avatar Mar 09 '15 19:03 fredericboivin

Just ran into pretty much this issue with spree 3.0.0 and variant_price

Changing line 112 to this seemed to do the trick @load_object.variants.each_with_index {|v, i| v.update_attributes(price: values[i].to_f) }

joeswann avatar May 26 '15 09:05 joeswann

@joeswann Thanks, that fixed it per your suggestion.

@load_object.variants.each_with_index {|v, i| v.update_attributes(sku: values[i].to_f) }

ping0xFF avatar Mar 08 '16 21:03 ping0xFF