omeka-s-modules / Collecting

GNU General Public License v3.0
2 stars 6 forks source link

Deleting a prompt or a form deletes the supplementary data tied to that prompt or form #65

Open kalbers opened 3 years ago

kalbers commented 3 years ago

There also currently isn't an indication (eg, prompt) that doing either will result in a loss of data.

jimsafley commented 3 years ago

This is expected behavior but I recognize that it's not optimal.

Here's why it happens. The module saves "Item Supplementary" (input), "User Private" (user_private), and "User Public" (user_public) text in the collecting_input table. (It also saves "Item Property" (property) text to the table, but it doesn't apply here because it's redundant.) The collecting_input table joins to the collecting_prompt table with ON DELETE CASCADE, so when the user deletes a prompt, the database automatically deletes all child inputs.

I could set ON DELETE SET NULL to preserve input text when a prompt is deleted, but the input would be orphaned and lose context - it would just be text without categorical meaning. I suppose the input could preserve some data about its prompt to avoid this, like its name, but that would require an elaborate migration path for existing installations. In addition, we would likely need a mechanism to delete orphaned inputs should the user want to do that. As you see, it's not a straightforward "fix" and may be more trouble then it's worth.

A possible better solution is that we could display an alert when the user flags an input, user_private, or user_public prompt for deletion. Something like, "Warning. Items that were created by using this form may lose user input if you delete this prompt."

zerocrates commented 3 years ago

Showing a warning about what will be deleted is an improvement for sure which doesn't require rearchitecture, but it will always be limited as it just depends on user attention to and understanding of the consequences implied by the warning, which is always uncertain at best with these kinds of warnings. Its simplicity makes it worth considering anyway, though.

There's the complicating factor that this all happens "all at once" when deleting a form, too, since it cascade-deletes the prompts, and then the process you described happens for each prompt.

On "going big" with the fix and actually preserving the data: Letting the prompts be orphaned from the form would probably be a better move in the "deleted a form" situation since it would automatically preserve context. But it has the same issues with needing "cleanup" capability and so on, and doesn't solve issues where you explicitly delete a prompt (I guess that would have to become "detaching" a prompt instead).

Instead, total decoupling of the answers/inputs from the form tables would be maybe the "best" option if actually allowing deletion while preserving the inputs: preserve all the current cascades, but add something like a JSON column on the collecting_item table that just stores the prompts and inputs at the time the item was collected. This has the upside of handling total deletion of the form, added/removed prompts, and changes to prompt text pretty gracefully: each collecting_item simply reflects the state of the form at the time the submission was made. You could possibly remove the collecting_input table completely in this model, too. Of course it's not space-efficient as the existing setup because it would duplicate prompt text. I think also collecting_item has the same issue with being cascade-deleted, but that would be a pretty simple fix.

jimsafley commented 3 years ago

It's a good solution. Change CollectingItem::$form to onDelete="SET NULL", add CollectingItem::$userInput as @Column(type="json"), and reflect "the state of the form at the time the submission was made" by adding something like the following to the CollectingItem::$userInput:

[
  {
    "prompt_text": "Title",
    "input_text": "My Contribution"
  }
]

Given that this data structure will be in place, there may be no need for CollectingInput (I'll have to see). An item's "Collected Inputs" section will draw entirely from CollectingItem::$userInput. The migration will be tricky, but doable.