Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG UploadField cannot save files on new objects until after save. #1827

Closed
tractorcow opened this issue May 2, 2013 · 12 comments
Closed

BUG UploadField cannot save files on new objects until after save. #1827

tractorcow opened this issue May 2, 2013 · 12 comments

Comments

@tractorcow
Copy link
Contributor

I'll consider this a bug, as it's an issue that impacts the usability of this field.

I am currently working on a fix for this. I have something that works well, but I need to rewrite all the test cases, as well as go through the ticket history for usability issues related to this field.

The method I'm choosing to use is the same approach used by the old dataobject manager, which is to store fileids on the form, and then write the references to the parent object on form submission.

This is a "please don't fix this behind my back" while I'm working on this, thank you! I'd like some time to polish this before I commit and pull.

@tractorcow
Copy link
Contributor Author

I've been developing to this branch in the meantime. There still seems to be a huge amount of work and testing to do.

https://github.com/tractorcow/sapphire/tree/3.1-uploadfield-enhancement

Compare mode here:

tractorcow/silverstripe-framework@silverstripe:3.1...3.1-uploadfield-enhancement

@chillu
Copy link
Member

chillu commented May 8, 2013

/cc @Zauberfisch

@chillu
Copy link
Member

chillu commented May 8, 2013

@tractorcow Zauberfisch is planning to rewrite UploadField based on GridField, since he thinks there's a lot of overlap. Its unclear whether this work can go into core (its very early stage), but maybe you two want to sync up? Zauberfisch is in GMT+1, and on IRC pretty frequently.

@chillu
Copy link
Member

chillu commented May 8, 2013

Can you maybe send some isolated pull requests based on these commits, to make review and merging easier? I guess there's only a few of these commits which require more testing, while others are pretty straightforward to integrate?

@tractorcow
Copy link
Contributor Author

Hi Ingo, thanks for the update.

UploadField is just about done. It behaves quite well as a standalone image uploader, and upload process has been greatly decoupled from the server. It'd be a shame to throw this away, but rather would it be better to develop the GridField based field as a separate field? (GridFieldImageManager or something to that effect?)

I'm just doing some last minute usability testing, making sure that all the javascript behaves well, and ironing out any usability issues and niggles from the original code.

@tractorcow
Copy link
Contributor Author

With regards to the individual breakdown of the commits, I'm not sure that they can be treated as distinct changes, but rather they are behavioural updates, and respective fixes for those updates. It would be hard to pull in many of them as standalone.

What I'll do is create another branch with the commits rebased and squashed, then pull request this. In this pull request I'll document each of the individual commits, and point out ones that can be pulled in individually (of which I think there may be 1 or 2 only).

@tractorcow
Copy link
Contributor Author

Also, you should check out https://github.com/colymba/GridFieldBulkEditingTools. It's an amazing module. :)

@chillu
Copy link
Member

chillu commented May 8, 2013

In regards to something replacing UploadField, that's all hypothetical at the moment. @Zauberfisch pushes in this direction, but its too early to tell - I like the power of UploadField, and look forward to your pull request :) No need to push too much effort into splitting up commits if they're reliant on each other, just thought maybe there's some quick wins to get out of the way. The bulk editing module looks interesting, but has a lot of JS copypaste from the UploadField module, which would emphasise Mr. Fisch's point about the functionality belonging into GridField though hehe.

@tractorcow
Copy link
Contributor Author

Bulk editing is already available in the UploadField, it just doesn't work with non-file objects, which is where the bulk editing tools works well (Similar to the old filedataobjectmanager). I think that having a good stand alone file upload tool, as well as a good extension for managing image-reliant dataobjects through the gridfield would be a good goal.

@tractorcow
Copy link
Contributor Author

Currently just struggling with fileupload validation on attach; It currently doesn't properly limit max number of files when attaching (especially with has_ones). I think once I've resolved this it'll be good to pull.

@tractorcow
Copy link
Contributor Author

Check it out now!

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

No branches or pull requests

2 participants