0

I have the following function

   var urls = '';
    handleFiles(f,function(url){
        urls = urls + url + ',';
        console.log("urls is " + urls);
    });

I get the url after uploading and update my urls. But my Urls never gets updated, it shows the url of the last file uploaded.

UPDATE 1 This is my complete code now.

var urls = '';
document.getElementById('question-file-selector').addEventListener('change',handleFileSelect,false);
    function handleFileSelect(evt){
        var files = evt.target.files; //File list object
            // Loop through file list, render image files as thumbnails
        for (var i = 0,f;f = files[i];i++){


        handleFiles(f,function(url){
                urls = urls + url + ',';
                console.log("urls is " + urls);
                });
        // Only process image files
        if (!f.type.match('image.*')){
            $('#list').append('<img class="file-thumb" src="/static/download168.png"/>');
            continue;
        }
        var reader = new FileReader();

        //Closure to capture file information
        reader.onload = (function(theFile){
                return function(e){
                    //Render thumbnail
                    $('#list').append('<img class="thumb" src="'+e.target.result+'" title="'+escape(theFile.name)+'"/>'); 
                    };
                    })(f);
        reader.readAsDataURL(f);
    }
    }

console.log("Url is",urls);`   

And my ajax function

    //Code for Image upload

// Custom jQuery xhr instance to support our progress bar.

var xhr_with_progress = function() {
     var xhr = new XMLHttpRequest();
     xhr.upload.addEventListener("progress",
         function(evt) {
             if (!evt.lengthComputable) return;
             var percentComplete = evt.loaded / evt.total;
             $("#progress-bar div.progress-bar").css('width', String(100*percentComplete) + "%");
         }, false);
     return xhr;
 };





$.support.cors = true;//For cross origin transfer

//Event listners to avoid default drag and drop reaction of browser
window.addEventListener("dragover",function(e){
  e = e || event;
  e.preventDefault();
},false);
window.addEventListener("drop",function(e){
  e = e || event;
  e.preventDefault();
},false);



function handleFiles(file,callback){

            var filename = file.name;
            $.ajax({
                type:'GET',
                data:{"filename":file.name, "FileType":"question_file"},
                url:'/dashboard/generateuploadurl/',
                contentType:"application/json",
                dataType:"json",
                async:false,
                success: function(data){ 
                    if(data.UploadUrl){
                      console.log("upload url successfully created for " + file.name + " file");
                        console.log(data.UploadUrl);
                        handleUpload(data.UploadUrl, file, data.Filename,callback);

                    }
                },
                error: function(data){ 
                    console.log("error occured while creating upload url for " + file.name + ' file');
                    console.log(data);
                },
            });
        }           
function handleUpload(UploadUrl, file, Filename,callback){
    $.ajax({
        xhr:xhr_with_progress,
        url:UploadUrl,
        type:'PUT',
        data:file,
        cache:false,
        contentType:false,
        processData:false,
        success: function(data){
            console.log('https://7c.ssl.cf6.rackcdn.com/'+ Filename);
            callback('https://7c.ssl.cf6.rackcdn.com/'+ Filename);
        },
        error: function(data){ 
            alert("error occured while uploading " + file.name );
            console.log(data);
        },
    }); 
}
learner
  • 39
  • 9
  • use : var urls = ''; – Sonu Sindhu Dec 30 '15 at 06:47
  • Why don't you do `var urls = []; urls.push(url);` and then `urls.join(',');`. About your current approach, we might get an idea if we see the code in which this code is wrapped. Might be some variable overwrite issue. It looks like `handleFiles` is an ajax method which calls back with url param. You might have to declare `var urls` in a outer scope. – Salman Dec 30 '15 at 06:49
  • 1
    @SonuSindhu That's good advice, but doesn't explain why this wouldn't work. @OP, I don't really know what `handleFiles` is supposed to do, but it is possible `handleFiles` never gets around to calling the function you're passing to it as the second argument. Look into that. – Asad Saeeduddin Dec 30 '15 at 06:49
  • Probably this is a timing issue and there is an async operation and you are looking at `urls` BEFORE it has been updated. It is always a danger sign anytime a global is being updated in a callback. If the callback is async, you're in trouble. – jfriend00 Dec 30 '15 at 06:52
  • If `console.log()` is really inside the callback, like you show in the question, it should show all the URLs. You'll have a problem if it's outside the callback. See http://stackoverflow.com/questions/23667086/why-is-my-variable-undefined-after-i-modify-it-inside-of-a-function-asynchron?newsletter=1&nlcode=97716%7c4ba7 – Barmar Dec 30 '15 at 06:55
  • 'handleFiles' is an AJAX function which needs to return the url of the file uploaded. So, I am using a callback to get the url and then update my variable urls. – learner Dec 30 '15 at 06:58
  • @SonuSindhu I have tried var. It does not work. – learner Dec 30 '15 at 06:59
  • @jfriend00 I have a column in my database which stores the urls of file/image uploaded. I am uploading the file via the AJAX function 'handleFiles' which callbacks the 'url'.I am using the 'url' to update my 'urls'. – learner Dec 30 '15 at 07:02
  • It sounds like you're trying to use the `urls` variable outside your callback which is just not going to work because you will never know when it has valid data. I'd suggest you read this [How do I return data from an asynchronous call](http://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call). – jfriend00 Dec 30 '15 at 07:03
  • Since you are only showing us a little bit of your code, I can't tell if the problem is in your `handleFiles()` function or in whatever code is attempting to use your `urls` variable. But, we will have to see that code to advise you more specifically. The mistake is likely in one of those two spots. – jfriend00 Dec 30 '15 at 07:05
  • @jfriend00 Yes. I am finally going to use my `urls` variable outside my callback. The link which you mentioned is for returning something from an AJAX function. But, I have made a callback to return `url` and now need to update my `urls` so that it has url of all the files uploaded. – learner Dec 30 '15 at 07:07
  • You are apparently not understanding what an ASYNC callback is. it happens at an indeterminate time sometime in the future. Outside the callback, you have NO idea when it occurs. So, you're probably trying to use the `urls` variable BEFORE the callback has been called and thus the variable is not yet updated. That timing issue is your problem and IS what my previous referenced link is all about. As I said earlier, we could help you quickly and completely with a nice answer if you showed us ALL the relevant code. As it now, we just have to debate with you rather than fix it for you. – jfriend00 Dec 30 '15 at 07:09
  • @jfriend00 Updated my code. – learner Dec 30 '15 at 07:16
  • Confirmation. Your issue is because you are treating an async operation like it is synchronous which will never work. Are you trying to do something with `urls` when ALL the `handleFiles()` calls are done? Is that the real end problem you're trying to solve here? Your code doesn't show trying to actually do something with the `urls` variable so I don't know what the end goal here is. The code will have to be restructured significantly so I'm trying to understand the end goal. – jfriend00 Dec 30 '15 at 07:22
  • I am trying to save the url of all the files I have uploaded in my `urls` . So that I can save the urls in my database. I have not yet used to `urls` variable to store in my database as I cannot get all the urls in `urls` . – learner Dec 30 '15 at 07:25

1 Answers1

0

Coordinating multiple asynchronous operations is a job best solved with tools like promises. So, in the long run, I'd suggest you read up about promises and how to use them.

Without promises, here's a brute force way you can tell when all your handleFiles() operations are done by using a counter to know when the last async operation is done and then using the urls variable INSIDE that last callback or calling a function from there and passing the urls variable to that function:

document.getElementById('question-file-selector').addEventListener('change', handleFileSelect, false);

function handleFileSelect(evt) {
    var files = evt.target.files; //File list object
    var urls = "";
    // Loop through file list, render image files as thumbnails
    var doneCnt = 0;
    for (var i = 0, f; f = files[i]; i++) {
        handleFiles(f, function (url) {
            urls = urls + url + ',';
            console.log("urls is " + urls);
            ++doneCnt;
            if (doneCnt === files.length) {
                // The last handleFiles async operation is now done
                // final value is in urls variable here
                // you can use it here and ONLY here

                // Note: this code here will get executed LONG after the
                // handleFileSelect() function has already finished executing
            }
        });
        // Only process image files
        if (!f.type.match('image.*')) {
            $('#list').append('<img class="file-thumb" src="/static/download168.png"/>');
            continue;
        }
        var reader = new FileReader();
        //Closure to capture file information
        reader.onload = (function (theFile) {
            return function (e) {
                //Render thumbnail
                $('#list').append('<img class="thumb" src="' + e.target.result + '" title="' + escape(theFile.name) + '"/>');
            };
        })(f);
        reader.readAsDataURL(f);
    }
}

// You can't use the urls variable here.  It won't be set yet.

Ohhh. Please remove the async: false from your ajax call. That's a horrible thing to code with for a variety of reasons.


Here's a version using the promises built into jQuery:

document.getElementById('question-file-selector').addEventListener('change', handleFileSelect, false);

function handleFileSelect(evt) {
    var files = evt.target.files; //File list object
    // Loop through file list, render image files as thumbnails
    var promises = [];
    for (var i = 0, f; f = files[i]; i++) {
        promises.push(handleFiles(f));
        // Only process image files
        if (!f.type.match('image.*')) {
            $('#list').append('<img class="file-thumb" src="/static/download168.png"/>');
            continue;
        }
        var reader = new FileReader();
        //Closure to capture file information
        reader.onload = (function (theFile) {
            return function (e) {
                //Render thumbnail
                $('#list').append('<img class="thumb" src="' + e.target.result + '" title="' + escape(theFile.name) + '"/>');
            };
        })(f);
        reader.readAsDataURL(f);
    }
    $.when.apply($, promises).then(function() {
        var results = Array.prototype.slice.call(arguments);
        // all handleFiles() operations are complete here
        // results array contains list of URLs (some could be empty if there were errors)
    });
}

function handleFiles(file) {
    var filename = file.name;
    return $.ajax({
        type: 'GET',
        data: {
            "filename": file.name,
            "FileType": "question_file"
        },
        url: '/dashboard/generateuploadurl/',
        contentType: "application/json",
        dataType: "json"
    }).then(function(data) {
        if (data.UploadUrl) {
            console.log("upload url successfully created for " + file.name + " file");
            console.log(data.UploadUrl);
            return handleUpload(data.UploadUrl, file, data.Filename);
        }
    }, function(err) {
        console.log("error occured while creating upload url for " + file.name + ' file');
        console.log(err);
        // allow operation to continue upon error
    });
}

function handleUpload(UploadUrl, file, Filename) {
    return $.ajax({
        xhr: xhr_with_progress,
        url: UploadUrl,
        type: 'PUT',
        data: file,
        cache: false,
        contentType: false,
        processData: false
    }).then(function(data) {
        console.log('https://7c.ssl.cf6.rackcdn.com/' + Filename);
        return 'https://7c.ssl.cf6.rackcdn.com/' + Filename;
    }, function(err) {
        console.log("error occured while uploading " + file.name);
        console.log(err);
        // allow operation to continue upon error
    });
}

Since I can't run this code to test it, there could be a mistake or two in here, but you should be able to debug those mistakes or tell us where the errors occur and we can help you debug it. These are conceptually how you solve these types of problems of coordinating multiple async operations.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Removed the `async: false`.Used this code. But it does not work. I am going to see what promises are. – learner Dec 30 '15 at 07:59
  • @learner - I added a version using the promises built into jQuery. If the first one doesn't work, then it's probably just a typo somewhere. If you debug it and tell us what you find, we can probably help you fix it. And, of course, you'd have to show us what code you were actually using to do something with the `urls` variable in the proper place. – jfriend00 Dec 30 '15 at 08:04
  • Do I need the callback function with promises? – learner Dec 30 '15 at 08:19
  • @learner - No. Promises use `.then()` handlers that take their own callback. Those are the callbacks you use with promises. I removed the `callback` argument from my promises implementation. It was not being used and was just accidentally left-over from your original code. – jfriend00 Dec 30 '15 at 08:21
  • I tried all combinations. The promise gives the last url only. I do `$.when.apply($,promises).then(function(results){ console.log(results.toString()); document.getElementById('id_file_urls').value = results.toString(); });` – learner Dec 30 '15 at 08:37
  • @learner - I fixed a couple typos in the code in my answer. I would need to see the actual code you are using to see what else might be wrong. You could put it in a jsFiddle and post a link in a comment. – jfriend00 Dec 30 '15 at 09:08
  • @learner - I see the issue. `$.when()` sends the results as separate arguments to the `.then()` handler, not as a single array. I've modified my answer to convert the arguments to an array where you can then use that array. – jfriend00 Dec 30 '15 at 10:04