hftlclub / node-iltis

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

Delete category with products #30

Closed fmalcher closed 6 years ago

fmalcher commented 7 years ago

❓ Am I right assuming that it should not be possible to delete a category when it still has products inside?

If I try doing this, the category is being deleted but the backend crahes. The products still keep the old category.

I expected an error message like "category is not empty, so you can't delete it"

fmalcher commented 7 years ago

Possible todo for the frontend: Only allow deletion if category is empty

rweisse commented 6 years ago

What do you mean with "the backend crashes"?

The category has got a deleted-tag. If the category is still in use this tag is setted. The category won't show up the category list any longer. But if you get a product or all products the deleted category should still be resolved inside this object correctly.

fmalcher commented 6 years ago

Well, what could I possibly mean by "crashing"? 😜 An error occurs and the backend process is being terminated.

bildschirmfoto 2017-10-17 um 20 18 12

bildschirmfoto 2017-10-17 um 20 18 17

fmalcher commented 6 years ago

Apart from this error I think it would be more suitable to not allow users to delete categories that have products in them. We only need the deleted flag for deleted products that still have that category assigned. "Deleting" a category and keeping this category for its products leads to inconsistency

rweisse commented 6 years ago

Oh that's bad. IMHO the deleted tag in categories is important: Imagine a product that gets deleted. It has been used before in some transactions. So we can not delete it completely. Its deleted tag will be set to true. By chance this product is part of a category we would like to delete in future. As long this product exists in the database we are not able to delete this category. So we need a deleted tag for categories, too.

fmalcher commented 6 years ago

Do we agree that it should be forbidden to "delete" a category with products inside? @rweisse

rweisse commented 6 years ago

@fmalcher yes. That makes sense. But in this special case I would prefer to disable the delete button in the frontend if the category isn't empty. The "deleted"-tag is only important when asking for all categories (product management, setting a category for a product). In all other usecases the category is automatically resolved with a product independent of its deleted-tag (even if it is setted).

fmalcher commented 6 years ago

Disabling the delete button for non-empty categories was done here: https://github.com/hftlclub/ng-iltis/commit/a65b87d12c92175a89c5f1c8b86bc383ede41cfe

Does the backend now also check this before deletion? @rweisse

rweisse commented 6 years ago

No - because it's not important. Deleting a category which is still in use won't have any effect on other functionalities. The only thing that changes is the dropdown menu of the product managements category window. Deleting a category prevents using this category for new products.

fmalcher commented 6 years ago

But there is no way of un-"deleting" a category. Thus, when you have accidentally deleted one, you can not recover it (i.e. reset the deleted flag)

rweisse commented 6 years ago

A deleted category is deleted and can't be reactived. Otherwise the flags name would be actived not deleted. If you like to use this category again you are free to recreate it. Deleting a category prevents the user to set this deleted category for products again. Products that are already assigned to this deleted category will keep it. That's the only way the use of a deleted-flag makes any sense. Otherwise we wouldn't have needed a deleted flag.

Explanation: Without a deleted-flag deleting a category that were still in use would either result in nulling the foreign key in products (in this case a product were allowed to have no category) or result in an error message. Using a deleted-flag offers the possibility to delete categories that are still in use and prevents the user to use them for future products. IMHO everythings working as expected.

fmalcher commented 6 years ago

Yeah, of course I dont want to recover deleted categories. I just dont want that users can accidentally delete it, because it's gone then. Just imagine there are two categories named "Beer" – one with all our beers and one empty. You want to delete the empty one but select the wrong one...

Is there a case where we actually WANT to delete a category with products? How can we guide our users to avoid them from accidentally deleting categories?

rweisse commented 6 years ago

There won't be a huge number of users who have the rights to make such changes. And I believe that these 3 people having such rights got something like a brain in their heads. And even if there are two categeries that have the same name - it doesn't matter which one gets deleted because you won't see any difference thanks to this feature.

rweisse commented 6 years ago

ok. no. if you delete the wrong one you will still have two times the same category in your product list - right.

fmalcher commented 6 years ago

Yeah, but it leads to logical inconsistency. A "deleted" category is still in use and that's not good. If a category is being deleted we should unassign it from all products OR make sure it can't get deleted if there are products in it. The frontend makes sure nobody can click the delete button when there are still products in a category. IMO the backend should also check this – not only to avoid suers doing it but because this leads to a state where "daleted" artifcats are still in use.

rweisse commented 6 years ago

This is not possible at the moment because this would result in undeletable categories (btw. its the same with units) if you like to delete a category that is used by an inactive or deleted product.

rweisse commented 6 years ago

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