search

citrus / spree_wholesale

A wholesale extension for Spree Commerce
http://rubygems.org/gems/spree_wholesale
BSD 3-Clause "New" or "Revised" License
36 stars 36 forks source link

clicking "update" destroys wholesaler's address #13

Closed patrickmcelwee closed 12 years ago

patrickmcelwee commented 12 years ago

There is an unfortunate bug in lib/spree_wholesale/wholesale_controller on the 0.60.x branch. In the update action, there is a line that reads

@wholesaler.ship_address.destroy unless @wholesaler.ship_address.nil? || @bill_address != @wholesaler.ship_address

On my live site ( :-( ) this caused the application to delete the only addresses I had for several wholesalers when I inadvertently clicked "Update" on their wholesaler/edit pages. These were wholesalers who had checked the "Use billing address for shipping" button.

I am something of a newbie, but I believe that the problem lies in the last condition, which should be @bill_address == @wholesaler.ship_address

Myself, I am just commenting this out, because I question whether it is necessary (or wise) to delete an address object here.

citrus commented 12 years ago

Sorry to hear about your addresses getting deleted.. can you restore them from a backup?

The logic in the update method is a bit finicky, in fact the wholesale_controller needs to be refactored and cleaned up quite a bit. (see issue #11)

Hopefully this weekend I'll be able to spend some time with it...

Cheers,

-Spencer

patrickmcelwee commented 12 years ago

I was able to restore the addresses via rails console.

This is a really useful extension, and I hope to be using it/extending it quite a bit in our store. I am very new to web development, so I'm not sure if I'll be much help while I still have the training wheels on, but at some point, I hope to be able to lend more of a hand.

Just one other note on the update method ... for some reason, an update marks the wholesaler as "accepted", but without going through the activate! method (I modified my activate! method to send out a "You have been accepted" email, so I'm pretty sure). To really make them active, I have to deny and then accept them.

Thanks for all your work on this!

Patrick

tavon commented 12 years ago

I've made a pull request (#14) to solve this issue.

tavon commented 12 years ago

I believe #14 resolves this issue and we should close this one.

patrickmcelwee commented 12 years ago

Thanks tavon! I agree. Very nice work.

citrus commented 12 years ago

Thanks @tavon!!!