Skip to content
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

add deprecation warnings in jbot for templates #7

Open
MrTango opened this issue Aug 30, 2023 · 3 comments
Open

add deprecation warnings in jbot for templates #7

MrTango opened this issue Aug 30, 2023 · 3 comments

Comments

@MrTango
Copy link

MrTango commented Aug 30, 2023

to move templates into plone.classicui, we need a way to show deprecation warnings, if someone is trying to override the template in the old path. It should still work, but show a message.
This needs to be build into jbot.

@mauritsvanrees
Copy link
Sponsor Member

Idea:

  1. In z3c.jbot create a new zcml directive that can handle a zcml like this, which we would add in plone.classicui/meta.zcml:
<browser:jbotDeprecated
    deprecated="path.to.deprecated_dict" />

Having this in meta.zcml should make sure it gets loaded before the regular configure.zcml of other packages get loaded, so before any <browser:jbot directives get parsed.

z3c.jbot would then have a handler that takes the path to the deprecated dictionary and stores it (or updates an own dictionary with this). We would fill the dictionary manually for each template that we have already moved, and it would contain for example as key Products.CMFPlone.browser.templates.main_template.pt with value plone.classicui.browser.templates.main_template.pt.

  1. In the registerDirectory method that gets called when an add-on registers a jbot overrides directory, we would change these lines to check if a filename in the directory matches any of the deprecated paths:
for filename in os.listdir(directory):
    filename = os.path.normcase(filename)
    full_path = normalize(
        "%s%s%s" %
        (directory, os.path.sep, filename))
    canonical_filename = deprecated_dict.get(filename)
    if canonical_filename:
        # maybe use zope.deprecated
        warnings.warn(f"template {filename} deprecated, use {canonical_filename}")
        self.paths[canonical_filename] = full_path
    # Probably still best to register the old file path always.
    self.paths[filename] = full_path

If doing this in meta.zcml does not work, we could also do this in configure.zcml, but then we need to iterate over all the jbot template managers that have already been registered, and iterate over their paths and update them.

mauritsvanrees added a commit to plone/classic-ui-team that referenced this issue Oct 25, 2023
@MrTango
Copy link
Author

MrTango commented Dec 13, 2023

very cool, so we need to write something like this ZCML directive https://github.com/zopefoundation/z3c.jbot/blob/2.0/src/z3c/jbot/meta.zcml for the jbotDeprecated part.

mauritsvanrees added a commit to zopefoundation/z3c.jbot that referenced this issue Mar 6, 2024
This can be used when you move templates and want an existing jbot
override for the old template path to still work.

Example:

* You move `old_package/test.pt` to `new_package/test.pt`.
* You register a dictionary with: `{"old_package.test.pt": "new_package.test.pt"}`
* If a third party package has a template override `old_package.test.pt`, we display a warning that the package should use `new_package.test.pt`.
* The override with the old name still works: instead of `new_package/test.pt`, the override is used.
* Of course an override with the new name works as well.

Sample warning for a template that is moved from plone.app.layout to plone.classicui:

UserWarning: Template /Users/maurits/zeelandia/plone60/src/zeelandia.theme/src/zeelandia/theme/browser/overrides/plone.app.layout.viewlets.searchbox.pt deprecated, use plone.classicui.viewlets.searchbox.pt

For background, see plone/plone.classicui#7
@mauritsvanrees
Copy link
Sponsor Member

I have implemented my idea in a PR:
zopefoundation/z3c.jbot#20

I tried it out in a client project where I had an override for the searchbox viewlet.
In a buildout with mr.developer you can try it out like this:

auto-checkout +=
    z3c.jbot
    plone.app.layout
    plone.classicui

[sources]
plone.app.layout = git git@github.com:plone/plone.app.layout.git branch=maurits-viewlets-searchbox
plone.classicui = git git@github.com:plone/plone.classicui.git branch=maurits-viewlets-searchbox
z3c.jbot = git git@github.com:zopefoundation/z3c.jbot.git branch=maurits-deprecated-templates-directives

[zeoclient]
eggs +=
    plone.classicui
zcml +=
    plone.classicui

This uses a branch of plone.app.layout where I have simply removed the searchbox viewlet, and a branch of plone.classicui where I have added this viewlet and I use the new jbotDeprecated zcml directive.

In some other package, you should add a jbot override with filename plone.app.layout.viewlets.searchbox.pt. It is enough to put something like 'Hello World' in there.

Two things should happen then:

  1. You get a warning on startup, saying you should override the new location.
  2. The old location override should still work: you see 'Hello World' instead of the search box.

The example might be bad: these layout templates might be perfectly fine to stay in plone.app.layout, because this package already seems mostly for Classic UI. Also, if we would move over the complete viewlets directory, we could get into circular dependencies if we want to have backwards compatibility imports in place.

But: this should work as a proof of concept for the idea.

mauritsvanrees added a commit to zopefoundation/z3c.jbot that referenced this issue Mar 6, 2024
This can be used when you move templates and want an existing jbot
override for the old template path to still work.

Example:

* You move `old_package/test.pt` to `new_package/test.pt`.
* You register a dictionary with: `{"old_package.test.pt": "new_package.test.pt"}`
* If a third party package has a template override `old_package.test.pt`, we display a warning that the package should use `new_package.test.pt`.
* The override with the old name still works: instead of `new_package/test.pt`, the override is used.
* Of course an override with the new name works as well.

Sample warning for a template that is moved from plone.app.layout to plone.classicui:

UserWarning: Template /Users/maurits/zeelandia/plone60/src/zeelandia.theme/src/zeelandia/theme/browser/overrides/plone.app.layout.viewlets.searchbox.pt deprecated, use plone.classicui.viewlets.searchbox.pt

For background, see plone/plone.classicui#7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

2 participants