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

Turn plone.app.contentrules in a core-addon #4119

Open
jensens opened this issue Feb 19, 2025 · 4 comments
Open

Turn plone.app.contentrules in a core-addon #4119

jensens opened this issue Feb 19, 2025 · 4 comments

Comments

@jensens
Copy link
Member

jensens commented Feb 19, 2025

PLIP (Plone Improvement Proposal)

Responsible Persons

Proposer: @jensens

Seconder: @mauritsvanrees

Abstract

Currently plone.app.contentrules (and plone.contentrules) are part of the Plone Core.

This PLIP proposes to move the two packages out of core as Core Addons.

Motivation

In order to clean up our architecture we identified already packages that are actually add-ons and moved them out (such as Discussion-/ Workingcopy-Support).

To further cleanup and shrink the core plone.app.contentrules should follow here.

Assumptions

Plone Core works fine without plone.app.contentrules.

Proposal & Implementation

  • create a GS profile and move the controlpanel registration there.
  • check usage of the plone.app.contentrules.ManageContentRules permission in Plone Core.
  • Remove dependencies from Products.CMFPlone to plone.app.contentrules.
  • Add dependencies of plone.app.contentrules to the meta package "Plone
  • Modify the test setup in buildout.coredev and plone.app.contentrules
  • check other packages in core if they accidentally use features from contentrules.
  • modify plone.restapi to only expose the endpoints for contentrules if the package is installed.
    This can be done by conditional imports. This seems already the case, but should be checked more thoroughly.
    Test would need to depend on pa.contentrules
  • check popular addons for plone.app.contentrules if they need extra dependencies
  • add/adapt the relevant documentation

for discussion:

  • merge plone.app.contentrules and plone.contentrules

Upgrade path:

Currently plone.app.contentrules does not install anything and is installed in the controlpanel by default.
We need to mimic in an upgrade step - if there is any rule defined - to install it without breaking existing installations
If there are no rules defined at all, we remove the controlpanel and mark it as not installed.
For inspiration, see this upgrade step for cleaning up plone.app.discussion settings.

We must emit a warning - or even an error - if there are rules defined, but plone.app.conentrules is not available.

Deliverables

New releases of packages

  • plone.app.contentrules/ plone.contentrules
  • Plone
  • Products.CMFPlone
  • plone.app.upgrade
  • plone.restapi
  • add-ons inc collective if needed

Documentation

Risks

Users of plone.app.contentrules would need to adapt their setup slighly.

Participants

  • TODO
@jensens
Copy link
Member Author

jensens commented Feb 19, 2025

hey @davisagli @mauritsvanrees @MrTango - any of you available as seconder or participant?
Please comment!

@mauritsvanrees
Copy link
Member

Seconded.

When I checked earlier this week, it seems plone.restapi already has zcml:condition="installed plone.app.contentrules" where needed. But this should be checked again when we start working on it.

I added a link to an upgrade step for cleaning up plone.app.discussion settings, which may serve as inspiration.

@mauritsvanrees
Copy link
Member

About merging plone.contentrules and plone.app.contentrules. This would be nice to try. The end target would probably be plone.app.contentrules. I am not sure if any classes from plone.contentrules are persistently stored in the ZODB, that may make things trickier.

The intended distinction between these two packages was probably:

  • plone.contentrules: define the infrastructure, interfaces, maybe base classes, of content rules. Theoretically usable in plain Zope or CMF. Indeed the readme says 'plone.contentrules provides a "pure Zope" implementation of a a rules engine', and the dependencies are only Zope and zope.componentvocabulary.
  • plone.app.contentrules: define actual standard rules that you can select in Plone, plus forms/UI and control panel.

There is something to be said for this distinction. plone.contentrules is small and likely easy (and fast) to test. And I expect that the base plone.contentrules does not get many changes. Indeed plone.contentrules has 238 commits, and plone.app.contentrules has 878. The last few real changes (disregarding fixes for deprecation warnings and dropping support for Plone or Python versions) for plone.contentrules were in 2021 ("Fix fields in the interface IRuleConfiguration: enabled, stop and cascading are not required."), 2018 ("zope.componentvocabulary") and then 2016 ("CSRF fix: safe write on read."). So plone.contentrules is stable, and most fixes/updates are needed in plone.app.contentrules.

The case where most is to be gained from merging, is when there is often a change needed in plone.contentrules and plone.app.contentrules at the same time. Given the stability of plone.contentrules, this is unlikely to be the case.

Still: having one less package would be good. And if you are trying to debug a problem, it is nicer if all the code is in one package instead of two.

So: +1, but it is a nice-to-have.

@mauritsvanrees
Copy link
Member

I asked on zopefoundation/meta and community.plone.org whether anyone is using plone.contentrules or other plone.something packages outside of Plone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: New (drafts)
Development

No branches or pull requests

2 participants