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

One level per group setting not enforced correctly at checkout #111

Open
dparker1005 opened this issue Jan 14, 2022 · 2 comments
Open

One level per group setting not enforced correctly at checkout #111

dparker1005 opened this issue Jan 14, 2022 · 2 comments

Comments

@dparker1005
Copy link
Member

dparker1005 commented Jan 14, 2022

Steps to replicate:

  1. Create a level group with multiple levels and set it so that users can only have one level from the group
  2. Check out normally for one of those levels
  3. Go to the checkout page for another level in that group
  4. Delete the dellevels parameter from the URL and reload the page
  5. Complete checkout
  6. See that you now have both levels even though you should only ever be able to have one

We try to address this kind of situation here, but it has 2 issues:

// OK, levels are added, levels are removed. Let's check once more for any conflict, and resolve them - with extreme prejudice.
$currentlevels = pmpro_getMembershipLevelsForUser( $user_id );
$currentlevelids = array();
if ( is_array( $currentlevels ) ) {
foreach ( $currentlevels as $curlevel ) {
$currentlevelids[] = $curlevel->id;
}
}
$levelsandgroups = pmprommpu_get_levels_and_groups_in_order();
$allgroups = pmprommpu_get_groups();
$levelgroupstoprune = array();
foreach ( $levelsandgroups as $curgp => $gplevels ) {
if ( array_key_exists( $curgp, $allgroups ) && $allgroups[ $curgp ]->allow_multiple_selections == 0 ) { // we only care about groups that restrict to one level within it
$conflictlevels = array();
foreach ( $gplevels as $curlevel ) {
if ( isset( $curlevel->id ) && in_array( $curlevel->id, $currentlevelids ) ) {
$conflictlevels[] = $curlevel->id;
}
}
if ( count( $conflictlevels ) > 1 ) {
$levelgroupstoprune[] = $conflictlevels;
}
}
}
if ( count( $levelgroupstoprune ) > 0 ) { // we've got some resolutions to do.
foreach ( $levelgroupstoprune as $curgroup ) {
foreach ( $curgroup as $idtodel ) {
pmpro_cancelMembershipLevel( $idtodel, $user_id, 'change' );
}
}
}

Issue 1: On lines 523 and 524, $curlevel->id does not exist. All instances of $curlevel->id should instead just be $curlevel.

Issue 2: After fixing this, going through the replication steps will result in users having none of the levels from that group, including the one that they were checking out for. This is still not the behavior that we are looking for. Intuitively, users should instead only have their most recent level for the group, which in this case is the level that they are checking out for.

@sc0ttkclark
Copy link

  • Issue 1 makes total sense that we should switch that to match the response of pmprommpu_get_levels_and_groups_in_order
  • Issue 2 would be solved if when we build the list of levels here to also add a conditional check that the level is not in $pmpro_checkout_levels array (which is a list of level objects btw)

@kimwhite
Copy link

I have recreated this on a Dev site. Reported by a user in ticket 401973.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants