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.

Sign up for GitHub

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

Jump to bottom

Add deleted_at to attachments #1698

Merged
merged 4 commits into from
Feb 19, 2020
Merged

Add deleted_at to attachments #1698

merged 4 commits into from
Feb 19, 2020

Conversation

WinnyTroy
Copy link
Contributor

@WinnyTroy WinnyTroy commented Oct 9, 2019 •
edited
Loading

Add deleted_at Field to Attachments.
Create soft_delete action that will populate deleted_at field for attachments.
Resolves #1696

pld
pld requested changes Jan 7, 2020
View reviewed changes
Copy link
Member

@pld pld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a test that deleted_at is set to the user that deleted it correctly? I didn't see it, maybe I missed it

@WinnyTroy
Copy link
Contributor Author

is there a test that deleted_at is set to the user that deleted it correctly? I didn't see it, maybe I missed it

I dont believe so. deleted_at is set to a datetime object.

@pld
Copy link
Member

pld commented Jan 14, 2020

is there a test that deleted_at is set to the user that deleted it correctly? I didn't see it, maybe I missed it

I dont believe so. deleted_at is set to a datetime object.

my mistake, I meant to write delete_by

can we please add this test if it doesn't exist

@WinnyTroy
Copy link
Contributor Author

is there a test that deleted_at is set to the user that deleted it correctly? I didn't see it, maybe I missed it

I dont believe so. deleted_at is set to a datetime object.

my mistake, I meant to write delete_by

can we please add this test if it doesn't exist

Sure, let me confirm, and include one if none exists

DavisRayM
DavisRayM previously approved these changes Jan 15, 2020
View reviewed changes
Copy link
Contributor

@DavisRayM DavisRayM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

pld
pld previously approved these changes Jan 16, 2020
View reviewed changes
@WNjihia
Copy link

WNjihia commented Jan 16, 2020

@WinnyTroy I uploaded a submission with an image then edited the submission and uploaded a different image. It seems like both images are still being fetched. Also kindly check on the photo tab view for the form - I noticed that the deleted image is still being fetched here too and in an instance where I edited the submission and deleted the image without uploading a new one, the deleted image still appears on the photo tab view
cc: @DavisRayM @pld

@WinnyTroy
Copy link
Contributor Author

@WinnyTroy I uploaded a submission with an image then edited the submission and uploaded a different image. It seems like both images are still being fetched. Also kindly check on the photo tab view for the form - I noticed that the deleted image is still being fetched here too and in an instance where I edited the submission and deleted the image without uploading a new one, the deleted image still appears on the photo tab view
cc: @DavisRayM @pld

this seems to be a deployment issue. I'll re-deploy the changes and request for QA once more. Appologies

@DavisRayM
Copy link
Contributor

Added a Blocked label as this is being blocked by the discussion ongoing here.

@WinnyTroy
Copy link
Contributor Author

Added a Blocked label as this is being blocked by the discussion ongoing here.

Removing this label

@WinnyTroy WinnyTroy removed the Blocked label Jan 27, 2020
@WinnyTroy WinnyTroy dismissed stale reviews from pld and DavisRayM via b03458f January 27, 2020 13:29
ivermac
ivermac reviewed Feb 12, 2020
View reviewed changes
onadata/apps/logger/models/attachment.py Outdated Show resolved Hide resolved
onadata/libs/utils/logger_tools.py Outdated Show resolved Hide resolved
@WinnyTroy WinnyTroy force-pushed the attachment_endpoint_fixes branch 3 times, most recently from 91d821f to 25ef8ef Compare February 13, 2020 07:08
pld
pld previously approved these changes Feb 18, 2020
View reviewed changes
ukanga
ukanga previously approved these changes Feb 18, 2020
View reviewed changes
@faith-mutua
Copy link

@WinnyTroy The fix has passed QA. Deleted images are no longer being displayed on the Photo's tab nor are their links in the Table tab. Also, both audio and video files are being replaced successfully.

@WinnyTroy
Add deleted_at Field to Attachments.
56e8be3 
Create soft_delete action that will populate deleted_at field for attachments. Fixes #1696
Call soft_delete action when replacing attachments
@WinnyTroy
Include deleted_by field to the attachments models
f98d2c7 
Make changes as requested from revies
Update failing tests
@WinnyTroy
Fix Flake8 errors
c3af830
@WinnyTroy
Included test showing replaced attachment functionality
953e0cf 
Moved soft delete action to the instance model
Restructured how we call the soft_delete action when editing submissions
@WinnyTroy WinnyTroy dismissed stale reviews from ukanga and pld via 953e0cf February 19, 2020 11:48
@ukanga ukanga merged commit e0123fa into master Feb 19, 2020
@ukanga ukanga deleted the attachment_endpoint_fixes branch February 19, 2020 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivermac ivermac ivermac left review comments

@DavisRayM DavisRayM DavisRayM left review comments

@ukanga ukanga ukanga approved these changes

@pld pld pld left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Submission attachments list is not updated correctly after replacing an old image/file with a new one.
7 participants
@WinnyTroy @pld @WNjihia @DavisRayM @faith-mutua @ukanga @ivermac