search

plone / Products.CMFPlone

The core of the Plone content management system
https://plone.org
GNU General Public License v2.0
254 stars 191 forks source link

Remove unused permission "Change portal events". #1975

Closed jensens closed 7 years ago

jensens commented 7 years ago

In Products.ATContentTypes/Products/ATContentTypes/permission.py line 25 there is a permission Change portal events defined.

It was introduced with ATContentTypes, in change log `2004-06-13 02:06 tiran.

Its usage was removed in svn revision r104169.

But all our workflows are still managing this permission.

Lets get rid of it. Ocurrences in

Further:

kakshay21 commented 7 years ago

I'm on it @jensens I didn't find the occurence in plone.app.workflow and Products.CMFPlone

jensens commented 7 years ago

My findings in a recent buildout.coredev are ..

... for Change portal events:

plone.app.discussion/plone/app/discussion/profiles/default/workflows/comment_one_state_workflow/definition.xml:4: <permission>Change portal events</permission>
plone.app.discussion/plone/app/discussion/profiles/default/workflows/comment_one_state_workflow/definition.xml:13:  <permission-map name="Change portal events" acquired="False">
plone.app.iterate/plone/app/iterate/profiles/test/workflows/workingcopy_workflow/definition.xml:11:    <permission>Change portal events</permission>
plone.app.iterate/plone/app/iterate/profiles/test/workflows/workingcopy_workflow/definition.xml:25:        <permission-map name="Change portal events" acquired="False">
plone.app.iterate/plone/app/iterate/profiles/test/workflows/workingcopy_workflow/definition.xml:64:        <permission-map name="Change portal events" acquired="False">
Products.ATContentTypes/Products/ATContentTypes/permission.py:25:ChangeEvents = 'Change portal events'
plone.app.workflow/plone/app/workflow/tests/test_one_state_workflow.py:7:ChangeEvents = 'Change portal events'
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:8:ChangeEvents = 'Change portal events'
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:8:ChangeEvents = 'Change portal events'
Products.CMFPlone/docs/HISTORY.txt:12210:- since CMFCalendar/Event.py uses a different set of Permissions, 'Change portal events'
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/intranet_workflow/definition.xml:15: <permission>Change portal events</permission>
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/intranet_workflow/definition.xml:36:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/intranet_workflow/definition.xml:77:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/intranet_workflow/definition.xml:116:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/intranet_workflow/definition.xml:153:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/intranet_workflow/definition.xml:185:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/plone_workflow/definition.xml:17: <permission>Change portal events</permission>
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/plone_workflow/definition.xml:33:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/plone_workflow/definition.xml:64:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/plone_workflow/definition.xml:98:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/plone_workflow/definition.xml:123:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/one_state_workflow/definition.xml:12: <permission>Change portal events</permission>
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/one_state_workflow/definition.xml:24:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/simple_publication_workflow/definition.xml:15: <permission>Change portal events</permission>
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/simple_publication_workflow/definition.xml:36:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/simple_publication_workflow/definition.xml:74:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/profiles/default/workflows/simple_publication_workflow/definition.xml:107:  <permission-map name="Change portal events"
Products.CMFPlone/Products/CMFPlone/tests/testSiteAdminRole.py:139:            'Change portal events':                                     1,
egrep: mockup/mockup/node_modules: No such file or directory
plone.app.upgrade/plone/app/upgrade/v41/alphas.py:57:        'Change portal events',

and for the ChangeEvents variable:

Products.ATContentTypes/Products/ATContentTypes/permission.py:25:ChangeEvents = 'Change portal events'
Products.ATContentTypes/Products/ATContentTypes/permission.py:26:setDefaultRoles(ChangeEvents, ('Manager', 'Site Administrator', 'Owner',))
plone.app.workflow/plone/app/workflow/tests/test_one_state_workflow.py:7:ChangeEvents = 'Change portal events'
plone.app.workflow/plone/app/workflow/tests/test_one_state_workflow.py:109:    def testChangeEventsIsNotAcquiredInPublishedState(self):
plone.app.workflow/plone/app/workflow/tests/test_one_state_workflow.py:110:        # since r104169 event content doesn't use `ChangeEvents` anymore...
plone.app.workflow/plone/app/workflow/tests/test_one_state_workflow.py:115:        self.assertTrue(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_one_state_workflow.py:118:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_one_state_workflow.py:121:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_one_state_workflow.py:124:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_one_state_workflow.py:127:        self.assertTrue(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_one_state_workflow.py:130:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:8:ChangeEvents = 'Change portal events'
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:217:        self.assertTrue(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:220:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:223:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:226:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:229:        self.assertTrue(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:232:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:238:        self.assertTrue(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:241:        self.assertTrue(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:244:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:247:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:250:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:253:        self.assertTrue(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_simple_publication_workflow.py:256:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:8:ChangeEvents = 'Change portal events'
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:345:        self.assertTrue(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:348:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:351:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:354:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:359:        self.assertTrue(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:362:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:365:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:368:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:373:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:376:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:379:        self.assertTrue(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:382:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:389:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:392:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:395:        self.assertFalse(checkPerm(ChangeEvents, self.ni))
plone.app.workflow/plone/app/workflow/tests/test_plone_workflow.py:398:        self.assertFalse(checkPerm(ChangeEvents, self.ni))

There may be some occurrences masked by a different variable, but running tests will probably show.

Regarding plone.app.upgrade - thats complicated. I would try to not touch old steps, but introduce a new step removing the permission from all installed workflow states in portal_workflow. I can't tell you exactly how this works, because I need to dig into the code there as well before.

kakshay21 commented 7 years ago

Do I have to remove all test functions on "change portal events" ? or do we have to modify those functions? @jensens

jensens commented 7 years ago

If we remove the permission the specific tests can be removed as well.

gforcada commented 7 years ago

@kakshay21 I already merged the one on plone.app.workflow 👍 one pull request less to merge!

jensens commented 7 years ago

Just to summarize, these are the PRs involved with need of a merge

Needs check:

kakshay21 commented 7 years ago

we have little trouble with 5.0 5.1 is fine BTW http://jenkins.plone.org/job/pull-request-5.1/1523/

2003 is good to merge

@jensens @gforcada

gforcada commented 7 years ago

@kakshay21 the problem at 5.0 jenkins job is that you added there the CMFPlone pull request as well, but if you look closely, CMFPlone master branch only targets 5.1 due to some incompatible changes introduced there, so to test it again:

5.1

https://github.com/plone/Products.CMFPlone/pull/2018
https://github.com/plone/Products.ATContentTypes/pull/45
https://github.com/plone/plone.app.iterate/pull/44
https://github.com/plone/plone.app.upgrade/pull/115
https://github.com/plone/plone.app.discussion/pull/120

5.0

https://github.com/plone/Products.CMFPlone/pull/2023
https://github.com/plone/Products.ATContentTypes/pull/48
https://github.com/plone/plone.app.iterate/pull/44
https://github.com/plone/plone.app.upgrade/pull/115
https://github.com/plone/plone.app.discussion/pull/120
https://github.com/plone/plone.app.workflow/pull/13

Lastly, a good thing is that there is quite a lot of activity in plone repositories, which unfortunately has the downside that on of your pull requests is already too old (the CMFPlone one) to be merged as it has some conflicts that need to be solved.

A good approach to mitigate this is of course to merge fast, but when that can not happen due to complexity or other factors, having the branches on plone organization helps others (in this specific case me) to help update the branches.

I will do so for the CMFPlone one so that I can resolve the conflicts and let jenkins run again.

gforcada commented 7 years ago

@jensens as for version bumps, should we rather make a minor release on all of them and instead update ATContentTypes on the versions that target 4.3 and 5.0 to add a deprecation warning (as the permission is defined there)?

gforcada commented 7 years ago

Now that I'm properly awake I get the error on 5.0: there was a missing pull request for CMFPlone, as the one that was created is only targeting 5.1.

I created the pull request already #2023