restrictcontentpro/restrict-content

Simplify access/user checks

Opened this issue · 11 comments

Let's get rid of this monstrosity:

function rcCheckUser() {
	if ( current_user_can( 'read' ) ) {
		if ( current_user_can( 'edit_posts' ) ) {
			if ( current_user_can( 'upload_files' ) ) {
				if ( current_user_can( 'moderate_comments' ) ) {
					if ( current_user_can( 'switch_themes' ) ) {
						//do nothing here for admin
					} else {
						add_filter( 'the_content', 'rcMetaDisplayEditor' );
					}
				} else {
					add_filter( 'the_content', 'rcMetaDisplayAuthor' );
				}
			} else {
				add_filter( 'the_content', 'rcMetaDisplayContributor' );
			}
		} else {
			add_filter( 'the_content', 'rcMetaDisplaySubscriber' );
		}
	} else {
		add_filter( 'the_content', 'rcMetaDisplayNone' );
	}
}

add_action( 'loop_start', 'rcCheckUser' );

I was planning on switching to just user_can() like we do in RCP, but the one thing about that change is it would remove the tiered system that's in place now (i.e. an "Author" can view "Contributor" and "Subscriber" content).

The tier would still work I believe. user_can( 'Contributor )` evaluates as true for authors and higher.

Nope. :( I thought it did too but I tried it earlier and it doesn't. For example, this returns false for an admin: user_can( get_current_user_id(), 'contributor' )

That's sad!

So is that something we want to preserve? It's not how it currently works in RCP and I did notice a few users thinking it was a bug. If we do want to preserve it we should probably at least clarify that's how it works on the restriction screen.

How about not preserving and adding an upgrade path of some kind. I don't think this is something we'll ever be able to "not break" if we want to ever support it.

I propose we update it to use user_can() and, after installing the update, show the admin a notice with information about the change. It can link to a doc on how to update the content to work (or perhaps run an upgrade routine to update automatically).

I like that idea 👍

Here's a PR I started with the new access checks: #12

@nosegraze is this ready for testing?

@mindctrl The restriction part is ready for a test. The PR doesn't yet have any kind of backwards compatibility or admin notice though.

The restriction stuff is working great!

Punting this to 2.3 to give a little more time for testing, the admin notice, and thoughts to back compat.