Listing Sync Revamp
Closed this issue · 6 comments
I believed we discussed potentially revamping the listing sync to move away from the legacy code as part of the image sync. Did you still want to clean this up?
Can you clarify what needs to be cleaned up, what the potential dangers are, and time estimates, thanks
The main potential danger is the requirement I see on this page: http://dev.reverb.com/docs/error-handling "If your client is handling errors, ensure that any 2xx response is considered successful"
Currently the api calls for findReverbListingUrlByMagentoSku, updateObject and createObject have hard coded status values which they check against. If a different 2xx code is returned than the ones which are hard coded, an exception will be thrown and the listing sync will not be processed as successful. I have created abstract functionality in Reverb_ReverbSync_Helper_Data::_processCurlRequestResponse() to handle validation of API call returns; eventually all of the API calls should use this functionality.
In more UX terms, having the listing sync queue tasks reside in a grid analogous to the order_update queue task grid would be more stable than the current Reverb Reports grid that the listing sync tasks are currently residing in. This Reverb Reports grid is legacy functionality that we patched with band-aids to make it stand up "for the time being". Creating a grid designed to show the listing sync queue tasks would provide a more stable solution. Additionally, if engineers are tasked with evaluating the stability of the Reverb module and assessing the apparent knowledge/expertise with which it was built, the listing sync code is currently the weakest code in the module. It may not prevent an engineer from placing the Reverb module on the site, but it would definitely be the likeliest "red flag". Reverb_ReverbSync_Helper_Data should be refactored to not contain the 3 api calls (findReverbListingUrlByMagentoSku, updateObject and createObject) and should instead be an abstract class which makes it easier to understand how to create new API call adapters.
In terms of a time estimate, I can't efficiently implement the 2xx response code fix without converting the listing sync to abandon the legacy code and fully adopt the queue task architecture (doing so would require more short-term fix patching).
- Implement 2xx status patch without fully adapting queue task architecture: 3 hours
- Fully adapt queue task architecture, including creating a new grid page for listing sync task objects: 5-8 hours.
- Move the findReverbListingUrlByMagentoSku, updateObject and createObject API calls out of Reverb_ReverbSync_Helper_Data and refactor Reverb_ReverbSync_Helper_Data to be abstract: 3-4 hours
Also forgot to point out that the error reporting in terms of assembling the error messages returning from listing API calls is incorrect. It will currently only take the first error message if multiple are returned. It also doesn't format them well
The 2xx issue is actually already occurring. I'm seeing 202 returned for updateObject in the logs, but the code has if ($status != 200) hard coded.
updateObject
{"title":"Casual Strings","sku":....}
202
Same for createObject
I agree with your assessments above, go ahead with the refactor and fix especially with handling the 202s
I would estimate this at 5-10 hours due to the amount of legacy code which will need to be decommissioned