-
-
Notifications
You must be signed in to change notification settings - Fork 24
Fix #228 -- Skip migration for objects without files #230
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.
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
Fix #228 -- Skip migration for objects without files #230
Conversation
|
There are existing tests already to ensure that the migrations are carried out for objects with pictures and that nothing is done for objects without pictures, this PR is just making that 2nd step more direct. |
codingjoe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! But just to be safe, be so kind and add a test case to prevent regression in future versions.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #230 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 13 13
Lines 495 495
=========================================
Hits 495 495
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
codingjoe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the liberty of adding them :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issue #228 by adding logic to skip database objects that have empty or null file fields during migrations. This prevents errors when the migration code attempts to process file operations on non-existent files.
- Added filtering to exclude objects with empty (
"") orNonevalues in the picture field before processing - Applied the fix consistently across all three migration methods
- Added comprehensive test coverage for each migration method to verify the fix
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pictures/migrations.py | Added .exclude(Q(**{self.name: ""}) | Q(**{self.name: None})) filtering to update_pictures, from_picture_field, and to_picture_field methods to skip objects without files |
| tests/test_migrations.py | Added three new test methods (test_update_pictures__with_empty_pictures, test_from_picture_field__with_empty_pictures, test_to_picture_field__with_empty_pictures) to verify that migrations correctly handle mixed scenarios with both populated and empty picture fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
Close #228