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.

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

feat: Snippet CMS 4.0 and djangocms-versioning Data migration #74

Conversation

adam-murray
Copy link
Contributor

Migration to support CMS4 transition from existing CMS3 installs

adam-murray and others added 30 commits August 9, 2021 13:12
Copy link
Collaborator

@Aiky30 Aiky30 left a comment

Choose a reason for hiding this comment

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

Sorry Adam, more changes required.

Different issues:

  • Migrations look to be out of sync to the code
  • Changes to test code that wasn't needed, only change what you have to on a previous project, especially when it's still maintained in a v3 branch.
  • Unused factories that can be cleaned

@@ -20,13 +20,13 @@ class Snippet(models.Model):
"""
name = models.CharField(
verbose_name=_('Name'),
unique=True,
max_length=255,
)
snippet_grouper = models.ForeignKey(
SnippetGrouper,
on_delete=models.PROTECT,
null=True,
Copy link
Collaborator

Choose a reason for hiding this comment

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

You shouldn't need the null or the blank here.

It's added initially for the migration here: https://github.com/django-cms/djangocms-snippet/pull/74/files#diff-7975630682285642c8665843c1405c2016fe129a1bc04adc016a3600bbbc3b49R38
It's then removed here: https://github.com/django-cms/djangocms-snippet/pull/74/files#diff-4ca21242eb21cfa24bd2e3f9e4848d967856c53f97e1cd1838f5dc194efcd14fR21

I think if you were to run the migrations again you would create a new migration file.

@@ -20,13 +20,13 @@ class Snippet(models.Model):
"""
name = models.CharField(
verbose_name=_('Name'),
unique=True,
max_length=255,
)
snippet_grouper = models.ForeignKey(
SnippetGrouper,
on_delete=models.PROTECT,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't match the migrations that you ran, did you change the protection bit later i.e. after creating the migrations?

https://github.com/django-cms/djangocms-snippet/pull/74/files#diff-4ca21242eb21cfa24bd2e3f9e4848d967856c53f97e1cd1838f5dc194efcd14fR21

If you ran make migrations this will create a new migration file. The migrations files will need to be updated from Cascade.

djangocms_snippet/models.py Outdated Show resolved Hide resolved
self.page.delete()
self.home.delete()
self.superuser.delete()
page_data = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should try and change as little as possible of the existing code, this makes maintenance easier. The master branch will receive updates for v3 and we will need to port those updates back in, if there is more noise than needed this can make the job harder.
Can you please move this back to match the original and only change what you need to change, Also the api published param is deprcated and will throw a warning:

    self.home = create_page(
            title="home",
            template="page.html",
            language=self.language,
            "created_by": self.superuser
        )

abstract = True


class TreeNodeFactory(factory.django.DjangoModelFactory):
Copy link
Collaborator

Choose a reason for hiding this comment

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

TreeNodeFactory is not used, please delete it

model = TreeNode


class PageFactory(factory.django.DjangoModelFactory):
Copy link
Collaborator

Choose a reason for hiding this comment

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

PageFactory is not used, please delete it

model = Page


class PageContentFactory(factory.django.DjangoModelFactory):
Copy link
Collaborator

Choose a reason for hiding this comment

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

PageContentFactory is not used, please delete it

model = PageContent


class PageVersionFactory(AbstractVersionFactory):
Copy link
Collaborator

Choose a reason for hiding this comment

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

PageVersionFactory is not used, please delete it

model = Version


class PageContentWithVersionFactory(PageContentFactory):
Copy link
Collaborator

Choose a reason for hiding this comment

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

PageContentWithVersionFactory is not used, please delete it

@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2021

Codecov Report

Merging #74 (e958a03) into support/django-cms-4.0.x (eeb8072) will decrease coverage by 5.25%.
The diff coverage is 71.15%.

Impacted file tree graph

@@                     Coverage Diff                      @@
##           support/django-cms-4.0.x      #74      +/-   ##
============================================================
- Coverage                     98.33%   93.07%   -5.26%     
============================================================
  Files                            16       20       +4     
  Lines                           180      231      +51     
  Branches                          8       12       +4     
============================================================
+ Hits                            177      215      +38     
- Misses                            2       12      +10     
- Partials                          1        4       +3     
Impacted Files Coverage Δ
...ocms_snippet/migrations/0009_auto_20210915_0445.py 100.00% <ø> (ø)
...ppet/migrations/0011_cms4_plugin_data_migration.py 60.00% <60.00%> (ø)
...ations/0010_cms4_grouper_version_data_migration.py 65.62% <65.62%> (ø)
djangocms_snippet/conf.py 100.00% <100.00%> (ø)
...ocms_snippet/migrations/0012_auto_20210915_0721.py 100.00% <100.00%> (ø)
djangocms_snippet/models.py 100.00% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eeb8072...e958a03. Read the comment docs.

Copy link
Collaborator

@Aiky30 Aiky30 left a comment

Choose a reason for hiding this comment

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

All good.

Ran this locally with a setup of snippets already created. They create fine and work as expected. :-)

@Aiky30 Aiky30 changed the title Snippet CMS4 Data migration feat: Snippet CMS 4.0 and djangocms-versioning Data migration Sep 15, 2021
@Aiky30 Aiky30 merged commit a441d5a into django-cms:support/django-cms-4.0.x Sep 15, 2021
fsbraun added a commit that referenced this pull request Aug 20, 2024
* djangocms-versioning compatibility - Initial Model and cms_config implementation (#75)

* feat: Snippet CMS 4.0 and djangocms-versioning Data migration (#74)

* feat: Moderation compatibility and version copy method implementation (#77)

* feat: Versioning Compatibility Changes (#79)

* fix: Fixed bug that causes ChangeView not to use SnippetForm (#80)

* feat: Reworked form fields and added the ability to show draft snippets (#81)

* fix: Adding valid HTML to the add snippet form throws an error (#82)

* fix: form initialisation error in read-only mode (#83)

* fix: History URL button broken link on the Snippets change form (#84)

* fix: Draft snippet render error due to draft snippets (#86)

* feat: Default versioning import user id setting can be set via environment variable (#89)

* Release 4.0.0.dev1 (#91)

* fix: django-cms 4.0.x - Remove breaking django-treebeard pinning issue (#93)

* Release 4.0.0.dev2 (#95)

* feat: Removing slug field and hyperlink from list display when versioning is enabled (#98)

* fix: Snippet plugin showing ID instead of name(#100)

* Fix: Compare view removed unnecessary template logic (#101)

* Release 4.0.0.dev3 (#103)

* feat: Preview icon renders content in read only  (#102)

* Release 4.0.0.dev4 (#109)

* fix: Added test coverage to admin preview view (#96)

* port-feat: pre-commit config added from the v3 workstream (#117)

Changes partially taken from this change: a56091c

Requires additional work when the django-cms 4.0 workstream is official, the version bump code has been ignored because it would generate official releases only: #116

* feat: django-cms 4.0.x - django 3.2 and Python 3.9 compatibility  (#92)

* Release 4.0.1.dev1 (#119)

* feat: Enable add button on the SnippetPluginForm (#127)

* Release 4.0.1.dev2 (#128)

* init support for django4.2 (#150)

* init support for django4.2

* add change log for add compatiable for django 4.2 and python 3.10

* Update CHANGELOG.rst

* Update .pre-commit-config.yaml

* fix lint config issue

* remove test code

* add blackline for flake8

* add pyproject.toml file

* add requirement files

* fix ci failed issue

* format setup.py import section

* upgrade isort version to fix pre commit ci issue

* upgrade the django-upgrade package to pass through the pre commit ci test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* upgrade packages in pre commit

* resort init file changes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* comment the django-upgrade from ci

* fix: add missing migration file

* fix: update previous file instead of creating new file

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Josh Peng Yu <[email protected]>

* fix pyproject.toml config error (#151)

* Added site field in models n site filter in admin (#159)

* Added site field in models n site filter in admin

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* updated versioning branch

* Add test cases and updated the implementation

* Update CHANGELOG.rst

Co-authored-by: Mark Walker <[email protected]>

* corrected formatting

* fixed flake8 issues

* removed list_filter

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Mark Walker <[email protected]>

* Release/4.1.0 (#155)

* bump version to 4.1.0

* bump version to 4.1.0 pyproject.toml

* replace test requirement url

* update changelog date

* update release date.

* Update README.rst

* Update README.rst

* Create PULL_REQUEST_TEMPLATE.md (#85)

Add PR template

* Only include badges at the top to avoid confusion & removed old badges like travis. (#106)

* Added pypi github actions (#108)

* FEATURE: adjust setup metadata (#110)

* correct doc about default value of DJANGOCMS_SNIPPET_CACHE (#120)

* Update to ace 1.9.6, load ace editor from static files if djangocms_static_ace is installed, add dark mode (#123)

Fix:		Load ace editor from static files if `djangocms-static-ace` is installed
Fix:		Respect if user has set dark mode
Add:	Weak dependency on djangocms-static-ace
Doc:	optional static-ace dependency

* ci: Update pypi action branch names (#125)

* Bump version: 3.0.0 → 3.1.0 (#126)

* fix: django-cms 3.x - Remove breaking django-treebeard pinning issue (#94)

* Removed pinning.

* Update setup.py

---------

Co-authored-by: Fabian Braun <[email protected]>

* fix: add noop django 4 migration (#134)

* fix: add noop django 4 migration

* fix: isort

* ci: Prepare release of version 3.1.1 (#139)

* Update CHANGELOG.rst

* Bump version

* Pin treebeard for Django CMS 3.7 tests

* Update dj30_cms37.txt

* chore: Package overhaul (#140)

* ci: Update pre-commit

* ci: Run ruff formatter

* build: Move package in to src dir

* test: Update requirements building

* chore: Keeping git happy after moving to src

* ci: Add dependabot config

* ci: Update tests to run tox

* build: Project config

* test: Update tox setup

* build: Update setup to build_meta

* ci: Update pypi workflows

* ci: Update linter to ruff

* chore: Fix RUF012

* ci: Update linter trigger

* chore: Drop python 3.8

* chore: Drop python 3.8

* chore: Adjust coverage config

* fix: Test pypi workflow

* Bump actions/setup-python from 4 to 5 (#148)

Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4 to 5.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@v4...v5)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: Test only for migrations within package

* Update tests

* run tests on all branches

* fix tests

* Update readme, changelog

* Fix common test suite for django CMS 3.11, 4.0, and 4.1

* Update CHANGELOG

* Merge branches together

* Update precommit-config

* fix: ruff precommit

* ci: auto fixes from pre-commit hooks

for more information, see https://pre-commit.ci

* fix: ruff issues

* More ruff fixes

* Update pyproject.toml

* ci: auto fixes from pre-commit hooks

for more information, see https://pre-commit.ci

* fix: Remove isort and flake8 from setup.cfg

* ci: auto fixes from pre-commit hooks

for more information, see https://pre-commit.ci

* Update setup.cfg

* Fix: universal branch (#167)

* fix: manifest.in

* fix: SnippetAdmin use overrdden change_form_template

* ci: auto fixes from pre-commit hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Update lint workflow

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Adam Murray <[email protected]>
Co-authored-by: Aiky30 <[email protected]>
Co-authored-by: Bernard Van Der Vyver <[email protected]>
Co-authored-by: Michael Collins <[email protected]>
Co-authored-by: 傅瑞曼 <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Josh Peng Yu <[email protected]>
Co-authored-by: Vipul Narang <[email protected]>
Co-authored-by: Mark Walker <[email protected]>
Co-authored-by: Nicolai <[email protected]>
Co-authored-by: Kaushal Dhungel <[email protected]>
Co-authored-by: Simon Krull <[email protected]>
Co-authored-by: Liumeo <[email protected]>
Co-authored-by: Mark Walker <[email protected]>
Co-authored-by: Patrik <[email protected]>
Co-authored-by: Mark Walker <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Josh Yu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants