Cannot delete enrollments with non 'order_X' trigger from the enrollment metabox in the order edit screen.

[eri-trabiccolo]

Reproduction Steps

• In admin open the order edit screen on an order which has a related enrollment to a course triggered by the order itself:

• Update the enrollment status to non active, e.g. Expired.

• Now go edit the course related to the order and from the students table re-enroll the student: The enrollment's trigger is now changed, as it became the "privileged" user who re-enrolled the student.

• Now click on the 'x' to unenroll (cancel) the user again, now that you can:

• Go back to the order screen, you'll see the Enrollment status and the trigger changed, also now you have the ability to delete the enrollment: the red button appears as the enrollment status is 'cancelled':

• Click on Delete Enrollment and wait for the page reload.

Expected Behavior

• I'd expect the Enrollment to be deleted, hence the meta box showing something of the sort:

Actual Behavior

• The Enrollment is not deleted, and the meta box looks like this:

The reason for this is that, in the order screen, we delete the enrollment passing "order_{$order_id}" as enrollment trigger, but the trigger has been changed, is not that one anymore.

I'm not sure this is completely intended as it was not so at the beginning (when the enrollments deletion was introduced but before the "button refactoring"). Also consider that when unenrolling, from the same meta box, we pass 'any' as trigger. We pass 'any' as trigger even when deleting the enrollment from the student's table mb in the course edit screen.

Of course keeping this different behavior for similar, but not equal, actions can make totally sense, and I actually think it does, but in this case we should not display the Delete Enrollment button in the order screen if the enrollment trigger is not the order itself. Do you agree?

Please also note that under Order Notes a new entry has been added, saying that the student enrollment records have been deleted, while they're not. The reason for this is that we don't expect a deletion failure and always log the success of that action: https://github.com/gocodebox/lifterlms/blob/3.36.1/includes/admin/post-types/meta-boxes/class.llms.meta.box.order.enrollment.php#L149

Error Messages / Logs

n/a

System Information

LifterLMS 3.36.1 (but this issue is there since the introduction of the Delete Enrollment button in v3.33.0 I guess)

[thomasplevy]

@eri-trabiccolo I think -- let me know if this makes sense -- that the enrollment metabox should only exist on the order when the order is the trigger for the enrollment.

If the order is no longer the trigger (because the admin has reactivated manually this way) then the order is no longer responsible for the enrollment and if they want the student to start paying again a new order needs to be initiated.

I can't determine if this is the right solution or not though. What do you think?

[eri-trabiccolo]

Ideally I agree but the thing is that I guess long term users are used, at least, to see the enrollment information there. Where else can they see it?

Also as of now consider that if an admin becomes the new trigger, and then cancels the enrollment, if you go change the enrollment status from the order edit screen the new trigger will be the order :D Also if someone deletes the enrollment from the order screen, as of now, he can re-add it by changing the status to Enrolled. Do we want to display the "empty" meta box in that case?

I'm not sure if removing these abilities will hurt someone...

Maybe we can leave only the enrollments info and prevent actionable elements from showing when the trigger is !== order_id, but still showing them if there's no trigger.

[thomasplevy]

I don't know...

Okay... what's the whole point of tracking triggers:

To prevent an admin from detaching an enrollment from an order accidentally... For example, if a student places an order and is enrolled and then an admin cancels the enrollment from the the student metabox the order would continue billing.

The "trigger" records what initiates the enrollment and then forces an admin to only manage the enrollment from the trigger source.

In this way, if an admin wants to cancel an enrollment they have to be on the screen where the order is so they can see that cancelling the enrollment and leaving the order "Active" could be problematic!

This kind of handcuffing though seems to be getting really really confusing.. Perhaps I'm working too hard to enforce some sort of intelligence here that doesn't really make all that much sense.

Additionally, the triggers function to ensure auto-enrollment courses are properly removed when someone is cancelled from a membership that enrolled them into courses automatically.

In the addition of deletion I missed some things... obviously... but I'm getting really lost on how to properly resolve these things...

Ultimately, if an admin wants to "break" things they should be able to but I don't want a system that allows admins to easily break things... It should be simple and it's obviously not.

I don't have a good solution here but what I KNOW IS IMPORTANT is that we don't write code that will introduce new areas of frustration and will then create more support tickets or bug reports.

Your questions:

Ideally I agree but the thing is that I guess long term users are used, at least, to see the enrollment information there. Where else can they see it?

Enrollment status should be visible here. You're right. We could mark the select box and button as "disabled" so that the status is visible (but not manageable).

Also as of now consider that if an admin becomes the new trigger, and then cancels the enrollment, if you go change the enrollment status from the order edit screen the new trigger will be the order

I'm not following this... Why would this happen? Is this another bug? I think the trigger is whatever the most recent event was... is that not the case?

Also if someone deletes the enrollment from the order screen, as of now, he can re-add it by changing the status to Enrolled. Do we want to display the "empty" meta box in that case?

I don't know. I'm confused.

I'm not sure if removing these abilities will hurt someone...

I don't know either... However, they can always get someone back into the course through a new order or enrollment. Right? So how much hurt is there actually? If we can "protect" admins from making mistakes and someone who's used to exploiting a bug needs to learn a new workflow I'm okay with it if the end result is a better UX. It has to be better though... I'm really confused by this whole thing which means we have something really poorly designed (by me) that needs fixing... I don't know.

Maybe we can leave only the enrollments info and prevent actionable elements from showing when the trigger is !== order_id, but still showing them if there's no trigger.

This seems okay. I think... I don't know