gocodebox / lifterlms-rest

LifterLMS REST API Feature Plugin
6 stars 7 forks source link

deprecated `llms_user_removed_from_membership_level` action hook #257

Closed pondermatic closed 2 years ago

pondermatic commented 2 years ago

Description

Replaced deprecated llms_user_removed_from_membership_level action hook with llms_user_removed_from_membership.

How has this been tested?

Screenshots

Types of changes

Checklist:

eri-trabiccolo commented 2 years ago

@pondermatic I see this needs to be done but actually the two hooks are not equivalent, as the deprecated one is fired after the student has been removed from a membership level, while the replacement is fired before. Shall we fire it after - requires a change in the core code base, I mean shoild this line: https://github.com/gocodebox/lifterlms/blob/6.0.0-alpha.1/includes/models/model.llms.student.php#L1553 be moved here: https://github.com/gocodebox/lifterlms/blob/6.0.0-alpha.1/includes/models/model.llms.student.php#L1573 is it safe?

pondermatic commented 2 years ago

@eri-trabiccolo, that's a good point. The enrollment.updated webhook would trigger before the user is unenrolled from the membership's courses. There is a small risk that the server that receives the webhook payload may immediately request the student's course enrollments before the user is unenrolled from the membership's courses. The list of enrollments would be wrong.

@thomasplevy, do you agree that we should move the do_action( "llms_user_removed_from_{$post_type}" ) from before the code that unenrolls the student from the membership's courses to after?

thomasplevy commented 2 years ago

Yes