validate that every relation has a relation type

[stefanprobst]

not sure if this has already been fixed, but the ICA instance has relationships without a relation type. i think saving these should be disallowed.

example:

https://ica.acdh-dev.oeaw.ac.at/apis/api/relations/personplace/542/

results in:

{
    "url": "https://ica.acdh-dev.oeaw.ac.at/apis/api/relations/personplace/542/",
    "id": 542,
    "review": false,
    "start_date": null,
    "start_start_date": null,
    "start_end_date": null,
    "end_date": null,
    "end_start_date": null,
    "end_end_date": null,
    "start_date_written": null,
    "end_date_written": null,
    "status": "",
    "references": null,
    "notes": null,
    "published": false,
    "source": null,
    "related_person": {
        "id": 540,
        "label": "Kraus, Karl",
        "url": "https://ica.acdh-dev.oeaw.ac.at/apis/api/entities/person/540/"
    },
    "related_place": {
        "id": 541,
        "label": "Jitschin",
        "url": "https://ica.acdh-dev.oeaw.ac.at/apis/api/entities/place/541/"
    },
    "relation_type": null
}

other example relations with the same issue: 532, 535, 538,

note that all of these have published: false

[sennierer]

requesting the webpage of the entities of this relation also result in 500: https://ica.acdh-dev.oeaw.ac.at/apis/entities/entity/person/540/detail

[sennierer]

tested it inlocal instance, its possible to create a relation without a relationtype. Shouldnt be and actually wasnt the case. we need fix that.

[stefanprobst]

note that this currently also crashes the UI, e.g. https://ica.acdh-dev.oeaw.ac.at/apis/entities/entity/person/540/detail

looking at the response from https://ica.acdh-dev.oeaw.ac.at/apis/api/entities/person/540/ there's a empty relation type

[stefanprobst]

@steffres are you working on this?

[SteffRhes]

Sorry, I'll try to get to it.

[apis-sentry]

Sentry issue: APIS-PMB-M

[gregorpirgie]

seems to be linked to sentry issue #APIS-PMB-M and issue #325

[SteffRhes]

I've added a check to relation class if any foreign keys are null. This check happens on save call and not on the field definition itself as it might be necessary to temporarily allow None values in memory.

However this does not fix the currently missing relation_type of existing objects. These need to be manually re-written.

As for issue #325 I can query succesfully https://pmb.acdh.oeaw.ac.at/apis/api/relations/placeevent/?format=json%2Bnet&limit=51%22 though I don't understand how these issues are supposed to be connected.

relevant commit:
6864fe8

[stefanprobst]

just fyi, curl "https://pmb.acdh.oeaw.ac.at/apis/api/relations/placeevent/?format=json%2Bnet&limit=51" is totally still returning 500 for me.

[SteffRhes]

True. When I had curled it I forgot quotation marks and the & escaped into bash.

@gregorpirgie how do you think this issue is related to #325 ? In that sentry https://sentry.acdh-dev.oeaw.ac.at/organizations/acdhch-sentry/issues/142/?project=3 I don't see anything that points to a empty relation_type? There the error is 'NoneType' object is not subscriptable indicating that something of the line d2["source"]["url"].split("/")[-3].title() is None and an index of it is being accessed. From that code context I don't see anything pointing to relation_type though.

[SteffRhes]

As discussed in JF and investigated by Gregor, this looks like it's caused by the json serializer (https://github.com/acdh-oeaw/apis-core/blob/master/apis_core/api_renderers.py). Matthias will look into this.

[sennierer]

ok, so I reproduced the error locally. And it is caused by PlaceEvent.objects.get(id=35228) not having assigned a related_place. Before 6864fe8 added by @steffres it was possible (at least in the ORM) to set related entities in relations to None (reproduced that in a4787ba). So the cause of the problem is fixed with @steffres fix, but the problem persist in the data. I will add a fix to leave out relations that do not contain source and target, but we should also fix the data.

[sennierer]

please note (@stefanprobst ) that this fix leaves out the broken entries from the db. so `curl "https://pmb.acdh.oeaw.ac.at/apis/api/relations/placeevent/?format=json%2Bnet&limit=51" wont break the API anymore, but until the broken entry is fixed return only 50 entries instead of 51

[sennierer]

btw @steffres , as @gregorpirgie pointed me to it. Is there a special reason you added the check function rather than setting the null=True field to null=False (and same for blank) in https://github.com/acdh-oeaw/apis-core/blob/master/apis_core/helper_functions/EntityRelationFieldGenerator.py#L133-L134 ?

[sennierer]

the fix mentioned above is deployed on our instances

[stefanprobst]

can you quickly clarify what has been fixed, and what has not yet?

• https://ica.acdh-dev.oeaw.ac.at/apis/entities/entity/person/540/detail still returns 500
• curl https://ica.acdh-dev.oeaw.ac.at/apis/api/relations/personplace/542/ still returns an entity with relation_type: null
• curl "https://ica.acdh-dev.oeaw.ac.at/apis/api/relations/personplace/?limit=10" | jq '.results[].relation_type' still shows 4 out of the first 10 results have relation_type: null

EDIT: i guess this is mostly about validation on-save, not the response data?

[sennierer]

yes, validation is now in place, but there is still invalid data