angular-restmod icon indicating copy to clipboard operation
angular-restmod copied to clipboard

Record received by $find should be bound to collection if possible

Open smashercosmo opened this issue 10 years ago • 3 comments

Ok, here is the case:

var collection = ItemModel.$collection();
collection.$refresh();

var item = collection.$find(1);
item.$destroy();

I expect that item record, received by $find is automatically bound to collection, so calling $destroy on the record will remove this item from collection. Are my expectations wrong? It seems to me quite logical.

smashercosmo avatar Jan 19 '15 11:01 smashercosmo

The item returned by $find is bound to the collection scope, but is not part of the collection (it's not even added to the collection when retrieved). To search for an item that is already contained by the collection you can use any array search function.

Maybe find should behave differently when called on a collection, searching first among the already loaded items and then executing a request to the api if no item is found... For some reason I don't find this approach very clean. What do you think?

iobaixas avatar Jan 21 '15 14:01 iobaixas

I believe the approach mentioned above - first looking in the collection and then executing a request if not found - could bring great value to RestMod users.

  1. Streamlines RestMod usage patterns
  2. Maintains Scope and RestMod functionality
  3. Fully populates models when only primary key is available
  4. Reduces number or API calls

Consider this use case:

  • app has a user object, with a list of trip ids.
  • activating a trip loads its details
  • there can only be one active trip at a time
  • user.trips only contains a list of trip ids, not full trip objects.

Here is the code for this use case. The code demonstrates two ways of activating a trip.

<div ng-app="demoApp">
    <div ng-controller="demoCtrl">
        <ul>
            <li ng-repeat="trip in user.trips">
                <a ng-click="setActiveByTripObj(trip)">{{trip.name}}</a>
                <a ng-click="setActiveById(trip.id)">{{trip.name}}</a>
            </li>
        </ul>
       <div>
             {{activeTrip.name}}
       </div>
    </div>
</div>
var app = angular.module('demoApp', []);

app.controller('demoCtrl', function() {

    // Query api to populate trips collection
    // API returns user with 3 trips: [{id: 1}, {id: 2}, {id: 3}];
    $scope.user = User.find(0);

    // sets active trip, which is only the trip id
    // we need to query the API to populate the trip
    $scope.setActiveByTripObj = function(tripObj) {
        $scope.activeTrip = tripObj; // sets the initial trip
        $scope.activeTrip.$fetch(); // populates the rest of the trip properties
    };

    // sets active trip by index
    // in my experience, this approach is susceptible to breaking scope
   // Grabbing a collection item by index, `$scope.user.trips[1]`, is not a good practice...
    $scope.setActiveByTripIndex = function(idx) {
        $scope.activeTrip = user.trips[idx]; // sets by index
        $scope.activeTrip.$fetch(); // populates the rest of the trip properties
    };

    // again sets active trip, except the connection to the trips collection is maintained.
    // The first time this is called, the trip is queried from the API
    // Subsequent calls don't need to query the API
    $scope.setActiveById = function(tripId) {
       $scope.activeTrip = trips.$find(tripId);
    };

});

facultymatt avatar Jan 21 '15 22:01 facultymatt

I would be a little worried about backward compatibility though... $find as rails find, always hits the api/database.

Maybe we could add a new method to the collection API: $resolveRecord(pk)..

Another approach would be to add a $lookup method to collections to search loaded items by primary key (and query api if not found) and then use $resolve, the code for the setActiveById would look like:

$scope.setActiveById = function(tripId) {
  $scope.activeTrip = trips.$lookup(tripId).$resolve();
};

What do you think @facultymatt?

iobaixas avatar Feb 02 '15 14:02 iobaixas