acdh-oeaw / apis-core

https://acdh-oeaw.github.io/apis-core/
MIT License
11 stars 3 forks source link

Draft: add new relations behavior on deletion of related objects (#343) #344

Open sennierer opened 2 years ago

sennierer commented 2 years ago

created a simple test that asserts that relations are deleted when entities are deleted. I think thats a foreseeable outcome (especially as the merge systematic takes care of copying relations). However, relations are also deleted if the relation_type is deleted. And while it makes sense that a relation shouldn't exist without a relation_type I think we shouldn't delete relations when the relation type is deleted. Researchers clean up there vocabs in the admin interface and while they are warned that relations will be deleted I think we should nonetheless set the relation_type to a configurable default and keep the relation. Thus the second test is currently failing. The behavior I suggest is:

sennierer commented 2 years ago

Can we have opinions on that? @gregorpirgie @steffres

skurzinz commented 2 years ago

Even though I'm only involved from users perspective, I think the reasoning is sound. Having relations "vanish" "just because" "I only deleted the relation_type" could certainly be an issue with the MPR users.

Am 21.07.2022 11:05 schrieb Matthias Schlögl @.***>:

Can we have opinions on that? @gregorpirgiehttps://github.com/gregorpirgie @steffreshttps://github.com/steffres

— Reply to this email directly, view it on GitHubhttps://github.com/acdh-oeaw/apis-core/pull/344#issuecomment-1191235611, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AI5U6VNHYISXHORCCZSJAGLVVEHF7ANCNFSM54G2VMOQ. You are receiving this because you are subscribed to this thread.Message ID: @.***>

gregorpirgie commented 2 years ago

I think @sennierer that the solution is very good. But I would also consider:

And I think that it would be important than that editors never change that name; or, even better, to implement a save mechanism that disallows changing that name – so that relations with the default relationtype can only be assigned a new relationtype, but not changed by editing the default value itself. Which might happen if editors see that at a current state of a project all relations with the default value fall into a certain category and think of just renaming the relationtype.

sennierer commented 2 years ago

@gregorpirgie I think you are right, we should aim for one string across all relationtypes. However, I think it could be configurable. Something along the lines of getattr(settings, "APIS_DEFAULT_RELATION_TYPE", "no relationtype specified") and I wouldnt care about users changing the default relation type. If the system doesnt find a relation_type with the set lable it just creates one. Gives the users the opportunity to change in batch if they made a mistake by deleting a relation_type and relieves us from implementing a probably rather complex logic to prevent the change.