Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

step review not implemented #133

Open
dpancic opened this issue Nov 16, 2022 · 7 comments
Open

step review not implemented #133

dpancic opened this issue Nov 16, 2022 · 7 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@dpancic
Copy link

dpancic commented Nov 16, 2022

In GitLab by @laureD19 on Nov 16, 2022, 21:02

In the review mode, it is currently not possible for a moderator to see the changes introduced by contributors on the steps of a workflow.

Is it complicated to implement review mode for steps? It would probably mean to have a visual clue of which step is concerned by a change on the following screen:
image

but for the rest, it seems the same approval/rejection comparison than other item types. Correct?

notify @KlausIllmayer @egray523 @vronk @tparkola

@dpancic
Copy link
Author

dpancic commented Nov 18, 2022

In GitLab by @laureD19 on Nov 18, 2022, 16:10

mock-up for this step screen? look into that

@KlausIllmayer
Copy link

Still to be solved before in SSHOC/sshoc-marketplace-backend#162

@stefanprobst Does the last comment mean that you would need a mock-up?

@laureD19 laureD19 added this to the Q3 milestone Aug 23, 2023
@stefanprobst
Copy link
Contributor

stefanprobst commented Oct 23, 2023

can we please clarify the expected api response for the following scenarios:

(i) deleted last step

# original
{ "steps": [{ "persistentId": 1 }, { "persistendId": 2 }, { "persistentId: 3" }] }
# suggested
{ "steps": [{ "persistentId": 1 }, { "persistendId": 2 }] }
API response for "deleted last step"
{
  "item": {
    "id": 57,
    "category": "workflow",
    "label": "The Test Workflow",
    "persistentId": "qzGZtH",
    "lastInfoUpdate": "2024-06-10T10:31:55+0000",
    "status": "approved",
    "informationContributor": {
      "id": 1,
      "username": "Administrator",
      "displayName": "Administrator",
      "status": "enabled",
      "registrationDate": "2020-08-04T12:29:00+0000",
      "role": "administrator",
      "email": "[email protected]",
      "config": true
    },
    "description": "For testing only.",
    "contributors": [],
    "properties": [],
    "externalIds": [],
    "accessibleAt": [],
    "relatedItems": [],
    "media": [],
    "composedOf": [
      {
        "id": 54,
        "category": "step",
        "label": "First step",
        "persistentId": "tCYrzh",
        "lastInfoUpdate": "2024-06-10T10:31:54+0000",
        "status": "approved",
        "informationContributor": {
          "id": 1,
          "username": "Administrator",
          "displayName": "Administrator",
          "status": "enabled",
          "registrationDate": "2020-08-04T12:29:00+0000",
          "role": "administrator",
          "email": "[email protected]",
          "config": true
        },
        "description": "First step.",
        "contributors": [],
        "properties": [],
        "externalIds": [],
        "accessibleAt": [],
        "relatedItems": [],
        "media": [],
        "composedOf": []
      },
      {
        "id": 56,
        "category": "step",
        "label": "Second step",
        "persistentId": "Vnp5lB",
        "lastInfoUpdate": "2024-06-10T10:31:54+0000",
        "status": "approved",
        "informationContributor": {
          "id": 1,
          "username": "Administrator",
          "displayName": "Administrator",
          "status": "enabled",
          "registrationDate": "2020-08-04T12:29:00+0000",
          "role": "administrator",
          "email": "[email protected]",
          "config": true
        },
        "description": "Second step.",
        "contributors": [],
        "properties": [],
        "externalIds": [],
        "accessibleAt": [],
        "relatedItems": [],
        "media": [],
        "composedOf": []
      },
      {
        "id": 58,
        "category": "step",
        "label": "Third step",
        "persistentId": "4LNx7w",
        "lastInfoUpdate": "2024-06-10T10:31:55+0000",
        "status": "approved",
        "informationContributor": {
          "id": 1,
          "username": "Administrator",
          "displayName": "Administrator",
          "status": "enabled",
          "registrationDate": "2020-08-04T12:29:00+0000",
          "role": "administrator",
          "email": "[email protected]",
          "config": true
        },
        "description": "Third step.",
        "contributors": [],
        "properties": [],
        "externalIds": [],
        "accessibleAt": [],
        "relatedItems": [],
        "media": [],
        "composedOf": []
      }
    ]
  },
  "equal": false,
  "other": {
    "id": 59,
    "category": "workflow",
    "persistentId": "qzGZtH",
    "lastInfoUpdate": "2024-06-10T10:32:55+0000",
    "status": "suggested",
    "informationContributor": {
      "id": 3,
      "username": "Contributor",
      "displayName": "Contributor",
      "status": "enabled",
      "registrationDate": "2020-08-04T12:29:00+0000",
      "role": "contributor",
      "email": "[email protected]",
      "config": true
    },
    "contributors": [],
    "properties": [],
    "externalIds": [],
    "accessibleAt": [],
    "relatedItems": [],
    "media": [],
    "composedOf": [
      {
        "id": 60,
        "category": "step",
        "label": "First step",
        "persistentId": "tCYrzh",
        "lastInfoUpdate": "2024-06-10T10:32:55+0000",
        "status": "suggested",
        "informationContributor": {
          "id": 3,
          "username": "Contributor",
          "displayName": "Contributor",
          "status": "enabled",
          "registrationDate": "2020-08-04T12:29:00+0000",
          "role": "contributor",
          "email": "[email protected]",
          "config": true
        },
        "description": "First step.",
        "contributors": [],
        "properties": [],
        "externalIds": [],
        "accessibleAt": [],
        "relatedItems": [],
        "media": [],
        "composedOf": []
      },
      {
        "id": 61,
        "category": "step",
        "label": "Second step",
        "persistentId": "Vnp5lB",
        "lastInfoUpdate": "2024-06-10T10:32:55+0000",
        "status": "suggested",
        "informationContributor": {
          "id": 3,
          "username": "Contributor",
          "displayName": "Contributor",
          "status": "enabled",
          "registrationDate": "2020-08-04T12:29:00+0000",
          "role": "contributor",
          "email": "[email protected]",
          "config": true
        },
        "description": "Second step.",
        "contributors": [],
        "properties": [],
        "externalIds": [],
        "accessibleAt": [],
        "relatedItems": [],
        "media": [],
        "composedOf": []
      }
    ]
  }
}

