Products.cmfplone: Move check_id skin script to function

[mauritsvanrees]

Part of https://github.com/plone/Products.CMFPlone/issues/1801.
The skins/plone_scripts/check_id.py script should be replaced by something else. A function in utils.py seems best.

• For Plone 5.1: add a function Products.CMFPlone.utils.check_id.
• For Plone 5.2: add the same, and remove the skin script.

The script is used in various places, so they need an update as well. Related issues for that:

• https://github.com/plone/Products.ATContentTypes/issues/60
• https://github.com/plone/Products.Archetypes/issues/118
• https://github.com/plone/Products.validation/issues/4
• https://github.com/plone/plone.app.content/issues/170

Several pull requests will need to be tested together. Let's use this issue to gather them.

---

For Plone 5.1:

https://github.com/plone/Products.CMFPlone/pull/2584
https://github.com/plone/Products.ATContentTypes/pull/62
https://github.com/plone/Products.Archetypes/pull/119
https://github.com/plone/Products.validation/pull/5
https://github.com/plone/plone.app.content/pull/172
https://github.com/plone/plone.dexterity/pull/92

Latest Jenkins for 5.1: https://jenkins.plone.org/job/pull-request-5.1/1581/

---

For Plone 5.2:

https://github.com/plone/Products.CMFPlone/pull/2583
https://github.com/plone/Products.ATContentTypes/pull/61
https://github.com/plone/Products.Archetypes/pull/119
https://github.com/plone/Products.validation/pull/5
https://github.com/plone/plone.app.content/pull/171
https://github.com/plone/plone.dexterity/pull/92

Latest Jenkins for 5.2: https://jenkins.plone.org/job/pull-request-5.2/1541/

[mauritsvanrees]

Note: it is tricky to have this running and get all tests to pass in both 5.1 and 5.2. A trick you do to test a corner case in one package may break a test in a different package.

What does not help, is that both Products.validation and plone.app.content have their own code for this. On current master, they both first look for context.check_id, and use a (different) simple fallback method if this is not there. This should normally find the CMFPlone script on 5.1. But in tests it may not be there, especially if you use a dummy class for testing.

In the PRs I have changed this to use the check_id attribute/script, then try the new check_id function, and then use the fallback. On 5.1, the function simply calls the script, which will then not be there in these cases, because otherwise the code in Products.validation and plone.app.content would have not even called the function.

It could help to move the fallback in both cases to the new function. But Products.validation might theoretically be used outside of Plone, in pure Zope2. So we may need to keep that fallback.

So there may be room for simplification.

Anyway, if Jenkins passes on both 5.1 and 5.2, it should be safe to merge everything.

[gforcada]

@mauritsvanrees wouldn't be a solution to move the skin script to an AT related package? As Archetypes related packages are not being ported to python 3 and are on the way to get deprecated and out of core, just pushing these scripts there while cleaning the other core packages that will stay might be simpler... I think @reinhardt used this approach, or thought about doing it

[mauritsvanrees]

The check_id script is also used by plone.app.content. When creating content, we do still have to do various checks on the id.

[mauritsvanrees]

This is ready for review.
Jenkins on 5.1 is happy.
Jenkins on 5.2 complains, but I think that is a general problem with the 5.2 Jenkins job since a few days. There are no test failures.

[mauritsvanrees]

Note: it is better to wait with merging until after Plone 5.1.4 has been released, which should be soonish.
But review can happen.

[ale-rt]

Also 5.2 is happy now: https://jenkins.plone.org/job/pull-request-5.2/1578/

Should we just merge all the PRs in a random order?

For Plone 5.1:

https://github.com/plone/Products.CMFPlone/pull/2584
https://github.com/plone/Products.ATContentTypes/pull/62
https://github.com/plone/Products.Archetypes/pull/119
https://github.com/plone/Products.validation/pull/5
https://github.com/plone/plone.app.content/pull/172
https://github.com/plone/plone.dexterity/pull/92

For Plone 5.2:

https://github.com/plone/Products.CMFPlone/pull/2583
https://github.com/plone/Products.ATContentTypes/pull/61
https://github.com/plone/Products.Archetypes/pull/119
https://github.com/plone/Products.validation/pull/5
https://github.com/plone/plone.app.content/pull/171
https://github.com/plone/plone.dexterity/pull/92

[mauritsvanrees]

Thanks for the review!
I will merge them all now.

[mauritsvanrees]

All are merged. CMFPlone 5.2 needed a manual merge.
A few Jenkins jobs may give failures now, but the last one should be good.
If there are problems and I don't notice them quickly enough, please ping me.

[mauritsvanrees]

The core jobs of Jenkins on 5.1 and 5.2 are passing, so all is well.