search

gocodebox / lifterlms-rest

LifterLMS REST API Feature Plugin
6 stars 7 forks source link

[Fix] Payloads are not sent for all the enrollments. #328

Closed actuallyakash closed 9 months ago

actuallyakash commented 9 months ago

Description

The array index was set to User ID instead of the Enrolment Post (Course / Membership) ID.

The $args consist of the below array:

( [0] => 97 (User ID) [1] => 2243 (Enrollment Post ID) )

That's why it was only processing the one request after marking it as processed as all the $args contain the same User ID.

Fixes https://github.com/gocodebox/lifterlms/issues/2568

How has this been tested?

Manually.

Types of changes

Bug fix

Checklist:

actuallyakash commented 9 months ago

This PR still needs work, as the incoming $args are not the same for every hook.

The Course Created hook contains two arguments but the post ID is on index 0.

and the for these hooks the post ID is at index 1.

Possible fix We need to use if statements to get the post ID from every hook and then send it to the should_deliver() method.

brianhogg commented 9 months ago

@actuallyakash I didn't see you already had a PR open and opened one here for the same issue :) Open to feedback on something I might have missed or feel free to grab the automated tests to verify your solution!

As per the comments it does appear that we may not need the is_already_processed check at all since Action Scheduler does not seem to allow scheduling two actions with the same arguments and the same webhook ID.

actuallyakash commented 9 months ago

As per the comments it does appear that we may not need the is_already_processed check at all since Action Scheduler does not seem to allow scheduling two actions with the same arguments and the same webhook ID.

Yeah, just noticed, that the Action scheduler doesn't allow duplicate actions. Nice catch.

I agree we can remove the is_already_processed.

actuallyakash commented 9 months ago

Closing in favour of #329