Ad filtering in Angular factory

Question

I'm an 'old school' programmer (C, C++), and I have step-by-step, algorithmic thinking. I'm writing some code in Angular + Django and I know that this code is ugly but I don't have any idea how to refactor it.

First of all, I have the models:

class Product(models.Model):
    name = models.CharField(max_length=100)
    status = models.CharField(max_length=10, choices=STATUSES.items(), default='new')

class Ad(models.Model):
    image = models.ImageField(upload_to="ad")
    width = models.IntegerField(blank=True)
    height = models.IntegerField(blank=True)
    product = models.ForeignKey(Product, null=True, blank=True)

class AdStatus(models.Model):
    status = models.IntegerField(default=1)
    ad = models.ForeignKey(Ad)

Next I have simple classes for serializing:

class AdSerializer(serializers.HyperlinkedModelSerializer):
    class Meta:
        model = Ad
        fields = ["url", "id", "image", "width", "height". "product"]
#etc...

And every model has its own factory in Angular:

  .factory('AdStatus',["$resource", function ($resource){
    var Resource = $resource(
        "/api/ad_status/:ad_id/",
        {ad_id: '@id'},
        {
            query: {
                isArray: true,
                transformResponse: function (data) {
                    var items = angular.fromJson(data);
                    return items.results;
                }
            },
            update: {
                method: "PUT",
            }
        },
        {
            stripTrailingSlashes: false
        }
    );
    Resource.prototype.changeStatus = function(status){
        this.status = status;
        this.$update();
    };
    return Resource;
  }])
/etc

This is the main code:

//controller isn't bad

angular.module('controllers.statuses', [])
.controller('StatusCtr', ["$scope", "$stateParams", "StatusSetter",  function($scope, $stateParams, StatusSetter) {
    $scope.click = function(){
        $scope.t = StatusSetter.change($stateParams.id);// this is id of product
    };
}]);

But this makes me a little sick:

.factory('StatusSetter', ["AdStatus", "Product", "Ad", function(AdStatus, Product, Ad){
    var changeStatus = function(id, productStatus, adStatus){
        var product = Product.get({product_id: id});
        product.$promise.then(function(data) {
            product.changeStatus(productStatus);
            var ads = Ad.query();
            var ad_status = AdStatus.query();
            var tmp = [];
            ads.$promise.then(function(data){
                for (var i = 0; i < ads.length; i++) {
                    if (ads[i].product == product.url){
                        tmp.push(ads[i]);
                    }
                }
                ad_status.$promise.then(function(data){
                    for (var i = 0; i < ad_status.length; i++){
                        for (var j = 0; j < tmp.length; j++){
                            if (ad_status[i].ad == tmp[j].url){
                                ad_status[i].changeStatus(adStatus);
                            }
                        }
                    }

                })
            });
        });
        return product;
    };
    return {
        change: function(id){
            return changeStatus(id, "new_status", 30);
        }
    }
  }])

The main philosophy of this code is:

change AdStatuses which points at Ads (which points at Products)

Any ideas? Maybe I can separate this 'filtering' to $resource or something.

Answer 1

I think what you want to do is:

• Get the product URL
• Find the Ads with that product URL
• Use each Ad URL to find the relevant AdStatus object
• Update each AdStatus object

This can be done by chaining the operations with the $resource.$promise

.factory('StatusSetter', ["AdStatus", "Product", "Ad", "$q" 
                          function(AdStatus, Product, Ad, $q)
{
    var changeStatus = function(id, newStatus, newValue){
        var filteredAdsList = [];
        var product = Product.get({product_id: id});
        product.$promise.then(function(productObj) {
            //chain product status change
            return productObj.changeStatus(newStatus).$promise;
        }) .then (function (productObj) {
            //wait for product status to finish change
            //chain Ad query
            return  Ad.query().$promise;
        }) .then (function (adsObj) (
            //push filtered list
            angular.forEach(adsObj, function (iAd)) {
                 if (iAd.product == product.url){
                     filteredAdsList.push(iAd);
                 }
             };
            //chain adStatus query 
            return AdStatus.query().$promise;
        }) .then (function (adStatusObj) {})
            //start with empty promise
            var qPromise = $q.when();
            angular.forEach(adStatusObj, function (iAdStatus) {
                angular.forEach(filteredAdsList, function(jAd) {
                     if (iAdStatus.ad == jAd.url){
                         //add to promise chain
                         qPromise = qPromise.then (function() {
                             var nextPromise =
                                iAdStatus.changeStatus(newValue).$promise;
                             return nextPromise;
                         });
                     };
                });
            });
            //return promise chain
            return qPromise;
        }) .then (function () {
            console.log("all status updated")
        })
        return product;
    };
    return {
        change: function(id){
            return changeStatus(id, "new_status", 30);
        }
    }
}])

By returning data and promises and chaining those promises, you can create code that uses asynchronous APIs in a way that looks familiar.

For more information on the AngularJS $q service and chaining promises see AngularJS $q Service API Reference - chaining promises

Also notice that I used angular.forEach for iteration. For more information on using angular.forEach see the AngularJS angular.forEach API Reference.