8000 fix: Replaced `languages` field from `Page` which used to become inco… · django-cms/django-cms@1031d20 · GitHub
[go: up one dir, main page]
Skip to content

Commit 1031d20

Browse files
fsbraunGithub Release Action
fsbraun
and
Github Release Action
authored
fix: Replaced languages field from Page which used to become inconsistent (#8080)
* Remove `languages` field from `Page` * add migration * Remove unused imports * Remove duplicates before asserting uniqueness constraint * Fix: sourcery complexity reduction recommendation * Fix linting * Fix tests --------- Co-authored-by: Github Release Action <info@django-cms.org>
1 parent 76c5bb0 commit 1031d20

File tree

13 files changed

+124
-101
lines changed

13 files changed

+124
-101
lines changed

‎cms/admin/pageadmin.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,14 +1227,16 @@ def delete_view(self, request, object_id, extra_context=None):
12271227
'language': force_str(get_language_object(language)['name'])
12281228
}
12291229
messages.success(request, message)
1230+
if language in page.admin_content_cache:
1231+
del page.admin_content_cache[language]
1232+
if language in page.page_content_cache:
1233+
del page.page_content_cache[language]
12301234

12311235
page_url.delete()
12321236
page_content.delete()
12331237
for p in saved_plugins:
12341238
p.delete()
12351239

