aldeed / meteor-cfs-autoform

Upload files using CollectionFS as part of AutoForm submission
MIT License
37 stars 32 forks source link

Update form #26

Open aj07mm opened 9 years ago

aj07mm commented 9 years ago

The update is still on todo?

aldeed commented 9 years ago

I think @dpankros may be working on it?

dpankros commented 9 years ago

I am, but due to some issues, like the reactivity issue we discussed, and demands on my time, it’s taking longer than anticipated. On January 29, 2015 at 4:51:21 PM, Eric Dobbertin (notifications@github.com) wrote:

I think @dpankros may be working on it?

— Reply to this email directly or view it on GitHub.

ndee commented 9 years ago

@dpankros: How will the upload work for multi-file fields? Will it be possible to add and remove previously uploaded files or will the update just replace files? For my app the first approach would be very useful. If you implement just the replace I would like to add the "add&remove" feature. What do you think?

dpankros commented 9 years ago

@ndee I will do my best to make the updates granular so you don't have to replace all the files in a batch, unless you want to. I'm going to get to that side of it soon but I have other work to complete before I can get back to it.

Right now, the single-file version allows adding, removing and updating. I wouldn't expect it to be too much more difficult to make the multiple-file version mirror that behavior. Of course, it's possible that I run into a showstopper when I get into the code, but I hope not.

Just a warning that the new version will likely require AutoForm 5.

ndee commented 9 years ago

@dpankros Sounds promising! Handling of multiple files in insert and update forms will be a core feature in my app. So, I'm motivated to support you. :-) Usage of AutoForm 5 will be no problem for me.

What recently came into my mind: How will the update work when I specify an array which contains items with file references. Like in the following example.

Will the update remove the file when I remove an array item containing a file reference?

ImageSchema = new SimpleSchema({
    name: {
        type: String,
        label: 'Comment'
    },
    fileId: {
        type: String,
        autoform: {
            afFieldInput: {
                type: 'cfs-file',
                collection: 'images'
            }
        }
    }
});

MySchema = new SimpleSchema(        {
    name: {
        type: String,
        label: "Name"
    },
    files: {
        type: [ImageSchema],
        label: 'Images',
        optional: true
    }
});
dpankros commented 9 years ago

"Delete on remove" is an option that you can specify in the new preferences object. If you want them to be removed, set it to true. Otherwise it will be up to you to handle the orphaned files (such as archiving them).

ndee commented 9 years ago

Very cool!

Neobii commented 9 years ago

https://github.com/aldeed/meteor-cfs-autoform/pull/27 I didn't get it to attach to the right issue. This is a pretty quick fix as I just copied the insert hooks to the update hooks but instead of the update I added the changes to the $set object.

This doesn't work for multiple files, however.

tcastelli commented 9 years ago

Sounds really cool, "Delete on remove" could be active by default. Any update on this part @dpankros? :)

dpankros commented 9 years ago

@tcastelli "Delete on Remove" is in my fork (not merged back into the main yet). I think the default behavior should be off, however, because I do not want to be the cause of data being lost accidentally. If a delete is performed, it should be a conscious decision by the developer of the app, not the package. It is simple to enable, however. E.g:

CfsAutoForm.prefs.set('deleteOnRemove', true);

This code is for the new, unreleased version. It will not work with the current version.

tcastelli commented 9 years ago

I see, seems reasonable for anyone who is considering restoring/versioning after updates. Which is the quickest way to import your fork in a project? I would like to test it since i don't need multiple updates (i guess i have to clone your repo in the package folder and add through package add right?)

dpankros commented 9 years ago

@tcastelli Yes. That's correct. I also have a test app, if you want to try it separately or use it an example.

I feel compelled to reiterate that you should consider this alpha/beta type code and it should NOT be used in production yet. It is not fully debugged.

tcastelli commented 9 years ago

Thanks for the test app link, even if in beta stage, I don't mind trying it out and discussing about possible bugs :)

petermikitsh commented 9 years ago

+1 for getting this PR'ed...

kpbird commented 9 years ago

@dpankros test app (https://github.com/dpankros/cfs-autoform-test) is not working when I remove insecure and autopublish packages

dpankros commented 9 years ago

The code hasn't been worked on in some time, mainly because it never not the testing it needed beyond what I and a few others had done. Because of the continued changes autoform and cfs-autoform, and not in my fork, I'm considering it all abandoned. Sorry. I don't think anyone is using it anyway.

If it's just insecure and autopublish that cause issues, have you checked the basic causes? E.g. failure to publish a collection.