MAE-318: Use queue to process memberships

ahed-compucorp commented 3 years ago

Overview

The OfflineAutoRenewalJob job is responsible for membership auto-renewal but there is a timeout issue when many records exist and the admin run the job for the first time on the test site.

Before

Peek 2021-02-13 20-24

After

Peek 2021-02-15 19-41

Technical Details

The feature will help the admin see the job's progress only when he executes it from CiviCRM UI. No change needed to the way we run the job (cv api, drush, HTTP request).

CiviCRM queue has Claim item concept that handles the case when the admin execute the job, and cronjob ran the job during the processing.

These are the keys points of this PR:

1st commit: I found out that any test that explicitly/implicitly calls CRM_Utils_System::currentPath will raise 'Undefined index: q' notice because PHPUnit converts PHP errors, warnings, and notices that are triggered during the execution of a test to an exception. I fixed that.
2nd commit: Added @throw line and fix typos in phpdoc.
3rd commit: Refactored getRecurringContributions to return an array of integers instead of an array of arrays because the number of installments is not used.
4th commit: Refactored the job OfflineAutoRenewal to use CiviCRM queue implicitly. I copied everything related as is to preserve the old functionality.
5th commit: Fixed failing tests because of the renamed files.
6th commit: Handled edge case when the job executed twice and the queue got populated with repeated tasks.
7th commit: A message "Failed to claim next task" will appear if we ran an empty queue. This commit handled that.
8th & 9th commits: Refactored the code for more clarity.
10th commit: Created JobLog with the description "Finished execution of Renew offline auto-renewal memberships with result: Success" on completion
11th commit: Added PostRunAllTask as the last task to solve a problem with queue web-runner that show 'Done' message instead of the last task's title.

erawat commented 3 years ago

@ana-compucorp Nice approach!

ahed-compucorp commented 3 years ago

I double-checked the commit Use queue to process memberships. while doing that, I moved any change outside the commit's scope to new commits.

ahed-compucorp commented 3 years ago

@erawat

Can you explain help more on the first commit on the technical details why we need to set the current path in order to resolve the 'undefined index' error?

Updated the commit message and the PR description to make it more clear.

Can you confirm if schedule job is working properly with cron job?

Yes, and no change needed if you run the job form the cron job.

if we use https(s) it will time out anyway if the queue is large.

Using the http(s) is a bad idea in general, and this PR will help continue processing from where it stopped.

ahed-compucorp commented 3 years ago

Will wait for https://github.com/compucorp/uk.co.compucorp.membershipextras/pull/355 to be merged then will rebase this PR.

compucorp / uk.co.compucorp.membershipextras