OpenConceptLab / ocl_issues

Issues for all OCL repos. NOTE: Install ZenHub Browser Extension and request access to the OCL Roadmap board to view all issues and to contribute
4 stars 1 forks source link

Mixed up concept IDs in collections #1860

Closed jamlung-ri closed 2 days ago

jamlung-ri commented 2 weeks ago

We discussed on OCL Chat how it is possible to put two of the same concept IDs in a single collection if they come from different sources. This can cause mappings to display incorrectly, potentially due to a bug in cascade where it does not consider the underlying source?

@snyaggarwal Please let us know if discussion is needed on this one, but it might be a straightforward fix to change $cascade to consider source more effectively.

snyaggarwal commented 1 week ago

@jamlung-ri this is an interesting one. This is not just a bug in $cascade but overall in all the collections->concept/mapping behaviors. Because we access concepts and mappings with ID, the collection doesn't know which concept to return. It returns the first one always. If you see the response of 1062:$cascade you will see its cascading the other 1062 concept because always as that the first one added. Though we have handled this in concept details API 1062 Adding this for discussion in our dev call tomorrow

snyaggarwal commented 1 week ago

@jamlung-ri @paynejd Here is whats happening with Collection:TestPhysio.

  1. It has MSFOCB:1062 and MSF:1062 concepts.
  2. On clicking on any one of these concepts the details are fetched of the correct concept because the URL has concept's version -- MSFOCB:1062 --> MSFOCB:1062:5448245.
  3. $cascade doesn't (yet!) work on resource version and also doesn't return conflict

Collection's Concept:1062 does return Conflict response and can be resolved by using uri param. This URI param works with both concept's URI and with source's URI

jamlung-ri commented 1 week ago

Thanks @snyaggarwal for the good insight. Should we queue this up for discussion at an upcoming call? Or do you feel that it'd be sufficient to cascade using the concept's version ID? It doesn't necessarily solve the underlying problem, but it would be a quick fix in TermBrowser.

snyaggarwal commented 1 week ago

@paynejd We can do a quick fix by exposing $cascade the same way as it is right now but the API route allow concept-version in it to recognize the right concept. For better solution we should add this to our discussion

jamlung-ri commented 1 week ago

We had some discussion about this today in OCL's Arch call, but I am unclear if we came up with any next steps or approaches. @snyaggarwal I imagine we need to continue the discussion before anything more can be done?

snyaggarwal commented 4 days ago

@jamlung-ri We decided that we will use the same behaviour as concept details.

  1. /$cascade/ should throw 409
  2. /$cascade/?uri= will resolve this.

This is deployed on QA.

jamlung-ri commented 4 days ago

@snyaggarwal This seems to be failing in QA. All cascade requests occurring in my test collection are returning with 409 errors.

Here's the full response that happens each time I try to open a concept to view its Associations:

{"detail":"Conflict."}
snyaggarwal commented 3 days ago

@jamlung-ri This should be fixed now on QA

jamlung-ri commented 3 days ago

This is now working great on QA! The UI is querying OCL's API using the URI parameters, which is clearing up the discrepancy of which source to query from.

Example queries for the same id in my test collection:

@snyaggarwal Let's move this forward.

snyaggarwal commented 2 days ago

@jamlung-ri This is deployed everywhere, closing this