Task action processor #5657

markusweigelt · 2023-05-08T14:51:46Z

This pr add the new TaskActionProcessor and following TaskActions to process.

COMMENT action adds a comment to the task
ERROR_OPEN action lock the task and add an error comment when task status is OPEN or INWORK
ERROR_CLOSE action set the task status of logged task to OPEN
PROCESS action set task status of open task to INWORK
CLOSE action close a task

Please consider the updated documentation https://github.com/kitodo/kitodo-production/wiki/Developer_3.x-Active-MQ#processors and for simplified use of ActiveMQ testing, the Docker image and the Java client https://github.com/slub/kitodo-production-activemq.

…o-production into step-state-processor

Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java

Kitodo/src/main/resources/kitodo_config.properties

solth · 2023-06-14T08:49:53Z

@henning-gerhardt would you mind reviewing this pull request? As I am not very familiar with ActiveMQ, I am not very well suited for this review, I think.

henning-gerhardt · 2023-06-14T09:14:42Z

@solth , @markusweigelt : I can review this pull but reviewing it will take some time.

henning-gerhardt

This is the first round of reviewing this pull request and did only contain a read code review. Testing and getting experience with this pull request is next.

But I'm unsure about the setting task state to 'lock' as this is dangerous and was a sign of a big error in the workflow in the past. I will talk to you in next days.

Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java

Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java

Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java

…k to state INWORK when correction id is not set

markusweigelt · 2023-06-23T15:34:03Z

@henning-gerhardt I have added your comments from the review and hints from our conversation.

Task Action ERROR_OPEN

now only works in task processing status INWORK
If no correction identifier is set, the task remains in processing state INWORK

Task Action ERROR_CLOSE

If no correction identifier is set, the task processing state should be INWORK
If correction identifier is set, the task processing state should be LOCKED

The tests and the documentation linked above is adapted too.

henning-gerhardt

This is now a functional review. I tested a few use scenarios but I did not test the usage of the correction id! Without a correction id the new functionality is working even I'm not convinced by setting a task to "LOCKED" (red) as this is in my current experience world wrong: after a "DONE" (green) task only a "OPEN" (yellow) or "INPROCESS" (orange) task state an follow and never a "LOCKED" (red) state.

One bigger issue is that if a task is set to "INPROCESS" a second time the state is not saved to the database and the new processing time is even not updated.

I did even read your documentation in the wiki (thumbs up for this) but is was for me at least at two places confusing or not understandable to read:

