750558 - When possible, avoid fetching places for social stacks

Status: RESOLVED FIXED

(Pancake Graveyard :: Front-end, defect)

Description

[Gordon Brander :gordonb]

In drawer.app.js, line 245:

        // TODO: create SiteModel and add to `StackModel.places()`,
        // preventing the need for a places fetch.

Because we don't create a PlaceModel from the data we already have on hand, every time we add a new place to the places for a social stack, we end up re-fetching the list.

We should probably fetch places when we first create the stack model (a brand new social stack), and after that, avoid it by default.

Comment 1

[Gordon Brander :gordonb]

These commits are working toward this goal

https://bitbucket.org/mozillapancake/pancake/changeset/d80bbe7efcdb
https://bitbucket.org/mozillapancake/pancake/changeset/1554afee9906
https://bitbucket.org/mozillapancake/pancake/changeset/0263c4ae73b5

Comment 2

[Gordon Brander :gordonb]

This problem is proving to be more complex than it ought to be. I think the issue may center around how we're doing data binding for place attributes in StackModels (wen the place_id is changed on the StackModl instance, we fetch the place if it doesn't exist in the PlaceCollection).

Here are some troubleshooting notes from stepping through the code (mostly unedited):


Wrapping up the fetch issue in the places collection.

We're getting back a valid stack_id and place_id.

`fetchStack` is also working as intended. The stack is returned from the collection if it already exists.

`fetchPlace` is called 3 times whenever I click a social item. It is missing the place once, and then finds it 2x. This creates a fetch every time. The Q is why it's coming back undefined 1x? It's there after all. It looks like fetchPlace is getting called before the social() call somehow.
 (Answer: I'm seeing it when it is invoked by fetchPlace when the place_id is changed after fetching via social() for the new StackModel).

0. Load page
1. Click social stack place for the first time (already created in Lattice).
   1. fetchPlace fetches. It is called by the place_id listener on StackModel. The first time the places route is entered, places is empty (this should be correct).
   2. fetchPlace is called by PlacesController::setActivePlace. Places is still empty because it it currently being fetched. lifecycle is "loading".
   3. fetchPlace is called again, by selectPlace => set place_id on stackModel => stackModel place_id:change event => fetchPlace. Places is populated.
2. Click a the same social stack place a second time.
   1. fetchPlace called by social() AJAX => StackModel::process => StackModel::set => trigger place_id:change => place_id:change event handler => fetchPlace. places() is empty because this is a brand new model, so the nodes are fetched. Our extra nodes fetch may actually be coming from here. Note that the place_id is the same here, so Backbone doesn't trigger an event for place_id:change.
3. Click a different (created) social place, in a stack that has already been loaded.
   1. fetchPlace called by social() AJAX => StackModel::process => StackModel::set => trigger place_id:change => place_id:change event handler => fetchPlace. places() is empty because this is a brand new model, so the nodes are fetched.
   2. fetchPlace is called by PlacesController::setActivePlace(). At this point places() is populated.
   3. And again, called by PlacesController::setActivePlace() => selectPlace => set place_id on stackModel => stackModel place_id:change event => fetchPlace. Places is populated. No fetch.

It looks like the place list is re-rendered after the second fetch.

After clicking btw stacks in the drawer, when you click a social stack place in the feed, it is not highlighted in the drawer. Also, no load is issued. However, clicking the SAME one as is now active resets the drawer -- suspicious. There is state being kept somewhere it should not be.

We should probably memoize the promise for in-flight collections and serve that up for fetch. Done. Let's see what changed. Update: nothing changed, but we're ensuring we don't issue more fetches than we should -- this is a good precaution.

Comment 3

[Gordon Brander :gordonb]

This is also causing a rendering issue -- namely, every time you click a social item from the feed, the places list is re-rendered completely.

Comment 4

[Gordon Brander :gordonb]

To side-step the StackModel event-based data binding, selectPlace could handle setting the correct information on StackModel. This way you wouldn't end up accidentally fetching places by simply editing properties on StackModel.

Comment 5

[Gordon Brander :gordonb]

Fixed by moving logic into selectPlace.