FOLIO: Post to a webhook after submitting a request

[maccabeelevine]

It's helpful for circulation staff to know when a new request is submitted, so they can pull it off the shelf etc. FOLIO has no way to notify staff when this happens. (It can notify the requesting patron, but not staff.) This allows calling an external service to provide that notification (via email, slack or whatever else).

Originally (#3995) I was going to built email notification functionality into VuFind, but it really seems beyond the scope since the ILS itself should be doing it. A webhook push is a much simpler addition.

[maccabeelevine]

I considered (as in #3995) doing this within HoldsTrait, so ILS agnostic, and maybe that's still the right call. But I think I want someone outside of FOLIO-land to indicate that they would actually care about this feature.

[maccabeelevine]

Re: the webhook call -- I am assuming that this will be very low latency, with the web service hosted in the same environment as VuFind. That said, if there's an easy path to make it async, that would be fine as well -- the event model seemed too heavyweight.

[demiankatz]

Thanks, @maccabeelevine -- see below for a few new thoughts (plus one that was hanging around from my original review attempt during GitHub glitchiness). :-)

[demiankatz]

Looks good to me now. I pushed up one small change to remove a whitespace edit to GuzzleService.php (since nothing else had changed). See below for a trivial question about a comment. Once that's sorted out, I'm happy to approve, though maybe we should give others an opportunity to share feedback on #vufind-folio-integrations on Slack before merging. Do you want to post there, or should I?

[maccabeelevine]

maybe we should give others an opportunity to share feedback on #vufind-folio-integrations on Slack before merging. Do you want to post there, or should I?

Sure I'll post.

[demiankatz]

Looks good to me now!

One more request: is it worth adding a data provider to testSuccessfulPlaceHold in the test, so we can test placing hold with the webhook setting on and off? We could adjust createConnector to accept a pre-built mock webhook service so we can set expectations appropriately for each scenario. If you're not sure what I'm talking about, I'm willing to take a stab at it on this end when time permits. :-)

[maccabeelevine]

One more request: is it worth adding a data provider to testSuccessfulPlaceHold in the test, so we can test placing hold with the webhook setting on and off?

Yes -- done!

[demiankatz]

Thanks, @maccabeelevine! I made a couple of tweaks to the tests -- I made the assertions more specific in the FolioTest, and I created a WebhookTest so we at least have a stub there.

I started trying to work on a test for the "bad HTTP status" case, but it got complicated (because of the stream interface used by the PSR response object) and I ran out of time. I'm a little concerned that there may be a problem there -- not sure if calling getBody() in the log message will work seamlessly, or if you'll end up with type conversion problems. If you have time to write another test, it might be worth the effort; if not, let me know and I can help further later in the week.