Skip to content

Implement per-plan role mappings feature #2

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Implement per-plan role mappings feature #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions wordpress/wp-content/plugins/memberful-wp/src/acl.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,11 +295,6 @@ function memberful_can_user_access_post( $user, $post ) {
return $user ? true : false;
}

// Grant access if user has a subscription and post or one of its terms allows access with any subscription
if ( memberful_wp_post_viewable_by_any_subscriber( $post, $terms_for_post )) {
return !empty( $user_subs );
}

// Get the set of restrictions for this post
$post_acl = get_post_meta( $post, 'memberful_acl', TRUE );
$plans_for_post = isset( $post_acl['subscription'] ) ? $post_acl['subscription'] : array();
Expand Down Expand Up @@ -331,7 +326,12 @@ function memberful_can_user_access_post( $user, $post ) {
$user_products = $user ? array_keys( memberful_wp_user_products( $user )) : array();
$product_intersect = array_intersect( $products_for_post, $user_products );

// If there are no specific plan/product restrictions, fall back to the broad rules
if (( empty( $plans_for_post ) ) && ( empty( $products_for_post ))) {
// Grant access if user has a subscription and post or one of its terms allows access with any subscription
if ( memberful_wp_post_viewable_by_any_subscriber( $post, $terms_for_post )) {
return !empty( $user_subs );
}
// Grant access if no restrictions
return true;
} elseif ( ! empty( $plan_intersect ) || ! empty( $product_intersect )) {
Expand Down
89 changes: 88 additions & 1 deletion wordpress/wp-content/plugins/memberful-wp/src/admin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,9 @@ function memberful_wp_advanced_settings() {
$allowed_roles = memberful_wp_roles_that_can_be_mapped_to();
$current_active_role = memberful_wp_role_for_active_customer();
$current_inactive_role = memberful_wp_role_for_inactive_customer();
$subscription_plans = memberful_subscription_plans();
$current_mappings = memberful_wp_get_all_plan_role_mappings();
$use_per_plan_roles = memberful_wp_use_per_plan_roles();

if ( ! empty( $_POST ) ) {
if ( isset( $_POST['role_mappings']['active_customer'] ) && array_key_exists( $_POST['role_mappings']['active_customer'], $allowed_roles ) ) {
Expand All @@ -386,17 +389,44 @@ function memberful_wp_advanced_settings() {
$new_inactive_role = sanitize_text_field($_POST['role_mappings']['inactive_customer']);
}

// Save active/inactive role mappings
if ( isset($new_active_role) && isset($new_inactive_role) ) {
update_option( 'memberful_role_active_customer', $new_active_role );
update_option( 'memberful_role_inactive_customer', $new_inactive_role );

memberful_wp_update_customer_roles( $current_active_role, $new_active_role, $current_inactive_role, $new_inactive_role );

Memberful_Wp_Reporting::report( __('Settings updated') );
Memberful_Wp_Reporting::report( __('Active/Inactive role settings updated') );
} else {
Memberful_Wp_Reporting::report( __('The roles you chose aren\'t in the list of allowed roles'), 'error' );
}

// Save per-plan role mappings
$new_use_per_plan_roles = isset( $_POST['use_per_plan_roles'] );
memberful_wp_set_use_per_plan_roles( $new_use_per_plan_roles );

if ( $new_use_per_plan_roles ) {
$new_plan_mappings = array();

if ( isset( $_POST['plan_role_mappings'] ) && is_array( $_POST['plan_role_mappings'] ) ) {
foreach ( $_POST['plan_role_mappings'] as $plan_id => $role ) {
$plan_id = intval( $plan_id );
$role = sanitize_text_field( $role );

if ( ! empty( $role ) && array_key_exists( $role, $allowed_roles ) && isset( $subscription_plans[ $plan_id ] ) ) {
$new_plan_mappings[ $plan_id ] = $role;
}
}
}

update_option( 'memberful_plan_role_mappings', $new_plan_mappings );
memberful_wp_update_all_user_roles_with_plan_mappings();
Memberful_Wp_Reporting::report( __('Per-plan role mappings updated') );
} else {
// If disabling, clear mappings
update_option( 'memberful_plan_role_mappings', array() );
}

wp_redirect( memberful_wp_plugin_advanced_settings_url() );
}

Expand All @@ -412,6 +442,9 @@ function memberful_wp_advanced_settings() {
),
),
'available_roles' => $allowed_roles,
'subscription_plans' => $subscription_plans,
'current_mappings' => $current_mappings,
'use_per_plan_roles' => $use_per_plan_roles,
);
memberful_wp_render( 'advanced_settings', $vars );
}
Expand Down Expand Up @@ -651,3 +684,57 @@ function memberful_wp_global_marketing() {
)
);
}

