feat: Preview icon renders content in read only #102

Bernardvdv · 2022-01-10T14:29:09Z

Description

When clicking on the preview icon against a snippet item, it should open the read only preview

Checklist

I have opened this pull request against master
I have added or modified the tests when changing logic
I have followed the conventional commits guidelines to add meaningful information into the changelog
I have read the contribution guidelines and I have joined #workgroup-pr-review on
Slack to find a “pr review buddy” who is going to review my pull request.

codecov-commenter · 2022-01-10T14:30:09Z

Codecov Report

Merging #102 (8decb3a) into support/django-cms-4.0.x (60746af) will decrease coverage by 0.79%.
The diff coverage is 78.57%.

@@                     Coverage Diff                      @@
##           support/django-cms-4.0.x     #102      +/-   ##
============================================================
- Coverage                     91.42%   90.62%   -0.80%     
============================================================
  Files                            24       23       -1     
  Lines                           373      384      +11     
  Branches                         58       59       +1     
============================================================
+ Hits                            341      348       +7     
- Misses                           24       26       +2     
- Partials                          8       10       +2

Impacted Files	Coverage Δ
djangocms_snippet/admin.py	`90.58% <78.57%> (-5.97%)`	⬇️

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 60746af...8decb3a. Read the comment docs.

Aiky30 · 2022-01-10T17:51:30Z

djangocms_snippet/admin.py

+        )
+        return change_url
+
+    def get_list_actions(self):


This is done automatically by the mixin, you shouldn't need to define this I don't think.

Was there a reason to adding this?

@Aiky30 Yes you are correct. Thought it was required but tested without it and it's not necessary.

Aiky30 · 2022-01-10T17:52:01Z

djangocms_snippet/admin.py

+        Return the change url which will be rendered in read only mode
+        :return: method or None
+        """
+        change_url = reverse(


Does this go into readonly mode always? Or does it load the edit view when you own the draft?

@Aiky30 The snippet view remains in read only so having another look at this

…gged in

…der_read_only_content

…_only_content' into feature_preview_icon_render_read_only_content

Aiky30

I think we need to double check the original use of locking to a user, does this actually happen with the other versioned packages i.e. navigation, cms pagecontent etc.

Also the test for locked versions has been broken.

Aiky30 · 2022-01-14T15:51:54Z

tests/test_admin.py

@@ -173,7 +173,7 @@ def test_admin_form_edit_when_locked(self):
        self.snippet_version.publish(user=self.superuser)
        with self.login_user_context(self.superuser):
            edit_url = reverse("admin:djangocms_snippet_snippet_change", args=(self.snippet.id,),)
-            response = self.client.get(edit_url)
+            response = self.client.get(edit_url + "?q=read_only")


We would need a separate test for this, by changing this test we lose the check on version locking and repurpose it for a new requirement for the preview read only functionality.

Aiky30 · 2022-01-14T15:59:01Z

djangocms_snippet/admin.py

        """
        if obj and djangocms_versioning_enabled:
-            created_by_id = self.get_version(obj).created_by_id


We need to keep this logic because it is for version locking.

We may end up in the situation where the parameter has not been supplied but the version is not locked to the current user.

May need to verify this logic and double check it to as without version locking I think that anyone can change it.

Aiky30

Some cleanup and needs at least one test.

Aiky30 · 2022-02-03T10:40:46Z

djangocms_snippet/admin.py

    if djangocms_versioning_enabled:
        snippet_admin_classes.insert(0, ExtendedVersionAdminMixin)
 except ImportError:
    djangocms_versioning_enabled = False

+TO_FIELD_VAR = '_to_field'


These are defined by django, we should reuse that definition:

from django.contrib.admin.options import TO_FIELD_VAR

Aiky30 · 2022-02-03T10:44:24Z

djangocms_snippet/admin.py

