-
Notifications
You must be signed in to change notification settings - Fork 30
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: allow reuse of status indicators #319
Merged
Merged
Changes from all commits
Commits
Show all changes
51 commits
Select commit
Hold shift + click to select a range
aa97881
Allow page indicators for any versioned model
fsbraun 5e29905
MySQL compatibility
fsbraun 679e2fc
Fix queryset initialization
fsbraun de56ba1
fix isort
fsbraun 7f18122
Generalize get_latest_admin_viewable_page_content
fsbraun 1de2317
fix flake8
fsbraun c948e5f
Fix: grouper can be model or instance
fsbraun 0125c2a
fix: identify correct inverse relation
fsbraun bcf56b2
Remove IndicatorMixin
fsbraun 5570b58
Add tests
fsbraun 5ca7d85
no message
fsbraun 60fe7f3
More flexible list_display option
fsbraun 9939a1b
Improve back functionality of get views
fsbraun c0346ab
Fix isort
fsbraun 43c6fc4
Add one more test, fix doc inconsistency
fsbraun c14bc6d
fix coverage
fsbraun 9c70b64
Let get_list_display return tuple
fsbraun 61107fe
Fix isort
fsbraun 9d5a90b
Fix test bugs
fsbraun a49a141
Remove empty line
fsbraun 1d6740f
Fix: Add MediaDefiningClass meta classes
fsbraun 8daf0c9
Refactor for more consistent api
fsbraun f2b4c3e
Update tests
fsbraun 4f186db
Fix 2 missing renames
fsbraun cb9cf6d
Remove spourious import cycle
fsbraun 6515cb5
fix: isort
fsbraun 7070ada
Update djangocms_versioning/helpers.py
fsbraun abdfaef
Update djangocms_versioning/helpers.py
fsbraun 91c7a46
Update docs/versioning_integration.rst
fsbraun 84e91a6
Consistent labels for "discard changes"
fsbraun edfb62c
Add more tests
fsbraun 6e793e6
Update release notes
fsbraun 6fb42e7
Fix: Clarify docs (page tree as example)
fsbraun ffe1f4c
Update docs
fsbraun 921367f
Merge branch 'master' into feat/code-reusability
fsbraun 8d98775
Merge branch 'master' into feat/code-reusability
fsbraun d193a0a
Update djangocms_versioning/helpers.py
fsbraun f31d501
Update tests/test_admin.py
fsbraun 033836b
Update tests/test_indicators.py
fsbraun 72bd397
Merge branch 'master' into feat/code-reusability
fsbraun 8f52295
fix indentation
fsbraun 6405b45
Update tests/test_indicators.py
fsbraun e7bea5c
Update tests/test_indicators.py
fsbraun 62e2cb1
Update tests/test_indicators.py
fsbraun b2ce48c
Update tests/test_admin.py
fsbraun 9eabf14
Move indicator names to constants, add tests for versionables module
fsbraun c379d74
fix flake8
fsbraun 50cc9e8
fix isort
fsbraun 0c2baa7
simpler imports
fsbraun ca30948
Fix: `get_{field}_from_request` now needs to be present in model admin
fsbraun f5c5fcb
fix 2 typos
fsbraun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import json | ||
from collections import OrderedDict | ||
from urllib.parse import urlparse | ||
|
||
|
@@ -8,6 +9,7 @@ | |
from django.contrib.contenttypes.models import ContentType | ||
from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist | ||
from django.db.models.functions import Lower | ||
from django.forms import MediaDefiningClass | ||
from django.http import Http404, HttpResponseNotAllowed | ||
from django.shortcuts import redirect, render | ||
from django.template.loader import render_to_string, select_template | ||
|
@@ -24,16 +26,18 @@ | |
|
||
from . import versionables | ||
from .conf import USERNAME_FIELD | ||
from .constants import DRAFT, PUBLISHED | ||
from .constants import DRAFT, INDICATOR_DESCRIPTIONS, PUBLISHED | ||
from .exceptions import ConditionFailed | ||
from .forms import grouper_form_factory | ||
from .helpers import ( | ||
get_admin_url, | ||
get_editable_url, | ||
get_latest_admin_viewable_content, | ||
get_preview_url, | ||
proxy_model, | ||
version_list_url, | ||
) | ||
from .indicators import content_indicator, content_indicator_menu | ||
from .models import Version | ||
from .versionables import _cms_extension | ||
|
||
|
@@ -42,16 +46,24 @@ class VersioningChangeListMixin: | |
"""Mixin used for ChangeList classes of content models.""" | ||
|
||
def get_queryset(self, request): | ||
"""Limit the content model queryset to latest versions only.""" | ||
"""Limit the content model queryset to the latest versions only.""" | ||
queryset = super().get_queryset(request) | ||
versionable = versionables.for_content(queryset.model) | ||
|
||
# TODO: Improve the grouping filters to use anything defined in the | ||
# apps versioning config extra_grouping_fields | ||
grouping_filters = {} | ||
if 'language' in versionable.extra_grouping_fields: | ||
grouping_filters['language'] = get_language_from_request(request) | ||
"""Check if there is a method "self.get_<field>_from_request" for each extra grouping field. | ||
If so call it to retrieve the appropriate filter. If no method is found (except for "language") | ||
no filter is applied. For "language" the fallback is versioning's "get_language_frmo_request". | ||
|
||
Admins requiring extra grouping field beside "language" need to implement the "get_<field>_from_request" | ||
method themselves. A common way to select the field might be GET or POST parameters or user-related settings. | ||
""" | ||
|
||
grouping_filters = {} | ||
for field in versionable.extra_grouping_fields: | ||
if hasattr(self.model_admin, f"get_{field}_from_request"): | ||
grouping_filters[field] = getattr(self.model_admin, f"get_{field}_from_request")(request) | ||
elif field == "language": | ||
grouping_filters[field] = get_language_from_request(request) | ||
return queryset.filter(pk__in=versionable.distinct_groupers(**grouping_filters)) | ||
|
||
|
||
|
@@ -116,7 +128,75 @@ def has_change_permission(self, request, obj=None): | |
return super().has_change_permission(request, obj) | ||
|
||
|
||
class ExtendedVersionAdminMixin(VersioningAdminMixin): | ||
class StateIndicatorMixin(metaclass=MediaDefiningClass): | ||
"""Mixin to provide state_indicator column to the changelist view of a content model admin. Usage:: | ||
|
||
class MyContentModelAdmin(StateIndicatorMixin, admin.ModelAdmin): | ||
list_display = [..., "state_indicator", ...] | ||
""" | ||
class Media: | ||
# js for the context menu | ||
js = ("admin/js/jquery.init.js", "djangocms_versioning/js/indicators.js",) | ||
# css for indicators and context menu | ||
css = { | ||
"all": (static_with_version("cms/css/cms.pagetree.css"),), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A ticket to create for a future change in the core and here is to separate the indicator styling out of this file. |
||
} | ||
|
||
indicator_column_label = _("State") | ||
|
||
@property | ||
def _extra_grouping_fields(self): | ||
try: | ||
return versionables.for_grouper(self.model).extra_grouping_fields | ||
except KeyError: | ||
return None | ||
|
||
def get_indicator_column(self, request): | ||
def indicator(obj): | ||
if self._extra_grouping_fields is not None: # Grouper Model | ||
content_obj = get_latest_admin_viewable_content(obj, include_unpublished_archived=True, **{ | ||
field: getattr(self, field) for field in self._extra_grouping_fields | ||
}) | ||
else: # Content Model | ||
content_obj = obj | ||
status = content_indicator(content_obj) | ||
menu = content_indicator_menu( | ||
request, | ||
status, | ||
content_obj._version, | ||
back=request.path_info + "?" + request.GET.urlencode(), | ||
) if status else None | ||
return render_to_string( | ||
"admin/djangocms_versioning/indicator.html", | ||
{ | ||
"state": status or "empty", | ||
"description": INDICATOR_DESCRIPTIONS.get(status, _("Empty")), | ||
"menu_template": "admin/cms/page/tree/indicator_menu.html", | ||
"menu": json.dumps(render_to_string("admin/cms/page/tree/indicator_menu.html", | ||
dict(indicator_menu_items=menu))) if menu else None, | ||
} | ||
) | ||
indicator.short_description = self.indicator_column_label | ||
return indicator | ||
|
||
def state_indicator(self, obj): | ||
raise ValueError( | ||
"ModelAdmin.display_list contains \"state_indicator\" as a placeholder for status indicators. " | ||
"Status indicators, however, are not loaded. If you implement \"get_list_display\" make " | ||
"sure it calls super().get_list_display." | ||
) # pragma: no cover | ||
|
||
def get_list_display(self, request): | ||
"""Default behavior: replaces the text "state_indicator" by the indicator column""" | ||
if versionables.exists_for_content(self.model) or versionables.exists_for_grouper(self.model): | ||
return tuple(self.get_indicator_column(request) if item == "state_indicator" else item | ||
for item in super().get_list_display(request)) | ||
else: | ||
# remove "state_indicator" entry | ||
return tuple(item for item in super().get_list_display(request) if item != "state_indicator") | ||
|
||
|
||
class ExtendedVersionAdminMixin(VersioningAdminMixin, metaclass=MediaDefiningClass): | ||
""" | ||
Extended VersionAdminMixin for common/generic versioning admin items | ||
|
||
|
@@ -125,6 +205,11 @@ class ExtendedVersionAdminMixin(VersioningAdminMixin): | |
""" | ||
|
||
change_list_template = "djangocms_versioning/admin/mixin/change_list.html" | ||
versioning_list_display = ( | ||
"get_author", | ||
"get_modified_date", | ||
"get_versioning_state", | ||
) | ||
|
||
class Media: | ||
js = ("admin/js/jquery.init.js", "djangocms_versioning/js/actions.js") | ||
|
@@ -269,11 +354,14 @@ def get_list_actions(self): | |
""" | ||
Collect rendered actions from implemented methods and return as list | ||
""" | ||
return [ | ||
actions = [ | ||
self._get_preview_link, | ||
self._get_edit_link, | ||
self._get_manage_versions_link, | ||
] | ||
] | ||
if "state_indicator" not in self.versioning_list_display: | ||
# State indicator mixin loaded? | ||
actions.append(self._get_manage_versions_link) | ||
return actions | ||
|
||
def get_preview_link(self, obj): | ||
return format_html( | ||
|
@@ -310,14 +398,9 @@ def extend_list_display(self, request, modifier_dict, list_display): | |
|
||
def get_list_display(self, request): | ||
# get configured list_display | ||
list_display = self.list_display | ||
list_display = super().get_list_display(request) | ||
# Add versioning information and action fields | ||
list_display += ( | ||
"get_author", | ||
"get_modified_date", | ||
"get_versioning_state", | ||
self._list_actions(request) | ||
) | ||
list_display += self.versioning_list_display + (self._list_actions(request),) | ||
# Get the versioning extension | ||
extension = _cms_extension() | ||
modifier_dict = extension.add_to_field_extension.get(self.model, None) | ||
|
@@ -326,6 +409,14 @@ def get_list_display(self, request): | |
return list_display | ||
|
||
|
||
class ExtendedIndicatorVersionAdminMixin(StateIndicatorMixin, ExtendedVersionAdminMixin): | ||
Check warning Code scanning / CodeQL Conflicting attributes in base classes
Base classes have conflicting values for attribute 'Media': [class Media](1) and [class Media](2).
|
||
versioning_list_display = ( | ||
"get_author", | ||
"get_modified_date", | ||
"state_indicator", | ||
) | ||
|
||
|
||
class VersionChangeList(ChangeList): | ||
def get_filters_params(self, params=None): | ||
"""Removes the grouper param from the filters as the main grouper | ||
|
@@ -697,7 +788,7 @@ def archive_view(self, request, object_id): | |
), | ||
args=(version.content.pk,), | ||
), | ||
back_url=version_list_url(version.content), | ||
back_url=self.back_link(request, version), | ||
) | ||
return render( | ||
request, "djangocms_versioning/admin/archive_confirmation.html", context | ||
|
@@ -777,7 +868,7 @@ def unpublish_view(self, request, object_id): | |
), | ||
args=(version.content.pk,), | ||
), | ||
back_url=version_list_url(version.content), | ||
back_url=self.back_link(request, version), | ||
) | ||
extra_context = OrderedDict( | ||
[ | ||
|
@@ -891,7 +982,7 @@ def revert_view(self, request, object_id): | |
), | ||
args=(version.content.pk,), | ||
), | ||
back_url=version_list_url(version.content), | ||
back_url=self.back_link(request, version), | ||
) | ||
return render( | ||
request, "djangocms_versioning/admin/revert_confirmation.html", context | ||
|
@@ -933,7 +1024,7 @@ def discard_view(self, request, object_id): | |
), | ||
args=(version.content.pk,), | ||
), | ||
back_url=version_list_url(version.content), | ||
back_url=self.back_link(request, version), | ||
) | ||
return render( | ||
request, "djangocms_versioning/admin/discard_confirmation.html", context | ||
|
@@ -969,14 +1060,6 @@ def compare_view(self, request, object_id): | |
), | ||
**persist_params | ||
) | ||
return_url = request.GET.get("back", version_list_url(v1.content)) | ||
try: | ||
# Is return url a valid url? | ||
resolve(urlparse(return_url)[2]) | ||
except Resolver404: | ||
# If not ignore | ||
return_url = None | ||
|
||
# Get the list of versions for the grouper. This is for use | ||
# in the dropdown to choose a version. | ||
version_list = Version.objects.filter_by_content_grouping_values( | ||
|
@@ -987,7 +1070,7 @@ def compare_view(self, request, object_id): | |
"version_list": version_list, | ||
"v1": v1, | ||
"v1_preview_url": v1_preview_url, | ||
"return_url": return_url, | ||
"return_url": self.back_link(request, v1), | ||
} | ||
|
||
# Now check if version 2 has been specified and add to context | ||
|
@@ -1015,6 +1098,18 @@ def compare_view(self, request, object_id): | |
request, "djangocms_versioning/admin/compare.html", context | ||
) | ||
|
||
@staticmethod | ||
def back_link(request, version=None): | ||
back_url = request.GET.get("back", None) | ||
if back_url: | ||
try: | ||
# Is return url a valid url? | ||
resolve(urlparse(back_url)[2]) | ||
except Resolver404: | ||
# If not ignore | ||
back_url = None | ||
return back_url or (version_list_url(version.content) if version else None) | ||
|
||
def changelist_view(self, request, extra_context=None): | ||
"""Handle grouper filtering on the changelist""" | ||
if not request.GET: | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check notice
Code scanning / CodeQL
Cyclic import