DMP-4216: Allow inactive tasks to be run manually #2238
Conversation
Ben-Edwards-cgi
requested review from
davet1985,
jackmaloney,
cakeben,
staceywilson,
cpareek and
mananpatel26
and removed request for
a team
hmcts-jenkins-cnp
bot
added
ns:darts-modernisation
prd:darts
rel:darts-api-pr-2238
labels
| @@ -71,8 +71,8 @@ void invokesTaskWhenTaskIsNotLocked() { | |||
|
|
|||
| adminAutomatedTaskService.runAutomatedTask(1); | |||
|
|
|||
| verify(automatedTaskRunner, times(1)).run(someAutomatedTask); | |||
| verify(auditApi, times(1)).record(AuditActivity.RUN_JOB_MANUALLY,"some-task-name"); | |||
| verify(automatedTaskRunner, times(1)).run(someAutomatedTask, true); | |||
There was a problem hiding this comment.
You have set isManual to true but is there a test where its set to false
There was a problem hiding this comment.
the is manual run false should be covered by the existing tests for automated tasks. As when it is not manually run we default this value to be false
| @@ -79,7 +79,7 @@ private void setupUserAuthentication() { | |||
| } | |||
|
|
|||
| @Override | |||
| public void run() { | |||
| public void run(boolean isManualRun) { | |||
There was a problem hiding this comment.
This code is making my head hurt. Why are we passing this isManualRun in as a parameter? Can't we just use the isManualTask variable that is already used in this class? Or just get rid of the isManualTask class variable and use your new parameter.
There was a problem hiding this comment.
I have removed the ManualTaskService as this no longer provided any value and updated the code to reflect this change
| @@ -90,7 +90,10 @@ public void run() { | |||
| String dbCronExpression = automatedTask.getCronExpression(); | |||
| // Check the cron expression hasn't been changed in the database by another instance, if so skip this run | |||
| if (isManualTask || getLastCronExpression().equals(dbCronExpression)) { | |||
| if (TRUE.equals(automatedTask.getTaskEnabled())) { | |||
| if (isManualRun || TRUE.equals(automatedTask.getTaskEnabled())) { | |||
| if (isManualRun && !TRUE.equals(automatedTask.getTaskEnabled())) { | |||
There was a problem hiding this comment.
Don't think you need the isManualRun && part here. If the second part of the if statement is met, then it means you can only have got there if it was manually triggered, so it's redundant.
There was a problem hiding this comment.
Agreed removed :)
Links
Change description
Summary of Changes
Updated manual task execution to bypass enabled / disabled checks. Meaning the task will be executed when run manually even if the task is disabled in the database.
Highlights
New Method for Running Tasks:
run(boolean isManualRun)was added to theAutomatedTaskinterface, allowing tasks to indicate whether they are running manually or automatically.run()was also added, which defaults to calling the new method withfalse(indicating an automated run).Modifications in Implementations:
AbstractLockableAutomatedTaskclass was updated to implement the newrun(boolean isManualRun)method.AutomatedTaskRunnerclass now takes into account whether a task is being run manually or automatically, logging the type of execution.Service Layer Updates:
AdminAutomatedTasksServiceImplclass now triggers the automated task with the new manual run parameter set totruewhen tasks are run manually.Test Adjustments:
These changes collectively improve the control and monitoring of automated tasks, making it easier for developers to distinguish between different execution modes.
Does this PR introduce a breaking change? (check one with "x")