Change TitleExtension to PageContentExtension
#7369
Conversation
marksweb
force-pushed
the
fix/pagecontentext
branch
from
cfe9bec
to
2f7fd07
Compare
There was a problem hiding this comment.
@marksweb Having had the time to think about this:
I think this change will also need to come along at the same time as djangocms-versioning, this is because the djangocms-versioning monkey patches rely on title_cache and get_title_obj.
It might be worth having a smaller PR just for TitleExtension to PageContentExtension renaming to allow that to come in quicker. As soon as you merge this you lose djangocms-versioning.
cms/api.py
Outdated
| @@ -204,7 +204,7 @@ def create_title(language, title, page, menu_title=None, slug=None, | |||
| created_by='python-api', soft_root=False, in_navigation=False, | |||
| template=TEMPLATE_INHERITANCE_MAGIC, | |||
| limit_visibility_in_menu=constants.VISIBILITY_ALL, | |||
| xframe_options=constants.X_FRAME_OPTIONS_INHERIT): | |||
| xframe_options=constants.X_FRAME_OPTIONS_INHERIT): # TODO: Deprecate this | |||
There was a problem hiding this comment.
It makes this much easier without changing the create_title api as you have done. We will need a plan of how we could replace this in the future, it will affect all internal tests, and if we have 2 at the same time the tests would need to cover both too. Very ugly.
This was left alone due to the sheer amount of changes that would be required to every other package and every other packages test suite.
|
@Aiky30 thanks for taking a look. I've got a PR on versoning to bring support for this change. So the two can come into play together. It'll just need a plan for how best to do that. |
Conflicts: cms/admin/pageadmin.py cms/extensions/__init__.py cms/templatetags/cms_tags.py cms/test_utils/project/extensionapp/admin.py cms/test_utils/project/extensionapp/cms_toolbars.py cms/test_utils/project/extensionapp/models.py cms/tests/test_admin.py cms/tests/test_apphooks.py cms/tests/test_extensions.py
…nto fix/pagecontentext
|
@fsbraun I'm currently installing from develop-4 and versioning from master. So when this merges, before my next build, I'll need a support branch here before this change or versioning merging to master as well. |
|
@marksweb It seems that the merge sneaked in |
|
@fsbraun Oh well spotted! That wasn't part of the conflicts either so I missed it. |
Description
This is a major change.
Currently we have a
TitleExtensionwhich hooks upPageContent. We've talked about how this should have been changed in the early days of v4, so I've done it now before we actually get to the point of official release to avoid it being something we wish we'd done but didn't.