Potential fragile usage of SpreadShop cart UI

[jtlapp]

The implementation of the item counter uses two features of the SpreadShop UI that may be unreliable over time:

(1) The implementation overwrites window.onSpreadShopLoaded as if no other software would ever use that event. Is that really the case? Is that event for the exclusive use of this plugin cart software, or is this a bug?

(2) The implementation attaches an event handler to the SpreadShop #addToBasket element. Can we be sure that SpreadShirt won't quietly change this without telling us, because they don't expect anyone to be using that?

Both of these features support the real-time item count, which my upgradeable-core pull request turns off partly in order to avoid these potential issues.

[RobertSpread]

Hi there,

we should discuss these topics in some detail and I will think about it. From my current knowledge, adding the event listeners should not affect anything as the onSpreadShopLoaded just triggers an additional event. The likeliness that the id of #addToBasket is changed is quite low but existing. I found no way so far to have an alternative.

I will work on it tonight and then let's see what we do.

[jtlapp]

Thanks for checking into this Robert. I have a thought too.

It would be nice if SpreadShirt provided a single event source for all shopping cart events. Multiple event handler functions could be registered with the source, and each registered function would get called with each new shopping cart event. The SpreadShop would hand the function an object that describes the event that occurred. An event object might look something like this (just thrown together):

{ eventType: "newQuantity", productID: "3882934729", articleID: "38387", color: "blue", quantity: 3 }

Other event types might be "newItem" and "removedItem". The interface is extensible for possible new events defined later. Or perhaps we only need an item-updated event that doesn't tell us exactly what changed, because we can figure it out by comparing to the previous cart, if need be.

The advantage here is that SpreadShirt would commit to preserving the interface of only one UI feature and commit to making sure developers are aware of changes to just this one thing. They won't have to commit to preserving numerous different event sources on different HTML elements or informing us about changes to all those sources. In fact, this universal event source might only serve third party plugins and hence not be subject to pressures that would otherwise change the UI.

[jtlapp]

Even simpler, all we need is the ability to subscribe to a notice that the shopping cart has changed. The plugin would just recompute the item count from scratch each time.

The SpreadShop javascript itself may need to subscribe to this same notice function if it is to update its item count in real-time for changes that our plugin makes to the shopping cart.

[RobertSpread]

We could solve the prolem with:

var fn = window.onSpreadShopLoaded;

window.onclick = function ( e ) { our function() fn( e ); }

This is not a 100% solution since who ever adds an event to the onSpreadShopLoaded will break this but we could prevent some isues with that. If there are no objections I will implement this tonight.

[RobertSpread]

The second topic I would like to ignore for now.

[jtlapp]

I think ajax provides a way to chain arbitrary event handlers, so as long as SpreadShop and other parties respect that chaining mechanism, there won't be any problems. At present, we aren't respecting it. I'll have to look into this.

[RobertSpread]

I added the

[RobertSpread]

I added the above described patch. I hope you agree in the approach for now. Each developer who implements a differnt plugin would nbeed to follow this approach. I appreciate that this is not an ideal solution but at least an improvement.

[jtlapp]

Your patch will help if Spreadshirt agrees to abide by the contract.

I think other issues with fragile dependencies on unpublished SpreadShop behavior remain:

(1) We are relying on you to know when developers stop using #addToBasket and to tell us in time for us to find a different solution. (2) We are relying on the locally-stored basket representation, which also might change in ways that break us. This becomes more of an issue as we rely on more of this data, such as for updating quantities.

I think the only way to alleviate these issues is for Spreadshirt to formally publish their contract with the developer, so we both know what is expected of us and what changes have to be communicated in advance.