(ii) deleted first step

# original
{ "steps": [{ "persistentId": 1 }, { "persistendId": 2 }, { "persistentId: 3" }] }
# suggested
{ "steps": [{ "persistentId": 2 }, { "persistendId": 3 }] }
API response for "deleted first step" (using the same workflow as in (i))
PUT localhost:8080/api/workflows/w5xeKr/steps/9dG91D?persistentId=w5xeKr&stepPersistentId=9dG91D&draft=true

results in 404

(iii) inserted first step

# original
{ "steps": [{ "persistentId": 1 }, { "persistendId": 2 }] }
# suggested
{ "steps": [{ "persistendId": 3 }, { "persistentId": 1 }, { "persistendId": 2 }] }

(iv) inserted last step

# original
{ "steps": [{ "persistentId": 1 }, { "persistendId": 2 }] }
# suggested
{ "steps": [{ "persistentId": 1 }, { "persistendId": 2 }, { "persistentId: 3" }] }

(v) changes to one step, others are unchanged

# original
{ "steps": [{ "persistentId": 1, "label": "one" }, { "persistendId": 2, "label": "two" }] }
# suggested
{ "steps": [{ "persistentId": 1, "label": "one" }, { "persistendId": 2, "label": "CHANGED" }] }

(vi) reordered steps

# original
{ "steps": [{ "persistentId": 1 }, { "persistendId": 2 }, { "persistentId: 3" }] }
# suggested
{ "steps": [{ "persistentId": 3 }, { "persistendId": 1 }, { "persistentId: 2" }] }

