Integration of VISIAN into DV

[Alorcus]

This branch makes it possible to import, export and work with scans from the Discovery Viewer (DV) from Mt. Sinai in Visian.

Due to the lack of an API interface from DV, the import still has to be done manually. An example import file has been provided for this purpose (see _libs\utils\src\lib\backend\dv\importExample_schema.json). This branch implements a new strategy: DVReviewStrategy with associated DVReviewTask (comparable to the integration of the WHO). In addition, a number of objects are introduced which are required for successful import and export. Finally, smaller methods have also been added to Visian, but their scope is very limited and does not further change the functionality of Visian.

Please note:
As it is not yet possible to recognise whether the DV strategy should be active, the code must be adapted manually. To do this under libs\utils\src\lib\url-utils.ts isFromDV change return false to return true, so that the DV strategy is used and the demo json is automatically imported.

[TimRiedel]

Flipping for rois is also fixed now. Additionally I renamed methods and classes of DV to reflect the concepts of VISIAN (such as layers and annotation groups) that are named differently in DV.

[TimRiedel]

Hey @richartkeil @Alorcus, I took the opportunity to document the review strategy and review task abstract classes. This should have been done months ago when we initially implemented them, but only now I realized that due to the complexity of the import logic, this is necessary for future developers.

During that process I noticed, that the intended behavior of the "strategy" is to handle the VISIAN concepts (layers, document, annotation groups, etc.) and that the "task" is usually only responsible for conversion to backend specific data formats and backend calling. It operates on generic types like files or metadata etc. For DV we handle it a bit differently, because the task directly operates on documents, layers etc.

The code for DV is well readable, but I am wondering whether we should enforce this responsibility division?
Especially because we have some methods like updateAnnotation on the task which does nothing at all right now, which looks a bit weird at a first glance. Furthermore, we provide the task access to the entire document instead of limiting its access only to the parts that it really needs.

What do you think? @Alorcus could you elaborate on why the interface of updateAnnotation in the DV Task was not suitable for the DV implementation?

[Alorcus]

The code for DV is well readable, but I am wondering whether we should enforce this responsibility division?

Thank you for all the improvements! That makes a lot of parts much smoother!
Regarding the updateAnnotation method. I have not yet managed to implement the updateAnnotation. This is because, on the one hand, I have focussed on the import and export for the fundamental integration of the DV connection and, on the other hand, it is not clear to me how this updateAnnotation method is supposed to work. You pass an annotationId and an array of files. So far so good. What kind of files? And what exactly should be updated? The entire task? Then we could use the existing updateTask() method within the ReviewTask.

Regarding the division of ReviewStrategy and ReviewTask, this can definitely be separated. During the implementation, it was not clear to me where exactly the boundary between the two classes lies. Since, as you have now also helpfully added, there was no documentation about it.

[TimRiedel]

...it is not clear to me how this updateAnnotation method is supposed to work. You pass an annotationId and an array of files. So far so good. What kind of files?

The update Annotation method should be called in order to store all the changes of one Annotation Group (remember, for MIA and the WHO an Annotation consists of multiple layers, which is called Annotation Group in Visian, because it groups the layers -> one could probably think of a more consistent naming here). Referencing the MIA and WHO strategy, such an annotation is uniquely identified by an ID, but may contain multiple layers (files). They are either zipped for MIA or stored in separate Annotation Layer objects in the WHO task (and usually contain some metadata like ids).

And what exactly should be updated? The entire task? Then we could use the existing updateTask() method within the ReviewTask.

For the WHO we store an intermediate task object that we update in this method. We do not send the changes to the backend yet in the updateAnnotation method. This is because the WHO only allows pushing the entire task to the /next endpoint on their side (save method), which pushes the task and automatically fetches the next one. For MIA we update the annotations directly in the backend from the updateAnnotation method, because we have the possibility to update each annotation individually and not just the entire task. That's why the save method is not implemented for MIA.

So the rough workflow for export would look like this:

1. Strategy iterates over annotations groups and gets the files (with metadata) for each group.
2. It calls updateAnnotation with the annotation ID and the files of the group.
3. Based on the metadata in the files the updateAnnotation method looks up the specific layers (in DVs case the ROIs) and stores them in the intermediate object
4. After all changes were made, we call the save method to finally push the entire task object to the DV backend.

Please let me know if the current documentation is sufficient to understand that or which improvements the documentation could need.

---

So the question now remains, how do we continue with this PR? Would you be able to implement some of the things mentioned before your holiday? I am unsure for two reasons:

• The code works and is understandable, so from a formality standpoint, the feature is done. Furthermore, when adding this additional layer of abstraction (responsibility division) it might be a bit more complex because then we have to work with generic metadata on files (which can be anything basically) and have to think about how to map this DV structure to a metadata representation that allows for easy retrieval in the export. Thirdly, more and more I think the review strategy/task would need a little refactoring in itself for it to not force the developers to implement methods that they do not need, but that would be out of scope here.
• On the other side the WHO and MIA both support the current interface and for them it makes sense to do it that way. Additionally, the review strategy implements generic methods like importAnnotationsWithMetadata that already do the heavy lifting for group and layer creation in VISIAN, which means that for new strategies one would not have to think about all that stuff again. In the end it would be more consistent with the current implementation and idea of strategy and task and could benefit readability if it is implemented the same everywhere.

[Alorcus]

So the question now remains, how do we continue with this PR? Would you be able to implement some of the things mentioned before your holiday?

As part of the goal of having VISIAN ready for a handover, we should focus on having a clean repo at the end. PRs that are still incomplete at the end - as we had at the beginning of the semester - are - as we also had to realize - more of a burden for subsequent teams than a gain. I therefore recommend leaving it at the current implementation. We now have a detailed description of the next steps for integration here in this PR. I can also refer to this in our report so that it is communicated transparently.
Personally, I will no longer have the time to make such major changes, and bear in mind that in addition to the pure implementation, there is at least one feedback loop, which often takes more time than you might intuitively expect.