strangerstudios / paid-memberships-pro

WordPress membership plugin to restrict access to content and charge recurring subscriptions using Stripe, PayPal, and more. Fully open source. 100% GPL.
https://www.paidmembershipspro.com
Other
460 stars 357 forks source link

pmpro_account shortcode atts causing issues #210

Open crabilld opened 9 years ago

crabilld commented 9 years ago

If I use this on the account page: [pmpro_account sections='membership,profile,invoices,links'] or [pmpro_account sections='membership,invoices'] instead of [pmpro_account] then it will no longer recognize it as the account page or assign the pmpro-account class to the body tag.

To fix that, line 115 in /includes/init.php needs to be changed from if(!empty($post->post_content) && strpos($post->post_content, "[pmpro_" . $pmpro_page_name . "]") !== false) to if(!empty($post->post_content) && strpos($post->post_content, "[pmpro_" . $pmpro_page_name) !== false)

Perhaps doing that would introduce a different bug... for instance if there is both a shortcode for pmpro_member and pmpro_membership.


Also, if I enter something like this into the content:

[pmpro_account]
[pmpro_account sections='profile']

then all sections will appear twice. Basically, if the sections attr is not in the shortcode the first time, then it cannot be used in subsequent calls to the shortcode.

So to accomplish the above, currently the only way to do that would be:

[pmpro_account sections='membership,profile,invoices,links']
[pmpro_account sections='profile']

As a side note, it would be nice if the account page was set from the Pages settings page, rather than by checking for a shortcode in the content. Right now, any page with [pmpro_account] in the content would be considered the account page, from a CSS point of view. And I myself would prefer defining the shortcode in my template file anyway, rather than the content, but I'm probably a rare case.

strangerstudios commented 9 years ago

If we change line 115 like that, we might load the pmpro_account preheader on pages that aren't the real account page.

The real need here is to be able to change the "real" pmpro_account page to only show certain sections. It makes sense to change the shortcode like you did. And I think in the case of the account page, we should just look for the ID from the page settings rather than the shortcode.

So I think the logic should be (in English) "if I see [pmpro_account] by itself OR this is the page set in the account page" then load the account preheader.

The problem, and part of the reason I check for the shortcode vs the page ID is that when running here on init, WordPress doesn't know what page it is yet. We can't do is_page checks.

I can't recall off the top of my head, but there were reasons we're doing this on init instead of something like wp or template_redirect where we would be able to check the page.

I'm open to suggestions to fix this that take into account the quirks of PMPro. Perhaps the account preheader can run on template_redirect, but the checkout one we'll still run on init. That probably won't impact anything.

crabilld commented 9 years ago

Sorry, just got back from a trip. I tried looking through Buddypress' code to see how they manage page settings, since they typically follow best-practices in code. There's a lot going on there, and I didn't fully grasp what they're doing, but it looks like they essentially load the body classes when the plugin loads, rather than hook them into an action, so this code runs very early. The downside of doing this is that the current page ID isn't available when they determine which body class to show, and I think that, rather than using info from WP, they utilize the url to determine what page they're looking at. I'm not completely sure about that last part.

Anyway, that would be a major deviation from the current setup of PMPro, and probably impractical. You load the body classes much later, which means that you do have the current page's ID available when you decide what body classes to show. Couldn't you just store the assigned page IDs in wp_options, and then use those to determine whether or not the current page is the real account page? This would mean that the body class would only show on one page on the site, regardless of whether the shortcode exists there or not. That would be my preference, because then I could style the account page without having to worry that those styles would affect other uses of the pmpro_account shortcode elsewhere on the site. The shortcode is already wrapped with the "pmpro_account" id, so I can style all uses of it without needing the global body class.

I'm not that familiar with the code, so I apologize if I'm greatly oversimplifying the issue.