8000 fix: Language chooser options pointing to the same language by fsbraun · Pull Request #7698 · django-cms/django-cms · GitHub
[go: up one dir, main page]
Skip to content

fix: Language chooser options pointing to the same language #7698

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Dec 8, 2023
Merged

fix: Language chooser options pointing to the same language #7698

merged 15 commits into from
Dec 8, 2023

Conversation

fsbraun
Copy link
Member
@fsbraun fsbraun commented Nov 22, 2023

Description

Fixes issue #7692 at least for content objects which do not have language as a grouping field or - if they have - allow for a language argument in their .get_absolute_url() method (such as, e.g., PageContent).

Overall, this still feels unsatisfying:

  • The user is only directed to the public URLs, leaving preview or edit mode.
  • Currently, there is no generic way for the django CMS core to identify content objects of a given language.
  • Since the language menu uses the same DefaultLanguageChanger class, new content objects (such as alias or blog) have to provide their own language menu, creating unnecessary code repetition.

I make this PR a draft to resolve the remaining shortcomings. Options include

  1. Add a complementary method get_for_language(language) to let a content model decide itself how to provide the appropriate language. Downside: Potentially inefficient since caching takes place in the grouper model (Page)
  2. Add a complementary property to indicate how to find a content object's grouper object (would be "page" for PageContent since page_content.page gets the grouper object) to call its get_content_object method. Downside: A (potentially) unnecessary implicit convention.
  3. Extend the CMS config for toolbar-enabled models to optionally provide the model's field to get to its grouper object.

I am favouring the third option, but would appreciate any feedback!

Related resources

Checklist

  • I have opened this pull request against develop-4
  • 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.

@fsbraun fsbraun added the 4.1 label Nov 22, 2023
@fsbraun fsbraun changed the title fix: Language chooser does not contain fix: Language chooser options pointing to the same language Nov 22, 2023
@fsbraun
Copy link
Member Author
fsbraun commented Nov 22, 2023

@marksweb @Aiky30 @benzkji Any thoughts?

@benzkji
Copy link
Contributor
benzkji commented Nov 22, 2023

Sorry, did not read the whole post.

  • I would not imply that a model has a grouper. All of my apps use django-modeltranslation, so, no grouper there (for now ?)
  • leaving edit mode is not very elegant, and could lead t 8000 o user frustration
  • I dont understand the "DefaultLanguageChanger", and how every app/model would need it's own?
  • What are "toolbar-enabled models"?

As you see, more questions than answers ;-)

@fsbraun
Copy link
Member Author
fsbraun commented Nov 22, 2023 •
edited
Loading

To your points:

  • It has to work for both models with (PageContent) and without a grouper.
  • yes
  • Supposedly it should be universal, but currently does not support edit or preview mode for frontend-editable models
  • django CMS introduces frontend-editable models besides PageContent. Example: AliasContent. They can be previewed and edited in the frontend editor without having to have their own URL. They need to register with djangoCMS using CMS config and could announce a grouper model iff they have one.

@fsbraun
Copy link
Member Author
fsbraun commented Nov 30, 2023

I now implemented options three:

  • Frontend-editable objects now have the option to register the name of the grouper field (e.g., "page" for PageContent, since page_content.page gives its grouper object)
  • The language menu now has a cached way of identifying sibling objects of the toolbar object if the name of the grouper field is registered.
  • Both language chooser and language menu now preserve preview or edit mode.

@fsbraun fsbraun marked this pull request as ready for review December 3, 2023 12:56
marksweb
marksweb approved these changes Dec 7, 2023
View reviewed changes
Copy link
Member
@marksweb marksweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been looking over this when I've found a time for a while. I haven't seen anything I don't like, but I've not ran it myself. Given I'm the bottleneck here I'll sign-off for the RC

