Remove queue based on the condition

[ramyakrishnai]

Proposed changes

JIRA: https://jira.newfold.com/browse/PRESS0-1495

Description: Right now we are clearing all the events from the queue when they are successfully sent to Hiive without checking whether they were successfully processed by Hiive. The request here is to remove only those events from the queue which are successfully processed by Hiive.
It will mean the failed events will be retried again in the next run. We will have to bring into place a retry mechanisim which will discard the event from queue once max retries reach for that event.

Type of Change

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation Update (if none of the other choices apply)

Checklist

• I have read the CONTRIBUTING doc
• Linting and tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Further comments

[circlecube]

@ramyakrishnai Looks good overall, can you take another look at the latest lint issues, please?

It would be good to set some limit or throttling too, I see it was a limit of 3 retries at one point, but maybe we add a delay for each retry so we don't end up hammering ourselves with requests.

[BrianHenryIE]

I have edited this so

• In EventManager::shutdown(), if the queued events are not sent, they are added to BatchQueue::push() which stores them in the database
• Events sent on the nfd_data_sync_cron cron job, via EventManager::send_batch() only call BatchQueue::remove() on success, otherwise call BatchQueue::release() to allow them be processed later

It still does not have:

• Throttling
• Expiration of events after a number of retries

Edit: ⚠️ I see it mentioned in the Jira ticket that the Hiive requests are non-blocking. I don't know what the return value of the function looks like in that case, so we must assume the code right now will not acknowledge any successes and all will be infinitely queued ⚠️ This was correct and has been addressed – non-blocking requests are assumed to be successful.

[wpscholar]

@BrianHenryIE We'll want to return an explicit HTTP status code here. The WP_Error get_error_code() method isn't a status code. It would be something like invalid_rest_request or some other string.

[BrianHenryIE]

I think it will be appropriate to return 500 between the browser and the WP plugin. The browser doesn't really need to know about 401 etc.