kitodo-production icon indicating copy to clipboard operation
kitodo-production copied to clipboard

Delete existing data editor settings for tasks of a changed workflow

Open BartChris opened this issue 3 years ago • 4 comments

When changing an already activated workflow (see: https://github.com/kitodo/kitodo-production/pull/5206) the template tasks constructed based on those workflow are deleted and then recreated. This however does not work if there are existing data editor setting for a specific task. (Mysql table dataeditor_setting)

In this case the change of the template tasks fails because of a foreign key constraint.

SqlExceptionHelper - Cannot delete or update a parent row: a foreign key constraint fails (`kitodo`.`dataeditor_setting`, CONSTRAINT `FK_dataeditorsetting_task_id` FOREIGN KEY (`task_id`) REFERENCES `task` (`id`))

The user interface reports the following error:

grafik

The pull requested tries to adress this by deleting all data editor settings related to template tasks which are changed during a workflow update. The disadvantage is that all editor settings for this task get lost, but since one cannot say what exactly has changed, it probably serves no purpose to preserve those settings.

Edit: An alternative approach would be to handle this on the database level by specifying "ON DELETE CASCADE" for this foreign key as dataeditor_setting does not hold critical data. The same setting could be applied for the foreign key pointing to the user table.

BartChris avatar Aug 17 '22 15:08 BartChris

I would prefer not to lose the DataEditorSettings and think we should strive for fixing the mentioned problem without losing any data, if possible. Shouldn't it be possible to first deatch the DataEditorSettings from the old task object before it is deleted and then re-attach them to the new, re-created task?

solth avatar Aug 22 '22 11:08 solth

That would be good, i agree. The problem is that it would be very hard to identify the same task if there were other tasks added in between or if a name of a task was changed. In the end this would require some mechanism to make sure that the new (recreated) task is attached to the right old settings. Edit: Maybe it is possible by using the Identifier of the task in the xml file, i will check that

BartChris avatar Aug 22 '22 13:08 BartChris

You are right. If the user completely changes the sequence of tasks it could be impossible to re-assign the data editor settings to the original tasks. Therefore I take back my first comment about trying to keep the data editor settings.

solth avatar Aug 23 '22 10:08 solth

On second thought I am not convinced that the current proposal is optimal, since users will loose there DataEditorSettings for no apparent reason. Even though these settings aren't critical for the system to work, it could easily be conceived as a bug in the system when previously saved settings are suddenly lost.

Perhaps a popup dialog could be shown when changing an active workflow in cases where DataEditorSettings exist that informs the workflow engineer that saving the workflow will reset user settings and that the users should be informed about this. The engineer can than decide whether he wants to abort changing the workflow or to change the workflow nonetheless and accept the loss of user data.

solth avatar Aug 23 '22 12:08 solth

@BartChris please add a test as requested by @henning-gerhardt

solth avatar Sep 23 '22 09:09 solth

Sorry for the delay, i will add those tests next week and will try to add a popup dialog.

On a general note on the problem of preserving old state (editor settings) attached to a task: While thinking about the problem of workflow changes in general (https://github.com/kitodo/kitodo-production/discussions/5351) i was also thinking about the possibility to compare older with newer workflow tasks as this would be necessary to eventually preserve the editor settings. In general one could compare the tasks either by their name or by their workflowId. (If you you do not want to compare every single property of a task). But i am not convinced wether this (see also discussion thread) gives any reliable indication if the editor settings should be preserved or not.

BartChris avatar Sep 27 '22 10:09 BartChris

I tried to make more clear what is happening in the test. The test should do, what you want it to do. The first template has a predefined workflow, tasks and dataeditor settings. The workflow gets replaced by a new workflow thereby updating all the tasks of this template. The settings attached to those tasks are deleted. The second template does not have tasks defined initially so i generate a task for that template and assign data editor settings to this task. Those settings are preserved.

BartChris avatar Oct 11 '22 09:10 BartChris

@BartChris One little Codacy problem still remains. Can you fix that issue? Then I will merge this pull request.

solth avatar Oct 11 '22 11:10 solth

~~Right now the check for existing data editor settings is also evaluated to true for compeltely new workflows, i have to inspect, why this happens.~~ Edit: It seems to work.

BartChris avatar Oct 13 '22 08:10 BartChris