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 24 commits into
Conversation
Tschuppi81
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Daverball
left a comment
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.
| self.append_entry_from_field_storage(value) | ||
|
|
||
|
|
||
| class FormcodeUploadField(UploadField): |
There was a problem hiding this comment.
Newly added classes FormcodeUploadField and FormcodeUploadMultipleField
Removes the FormcodeUpload* subclasses and the _rebuild_file_from_upload workaround in Directory.update. UploadField.process_resend now handles both cases generically: an already-stored '@<id>' reference is preserved, an unstored resend is decoded and exposed as an upload so it gets stored. Directory.update simply falls through to store the resent upload. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NyvouXerch757wofwGXyfF
| new_idx += 1 | ||
| continue | ||
| # new entry / added file: no stored file yet, fall | ||
| # through to create it from the resent upload below |
There was a problem hiding this comment.
I'm still pretty unhappy about how different this is from FormSubmissionCollection.update and that there's no code-reuse between the two at all. But I suppose we don't have to fix that right now.
| else: | ||
| form_field.data = data | ||
|
|
||
| def on_request(self) -> None: |
There was a problem hiding this comment.
All of this seems completely unnecessary. If UploadField works correctly, then with an action of keep it should leave its own data untouched, so it automatically should match the field.object_data we originally passed in via values when constructing the form object.
If the UploadField messes with it's data despite the action being keep, then that's a bug and you need to fix it there instead of trying to work around it here.
There was a problem hiding this comment.
Replaced with a second process as long is we are not aligned to the FormSubmission way doing it...
2 participants
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