folder_contents icons missing in classic UI after migration Products.CMFCore#3838

[szakitibi]

Fixes plone/Products.CMFPlone#3838, which is still an issue for Plone 5.2 sites migrated to Classic Plone 6.0.11.

What happens:

The two new icons plone-rearrange and plone-selection are not imported for existing Plone 5 sites migrated to Plone 6. As a result, the folder_contents view produces [Products.CMFPlone.browser.icons:103][waitress-2] Icon resolver lookup of 'plone-selection' failed, fallback to Plone icon. messages and fails to lookup the icons.

What should happen:

The icons are imported and the folder_contents view does not produce the icon lookup errors.

Steps to reproduce:

• go to https://github.com/collective/minimalplone5 and clone the repo
• run the buildout and create a Plone site
• go to localhost:8080/Plone/folder_contents, the icons look O.K.

• replace 5-latest with 6-latest in buildout.cfg and requirements.txt
• install the new rewquirements and run the buildout again
• open localhost:8080 and run the site upgrade
• go again to localhost:8080/Plone/foldercontents -> the icons will fail

---

!Important The problem only exists if the site is upgraded from Plone 5!

The plone.staticresources lacks an upgrade step to import the changes for the contents of plone/staticresources/profiles/default/registry/icons_plone.xml. It has been imported originally with upgrade step 203, but since Apr 22, 2021 there has been changes in the icons_plone.xml file, e.g. on Mar 23, 2022 - see commit 05ab4fd.

This PR's is created to import the missing icons.

[mister-roboto]

@szakitibi thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

[szakitibi]

Should we include a purge=False in anticipation of newly created Plone 6 sites which already have this record?

[yurj]

Should we include a purge=False in anticipation of newly created Plone 6 sites which already have this record?

Yes.

https://5.docs.plone.org/develop/addons/components/genericsetup.html#the-purge-attribute

[davisagli]

purge=False doesn't have any effect for a single value like this. It is used for sequence values (e.g. a list) where existing items in the should not be purged.

[mister-roboto]

Tibor Szakmany your emails are not known to GitHub and thus it is impossible to know if you have signed the Plone Contributor Agreement, which is required to merge this pull request.

Learn about the Plone Contributor Agreement: https://plone.org/foundation/contributors-agreement How to add more emails to your GitHub account: https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/adding-an-email-address-to-your-github-account

[szakitibi]

purge=False doesn't have any effect for a single value like this. It is used for sequence values (e.g. a list) where existing items in the should not be purged.

There is not a lot of documentation on this topic, I've just guessed, but indeed it does not work. Since GS is not capable of this, I've refactored the upgrade to use a python handler instead. The handler will apply the changes only if the records are missing.

[gforcada]

Suggested change

	- Fix missing folder_contents icons in classic UI after migration from Plone 5 - see also plone/Prodcuts.CMFPlone#3838
	[szakitibi]
	- Fix missing folder_contents icons in classic UI after migration from Plone 5 - see also plone/Prodcuts.CMFPlone#3838 [szakitibi]

It should be in a single line, otherwise it would break on pypi

[gforcada]

black complains about this file, you can use tox -e lint to fix that and commit it 👍🏾