Directory: Preserve attachments on entry save with past publication date - #2633
Directory: Preserve attachments on entry save with past publication date#2633Tschuppi81 wants to merge 12 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Daverball
left a comment
There was a problem hiding this comment.
Overall it seems like a reasonable solution, but it also makes an already messy situation even messier. I'm also not happy that some of the implementation details of directories have now leaked into the generic form module.
If we can clean this up and improve the situation, I'll be a lot happier.
| if raw_data.startswith('@'): | ||
| # reference to a persisted file: keep the loaded metadata | ||
| # (size, mimetype) so display and validation still work | ||
| original = self.object_data | ||
| if isinstance(original, dict) and \ | ||
| original.get('data') == raw_data: | ||
| self.data = cast('StrictFileDict', original) | ||
| else: | ||
| self.data = { | ||
| 'data': raw_data, | ||
| 'filename': str(valuelist[2]), | ||
| } |
There was a problem hiding this comment.
I'm not super happy that we have moved an implementation detail of how directory entries store their uploaded files into the generic UploadField. If you can make this work without adding any special-casing here, that would be a preferred solution.
There was a problem hiding this comment.
Right, in the generic UploadField makes only sense if we face the same issue for other form too.
There was a problem hiding this comment.
It turns out plain form submissions rely on this the same way directories do: saved uploads are referenced as @ and resent on edit — without the @ branch they get decoded as inline data and lost. Added test_pending_submission_file_survives_edit to cover it. So UploadField is the right location and fixes Directories and Form Submissions.
There was a problem hiding this comment.
If that's the case it makes even more sense to define formcode specific subclasses, since we still use UploadField in other places that aren't form submissions. And we probably want to refactor things so that file handling is reusable between FormSubmissionCollection.update and Directory.update, otherwise we will always have to fix bugs in two locations and be very careful to keep things in sync, which they may already not be.
| 'filename': data['filename'], | ||
| 'mimetype': new_file.reference.file.content_type, | ||
| 'size': new_file.reference.file.content_length | ||
| } |
There was a problem hiding this comment.
It's a little unfortunate how things here got even more messy and repetitive, I think we're at the point where a refactor might be necessary to keep things readable/understandable.
Directory: Preserve attachments on entry save with past publication date for single and multi upload fields
Uploading a file and saving/editing a directory entry with a publication date in the past triggered a validation error that discarded the attachments. The upload now survives the re-render (new entries) and edits keep the persisted files.
TYPE: Bugfix
LINK: OGC-3366