-                name="{}_{}_preview".format(*info),
-            ),
-        ] + super().get_urls()
+                   url(


This re-formatting is strange, how it was before was correct.

Aiky30 · 2022-02-03T10:46:46Z

tests/test_views.py

-from .utils.factories import SnippetWithVersionFactory
-
-
-class PreviewViewTestCase(CMSTestCase):


We should still have a test that the form renders and the fields are readonly or else we now have a whole new set of functionality without any tests. This is especially important as we are re-implementing django components, if django 4 changes anything on the changeform / changeform templates we need the test suite to tell us.

I think this can be achieved in one test that renders the form and checks the fields are readonly.

djangocms_snippet/admin.py

Co-authored-by: Aiky30 <Aiky30@users.noreply.github.com>

* 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 <joshyupeng@gmail.com> * 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 <theshow@gmail.com> * 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 <theshow@gmail.com> * 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 <fsbraun@gmx.de> * 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] <support@github.com> 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] <support@github.com> Co-authored-by: Adam Murray <adam-murray@hotmail.co.uk> Co-authored-by: Aiky30 <Aiky30@users.noreply.github.com> Co-authored-by: Bernard Van Der Vyver <Bvdv9075@gmail.com> Co-authored-by: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com> Co-authored-by: 傅瑞曼 <yxzfreeman@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Josh Peng Yu <joshyupeng@gmail.com> Co-authored-by: Vipul Narang <61502917+vipulnarang95@users.noreply.github.com> Co-authored-by: Mark Walker <theshow@gmail.com> Co-authored-by: Nicolai <69975577+NicolaiRidani@users.noreply.github.com> Co-authored-by: Kaushal Dhungel <57295839+Kaushal-Dhungel@users.noreply.github.com> Co-authored-by: Simon Krull <simondotunix@gmail.com> Co-authored-by: Liumeo <liumeo@pku.edu.cn> Co-authored-by: Mark Walker <mark.walker@realbuzz.com> Co-authored-by: Patrik <LuckyType@users.noreply.github.com> Co-authored-by: Mark Walker <mark@django-cms.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Josh Yu <joshyu@users.noreply.github.com>

Preview icon renders content in read only mode

339281e

Updated method comment

f5f0a55

Aiky30 reviewed Jan 10, 2022

View reviewed changes

Bernardvdv and others added 5 commits January 11, 2022 14:41

Updated permissions check to show snippet in edit mode if owner is lo…

59dcb78

…gged in

Merge branch 'support/django-cms-4.0.x' into feature_preview_icon_ren…

a0ff1fd

…der_read_only_content

Removed unused list actions method

833a0d1

Merge remote-tracking branch 'origin/feature_preview_icon_render_read…

01841fc

…_only_content' into feature_preview_icon_render_read_only_content

Added get paremeter to know when user selected preview

fa9dbe4

Aiky30 requested changes Jan 14, 2022

View reviewed changes

Bernardvdv added 4 commits January 31, 2022 13:30

Added custom preview endpoint to render form in a read only view

a9a3d19

isort and flake errors

1621568

isort spacing

837b19a

Added reference to django for the solution

dbc49a6

Aiky30 requested changes Feb 3, 2022

View reviewed changes

Bernardvdv added 3 commits February 3, 2022 14:23

Added a test to ensure preview endpoint fields are rendered read only

1377bb9

Removed unused imports

d12521e

Removed extra blank lines

8decb3a

Aiky30 reviewed Feb 3, 2022

View reviewed changes

djangocms_snippet/admin.py Outdated Show resolved Hide resolved

Update djangocms_snippet/admin.py

0c47c80

Co-authored-by: Aiky30 <Aiky30@users.noreply.github.com>

Aiky30 approved these changes Feb 3, 2022

View reviewed changes

Aiky30 merged commit 3c5af57 into django-cms:support/django-cms-4.0.x Feb 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Preview icon renders content in read only #102

feat: Preview icon renders content in read only #102

Bernardvdv commented Jan 10, 2022 •

edited

Loading

codecov-commenter commented Jan 10, 2022 •

edited

Loading

Aiky30 Jan 10, 2022

Bernardvdv Jan 11, 2022

Aiky30 Jan 10, 2022

Bernardvdv Jan 11, 2022

Aiky30 left a comment

Aiky30 Jan 14, 2022

Aiky30 Jan 14, 2022

Aiky30 left a comment

Aiky30 Feb 3, 2022

Aiky30 Feb 3, 2022

Aiky30 Feb 3, 2022

		from .utils.factories import SnippetWithVersionFactory


		class PreviewViewTestCase(CMSTestCase):

feat: Preview icon renders content in read only #102

feat: Preview icon renders content in read only #102

Conversation

Bernardvdv commented Jan 10, 2022 • edited Loading

Description

Checklist

codecov-commenter commented Jan 10, 2022 • edited Loading

Codecov Report

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bernardvdv commented Jan 10, 2022 •

edited

Loading

codecov-commenter commented Jan 10, 2022 •

edited

Loading