gocodebox / lifterlms

LifterLMS, a WordPress LMS Solution: Easily create, sell, and protect engaging online courses.
https://lifterlms.com
GNU General Public License v3.0
174 stars 134 forks source link

When the Topic is set to "Enrollment created" in WordPress Dashboard > LifterLMS > Settings > REST API > Webhooks, the payload only contains information on 1 course (even though there are more than one enrollments that occurred). #2568

Open dominiquemariano opened 8 months ago

dominiquemariano commented 8 months ago

Reproduction Steps

  1. Make at LifterLMS is the only plugin installed.
  2. Login to the WordPress site as an administrator.
  3. Create a membership called Membership 1.
  4. Create three courses called Course 1, Course 2, and Course 3.
  5. Open Membership 1 in the backend and add Course 1, Course 2, and Course 3 to Membership Settings > Auto Enrollment.
  6. Add a free access plan to Membership 1.
  7. Create a webhook in WordPress Dashboard > LifterLMS > Settings > REST API > Webhooks and set Topic to Enrollment created.
  8. Use a free service like webhook.site to generate the Delivery URL and to view the payload.
  9. Create a student account in WordPress Dashboard > Users > Add New.
  10. Log in as that student you created, and enroll on Membership 1 while logged in as that student.
  11. Got to webhook.site to look at the payload.

Expected Behavior

The payload should contain information about Course 1, Course 2, and Course 3.

Actual Behavior

The payload contains information about Course 1.

This issue has be recreated:

brianhogg commented 7 months ago

@ideadude Digging into this, the actual behavior should likely be to have three webhooks fired (once for each course), plus potentially another webhook for [ student_id, membership_id ].

An attempt is being made to queue up a webhook for delivery for each course, but LLMS_REST_Webhook::should_deliver() is returning false for the second and third courses. This is because when an attempt to queue the webhook is made, the first argument is added to the processed array (in this case, the student ID). is_already_processed() then returns true for the second and third hooks, with [ student_id, course_id ] as the arguments, and are rejected.

We could make this smarter by looking at all of the arguments to decide if it's already been enqueued instead of just the first one, depending on the resource and event of a particular webhook. The current "is already processed" check could also be causing some random uniqueness issues, ie. a webhook is enqueued for the first argument of a student (user) id of 5 when a different webhook is attempted with the first argument of 5 but for a different type of object.