(vii) reordered steps with changes to some

# original
{ "steps": [{ "persistentId": 1 }, { "persistendId": 2 }, { "persistentId: 3", "label": "three" }] }
# suggested
{ "steps": [{ "persistentId": 3, "label": "CHANGED" }, { "persistendId": 1 }, { "persistentId: 2" }] }

@laureD19
Copy link
Contributor

laureD19 commented Oct 24, 2023

better describe the scenarios - @KlausIllmayer @mkrzmr to seat with @stefanprobst sometime in June to better scenarios before sharing potential lproblems with Tomasz

@stefanprobst
Copy link
Contributor

stefanprobst commented Jun 10, 2024

detailed repro for scenario (i) and (ii) above - see the question at the very end!


1. sign in as "Administrator":

curl "http://localhost:8080/api/auth/sign-in" -X POST -H "content-type: application/json" -d '{"username":"Administrator","password":"q1w2e3r4t5"}' -i

get the token from Authorization header and save it to an env variable

export SSHOC_ADMIN="Bearer eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJBZG1pbmlzdHJhdG9yIiwiaWF0IjoxNzE4MDI4MDM2LCJleHAiOjE3MTgxMTQ0MzZ9.uGTCvWDPCoUQz2Vk3HlXxKDvncY0UdT7MpDYEA4X4ZjsHJ0L-XEVhl1UXpFDG6yoo1k_zanRk7pz2namFEEMug"

2. create a new workflow with 3 steps:

2.1 first create the workflow:

curl "http://localhost:8080/api/workflows" -X POST -H "Authorization: ${SSHOC_ADMIN}" -H "content-type: application/json" -d '{"label":"The test workflow","description":"For testing only."}' | jq

this will return:

JSON response
{
  "accessibleAt": [],
  "category": "workflow",
  "composedOf": [],
  "contributors": [],
  "description": "For testing only.",
  "externalIds": [],
  "id": 52,
  "informationContributor": {
    "config": true,
    "displayName": "Administrator",
    "email": "[email protected]",
    "id": 1,
    "registrationDate": "2020-08-04T12:29:00+0000",
    "role": "administrator",
    "status": "enabled",
    "username": "Administrator"
  },
  "label": "The test workflow",
  "lastInfoUpdate": "2024-06-10T14:01:02+0000",
  "media": [],
  "persistentId": "2mr9Rr",
  "properties": [],
  "relatedItems": [],
  "status": "approved"
}
we note the workflow's persistentId: "2mr9Rr"

2.2 proceed with creating 3 new steps:

curl "http://localhost:8080/api/workflows/2mr9Rr/steps" -X POST -H "Authorization: ${SSHOC_ADMIN}" -H "content-type: application/json" -d '{"label":"First step","description":"The first step."}' | jq
curl "http://localhost:8080/api/workflows/2mr9Rr/steps" -X POST -H "Authorization: ${SSHOC_ADMIN}" -H "content-type: application/json" -d '{"label":"Second step","description":"The second step."}' | jq
curl "http://localhost:8080/api/workflows/2mr9Rr/steps" -X POST -H "Authorization: ${SSHOC_ADMIN}" -H "content-type: application/json" -d '{"label":"Third step","description":"The third step."}' | jq

which returns:

Step 1
{
  "accessibleAt": [],
  "category": "step",
  "composedOf": [],
  "contributors": [],
  "description": "The first step.",
  "externalIds": [],
  "id": 54,
  "informationContributor": {
    "config": true,
    "displayName": "Administrator",
    "email": "[email protected]",
    "id": 1,
    "registrationDate": "2020-08-04T12:29:00+0000",
    "role": "administrator",
    "status": "enabled",
    "username": "Administrator"
  },
  "label": "First step",
  "lastInfoUpdate": "2024-06-10T14:01:52+0000",
  "media": [],
  "persistentId": "FoZEDZ",
  "properties": [],
  "relatedItems": [],
  "status": "approved"
}
Step 2
{
  "accessibleAt": [],
  "category": "step",
  "composedOf": [],
  "contributors": [],
  "description": "The second step.",
  "externalIds": [],
  "id": 56,
  "informationContributor": {
    "config": true,
    "displayName": "Administrator",
    "email": "[email protected]",
    "id": 1,
    "registrationDate": "2020-08-04T12:29:00+0000",
    "role": "administrator",
    "status": "enabled",
    "username": "Administrator"
  },
  "label": "Second step",
  "lastInfoUpdate": "2024-06-10T14:02:08+0000",
  "media": [],
  "persistentId": "f6LVoW",
  "properties": [],
  "relatedItems": [],
  "status": "approved"
}
Step 3
{
  "accessibleAt": [],
  "category": "step",
  "composedOf": [],
  "contributors": [],
  "description": "The third step.",
  "externalIds": [],
  "id": 58,
  "informationContributor": {
    "config": true,
    "displayName": "Administrator",
    "email": "[email protected]",
    "id": 1,
    "registrationDate": "2020-08-04T12:29:00+0000",
    "role": "administrator",
    "status": "enabled",
    "username": "Administrator"
  },
  "label": "Third step",
  "lastInfoUpdate": "2024-06-10T14:02:23+0000",
  "media": [],
  "persistentId": "ijQ1fh",
  "properties": [],
  "relatedItems": [],
  "status": "approved"
}

note the persistentIds of the steps: "FoZEDZ", "f6LVoW", "ijQ1fh"

3. sign in as "Contributor":

curl "http://localhost:8080/api/auth/sign-in" -X POST -H "content-type: application/json" -d '{"username":"Contributor","password":"q1w2e3r4t5"}' -i

get the token from Authorization header and save it to an env variable

export SSHOC_CONTRIB="Bearer eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJDb250cmlidXRvciIsImlhdCI6MTcxODAyODE3NiwiZXhwIjoxNzE4MTE0NTc2fQ.uadKiLhm0QMH6ct4LiYGZKI3pCpnL5JhynYHpN0cSD7J4JV6bt_FKrLXqC67SHdyi8Vdl3njJxcWcdMx0WhNKw"

4. edit the previously created workflow, and suggeset to delete the third step

we have noted the workflow's persistentId: "2mr9Rr". the third step has the persistentId:
"ijQ1fh".

note that we cannot simply use the DELETE method to delete the step (this is just for admins).
instead, we have to first apply all updates in draft mode, and then commit the changes to the
draft workflow
.

first, we need to set the workflow itself in draft mode:

curl "http://localhost:8080/api/workflows/2mr9Rr?draft=true" -X PUT -H "Authorization: ${SSHOC_CONTRIB}" -H "content-type: application/json" -d '{"label":"The test workflow","description":"For testing only."}' | jq

which returns:

JSON response
{
  "accessibleAt": [],
  "category": "workflow",
  "composedOf": [
    {
      "accessibleAt": [],
      "category": "step",
      "composedOf": [],
      "contributors": [],
      "description": "The first step.",
      "externalIds": [],
      "id": 54,
      "informationContributor": {
        "config": true,
        "displayName": "Administrator",
        "id": 1,
        "registrationDate": "2020-08-04T12:29:00+0000",
        "role": "administrator",
        "status": "enabled",
        "username": "Administrator"
      },
      "label": "First step",
      "lastInfoUpdate": "2024-06-10T14:01:52+0000",
      "media": [],
      "persistentId": "FoZEDZ",
      "properties": [],
      "relatedItems": [],
      "status": "approved"
    },
    {
      "accessibleAt": [],
      "category": "step",
      "composedOf": [],
      "contributors": [],
      "description": "The second step.",
      "externalIds": [],
      "id": 56,
      "informationContributor": {
        "config": true,
        "displayName": "Administrator",
        "id": 1,
        "registrationDate": "2020-08-04T12:29:00+0000",
        "role": "administrator",
        "status": "enabled",
        "username": "Administrator"
      },
      "label": "Second step",
      "lastInfoUpdate": "2024-06-10T14:02:08+0000",
      "media": [],
      "persistentId": "f6LVoW",
      "properties": [],
      "relatedItems": [],
      "status": "approved"
    },
    {
      "accessibleAt": [],
      "category": "step",
      "composedOf": [],
      "contributors": [],
      "description": "The third step.",
      "externalIds": [],
      "id": 58,
      "informationContributor": {
        "config": true,
        "displayName": "Administrator",
        "id": 1,
        "registrationDate": "2020-08-04T12:29:00+0000",
        "role": "administrator",
        "status": "enabled",
        "username": "Administrator"
      },
      "label": "Third step",
      "lastInfoUpdate": "2024-06-10T14:02:23+0000",
      "media": [],
      "persistentId": "ijQ1fh",
      "properties": [],
      "relatedItems": [],
      "status": "approved"
    }
  ],
  "contributors": [],
  "description": "For testing only.",
  "externalIds": [],
  "id": 59,
  "informationContributor": {
    "config": true,
    "displayName": "Contributor",
    "id": 3,
    "registrationDate": "2020-08-04T12:29:00+0000",
    "role": "contributor",
    "status": "enabled",
    "username": "Contributor"
  },
  "label": "The test workflow",
  "lastInfoUpdate": "2024-06-10T14:03:36+0000",
  "media": [],
  "persistentId": "2mr9Rr",
  "properties": [],
  "relatedItems": [],
  "status": "draft"
}
curl "http://localhost:8080/api/workflows/2mr9Rr/steps/ijQ1fh?draft=true" -X DELETE -H "Authorization: ${SSHOC_CONTRIB}" -i
curl "http://localhost:8080/api/workflows/2mr9Rr/commit" -X POST -H "Authorization: ${SSHOC_CONTRIB}" | jq

which returns:

JSON response
{
  "accessibleAt": [],
  "category": "workflow",
  "composedOf": [
    {
      "accessibleAt": [],
      "category": "step",
      "composedOf": [],
      "contributors": [],
      "description": "The first step.",
      "externalIds": [],
      "id": 54,
      "informationContributor": {
        "config": true,
        "displayName": "Administrator",
        "id": 1,
        "registrationDate": "2020-08-04T12:29:00+0000",
        "role": "administrator",
        "status": "enabled",
        "username": "Administrator"
      },
      "label": "First step",
      "lastInfoUpdate": "2024-06-10T14:01:52+0000",
      "media": [],
      "persistentId": "FoZEDZ",
      "properties": [],
      "relatedItems": [],
      "status": "approved"
    },
    {
      "accessibleAt": [],
      "category": "step",
      "composedOf": [],
      "contributors": [],
      "description": "The second step.",
      "externalIds": [],
      "id": 56,
      "informationContributor": {
        "config": true,
        "displayName": "Administrator",
        "id": 1,
        "registrationDate": "2020-08-04T12:29:00+0000",
        "role": "administrator",
        "status": "enabled",
        "username": "Administrator"
      },
      "label": "Second step",
      "lastInfoUpdate": "2024-06-10T14:02:08+0000",
      "media": [],
      "persistentId": "f6LVoW",
      "properties": [],
      "relatedItems": [],
      "status": "approved"
    }
  ],
  "contributors": [],
  "description": "For testing only.",
  "externalIds": [],
  "id": 59,
  "informationContributor": {
    "config": true,
    "displayName": "Contributor",
    "id": 3,
    "registrationDate": "2020-08-04T12:29:00+0000",
    "role": "contributor",
    "status": "enabled",
    "username": "Contributor"
  },
  "label": "The test workflow",
  "lastInfoUpdate": "2024-06-10T14:03:36+0000",
  "media": [],
  "persistentId": "2mr9Rr",
  "properties": [],
  "relatedItems": [],
  "status": "suggested"
}

5. still as "Contributor", make a different suggestion to delete the first step:

note that the workflow still has 3 steps (the previous suggestion has not been reviewed):

curl "http://localhost:8080/api/workflows/2mr9Rr"
JSON response
{
  "accessibleAt": [],
  "category": "workflow",
  "composedOf": [
    {
      "accessibleAt": [],
      "category": "step",
      "composedOf": [],
      "contributors": [],
      "description": "The first step.",
      "externalIds": [],
      "id": 54,
      "informationContributor": {
        "config": true,
        "displayName": "Administrator",
        "id": 1,
        "registrationDate": "2020-08-04T12:29:00+0000",
        "role": "administrator",
        "status": "enabled",
        "username": "Administrator"
      },
      "label": "First step",
      "lastInfoUpdate": "2024-06-10T14:01:52+0000",
      "media": [],
      "persistentId": "FoZEDZ",
      "properties": [],
      "relatedItems": [],
      "status": "approved"
    },
    {
      "accessibleAt": [],
      "category": "step",
      "composedOf": [],
      "contributors": [],
      "description": "The second step.",
      "externalIds": [],
      "id": 56,
      "informationContributor": {
        "config": true,
        "displayName": "Administrator",
        "id": 1,
        "registrationDate": "2020-08-04T12:29:00+0000",
        "role": "administrator",
        "status": "enabled",
        "username": "Administrator"
      },
      "label": "Second step",
      "lastInfoUpdate": "2024-06-10T14:02:08+0000",
      "media": [],
      "persistentId": "f6LVoW",
      "properties": [],
      "relatedItems": [],
      "status": "approved"
    },
    {
      "accessibleAt": [],
      "category": "step",
      "composedOf": [],
      "contributors": [],
      "description": "The third step.",
      "externalIds": [],
      "id": 58,
      "informationContributor": {
        "config": true,
        "displayName": "Administrator",
        "id": 1,
        "registrationDate": "2020-08-04T12:29:00+0000",
        "role": "administrator",
        "status": "enabled",
        "username": "Administrator"
      },
      "label": "Third step",
      "lastInfoUpdate": "2024-06-10T14:02:23+0000",
      "media": [],
      "persistentId": "ijQ1fh",
      "properties": [],
      "relatedItems": [],
      "status": "approved"
    }
  ],
  "contributors": [],
  "description": "For testing only.",
  "externalIds": [],
  "id": 57,
  "informationContributor": {
    "config": true,
    "displayName": "Administrator",
    "id": 1,
    "registrationDate": "2020-08-04T12:29:00+0000",
    "role": "administrator",
    "status": "enabled",
    "username": "Administrator"
  },
  "label": "The test workflow",
  "lastInfoUpdate": "2024-06-10T14:02:23+0000",
  "media": [],
  "persistentId": "2mr9Rr",
  "properties": [],
  "relatedItems": [],
  "status": "approved"
}

we set the workflow in draft mode again:

curl "http://localhost:8080/api/workflows/2mr9Rr?draft=true" -X PUT -H "Authorization: ${SSHOC_CONTRIB}" -H "content-type: application/json" -d '{"label":"The test workflow, second suggestion","description":"For testing only."}' | jq

which returns:

JSON response
{
  "accessibleAt": [],
  "category": "workflow",
  "composedOf": [
    {
      "accessibleAt": [],
      "category": "step",
      "composedOf": [],
      "contributors": [],
      "description": "The first step.",
      "externalIds": [],
      "id": 54,
      "informationContributor": {
        "config": true,
        "displayName": "Administrator",
        "id": 1,
        "registrationDate": "2020-08-04T12:29:00+0000",
        "role": "administrator",
        "status": "enabled",
        "username": "Administrator"
      },
      "label": "First step",
      "lastInfoUpdate": "2024-06-10T14:01:52+0000",
      "media": [],
      "persistentId": "FoZEDZ",
      "properties": [],
      "relatedItems": [],
      "status": "approved"
    },
    {
      "accessibleAt": [],
      "category": "step",
      "composedOf": [],
      "contributors": [],
      "description": "The second step.",
      "externalIds": [],
      "id": 56,
      "informationContributor": {
        "config": true,
        "displayName": "Administrator",
        "id": 1,
        "registrationDate": "2020-08-04T12:29:00+0000",
        "role": "administrator",
        "status": "enabled",
        "username": "Administrator"
      },
      "label": "Second step",
      "lastInfoUpdate": "2024-06-10T14:02:08+0000",
      "media": [],
      "persistentId": "f6LVoW",
      "properties": [],
      "relatedItems": [],
      "status": "approved"
    }
  ],
  "contributors": [],
  "description": "For testing only.",
  "externalIds": [],
  "id": 60,
  "informationContributor": {
    "config": true,
    "displayName": "Contributor",
    "id": 3,
    "registrationDate": "2020-08-04T12:29:00+0000",
    "role": "contributor",
    "status": "enabled",
    "username": "Contributor"
  },
  "label": "The test workflow, second suggestion",
  "lastInfoUpdate": "2024-06-10T14:12:24+0000",
  "media": [],
  "persistentId": "2mr9Rr",
  "properties": [],
  "relatedItems": [],
  "status": "draft"
}

Important

note that the user never explicitly saved changes as a draft. also note, that we committed the draft changes in the previous step. nevertheless the last response retured the intermediary draft from (4.), which has the third step deleted. is this expected behavior? should the previous draft have been garbage-collected when calling /commit?

@KlausIllmayer
Copy link

KlausIllmayer commented Jul 17, 2024

@stefanprobst and me where sitting together today and we looked into the scenario (iii) inserted first step. We tried it out based upon the last comment of Stefan where he explained in detail the scenario (i) and (ii):

  1. Login as moderator
  2. Create workflow with two steps and publish it
  3. Change login to contributor
  4. Add new step to the workflow on position 1 and suggest the change
  5. Create a diff of the suggestion to compare it with the published workflow from step 2 => GET /api/workflows/uK1QpB/versions/72109/diff?with=uK1QpB (uK1QpB is the persistentId of the workflow, 72109 is the version of the suggested workflow from step 4)

Step 5 should give us the difference between these two workflows. The only changes are the addition of a new step and that this new step is on first position (meaning that the others moved on position back). We expected to see this very clear in the output of the diff-call, but the result is disappointing:

JSON response ```json { "item": { "id": 72109, "category": "workflow", "label": "test dev insert first", "persistentId": "uK1QpB", "lastInfoUpdate": "2024-07-17T13:47:24+0000", "status": "suggested", "informationContributor": { "id": 3, "username": "Contributor", "displayName": "Contributor", "status": "enabled", "registrationDate": "2021-05-28T15:28:06+0000", "role": "contributor", "email": "[email protected]", "config": true }, "description": "insert a step on first place", "contributors": [], "properties": [], "externalIds": [], "accessibleAt": [], "relatedItems": [], "media": [], "composedOf": [ { "id": 72110, "category": "step", "label": "step 3", "persistentId": "RLvGPB", "lastInfoUpdate": "2024-07-17T13:47:25+0000", "status": "suggested", "informationContributor": { "id": 3, "username": "Contributor", "displayName": "Contributor", "status": "enabled", "registrationDate": "2021-05-28T15:28:06+0000", "role": "contributor", "email": "[email protected]", "config": true }, "description": "the third step", "contributors": [], "properties": [], "externalIds": [], "accessibleAt": [], "relatedItems": [], "media": [], "composedOf": [] }, { "id": 72111, "category": "step", "label": "step one", "persistentId": "2jHRYE", "lastInfoUpdate": "2024-07-17T13:47:25+0000", "status": "suggested", "informationContributor": { "id": 3, "username": "Contributor", "displayName": "Contributor", "status": "enabled", "registrationDate": "2021-05-28T15:28:06+0000", "role": "contributor", "email": "[email protected]", "config": true }, "description": "the first step", "contributors": [], "properties": [], "externalIds": [], "accessibleAt": [], "relatedItems": [], "media": [], "composedOf": [] }, { "id": 72112, "category": "step", "label": "step two", "persistentId": "ec8pK9", "lastInfoUpdate": "2024-07-17T13:47:25+0000", "status": "suggested", "informationContributor": { "id": 3, "username": "Contributor", "displayName": "Contributor", "status": "enabled", "registrationDate": "2021-05-28T15:28:06+0000", "role": "contributor", "email": "[email protected]", "config": true }, "description": "the second step", "contributors": [], "properties": [], "externalIds": [], "accessibleAt": [], "relatedItems": [], "media": [], "composedOf": [] } ] }, "equal": false, "other": { "id": 72109, "category": "workflow", "persistentId": "uK1QpB", "lastInfoUpdate": "2024-07-17T13:47:24+0000", "status": "suggested", "informationContributor": { "id": 3, "username": "Contributor", "displayName": "Contributor", "status": "enabled", "registrationDate": "2021-05-28T15:28:06+0000", "role": "contributor", "email": "[email protected]", "config": true }, "contributors": [], "properties": [], "externalIds": [], "accessibleAt": [], "relatedItems": [], "media": [], "composedOf": [ { "id": 72106, "category": "step", "label": "step one", "persistentId": "2jHRYE", "lastInfoUpdate": "2024-07-17T13:46:06+0000", "status": "approved", "informationContributor": { "id": 2, "username": "Moderator", "displayName": "Moderator", "status": "enabled", "registrationDate": "2021-05-28T15:28:06+0000", "role": "moderator", "email": "[email protected]", "config": true }, "description": "the first step", "contributors": [], "properties": [], "externalIds": [], "accessibleAt": [], "relatedItems": [], "media": [], "composedOf": [] }, { "id": 72108, "category": "step", "label": "step two", "persistentId": "ec8pK9", "lastInfoUpdate": "2024-07-17T13:46:06+0000", "status": "approved", "informationContributor": { "id": 2, "username": "Moderator", "displayName": "Moderator", "status": "enabled", "registrationDate": "2021-05-28T15:28:06+0000", "role": "moderator", "email": "[email protected]", "config": true }, "description": "the second step", "contributors": [], "properties": [], "externalIds": [], "accessibleAt": [], "relatedItems": [], "media": [], "composedOf": [] } ] } } ```

What astonished us: that some obviously not changed values are showing up, e.g. the persistentId, the informationContributor the label and description of the steps. It is also irritating, that the other.id is the same as the item.id. It is also clear, that there is lot of burden on the frontend part, to understand what is happening here. We can give some interpretations of the result (e.g. label and description is the same because there is no diff for the steps themselves, one would need to call an explicit diff of the steps => which would be annoying) and also think of possible workarounds.

All in all we came to the conclusion, that we would need a better documentation of the diff-behaviour by the backend team: first to confirm our interpretations and secondly to understand the diff-approach in general so that workaround are not necessary. One could also argue that the current diff-approach is to simplified and we would need better return values so that frontend can more easily visualize the differences and not doing much logic on its side, e.g. what about status-values in the diff that clearly mark what has changed.

@tparkola @laureD19 @mkrzmr We are proposing that someone from the backend team evaluates the current code of the diff-approach and by doing so writes down a documentation of the behaviour of the diff-calls, ideally based upon the identified scenarios by Stefan (see the comment above: #133 (comment)). The goal should be to rewrite the diff-approach so that there is less logic to be done by frontend as it would be currently the case. What do you think?

@vronk
Copy link
Contributor

vronk commented Oct 9, 2024

pending implementation of SSHOC/sshoc-marketplace-backend#470

@vronk vronk removed this from the Q3 milestone Oct 9, 2024
@laureD19 laureD19 added this to the Q4 2024 milestone Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants