Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trigger missing hooks when granting or revoking super admin permissions #20

Open
wants to merge 1 commit into
base: master
from

Conversation

@felixarntz
Copy link

felixarntz commented Feb 14, 2018

When granting or revoking super admin permissions in WordPress, hooks are triggered which are missing in this implementation. By calling grant_super_admin() or revoke_super_admin() respectively, the following actions are triggered:

  • grant_super_admin (before changes, always executed)
  • granted_super_admin (after changes, executed only on success)
  • revoke_super_admin (before changes, always executed)
  • revoked_super_admin (after changes, executed only on success)

This PR adds the hooks so that plugin functionality integrating with these hooks still works correctly.

@schlessera
Copy link
Member

schlessera commented Feb 14, 2018

Logic seems not sound yet. Tests throw a notice on line 210 (https://github.com/wp-cli/super-admin-command/pull/20/files#diff-d6a2d26b46771bf2fec60753e981f488R210) because it wants to trigger a removed_xxx action on a non-existent login.

@felixarntz
Copy link
Author

felixarntz commented Feb 15, 2018

I'll get back to this soon, to fix the issues.

@danielbachhuber
Copy link
Member

danielbachhuber commented Mar 20, 2018

@felixarntz Still planning to pick this up?

@schlessera
Copy link
Member

schlessera commented Jul 31, 2018

@felixarntz Are you still up for this?

@felixarntz
Copy link
Author

felixarntz commented Jul 31, 2018

@schlessera I don't have enough capabilities at the moment, but generally yes. I'll make a note to come back to it at some point the next month.

@schlessera
Copy link
Member

schlessera commented Dec 12, 2018

Just a gentle ping to see if you're still up for this, @felixarntz... :)

@felixarntz
Copy link
Author

felixarntz commented Dec 13, 2018

@schlessera Thanks for reminding me. It's not a big one, but I'll need a bit more time to tackle this. Should be able to between Christmas and New Years. :)

@schlessera
Copy link
Member

schlessera commented Apr 1, 2019

@felixarntz, a fresh ping for the fresh year... ;)

@felixarntz
Copy link
Author

felixarntz commented Apr 8, 2019

@schlessera Ah totally forgot about this, sorry. At the moment it's unlikely I can get to it in foreseeable future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.