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

Simplify access/user checks #10

Open
ashleyfae opened this issue Apr 20, 2017 · 11 comments
Open

Simplify access/user checks #10

ashleyfae opened this issue Apr 20, 2017 · 11 comments

Comments

@ashleyfae
Copy link
Member

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' );
@ashleyfae ashleyfae added this to the 2.1.4 milestone Apr 20, 2017
@ashleyfae
Copy link
Member Author

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).

@pippinsplugins
Copy link
Contributor

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

@ashleyfae
Copy link
Member Author

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' )

@pippinsplugins
Copy link
Contributor

That's sad!

@ashleyfae
Copy link
Member Author

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.

@pippinsplugins
Copy link
Contributor

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).

@ashleyfae
Copy link
Member Author

I like that idea 👍

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

@mindctrl
Copy link
Contributor

@nosegraze is this ready for testing?

@ashleyfae
Copy link
Member Author

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

@mindctrl
Copy link
Contributor

The restriction stuff is working great!

@mindctrl mindctrl modified the milestones: 2.1.4, 2.2 Sep 14, 2017
@mindctrl
Copy link
Contributor

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

@mindctrl mindctrl modified the milestones: 2.2, 2.3 Sep 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants