plone / plone.api

The Plone API
https://6.docs.plone.org/plone.api
Other
89 stars 53 forks source link

revoke_roles documentation wrong #446

Open jensens opened 3 years ago

jensens commented 3 years ago

Documentation says:

Arguments username and user are mutually exclusive. You can either set one or the other, but not both. if username and user are not given, the authenticated member will be used. https://docs.plone.org/develop/plone.api/docs/api/user.html#plone.api.user.revoke_roles

But if I omit the user and username like so: api.user.revoke_roles(obj=item, roles=["Owner"])

I got an

...
  Module kup.tfv.db.gfk.einzel, line 161, in action_add
  Module <decorator-gen-41>, line 2, in revoke_roles
  Module plone.api.validation, line 81, in wrapped
  Module <decorator-gen-40>, line 2, in revoke_roles
  Module plone.api.validation, line 116, in wrapped
  Module plone.api.user, line 414, in revoke_roles
  Module <decorator-gen-31>, line 2, in get
  Module plone.api.validation, line 116, in wrapped
  Module <decorator-gen-30>, line 2, in get
  Module plone.api.validation, line 146, in wrapped
plone.api.exc.MissingParameterError: At least one of these parameters must be supplied: userid, username.

Now, question is - is the documentation wrong or is it the implemenation?

mauritsvanrees commented 3 years ago

This happens because the function then calls get(username=username) and this requires at least one of username or userid.

I can imagine this code in revoke_roles:

if user is None:
    if username is None:
        user = get_current()
    else:
        user = get(username=username)

That seems the easiest way out of this.

I do wonder if it makes sense to be able to revoke roles from yourself...

jensens commented 3 years ago

Yes, it does make sense in my case. I remove the Owner role after adding content (in a subscriber) as manager (with plone.api.env.adopt_roles).