function memberful_wp_plan_role_mappings() {
$allowed_roles = memberful_wp_roles_that_can_be_mapped_to();
$subscription_plans = memberful_subscription_plans();
$current_mappings = memberful_wp_get_all_plan_role_mappings();
$use_per_plan_roles = memberful_wp_use_per_plan_roles();

if ( ! empty( $_POST ) && isset( $_POST['save_plan_role_mappings'] ) ) {
if ( ! memberful_wp_valid_nonce( 'memberful_options' ) ) {
return;
}

// Update the per-plan roles setting
$use_per_plan_roles = isset( $_POST['use_per_plan_roles'] );
memberful_wp_set_use_per_plan_roles( $use_per_plan_roles );

if ( $use_per_plan_roles && isset( $_POST['plan_role_mappings'] ) ) {
$new_mappings = array();

foreach ( $_POST['plan_role_mappings'] as $plan_id => $role ) {
$plan_id = intval( $plan_id );
$role = sanitize_text_field( $role );

// Only save if the role is valid and the plan exists
if ( ! empty( $role ) && array_key_exists( $role, $allowed_roles ) && isset( $subscription_plans[ $plan_id ] ) ) {
$new_mappings[ $plan_id ] = $role;
}
}

update_option( 'memberful_plan_role_mappings', $new_mappings );

// Update all existing users with the new role mappings
memberful_wp_update_all_user_roles_with_plan_mappings();

Memberful_Wp_Reporting::report( __( 'Plan role mappings updated successfully', 'memberful' ) );
} else {
// Clear all mappings if per-plan roles are disabled
update_option( 'memberful_plan_role_mappings', array() );
Memberful_Wp_Reporting::report( __( 'Per-plan roles disabled', 'memberful' ) );
}

wp_redirect( memberful_wp_plugin_plan_role_mappings_url() );
return;
Comment on lines +728 to +729
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add exit after redirect to prevent further execution.

Line 728 performs a redirect but doesn't exit, which means the script continues to execute and render the view. This can lead to unexpected behaviour and wasted resources.

Apply this diff:

     wp_redirect( memberful_wp_plugin_plan_role_mappings_url() );
-    return;
+    exit;
   }
🤖 Prompt for AI Agents 
In wordpress/wp-content/plugins/memberful-wp/src/admin.php around lines 728 to
729, the code calls wp_redirect(...) and returns but does not stop execution;
after performing the redirect you must immediately terminate script execution to
prevent further rendering or logic from running. Modify the flow so that after
the wp_redirect(...) call you call exit(); (or die();) to stop execution
(keeping the existing return/remove as appropriate) so no additional output or
processing occurs after the redirect.

}

$vars = array(
'subscription_plans' => $subscription_plans,
'available_roles' => $allowed_roles,
'current_mappings' => $current_mappings,
'use_per_plan_roles' => $use_per_plan_roles,
);