ERROR_OPEN (documentation at https://github.com/kitodo/kitodo-production/wiki/Developer_3.x-Active-MQ#error_open)

Some kind of confusing to read "This action set the processing status of a task to LOCKED when correctionTaskId is set and creates an error comment." and later "When correctionTaskId is set the correction task is set to OPEN."

ERROR_CLOSE (documentation at https://github.com/kitodo/kitodo-production/wiki/Developer_3.x-Active-MQ#error_close)

It is difficult to understand or confusion to read (at least for me): Name of the action ingest that the task got closed. But first sentence says "OPEN". Next sentence says "LOCKED" (kind of close), except under some conditions is set or stay "INWORK" and last sentence says "DONE" (kind of close).

Maybe you can explain this two actions in a better or other way. Maybe even renaming the action names to more meaning and understandable names.

Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java

…x is already call, set processing date all the time

markusweigelt · 2023-07-05T12:43:05Z

@henning-gerhardt have incorporated your comments and hints in the implemenation. If you are satisfied with it, then please approve.

henning-gerhardt

Works for me in my usage scenarios without using a correction id.

solth · 2023-09-28T11:00:57Z

@markusweigelt please rebase against current master.

markusweigelt · 2023-09-28T13:14:40Z

@solth Merged against the current master. ~~The current error in the CI is unrelated to this PR.~~ ( Occured only in my repo actions run https://github.com/markusweigelt/kitodo-production/actions/runs/6338902470/job/17216941936)

solth · 2023-09-29T06:45:43Z

Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java

+import org.kitodo.data.database.enums.TaskEditType;
+import org.kitodo.data.database.enums.TaskStatus;
+import org.kitodo.data.database.exceptions.DAOException;
+import org.kitodo.data.elasticsearch.exceptions.CustomResponseException;


Suggested change

import org.kitodo.data.elasticsearch.exceptions.CustomResponseException;

Unused import

solth · 2023-09-29T06:54:09Z

Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java

+                        currentTask.getProcessingStatus())) || (mapMessageObjectReader.hasField(
+                        KEY_CORRECTION_TASK_ID) && !TaskStatus.LOCKED.equals(currentTask.getProcessingStatus()))) {
+                    throw new ProcessorException(
+                            "Status of task is not INWORK if there is a no corrected task ID or LOCKED if there is a corrected task ID.");


Just curious: shouldn't this be "correction task ID" instead of "corrected task ID"? It is not yet corrected at this moment, is it?

solth · 2023-09-29T07:09:52Z

@markusweigelt I am not sure but I think the following line in https://github.com/markusweigelt/kitodo-production/blob/edfe9b1176d973a193d8fee99376db9c6e073321/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java#L243 can potentially throw a NullPointerException that needs to be handled because now Comment.correctionComment can be null, right?

if (!processComment.isCorrected()
                        && processComment.getCorrectionTask().getTitle().equals(comment.getCorrectionTask().getTitle())) {

markusweigelt · 2023-10-02T09:56:31Z

@solth I resolved a potential null pointer exception and checked if there were more occurrences with 'getCorrectionTask,' but I didn't find any.

markusweigelt added 18 commits January 30, 2023 19:59

Initial implementation of step state processor

2b3812c

Merge branch 'master' into step-state-processor

2aca882

Merge branch 'kitodo:master' into step-state-processor

a6d979b

Refactoring, improve documentation and checkstyle

234bdc4

Rename step to task

a294a17

Minor improvements

dfbec97

Update Kitodo/src/main/resources/kitodo_config.properties

623ce86

Revert WorkflowControllerService changes

464b229

Merge branch 'step-state-processor' of github.com:markusweigelt/kitod…

3d49255

…o-production into step-state-processor

Revert changes and add null check again

2c22629

Improve order of null check

55c1b80

Rename processor and enium to more unambiguous name, inital adding test

0a6d5d6

Renaming processor and enum, improve tests and processor

5a89fed

Improvements comments and add futher tests

b566b5f

Rename comment dao function

5fc60bc

Improve comment and fix bug

f85e32c

Remove unnecessary comment from kitodo_config.properties

ae85dfb

Improve comment

69d4f1a

markusweigelt marked this pull request as ready for review May 8, 2023 16:22

markusweigelt added 5 commits May 10, 2023 11:07

Use switch case, rename comment

b288bbe

Add default to switch case

b81be78

Improvments of error open and close behaviour

44ec676

Add javadoc

07989da

Add test for correction task

bfb6ea1

markusweigelt requested review from solth and matthias-ronge May 11, 2023 10:15

markusweigelt added 2 commits May 11, 2023 14:44

Change javadoc

f73738d

Merge branch 'kitodo:master' into task-action-processor

476f0a8

bertsky mentioned this pull request May 23, 2023

Send ocr processing messages to active mq slub/ocrd_manager#60

Merged

Merge branch 'kitodo:master' into task-action-processor

56cf1d5

markusweigelt requested a review from solth June 5, 2023 15:11

matthias-ronge suggested changes Jun 7, 2023

View reviewed changes

Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java Show resolved Hide resolved

Kitodo/src/main/resources/kitodo_config.properties Outdated Show resolved Hide resolved

Review changes

dc0395e

matthias-ronge approved these changes Jun 7, 2023

View reviewed changes

markusweigelt and others added 2 commits June 7, 2023 11:49

Revert change of table name

1d00f3f

Merge branch 'kitodo:master' into task-action-processor

d6ed2f6

markusweigelt requested a review from henning-gerhardt June 13, 2023 14:25

henning-gerhardt suggested changes Jun 16, 2023

View reviewed changes

markusweigelt added 2 commits June 23, 2023 16:52

Add code review improvements, change state to open errors and set tas…

1660538

…k to state INWORK when correction id is not set

Update task action comments

b3aa64e

henning-gerhardt suggested changes Jul 3, 2023

View reviewed changes

Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java Outdated Show resolved Hide resolved

Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java Outdated Show resolved Hide resolved

Remove seperate saveToIndex cause within save function the saveToInde…

12be95b

…x is already call, set processing date all the time

henning-gerhardt approved these changes Jul 5, 2023

View reviewed changes

solth added the feature label Jul 8, 2023

BartChris mentioned this pull request Jul 20, 2023

OCR-D Workflow integration #5697

Closed

aetherfaerber mentioned this pull request Aug 16, 2023

Restore the functionality to create processes using ActiveMQ #4837

Open

solth added this to the Kitodo.Production 3.7.0 milestone Sep 6, 2023

Merge branch 'kitodo:master' into task-action-processor

edfe9b1

solth reviewed Sep 29, 2023

View reviewed changes

add verify method to compare correction tasks

b7fa835

solth merged commit eb3d4df into kitodo:master Oct 2, 2023

solth mentioned this pull request Oct 4, 2023

Enable editing and delete info comments patch #5785

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Task action processor #5657

Task action processor #5657

markusweigelt commented May 8, 2023 •

edited

Loading

solth commented Jun 14, 2023

henning-gerhardt commented Jun 14, 2023

henning-gerhardt left a comment

markusweigelt commented Jun 23, 2023

henning-gerhardt left a comment

markusweigelt commented Jul 5, 2023 •

edited

Loading

henning-gerhardt left a comment

solth commented Sep 28, 2023

markusweigelt commented Sep 28, 2023 •

edited

Loading

solth Sep 29, 2023

solth Sep 29, 2023

solth commented Sep 29, 2023 •

edited

Loading

markusweigelt commented Oct 2, 2023

Task action processor #5657

Task action processor #5657

Conversation

markusweigelt commented May 8, 2023 • edited Loading

solth commented Jun 14, 2023

henning-gerhardt commented Jun 14, 2023

henning-gerhardt left a comment

Choose a reason for hiding this comment

markusweigelt commented Jun 23, 2023

henning-gerhardt left a comment

Choose a reason for hiding this comment

markusweigelt commented Jul 5, 2023 • edited Loading

henning-gerhardt left a comment

Choose a reason for hiding this comment

solth commented Sep 28, 2023

markusweigelt commented Sep 28, 2023 • edited Loading

solth Sep 29, 2023

Choose a reason for hiding this comment

solth Sep 29, 2023

Choose a reason for hiding this comment

solth commented Sep 29, 2023 • edited Loading

markusweigelt commented Oct 2, 2023

markusweigelt commented May 8, 2023 •

edited

Loading

markusweigelt commented Jul 5, 2023 •

edited

Loading

markusweigelt commented Sep 28, 2023 •

edited

Loading

solth commented Sep 29, 2023 •

edited

Loading