Turn this into a core addon #211

jensens · 2023-05-19T13:23:08Z

sync with CMFPlone, make installable and remove old files.

(to be) Merged on its own:

Need to be merged together

Jenkins paste to test together:

https://github.com/plone/plone.app.discussion/pull/211
https://github.com/plone/Products.CMFPlone/pull/3782
https://github.com/plone/plone.app.contenttypes/pull/665
https://github.com/plone/plone.app.dexterity/pull/371
https://github.com/plone/plone.app.upgrade/pull/330

jensens · 2023-05-19T13:30:56Z

pre-commit.ci autofix

gforcada · 2023-05-20T11:18:05Z

@jensens I see that you added already a 6.1 trove classifier, that's what makes pre-commit fail.

@ericof @mauritsvanrees I requested the new classifier: pypa/trove-classifiers#146

I was about to ask for the 7.0 as well, but hopefully it does not take too much time to process them anyway. 🍀

gforcada · 2023-05-20T11:23:17Z

@jensens great job! 😄 👍🏾

I have a few questions though, unfortunately we did not have enough time this week to talk about things in depth 😅 I was mostly 😵‍💫 all they long with too many conversations going on 😅

Anyway:

translations: what should we do with them? 🤔 @erral do you have a preference/suggestion?
should we add some QA tests to ensure that no package depends on these? plone.app.upgrade might be the only one that should be fine, all other packages should not import from plone.app.discussion, right? @ericof again, something that at some point we mentioned to discuss but we had way too many other things 😅

erral · 2023-05-20T11:29:47Z

This package's translations are already included in the plone domain, so we don't need to do anything. The pot file generation script will take the messages wherever they are.

ericof · 2023-05-21T09:32:10Z

One important point about Trove classifiers and CI: As soon as the classifier is accepted, w e need to bump the trove-classifiers package everywhere (or in meta)

gforcada · 2023-05-23T07:23:25Z

The classifier has been merged, but it does not appear, yet, on pypi, but hopefully is a matter of a few days/weeks 🍀

jensens · 2024-05-21T16:39:52Z

Note: This needs an uninstall as well.

jensens · 2024-05-27T11:35:26Z

pre-commit.ci autofix

… remove old files

updates: - [github.com/asottile/pyupgrade: v3.15.2 → v3.16.0](asottile/pyupgrade@v3.15.2...v3.16.0) - [github.com/PyCQA/flake8: 7.0.0 → 7.1.0](PyCQA/flake8@7.0.0...7.1.0)

Is that actually needed in Plone 6?

updates: - [github.com/asottile/pyupgrade: v3.16.0 → v3.17.0](asottile/pyupgrade@v3.16.0...v3.17.0) - [github.com/psf/black: 24.4.2 → 24.8.0](psf/black@24.4.2...24.8.0) - [github.com/PyCQA/flake8: 7.1.0 → 7.1.1](PyCQA/flake8@7.1.0...7.1.1) - [github.com/collective/i18ndude: 6.2.0 → 6.2.1](collective/i18ndude@6.2.0...6.2.1)

mauritsvanrees · 2024-08-13T17:40:55Z

I have added an upgrade step in plone/plone.app.upgrade#330. See testing hints there. I have merged main/master in the PR branches of the other packages. This seems ready now.

I noticed that on Jenkins only 142 robot tests were run instead of 149. The console showed the reason: ``` Set up plone.app.contenttypes.testing.PloneAppContenttypes in 0.020 seconds. Set up plone.app.robotframework.testing.SimplePublicationLayer in 0.001 seconds. Set up plone.app.robotframework.remote.RemoteLibraryBundle:RobotRemote in 0.000 seconds. Set up plone.testing.zope.WSGIServer in 0.043 seconds. Set up plone.app.robotframework.testing.RemoteLibrary:Robot in 0.000 seconds. Set up plone.app.discussion.testing.PloneAppDiscussionRobot Traceback (most recent call last): File "/Users/maurits/shared-eggs/cp311/zope.testrunner-6.4-py3.11.egg/zope/testrunner/runner.py", line 474, in run_layer setup_layer(options, layer, setup_layers) File "/Users/maurits/shared-eggs/cp311/zope.testrunner-6.4-py3.11.egg/zope/testrunner/runner.py", line 839, in setup_layer setup_layer(options, base, setup_layers) File "/Users/maurits/shared-eggs/cp311/zope.testrunner-6.4-py3.11.egg/zope/testrunner/runner.py", line 844, in setup_layer layer.setUp() File "/Users/maurits/shared-eggs/cp311/plone.app.testing-7.1.0-py3.11.egg/plone/app/testing/helpers.py", line 378, in setUp self.setUpPloneSite(portal) File "/Users/maurits/community/plone-coredev/6.1/src/plone.app.discussion/plone/app/discussion/testing.py", line 111, in setUpPloneSite settings = registry.forInterface(IDiscussionSettings) File "/Users/maurits/shared-eggs/cp311/plone.registry-2.0.1-py3.11.egg/plone/registry/registry.py", line 71, in forInterface raise KeyError( KeyError: 'Interface `plone.app.discussion.interfaces.IDiscussionSettings` defines a field `globally_enabled`, for which there is no record.' ```

We are already loading a file with the same name from plone.app.robotframework.

mauritsvanrees · 2024-08-13T21:30:53Z

Note for reviewers: several GitHub Actions tests fail in this PR. This is because in GitHub Actions we test against what is on https://dist.plone.org/release/6.1-dev/. And these tests will fail because we need several checkouts. That is what we do on Jenkins, so we can ignore the failing tests here.
We could probably add some mxdev configuration in here to fix this. But that would only be temporary until there is a new Products.CMFPlone release.

jensens · 2024-08-14T14:01:34Z

This should work https://github.com/plone/buildout.coredev/blob/6.1/plips/plip-padiscussion-addon.cfg
In past we had Jenkins jobs for the PLIP configurations, well, at least nowadays this can be used to be tested local.

gforcada · 2024-08-19T05:27:48Z

PLIP jobs in jenkins can be added, it's a matter of configuring them, they do not appear automatically though...

gforcada · 2024-08-19T05:51:05Z

plone/app/discussion/profiles/default/types/Discussion_Item.xml

-  <property name="content_icon">discussionitem_icon.png</property>
+  >Discussion Items are documents which reply to other content.
+  They should *not* be addable through the standard 'folder_factories' interface.</property>
+  <property name="content_icon">string:${portal_url}/discussionitem_icon.png</property>


This should be something like: string:chat-left-text ?

<property name="content_icon">string:chat-left-text</property>

i.e. this icon: https://icons.getbootstrap.com/icons/chat-left-text/

See https://6.docs.plone.org/classic-ui/icons.html

Actually, the content_icon property should no longer be used. At least I do not see it in the type definitions in plone.app.contenttypes.
Same for content_meta_type. I have removed them from this branch now.

We can still look at improving other icons, which you are partially already doing here: https://github.com/plone/plone.app.discussion/pull/222/files

Oops, removing those old properties is a bad idea. At least, when I instead add icon_expr, I get an error while creating a Plone Site. So I reverted that change.

@gforcada I have updated your PR 222 to update more icons and add an upgrade step. That is for Plone 6.0.

And I have copied that code in adapted form to the current PR for 6.1.

And yes, I took your proposed icon chat-left-text. :-)

plone/app/discussion/tests/functional_test_behavior_discussion.rst

This was moved here from `plone.app.dexterity`, but it has the exact same contents as `tests/functional_test_behavior_discussion.rst`. I compared again with the original in `plone.app.dexterity`, and it is indeed the same, except that the behavior name is fixed to `plone.allowdiscussion` instead of the former `plone.app.dexterity.behaviors.discussion.IAllowDiscussion`.

The FTIs in `plone.app.contenttypes` do not have them.

