Add specific capability for granular assignment.

[ethanclevenger91]

Also assign cap to administrators, who were already the only ones able to edit this via the manage_options capability.

[ethanclevenger91]

Thanks @jeffpaul. The only thing I'll say about this PR is that it currently fetches an option on every load because I don't really trust putting capability assignments behind register_activation_hook. The option is autoloaded so it shouldn't add any queries, but if you guys have preferences either way, I can update accordingly.

[jeffpaul]

Resolves #28

[adamsilverstein]

Hey @ethanclevenger91 - thanks for the PR!

This looks like it will work...

I don't really trust putting capability assignments behind register_activation_hook

Any reason in particular you don't trust this approach?

It would be nice of the plugin to also remove the custom capability when it is deactivated which makes me lean towards adding on activation. Thoughts?

[ethanclevenger91]

@adamsilverstein In a very edge case, it could be circumvented through directly editing the active plugins value in wp_options. The idea of setting permissions being tied to a dashboard action of any kind has always seemed flimsy, but I can certainly update to use register_activation_hook if preferred. In that case, yeah, it would make sense to add the permission removal as well.

[adamsilverstein]

I don't have a strong opinion either way and didn't find anything in our standards or prior art. I'm curious what @helen thinks.

[helen]

I don't know that I have a very strong opinion but to me it makes sense to tie it to activation and clean up on deactivation.

it could be circumvented through directly editing the active plugins value in wp_options

I think at the point of this happening you probably have a lot of other problems as well :) which isn't to say it's a dismissable concern, just that I wouldn't take that as the primary decider for an approach.

[adamsilverstein]

@ethanclevenger91 Based on Helen's feedback, can you please update this PR to add/remove the custom capabilities during plugin activation/deactivation?

[ethanclevenger91]

@adamsilverstein will do

[ethanclevenger91]

@adamsilverstein updated, and also moved to main plugin file, since it didn't really feel like it fell under the same purview as the other contents of the admin.php file.

[adamsilverstein]

@ethanclevenger91 thanks for updating this.

I am wondering about one potential issue with adding the capability in the plugin activation hook: users who already have the plugin activated will not have the capability added and therefore won't be able to access the Ads-txt admin page. (oops, apologies for not catching this earlier when I suggested moving to activation)

Two possible solutions for this issue would be:

1. Move adding the capability back to admin_init as you had it previously (while maintaining the removal in plugin deactivation to clean up)
2. Add an upgrade notice informing users they need to deactivate/reactivate the plugin after upgrading.

My preference at this point is 1 since that doesn't require user intervention. Perhaps we can have a plan to change to adding the capability during plugin activation in a future release, once we are sure existing users already have it.

[adamsilverstein]

I pushed some updates:

• add hook on admin_init, also keeping hook on activation - otherwise capability wasn't added in time on activation for menu to appear - requiring a second page refresh.
• switch check to avoid re-adding capability to use has_cap.
• move the capability name to a plugin constant.

[adamsilverstein]

Nice improvement @ethanclevenger91 - thanks!

[ethanclevenger91]

@adamsilverstein thanks for those improvements! I have criminally overlooked has_cap, great idea. Sorry I wasn't able to get to this myself sooner.

[helen]

Looks good!