hftlclub / node-iltis

Node.js backend for ILTIS
MIT License
1 stars 0 forks source link

Disallow deletion of crate types when assigned to a product #37

Closed fmalcher closed 6 years ago

fmalcher commented 6 years ago

ATM, we can delete crate types even if they're assigned to a product. This leads to inconsistent data and produces an error when loading the product list (possibly because those crate types cannot be found anymore) – see screenshot.

Expected behavior: Some domain error like "Crate type cannot be deleted because it is still assigned to a product"

Steps to reproduce:

Reasons FOR this proposal:

bildschirmfoto 2017-11-18 um 13 49 22

rweisse commented 6 years ago

I couldn't reproduce the crash. Maybe the reason for the crash was some sideeffect of the crash in issue #30 and #36. This should has been fixed with commit 78248c4

I have tested the following workflow:

There haven't been any errors. The advantage of this behavior is that you won't have to edit each product and delete everytime the cratetype you finally want to delete from the cratetype managements view. Deleting a cratetype like (11 slots, 0.5l) would be very fast and won't have to be done for each product.

Of course the disadvantage is that if you delete a cratetype by mistake you will have to recreate it and add it to all products again. But even if you don't do that this won't lead to inconstency. I would prefer a warning message when deleting the cratetype in the frontend: "Do you really want to delete this cratetype? Every product that is connected to this cratetype will lose it." - something like that plus a text challenge.

fmalcher commented 6 years ago

This could be a way. However, users should be warned then that there are still products assigned to the crate type. Is there a way to get the number of products assigned to each crate type?

Otherwise, it could be easy to do wrong things when you just want to delete an old (or duplicate) crate type – and accidentally delete the wrong one.

@rweisse

fmalcher commented 6 years ago

Same but different topic: I also think it should not be possible to delete a unit that is assigned to a product. Of course this is not a physical deletion but a logical one. I could delete all units and nothing will change in the running system, except for that I can't assign this unit anymore to some new products. However, that's problematic. Imagine someone deletes a unit, thinking it's not being used anymore. We then have some active "ghost" products with that deleted unit assigned. We should avoid inconsistent states like that and the only way, I think, is to disallow deleting a unit when it's assigned to some products.

fmalcher commented 6 years ago

When some entity got the deleted flag it should not be used anymore, except for in existing data that is not being changed anymore. Un-assigning "deleted" crate types from a product is fine. Letting "deleted" units be assigned to the product is not, since those products can still be used.

rweisse commented 6 years ago

See https://github.com/hftlclub/node-iltis/issues/41