Add back removed job and archive item tests

[jmsmkn]

In #154 two tests were removed as the functionality was removed. It is planned to add this functionality back in https://github.com/DIAGNijmegen/rse-roadmap/issues/335, so the tests also need to be added here. See #155.

• reintroduce test to replace test_update_interface_kind_of_archive_item_image_civ
• reintroduce test to replace test_create_job_with_upload

[amickan]

The functionality tested with test_update_interface_kind_of_archive_item_image_civ does not exist on GC anymore. It was removed in comic/grand-challenge.org#3156

It's not straight-forward to add this back now that the CIV creation is standardized everywhere and if this should only be possible for archive items. If we do want this, it will need to be done in a seperate PR from the main refactoring PR that I'm currently working on.

[amickan]

Having looked at this some more, I wonder if we really need this functionality? Users can just delete archive items and display sets and recreate them with the correct interface, if need be. So from a user perspective, updating the interface kind is no longer necessary. Of course, if they go this route, we end up with duplicate data, but how often does someone do this at the moment anyway? Implementing the option to update the interface is not straight-forward at all and introduces a lot of complexity to the CIV creation & validation code. I vote to drop this.

[jmsmkn]

I don't think they can do that as if they remove the image from the display set they lose permissions unless it is held in another display set.

[jmsmkn]

And this happens all the time, especially when people upload images as the wrong type, or want to use the image with another algorithm when the interface is different.

[amickan]

I don't think they can do that as if they remove the image from the display set they lose permissions unless it is held in another display set.

Ok, so we want this also for display sets? Previously this was only implemented for archive items, as far as I know. So then the requirement is that this works for display sets and archive items and should it also work for all interface types of only for images?

[amickan]

or want to use the image with another algorithm when the interface is different.

This is a different issue. They can select existing images when creating an algorithm job, so that's already possible.

[amickan]

should it also work for all interface types of only for images

Actually this is only feasible for images. For values and json files changing the interface might invalidate the value/file, so for those it doesn't make sense.

[jmsmkn]

Should work the same everywhere, so for jobs, archive items or display sets. Only doing this for images is fine, those data are much larger and something we do not want to duplicate or make the user wait a long time to re-import.

[amickan]

For jobs updating the interface does not make sense. A job can only be created with the interfaces that are defined on the algorithm at job creation time. We don't update job inputs later on. Or am I missing something?

[jmsmkn]

That's true, thanks.

[chrisvanrun]

AFAIK test_create_job_with_upload is already in the main branch atm.

[chrisvanrun]

Note: with the major refactor of helper functions, using an existing image or CIV for the create_job_with_upload comes out of the box.

[amickan]

Looking once again at how to implement updating the interface kind for images on archive items and display sets, I wonder the following:

Is it a (common) use case to have duplicate images in an archive item / display set (i.e. the same image attached to different interfaces in one and the same DS / AI)? I don't think it is, but it is currently possible / allowed (we do not enforce uniqueness on the value of attached CIVs, only on the interface).

Given that we cannot assume that an image is only linked to 1 interface on a given DS / AI at the moment, I don't think that the previous implementation of this feature was sound. The user used to just be able to update the interface like this, simply providing a new value pair.

client.update_archive_item(
      archive_item_pk="1234",
      values={"generic-overlay": image["api_url"]},
    )

If an archive item contains that image twice, it's impossible to know which of the existing CIVs to remove/ replace. The past implementation would have deleted both existing CIVs, but I think this is unexpected and inconsistent with the fact that we allow duplicate images on the CIVSets otherwise.

So, for a new implementation, I feel like it would be better to move this to its own router action and explicitly ask the user to provide old_interface_slug and new_interface_slug. They don't need to provide the image_pk, since we know that through the old interface slug. We would need to check that both interfaces are image kinds and throw an error if not. And otherwise, create and validate the new CIV, and then (if valid) add it and remove the old CIV.

[jmsmkn]

I don't think it's a common use case but I think the explicit replacement idea is really good, would make validation of old to new and what to replace really clear.