Correction comment state #6155
Correction comment state #6155
Conversation
2aa7bce
to
e7d15d5
Compare
| } catch (DAOException e) { | ||
| Helper.setErrorMessage("errorSaving", new Object[] {"comment"}, logger, e); | ||
| if (Objects.nonNull(comment.getProcess())) { | ||
| comment.setProcess(ServiceManager.getProcessService().getById(comment.getProcess().getId())); |
There was a problem hiding this comment.
We are setting the currentTask and the process again but do not save the comment object afterwards. If calling those setters is necessary should we not persist the comment object again?
There was a problem hiding this comment.
@BartChris Yes, you are right, this is quite confusing. The reason why we re-add the process (and task!) to the comment without saving it again is that we are making use of a side effect here. This Comment object is used again in the calling method afterwards, but with an updated currentTask and process property. It's not a different process or task that is added here, but rather updated objects, so re-setting these properties in the comment acts as a "refresh" of the object. Since the database entry of the comment only contains the ID of the process and the currentTask, there is no need to save it again, because the IDs didn't change (e.g. its the same process and currentTask as before, like I mentioned above).
This is indeed implemented in a sub-optimal way and should be refactored more fundamentally when we find the time to do so but right now I do not have the resources for that, unfortunately. I think the bug should be fixed anyway, even if its just an incremental improvement over the status quo and not yet the optimal solution. I can add a comment or TODO as a reminder to the code that this code should be refactored and improved, if you like.
There was a problem hiding this comment.
@solth I see, thanks a lot for the explanation. In think you can go ahead and merge. Adding a TODO would be helpful.
|
Functionality-wise everything works now, i just have a comment. |
This pull request slightly adjusts the
WorkflowControllerService.solveProblemfunction so that a comment'scorrectedflag with valueTRUEis saved to the database beforecloseTaskByUseris called. That method loads the comment from the database and requires the updatedcorrectedstate to work properly.The updated
correctedstate is then saved to the index fieldcorrectionCommentStatuswhich in turn is used to determine whether the grey or red exclamation mark icon should be displayed in the process and task lists. Thereby fixes #5676