memberful_wp_render( 'plan_role_mappings', $vars );
}
4 changes: 4 additions & 0 deletions wordpress/wp-content/plugins/memberful-wp/src/metabox.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ function memberful_wp_save_postdata( $post_id ) {
memberful_wp_set_post_available_to_any_registered_users( $post_id, $viewable_by_any_registered_users );

$viewable_by_anybody_subscribed_to_a_plan = isset($_POST['memberful_viewable_by_anybody_subscribed_to_a_plan']) && $_POST['memberful_viewable_by_anybody_subscribed_to_a_plan'] === '1';
// Enforce mutual exclusivity at save time: if specific plans are chosen, do not allow broad access
if ( !empty($subscription_plan_ids) ) {
$viewable_by_anybody_subscribed_to_a_plan = false;
}
Comment on lines +119 to +122
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify mutual exclusivity is also enforced for term metadata.

The enforcement of mutual exclusivity between specific plans and broad access is correct for posts. However, the memberful_wp_save_term_metadata function (lines 161-189) does not appear to implement the same logic. Ensure consistent behavior across posts and terms.

Run the following script to confirm that term metadata save lacks this enforcement:

🏁 Script executed:

#!/bin/bash
# Description: Check if term metadata save enforces mutual exclusivity

rg -n -A 5 -B 2 'memberful_wp_save_term_metadata' wordpress/wp-content/plugins/memberful-wp/src/metabox.php | grep -A 5 'viewable_by_anybody_subscribed_to_a_plan'

Length of output: 0

🏁 Script executed:

#!/bin/bash
rg -n 'function memberful_wp_save_term_metadata' -A 20 -B 2 wordpress/wp-content/plugins/memberful-wp/src/metabox.php

Length of output: 1610

Enforce mutual exclusivity in term metadata save
In memberful_wp_save_term_metadata (metabox.php lines 179–181), insert the same check as in post saves so that if $subscription_plan_ids is non-empty you force $viewable_by_anybody_subscribed_to_a_plan = false before calling memberful_wp_set_term_available_to_anybody_subscribed_to_a_plan. This ensures specific-plan ACLs override broad-access.

🤖 Prompt for AI Agents 
In wordpress/wp-content/plugins/memberful-wp/src/metabox.php around lines
119-122, the term metadata save path needs the same mutual-exclusivity guard as
post saves: before calling
memberful_wp_set_term_available_to_anybody_subscribed_to_a_plan inside
memberful_wp_save_term_metadata, check if $subscription_plan_ids is non-empty
and, if so, set $viewable_by_anybody_subscribed_to_a_plan = false; then proceed
to call memberful_wp_set_term_available_to_anybody_subscribed_to_a_plan so
specific-plan ACLs override broad-access.

memberful_wp_set_post_available_to_anybody_subscribed_to_a_plan( $post_id, $viewable_by_anybody_subscribed_to_a_plan );

if(!isset($_POST['memberful_marketing_content']))
Expand Down
2 changes: 2 additions & 0 deletions wordpress/wp-content/plugins/memberful-wp/src/options.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ function memberful_wp_all_options() {
'memberful_error_log' => array(),
'memberful_role_active_customer' => 'subscriber',
'memberful_role_inactive_customer' => 'subscriber',
'memberful_plan_role_mappings' => array(),
'memberful_use_per_plan_roles' => FALSE,
'memberful_posts_available_to_any_registered_user' => array(),
'memberful_hide_admin_toolbar' => TRUE,
'memberful_block_dashboard_access' => TRUE,
Expand Down
72 changes: 72 additions & 0 deletions wordpress/wp-content/plugins/memberful-wp/src/roles.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,75 @@ function memberful_wp_update_customer_roles( $old_active_role, $new_active_role,
$role_decision->update_user_role( $user );
}
}

/**
* Get the role mapping for a specific plan
* @param int $plan_id The plan ID
* @return string The WordPress role for this plan, or null if not mapped
*/
function memberful_wp_get_plan_role_mapping( $plan_id ) {
$mappings = get_option( 'memberful_plan_role_mappings', array() );
return isset( $mappings[ $plan_id ] ) ? $mappings[ $plan_id ] : null;
}

/**
* Set the role mapping for a specific plan
* @param int $plan_id The plan ID
* @param string $role The WordPress role to assign
*/
function memberful_wp_set_plan_role_mapping( $plan_id, $role ) {
$mappings = get_option( 'memberful_plan_role_mappings', array() );
$mappings[ $plan_id ] = $role;
update_option( 'memberful_plan_role_mappings', $mappings );
}

/**
* Remove the role mapping for a specific plan
* @param int $plan_id The plan ID
*/
function memberful_wp_remove_plan_role_mapping( $plan_id ) {
$mappings = get_option( 'memberful_plan_role_mappings', array() );
unset( $mappings[ $plan_id ] );
update_option( 'memberful_plan_role_mappings', $mappings );
}

/**
* Get all plan role mappings
* @return array Array of plan_id => role mappings
*/
function memberful_wp_get_all_plan_role_mappings() {
return get_option( 'memberful_plan_role_mappings', array() );
}

/**
* Check if per-plan roles are enabled
* @return bool
*/
function memberful_wp_use_per_plan_roles() {
return get_option( 'memberful_use_per_plan_roles', FALSE );
}

/**
* Enable or disable per-plan roles
* @param bool $enabled
*/
function memberful_wp_set_use_per_plan_roles( $enabled ) {
update_option( 'memberful_use_per_plan_roles', $enabled );
}

/**
* Update all existing users with the new plan role mappings
*/
function memberful_wp_update_all_user_roles_with_plan_mappings() {
$mapped_users = Memberful_User_Mapping_Repository::fetch_user_ids_of_all_mapped_members();

if ( empty( $mapped_users ) ) {
return;
}

$users = get_users( array( 'fields' => 'all', 'include' => $mapped_users ) );

foreach ( $users as $user ) {
Memberful_Wp_User_Role_Decision::ensure_user_role_is_correct( $user );
}
}
4 changes: 4 additions & 0 deletions wordpress/wp-content/plugins/memberful-wp/src/urls.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ function memberful_wp_plugin_protect_bbpress_url($no_header = FALSE) {
return memberful_wp_plugin_settings_url($no_header, 'protect_bbpress');
}

function memberful_wp_plugin_plan_role_mappings_url($no_header = FALSE) {
return memberful_wp_plugin_settings_url($no_header, 'plan_role_mappings');
}

/**
* Generate a URL to the Memberful site
*
Expand Down
38 changes: 38 additions & 0 deletions wordpress/wp-content/plugins/memberful-wp/src/user/role_decision.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ public static function build(array $extra_roles_memberful_is_allowed_to_change_f
);
}

private $active_role;
private $inactive_role;
private $roles_memberful_is_allowed_to_change_from;

public function __construct( $active_role, $inactive_role, $default_role, array $extra_roles_memberful_is_allowed_to_change_from = array() ) {
$this->active_role = $active_role;
$this->inactive_role = $inactive_role;
Expand All @@ -42,6 +46,40 @@ public function role_for_user($current_role, $current_subscriptions) {
return $current_role;
}

// Check if per-plan roles are enabled
if ( memberful_wp_use_per_plan_roles() && $is_active ) {
return $this->role_for_user_with_plan_mappings( $current_subscriptions );
}

return $is_active ? $this->active_role : $this->inactive_role;
}

/**
* Determine the role for a user based on their plan subscriptions
* @param array $current_subscriptions User's current subscription plans
* @return string The role to assign
*/
private function role_for_user_with_plan_mappings( $current_subscriptions ) {
$plan_mappings = memberful_wp_get_all_plan_role_mappings();

// Find the highest priority role based on user's subscriptions
$assigned_roles = array();

foreach ( $current_subscriptions as $plan_id => $subscription_data ) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove unused variable.

The variable $subscription_data is declared but never used in the loop. Remove it to clean up the code.

Apply this diff:

-    foreach ( $current_subscriptions as $plan_id => $subscription_data ) {
+    foreach ( $current_subscriptions as $plan_id => $_subscription_data ) {
       if ( isset( $plan_mappings[ $plan_id ] ) ) {
         $assigned_roles[] = $plan_mappings[ $plan_id ];
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
foreach ( $current_subscriptions as $plan_id => $subscription_data ) {
foreach ( $current_subscriptions as $plan_id => $_subscription_data ) {
if ( isset( $plan_mappings[ $plan_id ] ) ) {
$assigned_roles[] = $plan_mappings[ $plan_id ];
}
🧰 Tools
🪛 PHPMD (2.15.0)

68-68: Avoid unused local variables such as '$subscription_data'. (undefined)

(UnusedLocalVariable)

🤖 Prompt for AI Agents 
In wordpress/wp-content/plugins/memberful-wp/src/user/role_decision.php around
line 68, the foreach declares $subscription_data but never uses it; replace the
loop to iterate keys only (e.g. use foreach (array_keys($current_subscriptions)
as $plan_id) ) so the unused variable is removed and intent is clear.

if ( isset( $plan_mappings[ $plan_id ] ) ) {
$assigned_roles[] = $plan_mappings[ $plan_id ];
}
}

// If user has multiple plans with different roles, we need to determine priority
if ( ! empty( $assigned_roles ) ) {
// For now, we'll use the first role found
// In the future, this could be enhanced with role priority logic
return $assigned_roles[0];
}

// Fallback when per-plan roles are enabled but no mapping found
// Use the inactive role to avoid granting broad access unintentionally
return $this->inactive_role;
}
Comment on lines +62 to +84
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reconsider the fallback role for unmapped plans.

When per-plan roles are enabled but no mapping is found for a user's active subscriptions, the code falls back to $this->inactive_role (line 83). This seems incorrect because the user has active subscriptions and should not receive the inactive role.

Consider using $this->active_role instead, or maintaining the user's current role to avoid unintentionally downgrading active members.

Apply this diff to use the active role as fallback:

-    // Fallback when per-plan roles are enabled but no mapping found
-    // Use the inactive role to avoid granting broad access unintentionally
-    return $this->inactive_role;
+    // Fallback when per-plan roles are enabled but no mapping found
+    // Use the active role since user has active subscriptions
+    return $this->active_role;
🧰 Tools
🪛 PHPMD (2.15.0)

68-68: Avoid unused local variables such as '$subscription_data'. (undefined)

(UnusedLocalVariable)

🤖 Prompt for AI Agents 
In wordpress/wp-content/plugins/memberful-wp/src/user/role_decision.php around
lines 62 to 84, the fallback for users with active subscriptions but no per-plan
mapping currently returns $this->inactive_role which wrongly downgrades active
members; change the fallback to return $this->active_role (or preserve the
user's current role) so active subscribers aren't assigned the inactive role,
and ensure any tests or callers expecting inactive_role are updated accordingly.

}
39 changes: 38 additions & 1 deletion wordpress/wp-content/plugins/memberful-wp/views/acl_selection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<?php foreach($subscriptions as $id => $subscription): ?>
<li>
<label>
<input type="checkbox" name="memberful_subscription_acl[]" value="<?php echo esc_attr($id); ?>" <?php checked( $subscription['checked'] ); ?>>
<input type="checkbox" class="memberful-subscription-plan" name="memberful_subscription_acl[]" value="<?php echo esc_attr($id); ?>" <?php checked( $subscription['checked'] ); ?>>
<?php echo esc_html( $subscription['name'] ); ?>
</label>
</li>
Expand All @@ -48,3 +48,40 @@
<?php endif; ?>
</div>
</div>
<script type="text/javascript">
jQuery(function($){
var anyActive = $('input[name="memberful_viewable_by_anybody_subscribed_to_a_plan"]');
var specific = $('input.memberful-subscription-plan');

function syncFromAnyActive(){
var isOn = anyActive.is(':checked');
if (isOn) {
if (specific.filter(':checked').length) {
specific.filter(':checked').prop('checked', false).trigger('change');
}
specific.prop('disabled', true);
} else {
specific.prop('disabled', false);
}
}

function syncFromSpecific(){
if (specific.filter(':checked').length > 0) {
if (anyActive.is(':checked')) {
anyActive.prop('checked', false).trigger('change');
}
}
}

anyActive.on('change', function(){
syncFromAnyActive();
});

specific.on('change', function(){
syncFromSpecific();
});

// Initial state
syncFromAnyActive();
});
</script>
Loading