Open RFSH opened 1 year ago
The issue related to the ID
and Name
value format came up again. In this smite usecase, the Name
value also has :
in it. But we're assuming any string with :
is only ID
and therefore it's picking up the wrong ID
.
As a result, the link to the record app for the annotated term is wrong.
Summary
As we described in this diagram, the current implementation of the viewer app requires an "annotated term" column. This column must be a foreign key to a "Vocabulary" table, with "ID" and "Name" columns. We're also assuming that the "ID" column has a value in
<ID_domain>:<ID_value>
format.In this issue, I will go over why we're making this assumption and how we can potentially relax these rules. Because, in the end, we only need a label/name for the drawn annotation. And from the user's perspective, the label/name can be free-form and doesn't have to be a controlled vocabulary as long as there's some unique identifier for the annotation. And even if we want to keep the foreign key relationship, we shouldn't assume the format of
ID
value and it should work withRID
.Details
As I mentioned,
ID
andName
columns are required. The following are places where we're using these two columns:id
attributes based on the selected "Annotated term". Theid
will be in the following format:<ID>,<Name>
.id
attribute I mentioned in the previous bullet to find the database's correspondingImage_Annotation
record.<ID_domain>:<ID_value>
format and doesn't accept values of other formats.ID
is used in the list of annotations to create a link to theImage_Annotation
record.<Name>(<ID>)
.Other than the column requirement, we're assuming that the relationship between
Image_Annotation
andAnnotated_Term
is based onID
column. Given that we are already configuring the foreign key constraint, we should be able to derive this column by finding the foreign key object.The following are the improvements that we can make to this implementation:
Relax the
ID
value formatWe should be able to use the
RID
column as theannotated_term_id_column_name
. While most features work properly, and users can successfully create annotations with this configuration, the annotation load logic is "broken".When we're trying to match the
id
s in the SVG file with theImage_Annotation
records, instead of just using the value, we're also checking the format. Relaxing this format checking, should allow us to useRID
instead ofID
.This requires more thought as the
ID
value format checking was done to sanitize the old files we were getting from different vendors. If this is still an issue in some deployments, we should add a separate config property to enable it. Implementation-wise, this should be very simple.Relax the foreign key relationship requirement
As we mentioned, we only care about a "unique constraint" for each annotation group. So, in theory, the viewer app should be able to handle just a unique non-foreign key column for "annotated term" as well. But, as we listed, the foreign key relationship (and
ID
,Name
columns) assumption is baked into the viewer app. Therefore making this change would require a lot more redesign of how the viewer app works and configures. Since this is not a priority, we decided not to worry about this improvement. If use cases for this improvement arise, we need to discuss the required changes separately.