search

RobertSpread / spreadcart

simple plugin to display a cart for the spreadshop everywhere on your domain
1 stars 1 forks source link

* implement read option in proxy.php and make it work with refactorin… #29

Closed RobertSpread closed 8 years ago

RobertSpread commented 8 years ago

I was able to figure out the problem. This line var basketDoc = jQuery.parseXML(xhr.responseJSON.xml); does not work for me. Instead var basketDoc = jQuery.parseXML(xhr.responseJSON); does work perfectly. (note the difference .xml). I was not able to figure out where the .xml comes from, why it is needed or how I can make it work with this as well. Maybe you can give me a hint.

jtlapp commented 8 years ago

The proxy needs to put the XML returned from the API in a JSON response to the client. It puts the XML in a field called "xml" within the JSON response. You put it in that field.

We have to be consistent about the data format that the proxy returns to the client. It has to be always JSON or always XML. It was JSON for the delete action, so it would have to be JSON for the read action -- and any other action. JSON is more convenient for the client, because it automatically parses into Javascript.

Does that help?

jtlapp commented 8 years ago

Oh, I see that you are returning JSON. It's just that you're returning the XML as the JSON string rather than within a JSON field. That's one possible approach, but it precludes the proxy returning any additional information with the payload. That is all the information we return right now, and I'm not sure what other information might one day be useful, so we could do it either way.

jtlapp commented 8 years ago

The only reason I have the proxy returning JSON to the client is so that it can provide informative error messages. When an the proxy gets an error communicating with the API, it can return information about that error to the client via different JSON fields. The two fields of importance are a status code (the HTTP response status of the API to the proxy), and a user-readable error message.

No other responses need to be in JSON, but if any one response is to be in JSON, they all need to be in JSON. I was trying to be consistent by always labeling the data data returned in JSON. That is, I have the proxy always return JSON that translates into a Javascript object having labelled fields. This is not really necessary, however. We could instead just return an unlabeled string when there is only one data item possible in the return, as you seem to have done.

RobertSpread commented 8 years ago

Thank you very much for the explanation. I'll adjust the proxy accordingly.

RobertSpread commented 8 years ago

Changed the implementation according to your last suggestions. I close this pull request and open a new one with the needed adoptions. Please review. If you agree on my proposal I will merge the pull request and merge the refactoring to the master afterwards... Then we just need to get the update of the quantity working again ;)