-
Notifications
You must be signed in to change notification settings - Fork 51
fix: Allow for later adding of django CMS versioning #185
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
Conversation
Reviewer's GuideThis PR refactors the 0010 data migration to strip out all built-in versioning checks and dependencies—deferring version setup to run after migrations—and adds README entries for the new versioning feature flags. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @fsbraun - I've reviewed your changes - here's some feedback:
- Specify a reverse_code (or use migrations.RunPython.noop) for the cms4_grouper_version_migration to make the migration reversible.
- Use snippet.save(using=db_alias) when persisting to ensure writes go to the same database alias used for reads.
- Add an explicit dependency on the contenttypes app or re-introduce create_contenttypes to guarantee ContentType entries exist before this migration runs.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/djangocms_snippet/migrations/0010_cms4_grouper_version_data_migration.py
Outdated
Show resolved
Hide resolved
src/djangocms_snippet/migrations/0010_cms4_grouper_version_data_migration.py
Outdated
Show resolved
Hide resolved
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Description
This PR removes a conditional dependency from the migration files and allows to add versioning to snippets after having executed all migrations.
This is particular important when upgrading from django CMS 3 ( no versioning) to django CMS 4+ (potentially with versioning).
The settings are added to the README.
Related resources
Checklist
master
Discord to find a “pr review buddy” who is
going to review my pull request.
Summary by Sourcery
Simplify the existing migration to remove djangocms_versioning dependencies so migrations can execute without versioning installed and allow versioning to be added post-migration.
Enhancements:
Documentation: