leaguevine / leaguevine-ultistats

MIT License
18 stars 4 forks source link

Do not use ajax callback #66

Closed cboulay closed 12 years ago

cboulay commented 12 years ago

I've read elsewhere that it is generally a bad idea to use the ajax success callback. I also have a more important reason: after the WebSQL-AJAX plugin is used, the result of the local WebSQL query will be passed to the success/error callbacks but the result of the AJAX request (which may occur seconds to minutes later) will not trigger the success callback. Thus, if we want the result of the AJAX request to be reflected in the app as soon as it is returned, then we need to bind to the model/collection "changed"/"add" events instead of using the callbacks.

i.e., the view representation of the data should rely exclusively on binding to events.

If there is a situation where we need something from the data before we can even start to render, then we can use the jquery

$when(model.fetch()).then(function(){remaining tasks;});
cboulay commented 12 years ago

After a quick perusal of the code there seem to be 3 different types of problems to clean up.

  1. titlebar
  2. ajax requests that require authentication. We want to be sure the save/delete happens before the app is updated to reflect that.
  3. Getting the list of players (onfield and offield) for a trackedgame requires knowing the team_ids, but that can only be obtained after a successful fetch of the game.

I will look at 1 later. 2 is half related to #11. I will start there. 3 is a bit more complicated and will take some time.

cboulay commented 12 years ago

search_results.fetch() and fetch_more_items new_collection.fetch() both have success callbacks that will break when using WebSQL + AJAX. The best way to handle this is to take the Leaguevine.Utils.concat_collections function and put its equivalent into backbone-tastypie's Backbone.Collection.prototype.parse. The problem with that is Backbone.Collection.prototype.parse does not have the current list of models with which to compare. I need to get it context somehow. "this" should work but I'm having trouble with it.

cboulay commented 12 years ago

Actually these might not be an issue. These success callbacks will operate ONLY on the WebSQL data. The AJAX data will be parsed by the websqlajax plugin. Investigating...

cboulay commented 12 years ago

I fixed this a while ago in the chadapache branch. It is still a problem in the main branch but I'll close the issue for now.