…rom FTI." This reverts commit da5b2f5. I looked at `plone.app.contenttypes` for comparison, but those are Dexterity FTIs, and here we have an old-style FTI. When I locally added `icon_expr` in here, I actually got an error while adding a Plone Site: ``` ValueError: http://localhost:8080/@@plone-addsite Traceback (innermost last): Module ZPublisher.WSGIPublisher, line 181, in transaction_pubevents Module ZPublisher.WSGIPublisher, line 391, in publish_module Module ZPublisher.WSGIPublisher, line 285, in publish Module ZPublisher.mapply, line 98, in mapply Module ZPublisher.WSGIPublisher, line 68, in call_object Module Products.CMFPlone.browser.admin, line 299, in __call__ Module Products.CMFPlone.factory, line 165, in addPloneSite Module Products.GenericSetup.tool, line 393, in runAllImportStepsFromProfile - __traceback_info__: profile-Products.CMFPlone:plone Module Products.GenericSetup.tool, line 1513, in _runImportStepsFromContext Module Products.GenericSetup.tool, line 1360, in _doRunHandler Module Products.CMFPlone.setuphandlers, line 139, in importFinalSteps Module Products.GenericSetup.tool, line 393, in runAllImportStepsFromProfile - __traceback_info__: profile-Products.CMFPlone:dependencies Module Products.GenericSetup.tool, line 1504, in _runImportStepsFromContext Module Products.GenericSetup.tool, line 1316, in _doRunImportStep - __traceback_info__: typeinfo Module Products.CMFCore.exportimport.typeinfo, line 222, in importTypesTool Module Products.GenericSetup.utils, line 926, in importObjects - __traceback_info__: portal_types Module Products.GenericSetup.utils, line 922, in importObjects - __traceback_info__: types/Discussion_Item Module Products.GenericSetup.utils, line 525, in _importBody Module Products.CMFCore.exportimport.typeinfo, line 61, in _importNode Module Products.GenericSetup.utils, line 757, in _initProperties ValueError: undefined property 'content_icon' ``` So let's keep the old-style properties, although I am not sure they are actually used anywhere in current Plone.

The `view` action of comments had no icon on Plone 6.

Update the profile version to 3000. This leaves room for upgrades on the 4.x branch (Plone 6.0).

plone/app/discussion/setuphandlers.py

mauritsvanrees · 2024-09-02T14:53:52Z

I merged all PRs, except this one... Oops. :-) Doing so now.

This comment was marked as resolved.

Sign in to view

jensens marked this pull request as draft May 19, 2023 13:31

This was referenced May 19, 2023

move plone.allowdiscussion behavior to plone.app.discussion plone/plone.app.dexterity#371

Merged

depend here on plone.app.discussion and turn it in a core addon plone/Plone#29

Merged

jensens force-pushed the core-addon branch from 9457301 to ca196fe Compare June 6, 2023 17:23

mauritsvanrees mentioned this pull request May 15, 2024

PLIP: make plone.app.discussion a core add-on [6.1] plone/Products.CMFPlone#3958

Closed

4 tasks

jensens force-pushed the core-addon branch from 15e6d42 to fcf2a79 Compare May 15, 2024 13:38

jensens mentioned this pull request May 20, 2024

do not enable plone.allowdiscussion by default plone/plone.app.contenttypes#665

Merged

jensens force-pushed the core-addon branch from 0f670a2 to 6120678 Compare May 20, 2024 19:12

jensens mentioned this pull request May 27, 2024

Discussion core addon plone/Products.CMFPlone#3782

Merged

jensens mentioned this pull request May 30, 2024

Remove 'Discussion Item' from SearchSchema (helps also to turn pa.discussion into a core addon) plone/plone.base#65

Merged

jensens added 11 commits May 30, 2024 17:35

Turn this into a core addon, sync with CMFPlone, make installable and…

a3f9160

… remove old files

move behavior here

82328be

tune behavior and install it

ae0e9a1

fix behavior name usage

b499f9b

fix one test and finetuning

6378dce

fix after merge of master

798e2dc

add change log

018689d

move check for 'Reply to item' permission from Products.CMFPlone here.

1e74750

pre-commit fixes

8b09087

move test from pa.dexterity to here

63e6fbc

fix requirements

2a4b393

pre-commit-ci bot and others added 7 commits August 13, 2024 19:27

[pre-commit.ci] pre-commit autoupdate

a6d7a4d

