diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index c30908d0..9f05d488 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -27,16 +27,16 @@ jobs: uses: actions/checkout@v4 - name: Initialize CodeQL - uses: github/codeql-action/init@v2 + uses: github/codeql-action/init@v3 with: languages: ${{ matrix.language }} queries: +security-and-quality - name: Autobuild - uses: github/codeql-action/autobuild@v2 + uses: github/codeql-action/autobuild@v3 if: ${{ matrix.language == 'javascript' || matrix.language == 'python' }} - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v2 + uses: github/codeql-action/analyze@v3 with: category: "/language:${{ matrix.language }}" diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index c393f081..2fceee19 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -19,7 +19,7 @@ jobs: python-version: '3.11' cache: 'pip' - name: Cache dependencies - uses: actions/cache@v4.0.1 + uses: actions/cache@v4.0.2 with: path: ~/.cache/pip key: ${{ runner.os }}-pip-${{ hashFiles('docs/requirements.txt') }} @@ -44,7 +44,7 @@ jobs: python-version: '3.11' cache: 'pip' - name: Cache dependencies - uses: actions/cache@v4.0.1 + uses: actions/cache@v4.0.2 with: path: ~/.cache/pip key: ${{ runner.os }}-pip-${{ hashFiles('docs/requirements.txt') }} diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f7ab8665..f21c5790 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,26 @@ Changelog ========= +2.0.1 (2024-03-29) +================== + +* feat: Add content object level publish permissions by @fsbraun in https://github.com/django-cms/djangocms-versioning/pull/390 +* fix: Create missing __init__.py in management folder by @fsbraun in https://github.com/django-cms/djangocms-versioning/pull/366 +* fix #363: Better UX in versioning listview by @jrief in https://github.com/django-cms/djangocms-versioning/pull/364 +* fix: Several fixes for the versioning forms: #382, #383, #384 by @fsbraun in https://github.com/django-cms/djangocms-versioning/pull/386 +* fix: For Django CMS 4.1.1 and later do not automatically register versioned CMS Menu by @fsbraun in https://github.com/django-cms/djangocms-versioning/pull/388 +* fix: Post requests from the side frame were sent to wrong URL by @fsbraun in https://github.com/django-cms/djangocms-versioning/pull/396 +* fix: Consistent use of action buttons by @fsbraun in https://github.com/django-cms/djangocms-versioning/pull/392 +* fix: Avoid duplication of placeholder checks for locked versions by @fsbraun in https://github.com/django-cms/djangocms-versioning/pull/393 +* ci: Add testing against django main by @marksweb in https://github.com/django-cms/djangocms-versioning/pull/353 +* ci: Improve efficiency of ruff workflow by @marksweb in https://github.com/django-cms/djangocms-versioning/pull/378 +* Chore: update ruff and pre-commit hook by @raffaellasuardini in https://github.com/django-cms/djangocms-versioning/pull/381 +* build(deps): bump actions/cache from 4.0.1 to 4.0.2 by @dependabot in https://github.com/django-cms/djangocms-versioning/pull/397 + +New Contributors + +* @raffaellasuardini made their first contribution in https://github.com/django-cms/djangocms-versioning/pull/381 +* @jrief made their first contribution in https://github.com/django-cms/djangocms-versioning/pull/364 2.0.0 (2023-12-29) ================== diff --git a/djangocms_versioning/__init__.py b/djangocms_versioning/__init__.py index 8c0d5d5b..159d48b8 100644 --- a/djangocms_versioning/__init__.py +++ b/djangocms_versioning/__init__.py @@ -1 +1 @@ -__version__ = "2.0.0" +__version__ = "2.0.1" diff --git a/djangocms_versioning/admin.py b/djangocms_versioning/admin.py index 6b2a7e71..1d0ce04d 100644 --- a/djangocms_versioning/admin.py +++ b/djangocms_versioning/admin.py @@ -608,6 +608,9 @@ class VersionAdmin(ChangeListActionsMixin, admin.ModelAdmin, metaclass=MediaDefi # def get_queryset(self, request): # return super().get_queryset(request).prefetch_related('content') + class Media: + js = ["djangocms_versioning/js/versioning.js"] + def get_changelist(self, request, **kwargs): return VersionChangeList @@ -682,13 +685,13 @@ def _get_archive_link(self, obj, request, disabled=False): icon="archive", title=_("Archive"), name="archive", - disabled=not obj.can_be_archived(), + disabled=not obj.check_archive.as_bool(request.user), ) def _get_publish_link(self, obj, request): """Helper function to get the html link to the publish action """ - if not obj.check_publish.as_bool(request.user): + if not obj.can_be_published(): # Don't display the link if it can't be published return "" publish_url = reverse( @@ -701,14 +704,14 @@ def _get_publish_link(self, obj, request): title=_("Publish"), name="publish", action="post", - disabled=not obj.can_be_published(), + disabled=not obj.check_publish.as_bool(request.user), keepsideframe=False, ) def _get_unpublish_link(self, obj, request, disabled=False): """Helper function to get the html link to the unpublish action """ - if not obj.check_unpublish.as_bool(request.user): + if not obj.can_be_unpublished(): # Don't display the link if it can't be unpublished return "" unpublish_url = reverse( @@ -720,15 +723,12 @@ def _get_unpublish_link(self, obj, request, disabled=False): icon="unpublish", title=_("Unpublish"), name="unpublish", - disabled=not obj.can_be_unpublished(), + disabled=not obj.check_unpublish.as_bool(request.user), ) def _get_edit_link(self, obj, request, disabled=False): """Helper function to get the html link to the edit action """ - if not obj.check_edit_redirect.as_bool(request.user): - return "" - # Only show if no draft exists if obj.state == PUBLISHED: pks_for_grouper = obj.versionable.for_content_grouping_values( @@ -758,14 +758,14 @@ def _get_edit_link(self, obj, request, disabled=False): title=_("Edit") if icon == "pencil" else _("New Draft"), name="edit", action="post", - disabled=disabled, + disabled=not obj.check_edit_redirect.as_bool(request.user) or disabled, keepsideframe=keepsideframe, ) def _get_revert_link(self, obj, request, disabled=False): """Helper function to get the html link to the revert action """ - if not obj.check_revert.as_bool(request.user): + if obj.state in (PUBLISHED, DRAFT): # Don't display the link if it's a draft or published return "" @@ -778,13 +778,13 @@ def _get_revert_link(self, obj, request, disabled=False): icon="undo", title=_("Revert"), name="revert", - disabled=disabled, + disabled=not obj.check_revert.as_bool(request.user) or disabled, ) def _get_discard_link(self, obj, request, disabled=False): """Helper function to get the html link to the discard action """ - if not obj.check_discard.as_bool(request.user): + if obj.state != DRAFT: # Don't display the link if it's not a draft return "" @@ -797,7 +797,7 @@ def _get_discard_link(self, obj, request, disabled=False): icon="bin", title=_("Discard"), name="discard", - disabled=disabled, + disabled=not obj.check_discard.as_bool(request.user) or disabled, ) def _get_unlock_link(self, obj, request): @@ -808,12 +808,6 @@ def _get_unlock_link(self, obj, request): if not conf.LOCK_VERSIONS or obj.state != DRAFT or not version_is_locked(obj): return "" - disabled = True - # Check whether the lock can be removed - # Check that the user has unlock permission - if request.user.has_perm("djangocms_versioning.delete_versionlock"): - disabled = False - unlock_url = reverse(f"admin:{obj._meta.app_label}_{self.model._meta.model_name}_unlock", args=(obj.pk,)) return self.admin_action_button( unlock_url, @@ -821,7 +815,7 @@ def _get_unlock_link(self, obj, request): title=_("Unlock"), name="unlock", action="post", - disabled=disabled, + disabled=not obj.check_unlock.as_bool(request.user), ) def get_actions_list(self): @@ -978,7 +972,6 @@ def publish_view(self, request, object_id): # Redirect to published? if conf.ON_PUBLISH_REDIRECT == "published": - redirect_url = None if hasattr(version.content, "get_absolute_url"): redirect_url = version.content.get_absolute_url() or redirect_url @@ -1323,10 +1316,7 @@ def changelist_view(self, request, extra_context=None): # Check if custom breadcrumb template defined, otherwise # fallback on default breadcrumb_templates = [ - "admin/djangocms_versioning/{app_label}/{model_name}/versioning_breadcrumbs.html".format( - app_label=breadcrumb_opts.app_label, - model_name=breadcrumb_opts.model_name, - ), + f"admin/djangocms_versioning/{breadcrumb_opts.app_label}/{breadcrumb_opts.model_name}/versioning_breadcrumbs.html", "admin/djangocms_versioning/versioning_breadcrumbs.html", ] extra_context["breadcrumb_template"] = select_template(breadcrumb_templates) diff --git a/djangocms_versioning/apps.py b/djangocms_versioning/apps.py index c1ebdcfd..a5019d4c 100644 --- a/djangocms_versioning/apps.py +++ b/djangocms_versioning/apps.py @@ -12,15 +12,18 @@ def ready(self): from cms.models import contentmodels, fields from cms.signals import post_obj_operation, post_placeholder_operation + from .conf import LOCK_VERSIONS from .handlers import ( update_modified_date, update_modified_date_for_pagecontent, update_modified_date_for_placeholder_source, ) - from .helpers import is_content_editable + from .helpers import is_content_editable, placeholder_content_is_unlocked_for_user # Add check to PlaceholderRelationField fields.PlaceholderRelationField.default_checks += [is_content_editable] + if LOCK_VERSIONS: + fields.PlaceholderRelationField.default_checks += [placeholder_content_is_unlocked_for_user] # Remove uniqueness constraint from PageContent model to allow for different versions pagecontent_unique_together = tuple( diff --git a/djangocms_versioning/cms_config.py b/djangocms_versioning/cms_config.py index 92c66fe4..e2894084 100644 --- a/djangocms_versioning/cms_config.py +++ b/djangocms_versioning/cms_config.py @@ -25,7 +25,6 @@ from . import indicators, versionables from .admin import VersioningAdminMixin -from .conf import LOCK_VERSIONS from .constants import INDICATOR_DESCRIPTIONS from .datastructures import BaseVersionableItem, VersionableItem from .exceptions import ConditionFailed @@ -33,7 +32,6 @@ get_latest_admin_viewable_content, inject_generic_relation_to_version, is_editable, - placeholder_content_is_unlocked_for_user, register_versionadmin_proxy, replace_admin_for_models, replace_manager, @@ -160,12 +158,6 @@ def handle_admin_field_modifiers(self, cms_config): for key in modifier.keys(): self.add_to_field_extension[key] = modifier[key] - def handle_locking(self): - if LOCK_VERSIONS: - from cms.models import fields - - fields.PlaceholderRelationField.default_checks += [placeholder_content_is_unlocked_for_user] - def configure_app(self, cms_config): if hasattr(cms_config, "extended_admin_field_modifiers"): self.handle_admin_field_modifiers(cms_config) @@ -188,7 +180,6 @@ def configure_app(self, cms_config): self.handle_version_admin(cms_config) self.handle_content_model_generic_relation(cms_config) self.handle_content_model_manager(cms_config) - self.handle_locking() def copy_page_content(original_content): diff --git a/djangocms_versioning/cms_toolbars.py b/djangocms_versioning/cms_toolbars.py index 94337cff..80173f9a 100644 --- a/djangocms_versioning/cms_toolbars.py +++ b/djangocms_versioning/cms_toolbars.py @@ -127,8 +127,8 @@ def _add_unlock_button(self): if LOCK_VERSIONS and self._is_versioned(): item = ButtonList(side=self.toolbar.RIGHT) proxy_model = self._get_proxy_model() - version = Version.objects.get_for_content(self.toolbar.obj) - if version.check_unlock.as_bool(self.request.user): + version = Version.objects.filter_by_content_grouping_values(self.toolbar.obj).filter(state=DRAFT).first() + if version and version.check_unlock.as_bool(self.request.user): unlock_url = reverse( f"admin:{proxy_model._meta.app_label}_{proxy_model.__name__.lower()}_unlock", args=(version.pk,), diff --git a/djangocms_versioning/conditions.py b/djangocms_versioning/conditions.py index c73a8c14..fd76a007 100644 --- a/djangocms_versioning/conditions.py +++ b/djangocms_versioning/conditions.py @@ -76,3 +76,22 @@ def inner(version, user): else: raise ConditionFailed(message) return inner + +def user_can_unlock(message: str) -> callable: + def inner(version, user): + if not user.has_perm("djangocms_versioning.delete_versionlock"): + raise ConditionFailed(message) + return inner + +def user_can_publish(message: str) -> callable: + def inner(version, user): + if not version.has_publish_permission(user): + raise ConditionFailed(message) + return inner + + +def user_can_change(message: str) -> callable: + def inner(version, user): + if not version.has_change_permission(user): + raise ConditionFailed(message) + return inner diff --git a/djangocms_versioning/conf.py b/djangocms_versioning/conf.py index 40030005..1188780e 100644 --- a/djangocms_versioning/conf.py +++ b/djangocms_versioning/conf.py @@ -1,7 +1,8 @@ +from cms import __version__ as CMS_VERSION from django.conf import settings ENABLE_MENU_REGISTRATION = getattr( - settings, "DJANGOCMS_VERSIONING_ENABLE_MENU_REGISTRATION", True + settings, "DJANGOCMS_VERSIONING_ENABLE_MENU_REGISTRATION", CMS_VERSION <= "4.1.0" ) USERNAME_FIELD = getattr( @@ -31,4 +32,4 @@ ON_PUBLISH_REDIRECT = getattr( settings, "DJANGOCMS_VERISONING_ON_PUBLISH_REDIRECT", "published" ) -# Allowed values: "versions", "published", "preview" +#: Allowed values: "versions", "published", "preview" diff --git a/djangocms_versioning/models.py b/djangocms_versioning/models.py index 0a5ef6a2..8bfa1009 100644 --- a/djangocms_versioning/models.py +++ b/djangocms_versioning/models.py @@ -16,6 +16,9 @@ draft_is_not_locked, in_state, is_not_locked, + user_can_change, + user_can_publish, + user_can_unlock, ) from .conf import ALLOW_DELETING_VERSIONS, LOCK_VERSIONS from .operations import send_post_version_operation, send_pre_version_operation @@ -29,7 +32,7 @@ not_draft_error = _("Version is not a draft") lock_error_message = _("Action Denied. The latest version is locked by {user}") lock_draft_error_message = _("Action Denied. The draft version is locked by {user}") - +permission_error_message = _("You do not have permission to perform this action") def allow_deleting_versions(collector, field, sub_objs, using): if ALLOW_DELETING_VERSIONS: @@ -278,7 +281,7 @@ def copy(self, created_by): Allows customization of how the content object will be copied when specified in cms_config.py - This method needs to be ran in a transaction due to the fact that if + This method needs to be run in a transaction due to the fact that if models are partially created in the copy method a version is not attached. It needs to be that if anything goes wrong we should roll back the entire task. We shouldn't leave this to package developers to know to add this feature @@ -296,6 +299,7 @@ def copy(self, created_by): check_archive = Conditions( [ + user_can_change(permission_error_message), in_state([constants.DRAFT], _("Version is not in draft state")), is_not_locked(lock_error_message), ] @@ -345,7 +349,10 @@ def _set_archive(self, user): pass check_publish = Conditions( - [in_state([constants.DRAFT], _("Version is not in draft state"))] + [ + user_can_publish(permission_error_message), + in_state([constants.DRAFT], _("Version is not in draft state")), + ] ) def can_be_published(self): @@ -418,6 +425,7 @@ def is_visible(self): ) check_unpublish = Conditions([ + user_can_publish(permission_error_message), in_state([constants.PUBLISHED], _("Version is not in published state")), draft_is_not_locked(lock_draft_error_message), ]) @@ -468,14 +476,60 @@ def _set_unpublish(self, user): possible to be left with inconsistent data)""" pass + def has_publish_permission(self, user) -> bool: + """ + Check if the given user has permission to publish. + + Args: + user (User): The user to check for permission. + + Returns: + bool: True if the user has publish permission, False otherwise. + """ + return self._has_permission("publish", user) + + def has_change_permission(self, user) -> bool: + """ + Check whether the given user has permission to change the object. + + Parameters: + user (User): The user for which permission needs to be checked. + + Returns: + bool: True if the user has permission to change the object, False otherwise. + """ + return self._has_permission("change", user) + + def _has_permission(self, perm: str, user) -> bool: + """ + Check if the user has the specified permission for the content by + checking the content's has_publish_permission, has_placeholder_change_permission, + or has_change_permission methods. + + Falls back to Djangos change permission for the content object. + """ + if perm == "publish" and hasattr(self.content, "has_publish_permission"): + # First try explicit publish permission + return self.content.has_publish_permission(user) + if hasattr(self.content, "has_change_permission"): + # First fallback: change permissions + return self.content.has_change_permission(user) + if hasattr(self.content, "has_placeholder_change_permission"): + # Second fallback: placeholder change permissions - works for PageContent + return self.content.has_placeholder_change_permission(user) + # final fallback: Django perms + return user.has_perm(f"{self.content_type.app_label}.change_{self.content_type.model}") + check_modify = Conditions( [ in_state([constants.DRAFT], not_draft_error), draft_is_not_locked(lock_draft_error_message), + user_can_unlock(permission_error_message), ] ) check_revert = Conditions( [ + user_can_change(permission_error_message), in_state( [constants.ARCHIVED, constants.UNPUBLISHED], _("Version is not in archived or unpublished state"), @@ -508,6 +562,7 @@ def _set_unpublish(self, user): [ in_state([constants.DRAFT, constants.PUBLISHED], not_draft_error), draft_is_locked(_("Draft version is not locked")) + ] ) diff --git a/djangocms_versioning/static/djangocms_versioning/css/versioning.css b/djangocms_versioning/static/djangocms_versioning/css/versioning.css index f66b94a0..bc19b68b 100644 --- a/djangocms_versioning/static/djangocms_versioning/css/versioning.css +++ b/djangocms_versioning/static/djangocms_versioning/css/versioning.css @@ -207,6 +207,3 @@ ins.cms-diff img { .cms-select::-ms-expand { opacity: 0; } -input.button.revert-button { - margin: 5px; -} diff --git a/djangocms_versioning/static/djangocms_versioning/js/admin/versioning-actions.js b/djangocms_versioning/static/djangocms_versioning/js/admin/versioning-actions.js new file mode 100644 index 00000000..632032fa --- /dev/null +++ b/djangocms_versioning/static/djangocms_versioning/js/admin/versioning-actions.js @@ -0,0 +1,22 @@ +(function () { + "use strict"; + + function closeSideFrame() { + try { + window.top.CMS.API.Sideframe.close(); + } catch (err) {} + } + + document.addEventListener('DOMContentLoaded', function () { + document.querySelectorAll('form.js-close-sideframe').forEach(el => { + el.addEventListener("submit", (ev) => { + ev.preventDefault(); + ev.target.action = ev.target.action; // save action url + closeSideFrame(); + const form = window.top.document.body.appendChild(ev.target); // move to top window + form.style.display = 'none'; + form.submit(); // submit form + }); + }); + }); +})(); diff --git a/djangocms_versioning/static/djangocms_versioning/js/versioning.js b/djangocms_versioning/static/djangocms_versioning/js/versioning.js new file mode 100644 index 00000000..0704f537 --- /dev/null +++ b/djangocms_versioning/static/djangocms_versioning/js/versioning.js @@ -0,0 +1,37 @@ +(function() { + var firstChecked, lastChecked; + + function handleVersionSelection(event) { + if (firstChecked instanceof HTMLInputElement && firstChecked.checked) { + firstChecked.checked = false; + firstChecked.closest('tr').classList.remove('selected'); + firstChecked = lastChecked; + } + if (event.target instanceof HTMLInputElement) { + if (event.target.checked) { + firstChecked = lastChecked; + lastChecked = event.target; + } else if (firstChecked === event.target) { + firstChecked = null; + } else { + lastChecked = null; + } + } + } + + document.addEventListener('DOMContentLoaded', function(){ + var selectedVersions = document.querySelectorAll('#result_list input[type="checkbox"].action-select'); + var selectElement = document.querySelector('#changelist-form select[name="action"]'); + if (selectElement instanceof HTMLSelectElement) { + for (var i = 0; i < selectElement.options.length; i++) { + if (selectElement.options[i].value && selectElement.options[i].value !== 'compare_versions') { + // for future safety: do not restrict on two selected versions, since there might be other actions + return; + } + } + } + selectedVersions.forEach(function(selectedVersion){ + selectedVersion.addEventListener('change', handleVersionSelection); + }); + }); + })(); diff --git a/djangocms_versioning/templates/djangocms_versioning/admin/archive_confirmation.html b/djangocms_versioning/templates/djangocms_versioning/admin/archive_confirmation.html index 40ab5cd3..4a70749c 100644 --- a/djangocms_versioning/templates/djangocms_versioning/admin/archive_confirmation.html +++ b/djangocms_versioning/templates/djangocms_versioning/admin/archive_confirmation.html @@ -6,6 +6,7 @@ {{ block.super }} {{ media }} + {% endblock %} {% block breadcrumbs %}{% endblock %} @@ -15,15 +16,17 @@
{% translate "Are you sure you want to archive the following version?" %}