Skip to content

Fix bug that caused search objects not to delete #5487

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

Merged
merged 3 commits into from
Mar 20, 2019
Merged

Fix bug that caused search objects not to delete #5487

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7d82d7
Fix bug that caused search objects not to delete
ericholscher Mar 18, 2019
d4a4b8e
Remove redundent logic from below.
ericholscher Mar 19, 2019
3d75dc3
Revert "Remove redundent logic from below."
ericholscher Mar 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions readthedocs/projects/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1324,17 +1324,13 @@ def _manage_imported_files(version, path, commit):
)
# Keep the objects into memory to send it to signal
instance_list = list(delete_queryset)
Copy link
Member

Choose a reason for hiding this comment

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

Doing list() doesn't make to hit the db already?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, we can probably remove this if we want, since the objects should exist now.

# Safely delete from database
delete_queryset.delete()
# Always pass the list of instance, not queryset.
# These objects must exist though,
# because the task will query the DB for the objects before deleting
bulk_post_delete.send(sender=HTMLFile, instance_list=instance_list)
# Safely delete from database
delete_queryset.delete()

# Delete ImportedFiles from previous versions
(
Copy link
Member

Choose a reason for hiding this comment

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

All HTMLFile are ImportedFile, but not all ImportedFile are HTMLFile. So we should delete the ImportedFile also. The HTMLFile was deleted above for dispatching signals as HTMLFile is a proxy model.

ImportedFile.objects.filter(project=version.project,
version=version).exclude(commit=commit
).delete()
)
changed_files = [
resolve_path(
version.project,
Expand Down