@fsbraun fsbraun merged commit 2d17eaa into django-cms:develop-4 Dec 8, 2023
fsbraun added a commit that referenced this pull request Dec 10, 2023
@fsbraun @marksweb
@Will-Hoey @wfehr @dwintergruen @jrief
ci: Merge develop-4 into release/4.1.x (#7714)
efacd30 
* ci: Merge back `release/4.1.x` (4.1.0rc4) into `develop-4` (#7640)

* Fix: Debug toolbar action button has too low contrast in dark mode (#7642)

* feat: django 5 support (#7648)

* Support for Django 5.0

* Fix: test.yml and django Promise handling

* Shorten github action names

* Update test.yml

* Update _cms.scss

* Update setup.py

* test: Run tests against django main branch (#7650)

* ci: Add testing against django main branch

* ci: Add dependabot github action updates

* ci: Test all dbs on django main

* Update postgres

Co-authored-by: Fabian Braun <fsbraun@gmx.de>

* Output django version

Co-authored-by: Fabian Braun <fsbraun@gmx.de>

* Output django version

Co-authored-by: Fabian Braun <fsbraun@gmx.de>

* ci: install requirements before django

* Remove duplicate

Co-authored-by: Fabian Braun <fsbraun@gmx.de>

* Replace get_storage_class with import_string

* Fix ruff

* remove unused code from util. __init__.py

* Fix incomplete property overwrite

* Fix: Lazy choice field implementation was wrong. Added test coverage

* Fix: Add `on-error-continue: true` to django-main-postgres and django-main-mysql github actions

---------

Co-authored-by: Fabian Braun <fsbraun@gmx.de>

* fix: When opening structure board page menu disappears or shows wrong page template (#7671)

* Fix: add .current_page to request for structure board

* Fix caching issue showing wrong selected page template

* Fix: add .current_page to request for structure board

* Fix caching issue showing wrong selected page template

* Add tests

* fix: Update RTD config (#7647)

* Add comments

---------

Co-authored-by: Mark Walker <mark@django-cms.org>

* fix: #7662, add support for python 3.12 and upgrade github actions (#7680)

* fix: #7662, add support for python 3.12 and upgrade github actions

* fix: deprecations

* fix: make some improvements to codespell and the tests for docs

* fix: add updated reqs for docs

* fix: code spell typos

* fix: code spell typos for docs

* fix: some more spell fixes

* fix: pretty much all spelling mistakes

* fix: some more spelling mistakes

* fix: skiplist further

* fix: issue with outdated deps at docs/requirements

* fix: downgrade matplotlib to a version that also builds on python 3.8

* fix: 3.8 build

* fix: try to fix some test failures

* fix: drop 3.8

* fix: upgrade sphinxcontrib-spelling to 8.0.0 so that 3.12 builds

* fix: no need to hardcode sphinx in test requirements

* fix: get the tests passing for sqlite

* fix: sqlite tests finally

* fix: use furo theme for cms 4 as well

:

* fix: use cms 4 for building docs documentation

* fix: same as in develop for cms 3.11.x

* fix: abbreviation into full form to not confused codespell

* fix: `.load` jQuery method erroneously replaced by `.on('load')` (#7679)

* Fix: `.load` jQuery method erroneously replaced by `on('load')`

* fix: Remove `can_publish` permission from django CMS 4 core (#7635)

* Fix css glitch

* Update translations source fill which was missing strings

* Fix: Remove can_publish permission

* Update _toolbar.scss

* Fix: Use svg icons for boolean values in admin (present since Dango 1.11)

* Fix: Update to use .format for icon base

* Fix: missing alt attribute

* fix: Port forward #7664 and #7657 (#7695)

* fix: preserve `view_class` in decorated views (#7664)

* Fix tests

* Bugfix: avoid InvalidCacheKey (memcached) for key-length ~249 (fixes #7595) (#7657)

* Remove docs test from test suite (since covered by separate github action)

* Remove docs requirements from the django-main test

* Add setuptools to requirements for python 3.12

* Add setuptools to requirements.txt

* Fix: Test with current ckeditor

* Undo unnecessary change

* Fix tests for Django 5.1

* Fix: Missing output_field

---------

Co-authored-by: Will Hoey <48737592+Will-Hoey@users.noreply.github.com>
Co-authored-by: wfehr <24782511+wfehr@users.noreply.github.com>

* fix: validate endpoint languages and redirect if not editable (#7691)

* Fix: align language settings for preview and edit endpoints

* Rename to existing "is_editable" method.

* Add "object_is_editable" to toolbar as a common interface to decide if an object is editable

* Fix identation

* fix: Add check for Django's i18n context processor needed for wizards to work (#7699)

* Check for `django.template.context_processors.i18n` preprocessor in the i18n check (required for wizards to work)

* Fix tests ;-)

* feat: Add `djangocms` command to quickly start a project (#7702)

* fix: Localization of permission checks on deleting (#7683)

* Permission in the german version have sometimes german names.

* change django-cms to cms

* Remove specific German locale test

* Update pageadmin.py

* Undo renaming

---------

Co-authored-by: wintergruen <dirk.wintergruen@klassik-stiftung.de>
Co-authored-by: Fabian Braun <fsbraun@gmx.de>

* fix: django 5's choice widget is not lazy (#7707)

* Support for Django 5.0

* Fix: test.yml and django Promise handling

* Shorten github action names

* Update test.yml

* Update _cms.scss

* Update setup.py

* Fix: Django 5 choice widget is not lazy either

* Add test

* Fix: Swapped underscore

* Deprecate SuperLazyIterator and LazyChoiceField

* fix: Page Content Extension toolbar (#7708)

* Fix: page content extension toolbar naming and use latest_content filter

* Align page content extension menu with shown page content

* Deprecate the use for more than one page content object

* fix linting issue

* Accommodate review feedback

* Add some comments

* Readability improvement `ruff format`

* One more readability improvement

* Doc-string update

* Fix typo

Co-authored-by: Jacob Rief <jacob.rief@gmail.com>

* fix: Language chooser options pointing to the same language (#7698)

* Fix: Language chooser

* Ensure page language uniqueness

* Fix linter issue

* Add edit mode and preview mode to language chooser

* Add option to register grouper field for frontend-editable models

* Undo changes to cms_toolbars to avoid code redundancy

* Simplify toolbar utils.

* Remove not util function

* Remove unneeded imports

* fix: allow for `EmptyPageContent`

* Add CONTRIBUTING.rst and CODE_OF_CONDUCT.rst to develop-4 branch (#7713)

---------

Co-authored-by: Mark Walker <mark@django-cms.org>
Co-authored-by: Vinit Kumar <vinit.kumar@kidskonnect.nl>
Co-authored-by: Will Hoey <48737592+Will-Hoey@users.noreply.github.com>
Co-authored-by: wfehr <24782511+wfehr@users.noreply.github.com>
Co-authored-by: dwintergruen <dwinter@comp-hum.de>
Co-authored-by: wintergruen <dirk.wintergruen@klassik-stiftung.de>
Co-authored-by: Jacob Rief <jacob.rief@gmail.com>
fsbraun added a commit to fsbraun/django-cms that referenced this pull request Feb 7, 2024
@fsbraun
fix: Language chooser options pointing to the same language (django-c…
21386d4 
…ms#7698)

* Fix: Language chooser

* Ensure page language uniqueness

* Fix linter issue

* Add edit mode and preview mode to language chooser

* Add option to register grouper field for frontend-editable models

* Undo changes to cms_toolbars to avoid code redundancy

* Simplify toolbar utils.

* Remove not util function

* Remove unneeded imports

* fix: allow for `EmptyPageContent`
@fsbraun fsbraun deleted the fix/language-chooser branch October 5, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marksweb marksweb marksweb approved these changes

Assignees
No one assigned
Labels
4.1
Projects
django CMS v4
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fsbraun @benzkji @marksweb
0