Skip to content

Make notify->_notify_admin a public function.

Robert Sinton requested to merge feature/public_notify_admin_method into master

What I did

  • Converted _notify_admin into a public function.

Implications

I find I often want to flick off a notification email to an admin, in contexts where I don't need the other notify functionality.

Example I'm working on right now is a cron task handler, which will exit immediately after triggering emails like this.

This change means I can do this:

$this->notify->notify_admin('error', 'Xero PO not created', 'There was no applicable Xero file for the gallery for loan ID ' . $loan->id . ' (' . $loan->display_full_name() . '), so no Xero PO has been raised.');

Instead of having to do this, which is pretty obscure in the context:

$this->notify->set('error', 'Xero PO not created', null, true, 'There was no applicable Xero file for the gallery for loan ID ' . $loan->id . ' (' . $loan->display_full_name() . '), so no Xero PO has been raised.');

I've also noted a number of times where devs have achieved a standalone email to admin by writing code like this:

$this->notify->set('error', 'The thing failed', null, true, 'It had its reasons');
$this->notify->delete('error');

… which seems ripe for losing error notifications unintentionally.

Setup

None

How to test

  • Code review.
  • Call it from any controller or model, and ensure you receive the email.
Edited by Robert Sinton

Merge request reports