search

SSHOC / sshoc-marketplace-backend

Code for the backend
Apache License 2.0
2 stars 0 forks source link

Status of a step is ignored in relations #401

Closed KlausIllmayer closed 3 months ago

KlausIllmayer commented 1 year ago

It seems that for the item category STEP the status of such items is ignored when it comes to relations. It must - like for the other item categories - only show the steps to the public that do have the status APPROVED but it also shows at least the status SUGGESTED. This is not only relevant on level of visibility but also on the way how relations are processed. This leads to cascading effects that ultimately breaks the view of items with such relations. We already have two such items in the production version. When opening them you will get a 500 error. Not only on the frontend but also in many of the API calls, e.g. there is tool in such a corrupt state that currently breaks also the API call /api/tools-services. Thus this bug has high priority to prevent further breaks. We also need to find a way to fix the current broken items.

@tparkola Can you have a look into the backend code and identify, why relations to steps ignore the status? The bug itself is complicated to understand and to explain. I will try to extend the issue with new findings. I will also send you a dump of the database so that you can debug it.

How to replicate:

As the errors only show up when you have in the history of an item a relation to a suggested as well as an approved step, it is important to operate on two permission levels as a contributor and as a moderator.

  1. Moderator creates a Tool which should be related from the Step of a Workflow. After saving it, it is immediately published as the role is Moderator.
  2. Moderator creates a Workflow without a step. After saving it, it is immediately published
  3. Switch to Contributor and edit the created Workflow from step 2. Within the Workflow create a new Step that has a relation to the Tool that was created in step 1. After saving it, the Workflow/Step get the status SUGGESTED as the role is a Contributor.
  4. If you open the public version of the Tool created in step 1, you already see the relation to the Step even if the Step itself is not APPROVED. Clicking on it will show the Workflow without the Step.

This is the easiest way to get a proof of the bug. There are many variants which leads to different weird states. As an example, if you create the Workflow in step 2 as a Contributor, this also gets the status SUGGESTED. After adding the Step to this suggested workflow as same Contributor including the relation to the created Tool from step 1, the public will again see the suggested Step as explained in step 4. Additionally, the suggested workflow is exposed by this and clicking on the step will try to show the suggested workflow which ends up in an error message.

Additionally, we also see further weird error message, e.g. after approving the Workflow/Step as Moderator it is not possible anymore to edit the created Tool from step 1. After saving it it will give you an error message and all changes are lost. You can delete the relation, which looks like to address the issue, but if again applying the relation you will run into the same error. This makes sense, as the suggested version is still in the history and for the relation is dealt the same way as the approved version.

KlausIllmayer commented 11 months ago

@tparkola pointed out in another communication channel that the replicate step 4 is only true if contributor is still logged in. This means for the public version you won't see the suggested relation. As the contributor sees the suggested items, it is a correct behaviour. If we like to change it, we need again to think about visibility of such suggestions for the person who suggested them. But this is not really the point of this issue, which is thus wrongly labeled. It is more about the corrupt state of the /api/tools-services-list where I had hopes, that this is connected with the later described weird states. I will describe the problematic errors in a dedicated comment, because they are more troubling and should be focused. For the visibility states I won't introduce new behaviour, we may need to think about describing it in the documentation in more detail.