Add system status warning to display scheduled job failures

[jitendrapurohit]

Overview

Add alert to display scheduled job errors

Before

Schedule job fails silently. Civi core doesn't provide an inbuilt option to notify admins.

From https://chat.civicrm.org/civicrm/pl/8wbq1y3j4tdeumnf1iuie44a5o

anyone else wondered about having Sch Reminder errors listed as problems in System Status? Just found a case where 5 months back 'somehow' a membership was given an End Date of 0000:00:00 and guess what. that stops renewal reminders being sent out. sheesh.

After

Alert displayed on the system status page. Eg

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@jitendrapurohit style warning

[sunilpawar]

i have built similar functionality using extension : https://github.com/Skvare/com.skvare.jobchecker

[eileenmcnaughton]

@sunilpawar your extension looks nice & slightly more advanced than this. I think it's OK to have some functionality in core & an extension if people want more - I'm wondering what your thoughts are?

[sunilpawar]

@sunilpawar your extension looks nice & slightly more advanced than this. I think it's OK to have some functionality in core & an extension if people want more - I'm wondering what your thoughts are?

@eileenmcnaughton
Actually i was in planing to publish this extension.
If extension configuration is useful then you can use it, so i do not need to publish the same functionality through extension.

[wmortada]

@jitendrapurohit this sounds great! It would be a start in addressing the following two issues:

• https://lab.civicrm.org/dev/core/-/issues/1933
• https://lab.civicrm.org/dev/core/-/issues/2775

[mlutfy]

Could you wrap the strings in ts() ? (this line, and other lines around it)

[jitendrapurohit]

Sorry for the late update @mlutfy. I've added the ts() now. Thanks.

[mlutfy]

@jitendrapurohit Unfortunately it's not quite right - please see: https://docs.civicrm.org/dev/en/latest/translation/#best-practices

For example:

• $message = ts("<p>The following scheduled jobs failed on the last run:</p> - wrap just the string, not the HTML (when possible)
• ts($message), - do not include variables in ts (unless it uses placeholders), but besides it is already translated a few lines above.

[jitendrapurohit]

Thanks @mlutfy I've updated the usage in the latest commit 👍

[mlutfy]

Can you do instead:

<td><a href="' . $viewLogURL . '"> . ts('View Job Log') . '</a></td>

This way it avoids adding a new string to translate (and HTML should be avoided when possible).

$ grep -r 'View Job Log'
CRM/Admin/Page/Job.php:          'name' => ts('View Job Log'),

[jitendrapurohit]

Updated with the above suggestion 👍

[mlutfy]

ts is not necessary here. Job names are not translatable (they are set/translated during the installation, in civicrm_data.xx_XX.mysql).

The other changes are good, so just these tiny 3 edits and we're done :)

[jitendrapurohit]

Updated

[mlutfy]

ts not necessary for the above two lines.

[jitendrapurohit]

Removed now. Thanks.

[mlutfy]

@jitendrapurohit Thank you!

Merging based on code review, feedback from other reviewers, and also r-run:

[demeritcowboy]

I remember seeing this PR and not really paying attention because I always dealt with this issue another way, but after rolling out 5.46 I see this is a good thing™. 🎉