[Issue #3500] Handle opportunity attachments when opp deleted or is published #3503
Conversation
| @@ -53,11 +66,18 @@ def process_opportunity( | |||
| extra, | |||
| ) | |||
|
|
|||
| # Cleanup the attachments from s3 | |||
| if target_opportunity is not None: | |||
| for attachment in target_opportunity.opportunity_attachments: | |||
There was a problem hiding this comment.
Just making sure I understand the flow here. target_opportunity is an existing record on our side, that we're replacing due to an update. So this is effectively the "old" attachments and so we're cleaning those out and then we'll put "back" any attachments that are still referenced, and any net new attachments.
There was a problem hiding this comment.
I see now these are deleted Opportunities only so there's no putting back, deleting is the final state.
| # If an opportunity went from being a draft to not a draft (published) | ||
| # then we need to move all of its attachments to the public bucket | ||
| # from the draft s3 bucket. | ||
| if was_draft and transformed_opportunity.is_draft is False: |
There was a problem hiding this comment.
Maybe I am mis-understanding in my comment above, because if I'm reading that right, would we not have deleted the draft entries above?
There was a problem hiding this comment.
Never mind, I saw in the other review a little more context around the above section and see now that it's only deleting them if the record is marked as deleted.
| # Cleanup the attachments from s3 | ||
| if target_opportunity is not None: | ||
| for attachment in target_opportunity.opportunity_attachments: | ||
| file_util.delete_file(attachment.file_location) |
There was a problem hiding this comment.
Does this delete all revisions of a file (is the bucket using versioning?) If not, we may need to delete all past revisions as well.
There was a problem hiding this comment.
We don't have versioning on the bucket.
…draft-delete-opp-attachments
…draft-delete-opp-attachments
…draft-delete-opp-attachments
Summary
Fixes #3500
Time to review: 10 mins
Changes proposed
Handle the following two cases during the transformation process
Context for reviewers
Mostly just some additional file utils for moving files to handle the above scenarios. Only noteworthy callout is that there isn't really a concept of "moving" a file on s3, it's just a copy+delete.