Skip to content
This repository has been archived by the owner on Mar 25, 2020. It is now read-only.

Insert only? #7

Open
SachaG opened this issue Oct 5, 2014 · 9 comments
Open

Insert only? #7

SachaG opened this issue Oct 5, 2014 · 9 comments

Comments

@SachaG
Copy link

SachaG commented Oct 5, 2014

The readme mentions this package works with insert forms.

So how would you deal with someone wanting to change a file they previously uploaded? A typical use case would be someone uploading an avatar to their profile.

@aldeed
Copy link
Owner

aldeed commented Oct 5, 2014

I haven't thought it through much yet. CFS does not have great built-in support for updating files at the moment, so I think I would probably use the same form with a hook to delete the existing avatar file once the new one is uploaded.

This pkg is really at more of a POC status right now. Seems to work fine, but I'm open to improvement ideas.

@macrozone
Copy link

I created a fork and implemented an update behaviour there.
For every cfs-field, I fetch the corresponding property in the document that should be updated and delete the files, that had been attached to this property.

It's not tested well, in particular,I did not test it with fields that contain multipe files:
https://github.com/panter/meteor-cfs-autoform

@dpankros
Copy link
Collaborator

How does it handle failure cases? That is, when are you deleting the files? Are you deleting the old files on a successful update and the new files on failure?

On Dec 12, 2014, at 6:07 AM, Marco Wettstein notifications@github.com wrote:

I created a fork and implemented an update behaviour there.
For every cfs-field, I fetch the corresponding property in the document that should be updated and delete the files, that had been attached to this property.

It's not tested well, in particular,I did not test it with fields that contain multipe files:
https://github.com/panter/meteor-cfs-autoform


Reply to this email directly or view it on GitHub.

@macrozone
Copy link

Deleting files occure before inserting the new ones. So if insertion of a new file fails, the old files are deleted nevertheless. (by the way: i forget to push the current version, it should now be updated on the fork)

We could consider performing some kind of rollback: do not delete the files, but mark them as to be deleted. If insert of the new files succeed, delete the marked files.

@dpankros
Copy link
Collaborator

I'm going to leave this one to @aldeed. Personally, I think we need to handle the failure cases slightly better in that a failure should put the database back to it's original state and not partially updated. For the apps I tend to work on, a failure could snowball into a disaster if I lose a file.

The other difficulty here is that we're trying to merge this project with another similar project that already has update capability. I'm hesitant to add more code into the mix and possibly create a more difficult merge of the codebases when it may be removed in favor of the other project's code anyway. Thus, I'm going to defer to @aldeed here.

Thanks for your work!

@macrozone
Copy link

It is a prove-of-concept.

@aldeed, I am testing my fork currently and try to improve the failure-handling. It also needs refactoring. We might could merge back, if it is in a appropriate state. What do you think?

@aldeed
Copy link
Owner

aldeed commented Dec 12, 2014

I think we can take the best of @macrozone and @yogiben update implementations and manually write a merged implementation into this package. I think @yogiben said his is fairly simple, adding/deleting the file immediately without waiting for form submission. Ideally we should support both ways, with the comprehensive rollback way being the default.

I would personally be interested in knowing more about specific use cases. When I handle updates, I tend to just handle them directly through CFS code, not as part of an update form. I think the main reason people are wanting this is for managing attached files in auto-CRUD like yogiben:admin. If that's true, then we should initially target toward that use case.

@sf-wind
Copy link

sf-wind commented Dec 27, 2014

"Insert only" means if I update the form, without touching the file field, I would lose the uploaded files? And that file become dangling around? That's what happened to me on my form. After I update the form, the file field, which should be an _id of the file collection, becomes "dummyId".

Allowing updating is a very useful feature. I would love to see it implemented...

@aldeed
Copy link
Owner

aldeed commented Dec 28, 2014

@sf-wind, yes probably, although there should be a way for us to make sure the "dummyId" doesn't make it through. I thought we were already doing that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants