diff data for workflows does not include info about added/removed steps #162

Closed dpancic closed 1 year ago

In GitLab by @stefanprobst on Mar 4, 2022, 16:14

retrieving the diff between two workflow versions does not provide info about inserted or removed workflow steps.

i realize that i can get the diff for individual steps via /workflow/:workflowId/steps/:stepId/diff, but there currently seems no way to know about deleted/inserted steps, e.g. when the current workflow version has:

{ composedOf: [{ persistentId: 'step1' }, { persistentId: 'step2' }] }

and the suggested version has:

{ composedOf: [{ persistentId: 'step2' }] }

let me know if you need more detailed reproduction steps.

In GitLab by @stefanprobst on Mar 4, 2022, 19:28

we could do this manually client-side, if the diff data for workflows did include diff.item.composedOf

In GitLab by @tparkola on Mar 10, 2022, 09:59

Diff response for both workflow entity does now include composedOf field to compare with. Changes on dev branch.

In GitLab by @laureD19 on Jun 22, 2022, 10:43

marked this issue as related to sshoc-marketplace-frontend#84

In GitLab by @KlausIllmayer on Jun 28, 2022, 17:09

@stefanprobst can you have a look if this is solved as you expected? if so, please close the issue.

I've tested it on stage and it seems that it is not solved, re-assigning to @tparkola

Created as admin a workflow without a step
Suggested as contributor to change the workflow: added one additional contributor and added two steps
Calling API GET /api/workflows/[persistent_id-workflow]/diff?with=[persistent_id-workflow]&otherVersionId=[last-version-id-suggested-by-contributor]: it says equal=false and for contributor it identifies the change but not for steps. In the published workflow - which is delivered back - there is "composedOf": [] which is correct (see 1.) but in the other-section of the diff it says "composedOf": [null, null] not giving the step-id back of the changes.

I've tried also a different workflow without adding additional information (like the contributor in 2.) and only adding a new step. In such a case the diff tells me that equal=true. So for me it seems that added steps are not delivered back in the diff.

Also deleting a step is not covered by the diff. Here also the information about the current approved version in item delivers back wrong information about composedOf, saying that there is no step inside where there should be one (I created a workflow with a step as admin and suggested a new version as contributor where I delete the step, when looking into the result of the diff from 3. it says that the version of the admin does not have a step but a dedicated GET /api/workflows/[persistent_id-workflow]/versions/[version-id-of-admin-created-version] on this version shows, that there is a step).

Additionally, I found out that the diff does not check, if the user is allowed to access the version. A GET /api/workflows/[persistent_id-workflow]/versions/[version-id-of-suggested-version-by-contributor] without a bearer token gives a 403 and an error about not authorized. But when calling GET /api/workflows/[persistent_id-workflow]/diff?with=[persistent_id-workflow]&otherVersionId=[version-id-of-suggested-version-by-contributor] gives no authorization error and shows differences.

Additional information what I would expect with real data from stage (you can try it out there):

In the case of an added step calling GET /api/workflows/3RLXrl/diff?with=3RLXrl&otherVersionId=53066 currently delivers:

{
  "item": {
    "id": 53065,
    "category": "workflow",
    "label": "workflow test diff",
    "persistentId": "3RLXrl",
    "lastInfoUpdate": "2023-03-17T16:24:21+0000",
    "status": "approved",
    [...]
    "composedOf": [
      {
        "id": 53067,
        "category": "step",
        "label": "contributor added step",
        "persistentId": "rb6NNY",
        "lastInfoUpdate": "2023-03-17T16:26:45+0000",
        "status": "suggested",
        "informationContributor": {
          [...]
        },
        "description": "like to see it in diff",
        "contributors": [],
        "properties": [],
        "externalIds": [],
        "accessibleAt": [],
        "relatedItems": [],
        "media": [],
        "composedOf": []
      }
    ]
  },
  "equal": true,
  "other": {
    "id": 53066,
    "category": "workflow",
    "persistentId": "3RLXrl",
    "lastInfoUpdate": "2023-03-17T16:26:45+0000",
    "status": "suggested",
    [...]
    "composedOf": [
      null
    ]
  }
}

whereas it should be in my understanding the opposite way:

{
  "item": {
    "id": 53065,
    "category": "workflow",
    "label": "workflow test diff",
    "persistentId": "3RLXrl",
    "lastInfoUpdate": "2023-03-17T16:24:21+0000",
    "status": "approved",
    [...]
    "composedOf": []
    ]
  },
  "equal": false,
  "other": {
    "id": 53066,
    "category": "workflow",
    "persistentId": "3RLXrl",
    "lastInfoUpdate": "2023-03-17T16:26:45+0000",
    "status": "suggested",
    [...]
    "composedOf": [
      {
        "id": 53067,
        "category": "step",
        "label": "contributor added step",
        "persistentId": "rb6NNY",
        "lastInfoUpdate": "2023-03-17T16:26:45+0000",
        "status": "suggested",
        "informationContributor": {
          [...]
        },
        "description": "like to see it in diff",
        "contributors": [],
        "properties": [],
        "externalIds": [],
        "accessibleAt": [],
        "relatedItems": [],
        "media": [],
        "composedOf": []
      }
    ]
  }
}

equal must be false, for the composedOf it seems to me that it is mixed up the versions.

I investigated the issue and I found the cause, i.e. when using /api/workflows/[persistent_id-workflow]/diff?with=[persistent_id-workflow]&otherVersionId=[last-version-id-suggested-by-contributor] the system loaded wrong versions to compare in case of workflow steps - it compared steps of the latest workflow regardless of its approved state - so in case of steps it compared the same things and no changes were found. I fixed that - now it loads the latest approved version, as in case of the workflow itself. There was also a bug in the code, which complicated even more the response. Visibility issue has been fixed as well. Please check if it works for you now as expected.

Former example was deleted from stage, created a new one (Admin creates a workflow without a step and publishes it, Contributor edits the workflow and add a new step) and tested the API call api/workflows/[persistentId]/diff?with=[persistentId]&otherVersionId=[versionIdOfChangedWorkflowByContributor]: now the result looks like it was expected.

SSHOC / sshoc-marketplace-backend

diff data for workflows does not include info about added/removed steps #162