UploadServer.delete method does not delete the thumbnails

[s7dhansh]

Kindly fix :)

[tomitrescak]

?? The delete method is not implemented at all and is left for anyone to implement since server does not know where you store upload info. Sending path of a deleted file is a major security threat.

[s7dhansh]

I just tried this code:

'deleteFile': function (fileNameOnServer) {
    ...
    if (Meteor.isServer) UploadServer.delete(fileNameOnServer);
   ...
  }

and it is deleting the file on the server, but not the thumbnails. I got the idea from https://github.com/tomitrescak/meteor-uploads/issues/21.

This is a server bug (https://github.com/tomitrescak/meteor-tomi-upload-server). If you want I can raise it there.

[s7dhansh]

@tomitrescak any thoughts?

[nonissue]

@s7dhansh This is not a bug, it is a feature request, and something you can fix on your own. Reporting an issue on an open source project with the body of the post being "kindly fix :)" and then insisting it is a bug after the project creator has closed it is quite disrespectful IMO.

Anyway, it is possible to delete the thumbnails without this being added as a feature to the project. If you add the following to your deleteFile server-side method, it should delete images in the appropriate subdirectories. Note that you'll still have to call UploadServer.delete(upload.path) to delete the original.

var subFolders = Object.keys(UploadServer.getOptions().imageVersions);
for(i in subFolders) {
    UploadServer.delete(upload.subDirectory + "/" + subFolders[i] + "/" + upload.name);
};

[s7dhansh]

@nonissue With due respect, it is an incomplete feature (which I do put in the category of bugs). If thumbnails are created automatically, wouldn't one expect them to be deleted too? I didn't think there was too much to tell about it, it was easily reproducible. When I figured out that tomi had not understood what I meant, I gave the detailed explanation. Now it was up to tomi to mark it as enhancement, but I did not get any response.

Well, my intention was not to disrespect you guys. Thank you for the answer. I just had a look in the code. Since it is already prefixing the uploadDir path (https://github.com/tomitrescak/meteor-tomi-upload-server/blob/master/upload_server.js : 136), we can simply give this:

var subFolders = Object.keys(UploadServer.getOptions().imageVersions);
for(i in subFolders) {
    UploadServer.delete(subFolders[i] + "/" + upload.name);
}

Considering there is no extra parameter being passed, this code can be a part of the default UploadServer.delete method, no? Am I missing something?

[nonissue]

@s7dhansh It's not my project and in no way do I deserve any recognition and I have no connection to tomi, I just try and answer questions when I can to pay him back for his hard work.

You have to look at it this way: no project will ever be feature complete, ever. You have to accept some of the responsibility and recognize that with packages for your projects, you may have to write some of the methods and integrations to meet your requirements. This is not a file management package, it is an uploads package. @tomitrescak generously integrated a simple delete method as a few people requested it, but that does not mean the onus is on him to implement some massively complex method that meets every requirement anyone can ever imagine. I personally prefer he integrate small methods when he can, and if you have an idea for improving it, I'm sure he would welcome your PR. I do not mean this in a snarky or rude way, I just think you have to recognize he does this in his own free time and does not get paid for it.

You are correct in that for your use case, the upload.subDirectory is not necessary. I forgot that in my implementation, I specify a subdirectory for a project that is the parent of each uploaded image.

[nonissue]

@s7dhansh Also, if you are generating the thumbnails for the purposes of serving responsive images based on viewport width/device, I may have some improvements I can share with you that may make your life easier, so let me know if you are interested! I hope the code I have provided helps.

[s7dhansh]

@nonissue I completely understand that and agree with you. I did not look at the code earlier (fear of the unknown) and was waiting for an entry point, which you gave me. So thank you again.

Sure I would love the improvements!

[nonissue]

@s7dhansh If you follow me back on twitter, we can DM so I can share that code.

[s7dhansh]

@nonissue done

@tomitrescak https://github.com/tomitrescak/meteor-tomi-upload-server/pull/22. Please review.