search

reverbdotcom / reverb-magento

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

Sync description on update #226

Closed skwp closed 8 years ago

skwp commented 8 years ago

Does not provide support for mass-sync description updates

skwp commented 8 years ago

I hacked this up by following existing patterns. Needs testing

zztimur commented 8 years ago

This will work, but change of description won’t trigger an update. So if you change “Description of a guitar” to a “Description of a guitar with some more info” that wouldn’t be pushed to reverb. Is this the intended behavior?

On Mar 30, 2016, at 12:45 PM, Yan Pritzker notifications@github.com wrote:

I hacked this up by following existing patterns. Needs testing

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/reverbdotcom/reverb-magento/pull/226#issuecomment-203548113

skwp commented 8 years ago

No I probably missed something. If you want to finish this up and it's relevant to you guys I would love the help

Sent from my iPhone

On Mar 30, 2016, at 5:01 PM, zztimur notifications@github.com wrote:

This will work, but change of description won’t trigger an update. So if you change “Description of a guitar” to a “Description of a guitar with some more info” that wouldn’t be pushed to reverb. Is this the intended behavior?

On Mar 30, 2016, at 12:45 PM, Yan Pritzker notifications@github.com wrote:

I hacked this up by following existing patterns. Needs testing

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/reverbdotcom/reverb-magento/pull/226#issuecomment-203548113

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

zztimur commented 8 years ago

To Confirm: You expect that updating description would trigger listing update, correct?

On Mar 30, 2016, at 6:22 PM, Yan Pritzker notifications@github.com wrote:

No I probably missed something. If you want to finish this up and it's relevant to you guys I would love the help

Sent from my iPhone

On Mar 30, 2016, at 5:01 PM, zztimur notifications@github.com wrote:

This will work, but change of description won’t trigger an update. So if you change “Description of a guitar” to a “Description of a guitar with some more info” that wouldn’t be pushed to reverb. Is this the intended behavior?

On Mar 30, 2016, at 12:45 PM, Yan Pritzker notifications@github.com wrote:

I hacked this up by following existing patterns. Needs testing

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/reverbdotcom/reverb-magento/pull/226#issuecomment-203548113

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/reverbdotcom/reverb-magento/pull/226#issuecomment-203679142

skwp commented 8 years ago

Correct I would expect the update to any field that matters to trigger a sync including description.

Sent from my iPhone

On Mar 30, 2016, at 6:25 PM, zztimur notifications@github.com wrote:

To Confirm: You expect that updating description would trigger listing update, correct?

On Mar 30, 2016, at 6:22 PM, Yan Pritzker notifications@github.com wrote:

No I probably missed something. If you want to finish this up and it's relevant to you guys I would love the help

Sent from my iPhone

On Mar 30, 2016, at 5:01 PM, zztimur notifications@github.com wrote:

This will work, but change of description won’t trigger an update. So if you change “Description of a guitar” to a “Description of a guitar with some more info” that wouldn’t be pushed to reverb. Is this the intended behavior?

On Mar 30, 2016, at 12:45 PM, Yan Pritzker notifications@github.com wrote:

I hacked this up by following existing patterns. Needs testing

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/reverbdotcom/reverb-magento/pull/226#issuecomment-203548113

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/reverbdotcom/reverb-magento/pull/226#issuecomment-203679142

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

skwp commented 8 years ago

Not sure what I need to do to get the description update to trigger. @zztimur any pointers?

zztimur commented 8 years ago

I’ll look into that tonight.

On Apr 6, 2016, at 11:04 AM, Yan Pritzker notifications@github.com wrote:

Not sure what I need to do to get the description update to trigger. @zztimur https://github.com/zztimur any pointers?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/reverbdotcom/reverb-magento/pull/226#issuecomment-206443084

skwp commented 8 years ago

that would be great. I'd like to bundle this for the next release with the category changes

skwp commented 8 years ago

@dunagan5887 would you be able to help take this to the finish line

dunagan5887 commented 8 years ago

Which version(s) are you guys seeing this issue on? I'm using 1.9 and 1.7, and any time I update only the description field on a product, a listing sync background task is created

zztimur commented 8 years ago

1.9.0.1 - updating description doesn't trigger a sync task

dunagan5887 commented 8 years ago

Does this product have its "Sync to Reverb" field set to "Yes"?

zztimur commented 8 years ago

Yes

On Apr 12, 2016, at 11:00 AM, Sean Dunagan notifications@github.com wrote:

Does this product have its "Sync to Reverb" field set to "Yes"?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/reverbdotcom/reverb-magento/pull/226#issuecomment-208981008

zztimur commented 8 years ago

I have too look into this on our instance

On Apr 12, 2016, at 11:00 AM, Timur Zaynullin zztimur@gmail.com wrote:

Yes

On Apr 12, 2016, at 11:00 AM, Sean Dunagan <notifications@github.com mailto:notifications@github.com> wrote:

Does this product have its "Sync to Reverb" field set to "Yes"?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/reverbdotcom/reverb-magento/pull/226#issuecomment-208981008

dunagan5887 commented 8 years ago

I was testing on 1.9.2.0. I can set up an instance of 1.9.0.1 and test if you'd like

zztimur commented 8 years ago

It works, the issue was on our end. Thanks

On Tue, Apr 12, 2016 at 11:03 AM, Sean Dunagan notifications@github.com wrote:

I was testing on 1.9.2.0. I can set up an instance of 1.9.0.1 and test if you'd like

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/reverbdotcom/reverb-magento/pull/226#issuecomment-208982414

-- Sent from my iPhone

skwp commented 8 years ago

Do we need to do any handling for a bulk update or is this ok?

dunagan5887 commented 8 years ago

Bulk updating of the description field doesn't sound like a common occurrence, but I could be wrong

skwp commented 8 years ago

Even if not common i'm worried that we support bulk on most fields but not this one so it might be a confusing support issue. related thought: If we implement arbitrary field mapping do we need to somehow support those arbitrary fields being bulk updated also?

dunagan5887 commented 8 years ago

That sounds like a business question to me. It wouldn't be too hard to tie those fields in to the trigger

skwp commented 8 years ago

Sorry to rephrase, I do want everything to be bulk updateable or customers will be confused and come to us for support. I'm just making sure there's not a technical limitation to doing that for our arbitrary mapped fields as well. For this PR, can you please add whatever hooks we need for bulk updates to work? Thanks!

skwp commented 8 years ago

I'm going to merge this and wait on bulk support for descriptoin in a separate PR @dunagan5887