1236-
page.remove_language(language)
1237-
12381240
send_post_page_operation(
12391241
request=request,
12401242
operation=operations.DELETE_PAGE_TRANSLATION,

‎cms/api.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -313,12 +313,10 @@ def create_page_content(language, title, page, menu_title=None, slug=None,
313313
_thread_locals.user = created_by
314314
created_by = get_clean_username(created_by)
315315

316-
page.urls.create(
317-
slug=slug,
318-
path=path,
316+
page.urls.update_or_create(
319317
page=page,
320-
managed=not bool(overwrite_url),
321318
language=language,
319+
defaults=dict(slug=slug, path=path, managed=not bool(overwrite_url)),
322320
)
323321

324322
# E.g., djangocms-versioning needs an User object to be passed when creating a versioned Object
@@ -340,11 +338,8 @@ def create_page_content(language, title, page, menu_title=None, slug=None,
340338
xframe_options=xframe_options,
341339
)
342340
page_content.rescan_placeholders()
341+
page._clear_internal_cache()
343342

344-
page_languages = page.get_languages()
345-
346-
if language not in page_languages:
347-
page.update_languages(page_languages + [language])
348343
return page_content
349344

350345

‎cms/cms_menus.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,6 @@ def get_nodes(self, request) -> list[NavigationNode]:
333333
"soft_root",
334334
"in_navigation",
335335
"page__site_id",
336-
"page__languages",
337336
"page__parent_id",
338337
"page__is_home",
339338
"page__login_required",
@@ -352,7 +351,7 @@ def prefetch_urls(page_content: PageContent) -> PageContent:
352351
else:
353352
preview_url = None # No short-cut here
354353
prefetched_urls = PageUrl.objects.filter(
355-
language__in=self.languages,
354+
language__in=(page_content.language for page_content in page_contents),
356355
page_id__in=(page_content.page.pk for page_content in page_contents),
357356
) # Fetch the PageUrl objects
358357
# Prepare for filling urls_cache

‎cms/management/commands/subcommands/copy.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,6 @@ def handle(self, *args, **options):
8989
new_title["page"] = page
9090
PageContent.objects.with_user(user).create(**new_title)
9191

92-
if to_lang not in page.get_languages():
93-
page.update_languages(page.get_languages() + [to_lang])
94-
9592
if copy_content:
9693
# copy plugins using API
9794
if verbose:

‎cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Generated by Django 4.2. 12B5 16 on 2024-11-14 06:48
2+
3+
from django.db import migrations
4+
from django.db.models import Count
5+
6+
7+
def remove_pageurl_duplicates(apps, schema_editor):
8+
PageUrl = apps.get_model("cms", "PageUrl")
9+
non_unique = PageUrl.objects.values("page_id", "language").annotate(total=Count("language")).filter(total__gt=1)
10+
for item in non_unique:
11+
for url in PageUrl.objects.filter(page_id=item["page_id"], language=item["language"]).order_by("-pk")[1:]:
12+
url.delete()
13+
14+
15+
class Migration(migrations.Migration):
16+
dependencies = [
17+
("cms", "0038_alter_page_site"),
18+
]
19+
20+
operations = [
21+
migrations.RunPython(remove_pageurl_duplicates),
22+
migrations.RemoveField(
23+
model_name="page",
24+
name="languages",
25+
),
26+
migrations.AlterUniqueTogether(
27+
name="pageurl",
28+
unique_together={("language", "page")},
29+
),
30+
]

‎cms/models/pagemodel.py

Lines changed: 57 additions & 70 deletions
809
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,11 @@
1616
from treebeard.mp_tree import MP_Node
1717

1818
from cms import constants
19-
from cms.exceptions import LanguageError
2019
from cms.models.managers import PageManager, PageUrlManager
2120
from cms.utils import i18n
2221
from cms.utils.compat.warnings import RemovedInDjangoCMS43Warning
2322
from cms.utils.conf import get_cms_setting
24-
from cms.utils.i18n import get_current_language, get_fallback_languages
23+
from cms.utils.i18n import get_current_language
2524
from cms.utils.page import get_clean_username
2625
from menus.menu_pool import menu_pool
2726

@@ -114,12 +113,6 @@ class Page(MP_Node):
114113
blank=True,
115114
null=True,
116115
)
117-
languages = models.CharField(
118-
max_length=255,
119-
editable=False,
120-
blank=True,
121-
null=True,
122-
)
123116
is_page_type = models.BooleanField(
124117
default=False,
125118
help_text=_("Mark this page as a page type"),
@@ -143,6 +136,7 @@ class Meta:
143136
def __init__(self, *args, **kwargs):
144137
super().__init__(*args, **kwargs)
145138
self.urls_cache = {}
139+
#: Internal cache for page urls
146140
self.page_content_cache = {}
147141
#: Internal cache for page content objects visible publicly
148142
self.admin_content_cache = AdminCacheDict()
@@ -438,7 +432,6 @@ def move_page(self, target_page, position='first-child'):
438432
languages = (
439433
self
440434
.urls
441-
.filter(language__in=self.get_languages())
442435
.values_list('language', flat=True)
443436
)
444437

@@ -524,7 +517,6 @@ def copy(self, site, parent_page=None, parent_node=None, language=None,
524517
)
525518
placeholder.copy_plugins(new_placeholder, language=new_title.language)
526519
new_page.page_content_cache[new_title.language] = new_title
527-
new_page.update_languages([trans.language for trans in translations])
528520

529521
if extensions:
530522
from cms.extensions import extension_pool
@@ -692,30 +684,46 @@ def get_parent_page(self):
692684
)
693685
return self.parent
694686

695-
def get_languages(self):
696-
if self.languages:
697-
return sorted(self.languages.split(','))
698-
else:
699-
return []
687+
@property
688+
def languages(self):
689+
warnings.warn(
690+
"Attribute `languages` is deprecated. Use `get_languages` instead.",
691+
RemovedInDjangoCMS43Warning,
692+
stacklevel=2
693+
)
694+
return ",".join(self.get_languages())
695+
696+
def get_languages(self, admin_manager=True):
697+
"""Returns available languages for the page. This is potentially costly."""
698+
if admin_manager:
699+
if not self.admin_content_cache:
700+
self.set_admin_content_cache()
701+
return list(self.admin_content_cache.keys())
702+
if not self.page_content_cache:
703+
self._get_page_content_cache(get_language(), fallback=False, force_reload=False)
704+
return list(self.page_content_cache.keys())
700705

701706
def remove_language(self, language):
702-
page_languages = self.get_languages()
703-
704-
if language in page_languages:
705-
page_languages.remove(language)
706-
self.update_languages(page_languages)
707+
warnings.warn(
708+
"Method `remove_language` is deprecated and has no effect any more.",
709+
RemovedInDjangoCMS43Warning,
710+
stacklevel=2
711+
)
707712

708713
def update_languages(self, languages):
709-
languages = ",".join(set(languages))
710-
# Update current instance
711-
self.languages = languages
712-
# Commit. It's important to not call save()
713-
# we'd like to commit only the languages field and without
714-
# any kind of signals.
715-
self.update(languages=languages)
714+
warnings.warn(
715+
"Method `update_languages` is deprecated and has no effect any more.",
716+
RemovedInDjangoCMS43Warning,
717+
stacklevel=2
718+
)
716719

717720
def get_published_languages(self):
718-
return self.get_languages()
721+
warnings.warn(
722+
"Method `get_published_languages` is deprecated. Use `get_languages(admin_manager=False)` instead.",
723+
RemovedInDjangoCMS43Warning,
724+
stacklevel=2,
725+
)
726+
return self.get_languages(admin_manager=False)
719727

720728
def set_translations_cache(self):
721729
warnings.warn(
@@ -800,58 +808,36 @@ def get_page_content_obj_attribute(self, attrname, language=None, fallback=True,
800808
except AttributeError:
801809
return None
802810

803-
def get_path(self, language, fallback=True):
811+
def get_url_obj(self, language, fallback=True):
804812
"""Get the path of the page depending on the given language"""
805813
languages = [language]
806-
807814
if fallback:
808815
languages.extend(self.get_fallbacks(language))
816

810-
page_languages = self.get_languages()
811-
812-
for _language in languages:
813-
if _language in page_languages:
814-
language = _language
815-
break
816-
817817
if language not in self.urls_cache:
818-
self.urls_cache.update({
819-
url.language: url for url in self.urls.all() if url.language in languages # TODO: overwrites multiple urls
820-
})
821-
822-
for _language in languages:
823-
self.urls_cache.setdefault(_language, None)
818+
# `get_page_from_request` will fill the cache only for the current language
819+
# Here, we fully fill it and try again
820+
self.urls_cache = {
821+
url.language: url for url in self.urls.all() if url.language in languages
822+
}
823+
824+
return next(
825+
(self.urls_cache[lang] for lang in languages if lang in self.urls_cache),
826+
None
827+
)
824828

825-
try:
826-
return self.urls_cache[language].path
827-
except (AttributeError, KeyError):
828-
return None
829+
def get_path(self, language, fallback=True):
830+
url = self.get_url_obj(language, fallback)
831+
if url:
832+
return url.path
833+
return None
829834

830835
def get_slug(self, language, fallback=True):
831-
languages = [language]
836+
url = self.get_url_obj(language, fallback)
837+
if url:
838+
return url.slug
839+
return None
832840

833-
if fallback:
834-
languages.extend(self.get_fallbacks(language))
835-
836-
page_languages = self.get_languages()
837-
838-
for _language in languages:
839-
if _language in page_languages:
840-
language = _language
841-
break
842-
843-
if language not in self.urls_cache:
844-
self.urls_cache.update({
845-
url.language: url for url in self.urls.filter(language__in=languages)
846-
})
847-
848-
for _language in languages:
849-
self.urls_cache.setdefault(_language, None)
850-
851-
try:
852-
return self.urls_cache[language].slug
853-
except (AttributeError, KeyError):
854-
return None
855841

856842
def get_title(self, language=None, fallback=True, force_reload=False):
857843
"""
@@ -1099,6 +1085,7 @@ class PageUrl(models.Model):
10991085
class Meta:
11001086
app_label = 'cms'
11011087
default_permissions = []
1088+
unique_together = ('language', 'page')
11021089

11031090
def __str__(self):
11041091
return f"{self.path or self.slug} ({self.language})"

‎cms/templatetags/cms_admin.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ def get_page_display_name(cms_page):
9898
page_content = cms_page.get_admin_content(language, fallback="force")
9999
title = page_content.title or page_content.page_title or page_content.menu_title
100100
if not title:
101-
title = cms_page.get_slug(language)
101+
title = cms_page.get_slug(language) or _("Empty")
102102
return title if page_content.language == language else mark_safe(f"<em>{title} ({page_content.language})</em>")
103103

104104

‎cms/tests/test_templatetags.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ def test_get_preview_url(self):
8181

8282

8383
def test_get_admin_tree_title(self):
84-
page = create_page("page_a", "nav_playground.html", "en")
84+
page = create_page("page_a", "nav_playground.html", "en", slug="slug-test2")
8585
self.assertEqual(get_page_display_name(page), 'page_a')
8686
languages = {
8787
1: [
@@ -108,7 +108,6 @@ def test_get_admin_tree_title(self):
108108
page.admin_content_cache = {'en': PageContent(menu_title="menu test2", language="en")}
109109
self.assertEqual('<em>menu test2 (en)</em>', force_str(get_page_display_name(page)))
110110
page.admin_content_cache = {'en': PageContent(language="en")}
111-
page.urls_cache = {'en': PageUrl(slug='slug-test2')}
112111
self.assertEqual('<em>slug-test2 (en)</em>', force_str(get_page_display_name(page)))
113112
page.admin_content_cache = {'en': PageContent(language="en"), 'fr': EmptyPageContent('fr')}
114113
self.assertEqual('<em>slug-test2 (en)</em>', force_str(get_page_display_name(page)))

‎cms/tests/test_toolbar.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,7 @@ def get_delete_url(pk):
533533
page = create_page("english-page", "nav_playground.html", "en")
534534
german_content = create_page_content("de", "german content", page)
535535
english_content = page.get_content_obj('en')
536+
page_languages = page.get_languages()
536537
edit_url = get_object_edit_url(english_content)
537538
staff = self.get_staff()
538539
self.client.force_login(staff)
@@ -541,8 +542,8 @@ def get_delete_url(pk):
541542
menus = response.context['cms_toolbar'].menus
542543
language_menu = menus['language-menu']
543544
delete = language_menu.items[-2]
544-
german_delete = delete.items[0]
545-
english_delete = delete.items[1]
545+
german_delete = delete.items[page_languages.index("de")]
546+
english_delete = delete.items[page_languages.index("en")]
546547

547548
copy = language_menu.items[-1]
548549
copy_german = copy.items[0]

‎cms/tests/test_views.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,18 @@ def test_malicious_content_login_request(self):
366366

367367
self.assertNotIn(response.url, "<script>alert('Attack')</script>")
368368

369+
def test_queries(self):
370+
create_page("home", "simple.html", "en")
371+
cms_page = create_page("dreinhardt", "simple.html", "en")
372+
url = cms_page.get_absolute_url()
373+
with self.assertNumQueries(5):
374+
# 1. get_page_from_request: checks PageUrl
375+
# 2. get page contents: PageContent
376+
# 3. Check permissions
377+
# 4. Get placeholders
378+
# 5. Get plugins
379+
self.client.get(url)
380+
369381

370382
@override_settings(ROOT_URLCONF='cms.test_utils.project.urls')
371383
class ContextTests(CMSTestCase):

0 commit comments

Comments
 (0)
0