Additional Versioning Compatibility Changes #79

adam-murray · 2021-09-16T10:27:18Z

This PR includes additional changes required to implement versioning and moderation, as well as making it fully compatible without.

…gement command for cms3-cms4

… enabled

…igration

…ned compatibility.

Co-authored-by: Aiky30 <[email protected]>

codecov-commenter · 2021-09-17T16:28:56Z

Codecov Report

Merging #79 (c5dcb04) into support/django-cms-4.0.x (4fd5662) will decrease coverage by 1.96%.
The diff coverage is 84.09%.

@@                     Coverage Diff                      @@
##           support/django-cms-4.0.x      #79      +/-   ##
============================================================
- Coverage                     92.18%   90.21%   -1.97%     
============================================================
  Files                            21       24       +3     
  Lines                           243      327      +84     
  Branches                         13       21       +8     
============================================================
+ Hits                            224      295      +71     
- Misses                           14       25      +11     
- Partials                          5        7       +2

Impacted Files	Coverage Δ
djangocms_snippet/rendering.py	`40.00% <40.00%> (ø)`
djangocms_snippet/models.py	`94.28% <75.00%> (-5.72%)`	⬇️
djangocms_snippet/forms.py	`87.17% <87.17%> (ø)`
djangocms_snippet/views.py	`87.50% <87.50%> (ø)`
djangocms_snippet/admin.py	`93.10% <88.23%> (-6.90%)`	⬇️
djangocms_snippet/cms_config.py	`84.21% <100.00%> (+9.21%)`	⬆️

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 4fd5662...c5dcb04. Read the comment docs.

Aiky30

I haven't had chance to look at the tests yet due to the support issues that are still open.

Aiky30 · 2021-10-11T15:21:08Z

djangocms_snippet/admin.py

+)
+
+snippet_admin_classes = [
+    AbstractSnippetAdmin,


I'm confused as to why we have an abstract class that is not reused, could this not be:

Suggested change

AbstractSnippetAdmin,

admin.ModelAdmin,

Aiky30 · 2021-10-11T15:22:14Z

djangocms_snippet/admin.py

+        abstract = True
+
+
+djangocms_versioning_enabled = getattr(


This is normally done by the cms_config, you can then just call the config for the package and see if it's enabled rather than repeat the settings implementation: https://github.com/divio/djangocms-alias/blob/master/djangocms_alias/cms_config.py#L22

Aiky30 · 2021-10-11T15:26:43Z

djangocms_snippet/forms.py

+except ImportError:
+    is_versioning_installed = False
+
+djangocms_versioning_enabled = getattr(


Same as above, this is set in the cms config. If we just had a setting it would live in conf.py but as this is in the cms_config we should use that as the centralised control.

Aiky30 · 2021-10-11T15:29:15Z

djangocms_snippet/forms.py

+        name = data.get("name")
+        slug = data.get("slug")
+        snippet_grouper = data.get("snippet_grouper")
+        published_snippet_queryset = Snippet.objects.all()


The phrase published_snippet_queryset is misleading because this is not a published queryset if versioning is not enabled.

djangocms_snippet/forms.py

Aiky30 · 2021-10-11T15:36:03Z

djangocms_snippet/urls.py

+from djangocms_snippet.views import SnippetPreviewView
+
+
+urlpatterns = [


What is this used for?

The url already exists to preview: https://github.com/django-cms/djangocms-snippet/pull/79/files#diff-aabaa98ef870e0884628a1bb3ddce0de62a156e640798d77f6cbbe6cd6c7cbacR79

djangocms_snippet/views.py

Co-authored-by: Aiky30 <[email protected]>

…am-murray/djangocms-snippet into feature/versionining-extra-code-tasks

Aiky30

Overall a really good effort and good test coverage.

A few minor points:

Some reworking of the logic for visual
I don't think that the settings overrides are actually working. I tend to have to patch to get them to work. Changing the default settings will confirm this.
An additional integration test

djangocms_snippet/admin.py

tests/test_admin.py

tests/test_forms.py

tests/test_views.py

Co-authored-by: Aiky30 <[email protected]>

djangocms_snippet/models.py

Aiky30 · 2021-10-14T12:11:09Z

tests/test_forms.py

+        """
+        Snippet forms should be valid regardless of the versions, or states which already exist within its grouper.
+        """
+        snippet_to_archive = SnippetWithVersionFactory()


I'm confused by this logic. snippet_to_publish is draft with the same grouper and there is then another draft created on line 23.

The same grouper has 1 published version and 2 drafts which doesn't make sense.

It's easier to follow if you force the version state:
archived = SnippetWithVersionFactory(version__state=ARCHIVED)
published = SnippetWithVersionFactory(version__state=PUBLISHED, snippet_grouper=archived.snippet_grouper)
draft = SnippetWithVersionFactory(version__state=DRAFT, snippet_grouper=archived.snippet_grouper)

The test should also prove when using the slug is invalid too, it would go on to add:

# Another grouper tries to use the same slug value new_grouper_published = SnippetWithVersionFactory(version__state=PUBLISHED) form_data = { "name": new_grouper_published.name, "slug": draft.slug, "html": new_grouper_published.html, "snippet_grouper": new_grouper_published.snippet_grouper.id, }

I don't think that's actually the case, I tested in the debugger. The created Snippets have the correct states.

# snippet_to_archive starts draft snippet_to_archive = SnippetWithVersionFactory() # snippet_to_archive then gets published snippet_to_archive.versions.first().publish(user=self.get_superuser()) # snippet_to_publish is a new draft snippet_to_publish = SnippetWithVersionFactory(snippet_grouper=snippet_to_archive.snippet_grouper) # We now create another new draft int he same grouper SnippetWithVersionFactory(snippet_grouper=snippet_to_publish.snippet_grouper)

Also in none of the new versions are the slugs the same

I also missed that before, the slug should be the same in each version.

Co-authored-by: Aiky30 <[email protected]>

Aiky30 · 2021-10-14T13:08:24Z

tests/test_forms.py

+
+        form = SnippetForm(form_data)
+
+        self.assertTrue(form.is_valid())


The test should also prove when using the slug is invalid too, it would go on to add:

# Another grouper tries to use the same slug value new_grouper_published = SnippetWithVersionFactory(version__state=PUBLISHED) form_data = { "name": new_grouper_published.name, "slug": draft.slug, "html": new_grouper_published.html, "snippet_grouper": new_grouper_published.snippet_grouper.id, }

* 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]>

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

Added cms_config versionable item and grouper

6368d7e

Added admin, requirements for testing and requirements for running

75449d6

Update

9bc367c

Fixed issue in setup.py and added basic first migration

7c9455c

Added migration to cms4

4954ee4

Added migration to cms4

8defe7f

Began to implement versioning switch, added migration as part of mana…

b9f822f

…gement command for cms3-cms4

Moved data migration to management command, added util for versioning…

49e4bb3

… enabled

Added tests for config as well as util for versioning check

dceee9f

Merge branch 'feature/versioned-snippets' into feature/snippet-data-m…

7dfd721

…igration

Updated cms_config

56adbdb

Updated management command

48e125f

Added form, updated model and forms to improve versioned and unversio…

5e12a63

…ned compatibility.

Updated migration, removed unecessary plugin

fdcde12

Merged

13547ae

Removed additional copies of files

9b73d59

Split migration command into migration and command

680ce70

Updates to migration

8dc7f91

Updates to migration

6e937e8

Update migration for review

2df658b

Remove pdb

11a7d3e

Migration upgrade

d7c5dbb

Remove plugin from migration 10, added version creation

8898232

Implemented proper plugin migration

225b9f9

Removed management command

500cf72

Migration 10 now handles lack of versioning gracefully.

2017d90

Add snippet property for backwards compatibility

25e552e

Co-authored-by: Aiky30 <[email protected]>

Remove old foreign key from plugin, and rename new one

59f82a8

Cleaned up migration and models

25048ff

Removed unecessary logging

649c4af

Merged with updated moderation integration

8e4db70

adam-murray added 8 commits September 17, 2021 17:32

Linting

65a3175

iSort

c61190c

Added test criteria and comments for admin

a500c9a

Fixed form save method issue found in manual testing

b8f7367

Push or placeholder problems

4ac015d

Preview view and testcase

e251685

Linting

426510f

Linting and additional test case

47dfedf

Aiky30 requested changes Oct 11, 2021

View reviewed changes

adam-murray and others added 4 commits October 12, 2021 10:39

Update djangocms_snippet/views.py

a6919e8

Co-authored-by: Aiky30 <[email protected]>

PR updates

7f65404

Merge branch 'feature/versionining-extra-code-tasks' of github.com:ad…

ade6c52

…am-murray/djangocms-snippet into feature/versionining-extra-code-tasks

Linting

7dea453

Aiky30 requested changes Oct 12, 2021

View reviewed changes

adam-murray and others added 4 commits October 13, 2021 08:28

Clean up admin class extension

7570829

Co-authored-by: Aiky30 <[email protected]>

PR updates

6cb31ef

Linitng

9e5433f

Added comment to explain why cms_toolbar_models are required

68f537f

Aiky30 reviewed Oct 14, 2021

View reviewed changes

djangocms_snippet/models.py Outdated Show resolved Hide resolved

Aiky30 reviewed Oct 14, 2021

View reviewed changes

adam-murray and others added 4 commits October 14, 2021 13:41

Update djangocms_snippet/models.py

c5dcb04

Co-authored-by: Aiky30 <[email protected]>

Added commments to make end-to-end test clearer

d7e693e

Merged

b9fe248

Comment consistency

e9e3394

Aiky30 reviewed Oct 14, 2021

View reviewed changes

Aiky30 approved these changes Oct 14, 2021

View reviewed changes

Aiky30 merged commit e215e01 into django-cms:support/django-cms-4.0.x Oct 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Additional Versioning Compatibility Changes #79

Additional Versioning Compatibility Changes #79

adam-murray commented Sep 16, 2021

codecov-commenter commented Sep 17, 2021 •

edited

Loading

Aiky30 left a comment

Aiky30 Oct 11, 2021

Aiky30 Oct 11, 2021

Aiky30 Oct 11, 2021

Aiky30 Oct 11, 2021

Aiky30 Oct 11, 2021

Aiky30 left a comment

Aiky30 Oct 14, 2021 •

edited

Loading

adam-murray Oct 14, 2021

Aiky30 Oct 14, 2021 •

edited

Loading

Aiky30 Oct 14, 2021 •

edited

Loading

Aiky30 Oct 14, 2021

Aiky30 Oct 14, 2021

		from djangocms_snippet.views import SnippetPreviewView


		urlpatterns = [


		form = SnippetForm(form_data)

		self.assertTrue(form.is_valid())

Additional Versioning Compatibility Changes #79

Additional Versioning Compatibility Changes #79

Conversation

adam-murray commented Sep 16, 2021

codecov-commenter commented Sep 17, 2021 • edited Loading

Codecov Report

Aiky30 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aiky30 left a comment

Choose a reason for hiding this comment

Aiky30 Oct 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aiky30 Oct 14, 2021 • edited Loading

Choose a reason for hiding this comment

Aiky30 Oct 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Sep 17, 2021 •

edited

Loading

Aiky30 Oct 14, 2021 •

edited

Loading

Aiky30 Oct 14, 2021 •

edited

Loading

Aiky30 Oct 14, 2021 •

edited

Loading