updates: - [github.com/asottile/pyupgrade: v3.15.2 → v3.16.0](asottile/pyupgrade@v3.15.2...v3.16.0) - [github.com/PyCQA/flake8: 7.0.0 → 7.1.0](PyCQA/flake8@7.0.0...7.1.0)

Test drop five:registerPackage

c8e7340

Is that actually needed in Plone 6?

Uninstall profile: remove Discussion Item from portal_types tool.

e66c836

Uninstall profile: remove our workflow definitions.

f9c6469

Merge branch 'main' into core-addon

c5711cf

Configuring with plone/meta

f896cb4

mauritsvanrees added 2 commits August 13, 2024 21:46

Robot tests: we have no own keywords.robot file.

5ed2438

We are already loading a file with the same name from plone.app.robotframework.

gforcada requested changes Aug 19, 2024

View reviewed changes

mauritsvanrees and others added 7 commits August 19, 2024 14:42

Remove unused content_icon and content_meta_type properties from FTI.

da5b2f5

The FTIs in `plone.app.contenttypes` do not have them.

FTI: use @@view alias instead non-existing discussionitem_view.

a8f000d

Add missing icon

473224a

The `view` action of comments had no icon on Plone 6.

Add news entry

840a445

Register contenttype icon. Add upgrade step for updating the icons.

ebd965c

Update the profile version to 3000. This leaves room for upgrades on the 4.x branch (Plone 6.0).

mauritsvanrees requested a review from gforcada August 19, 2024 14:32

gforcada requested changes Aug 20, 2024

View reviewed changes

plone/app/discussion/setuphandlers.py Outdated Show resolved Hide resolved

mauritsvanrees added 3 commits August 20, 2024 10:22

Removed reference to unneeded test file that I removed yesterday.

aa64bd1

Fix hidden profile id.

7e32976

Rename profiles/to_2002 to profiles/to_3000.

98aee4a

gforcada approved these changes Aug 21, 2024

View reviewed changes

mauritsvanrees merged commit 9648d82 into main Sep 2, 2024
8 of 12 checks passed

mauritsvanrees deleted the core-addon branch September 2, 2024 14:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Turn this into a core addon #211

Turn this into a core addon #211

jensens commented May 19, 2023 •

edited by mauritsvanrees

Loading

This comment was marked as resolved.

jensens commented May 19, 2023

gforcada commented May 20, 2023

gforcada commented May 20, 2023

erral commented May 20, 2023

ericof commented May 21, 2023

gforcada commented May 23, 2023

jensens commented May 21, 2024

jensens commented May 27, 2024

mauritsvanrees commented Aug 13, 2024

mauritsvanrees commented Aug 13, 2024

jensens commented Aug 14, 2024 •

edited

Loading

gforcada commented Aug 19, 2024

gforcada Aug 19, 2024

mauritsvanrees Aug 19, 2024

mauritsvanrees Aug 19, 2024

mauritsvanrees Aug 19, 2024

mauritsvanrees Aug 19, 2024

mauritsvanrees commented Sep 2, 2024

Turn this into a core addon #211

Turn this into a core addon #211

Conversation

jensens commented May 19, 2023 • edited by mauritsvanrees Loading

This comment was marked as resolved.

jensens commented May 19, 2023

gforcada commented May 20, 2023

gforcada commented May 20, 2023

erral commented May 20, 2023

ericof commented May 21, 2023

gforcada commented May 23, 2023

jensens commented May 21, 2024

jensens commented May 27, 2024

mauritsvanrees commented Aug 13, 2024

mauritsvanrees commented Aug 13, 2024

jensens commented Aug 14, 2024 • edited Loading

gforcada commented Aug 19, 2024

gforcada Aug 19, 2024

Choose a reason for hiding this comment

mauritsvanrees Aug 19, 2024

Choose a reason for hiding this comment

mauritsvanrees Aug 19, 2024

Choose a reason for hiding this comment

mauritsvanrees Aug 19, 2024

Choose a reason for hiding this comment

mauritsvanrees Aug 19, 2024

Choose a reason for hiding this comment

mauritsvanrees commented Sep 2, 2024

jensens commented May 19, 2023 •

edited by mauritsvanrees

Loading

jensens commented Aug 14, 2024 •

edited

Loading