Hyrax: Modifications to workflows cannot happen if there are items already in the workflow

[mjgiarlo]

Issue by vantuyls
_Friday Sep 02, 2016 at 16:34 GMT_
_Originally opened as https://github.com/projecthydra/sufia/issues/2623_

---

If there are items currently in a Mediated Deposit Workflow, no changes can be made to the workflow until in-process items are cleared out.

This is a first step towards a smarter way of being able to modify workflows with items in-process. In the future, we're hoping to be able to modify workflows with items in-process without orphaning or otherwise "screwing up" those in-process items.

Decomposes #2442

[mjgiarlo]

Comment by jeremyf
_Friday Sep 02, 2016 at 17:02 GMT_

---

@vantuyls There is an implication that Workflows should be time based. For a given Administrative Set and a given date there exists one associated workflow. However, for a different given date, there can be another workflow.

When we determine which workflow for a given item, we query the Workflows by Admin Set and an "As of Date" (e.g. a deposit started on this date).

I'm bringing in @mjgiarlo as this is a larger conversation regarding the longitudinal behavior of the application.

[mjgiarlo]

Comment by vantuyls
_Friday Sep 02, 2016 at 17:14 GMT_

---

Good idea. It's a bit delicate to build out these tickets with so much parallel work happening. @mjgiarlo thoughts?

[mjgiarlo]

Comment by mjgiarlo
_Friday Sep 02, 2016 at 18:05 GMT_

---

@jeremyf We were thinking the initial target would be a single workflow for the entire site, and it's either on or off. That lets us build out prerequisites and a first design, upon which we can iterate and make improvements. Which is to say, we'd approach admin set-related workflow in follow-up tickets. So, the first target is: there is one and only one workflow, and you can only modify it (add step, remove step, reorder steps) once 0 items are in the workflow. This gets us out of the business of, say, the items themselves needing to keep track of the state of the workflow.

Should we set up a time to chat, @jeremyf @vantuyls? Steve and I were talking yesterday about wanting to pull you in, to the extent that you are available of course, because you've already done some of this work so your head's already in this space.

[mjgiarlo]

Comment by jeremyf
_Friday Sep 02, 2016 at 19:03 GMT_

---

Yes. Pull me in. I can "consult" and provide guidance. I have a ridiculous
amount of code that can be referenced and leveraged. It is also possible
that we could extract Sipity's workflow into a gem/engine.

There are points of indirection that help it be AuthN/authz agnostic as
well as object in workflow agnostic.

Jeremy Friesen

On Sep 2, 2016 2:05 PM, "Michael J. Giarlo" [email protected]
wrote:

@jeremyf https://github.com/jeremyf We were thinking the initial target
would be a single workflow for the entire site, and it's either on or off.
That lets us build out prerequisites and a first design, upon which we can
iterate and make improvements. Which is to say, we'd approach admin
set-related workflow in follow-up tickets. So, the first target is: there
is one and only one workflow, and you can only modify it (add step, remove
step, reorder steps) once 0 items are in the workflow. This gets us out of
the business of, say, the items themselves needing to keep track of the
state of the workflow.

Should we set up a time to chat, @jeremyf https://github.com/jeremyf
@vantuyls https://github.com/vantuyls? Steve and I were talking
yesterday about wanting to pull you in, to the extent that you are
available of course.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/projecthydra/sufia/issues/2623#issuecomment-244447353,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAIUlhJuxuRciBFNyII-vGZEa980AT-ks5qmGV3gaJpZM4Jz4T8
.

[mjgiarlo]

Comment by mjgiarlo
_Friday Oct 28, 2016 at 22:49 GMT_

---

I can't think of a reason the 0-step workflow, the one we're currently working towards in Sufia, would change. Defer this to when 1+-step workflow is working.

[mjgiarlo]

Comment by mjgiarlo
_Friday Oct 28, 2016 at 22:50 GMT_

---

This should probably move into CC. Confirm/deny, @jcoyne?

[jeremyf]

This is a very big challenge. Dynamic workflows means there may be no real knowledge of "in-process" states; Think of a very simple "Published -> Redacted -> Published" workflow. There is no "in-process" state. They are all valid.

In Sipity, one constraint is that workflow actions are never deleted/removed from the database (even if the workflow JSON document is changed to remove existing steps).

I believe the following rules should be enforced:

Don't allow a Sipity::WorkflowState to be deleted if there are any objects in that state

With that rule in place, you should be able to modify a Sipity::WorkflowAction to have a different #resulting_workflow_state and thus enter a new path in the workflow.

[carolyncole]

@vantuyls @mjgiarlo @jeremyf From The information above it seems I have three choices.

1. We could configure which states it would be OK to reload/change the workflow
   
   
   
   1. add an optional configuration option to the workflow
   
   
   2. check for either no items, or items only in the configured states before allowing the workflow to change
   
   
   3. error on the workflow update if the above check fails otherwise change the workflow in place
   
   
2. Only change the workflow if no object has been created with it
   
   
   
   1. Rename the current workflow to include version information
   
   
   2. Move all existing AdminSets to point at the old workflow
   
   
   3. create a new workflow with the same name
   
   
3. Change the workflow if there is no object in the state being changed or removed.
   
   
   
   1. Compare the current workflow with the old workflow
   
   
   2. For any states being removed verify that no item is in that state
   
   
   3. error on the workflow update if the above check fails otherwise change the workflow in place
   
   

I think the first option being the simplest (I believe) to implement and possibly the simplest to explain to user long term.

The second one has the advantage of a history of all the different workflow states, but we would need to manage the old version in a way that new Admin Sets do not get assigned to those workflows. Possibly need a new state like archived or historical for a workflow so that the UI only shows those states.

The third one has the most flexibility for changing workflows, but may be a bit complex to understand.
What way do we want to go?

[jeremyf]

@cam156 Of the above options in order of preference:

1. Only change the workflow if no object has been created with it

As you said, that is perhaps infeasible, but is the cleanest. We could allow items to move from one workflow to another (i.e. if the item has a WorkflowState#name that is present in both workflows, then moves are possible)

1. Change the workflow if there is no object in the state being changed or removed.

This one creates a concrete rule in which we can rely on the system to enforce (e.g. "I'm Sorry Dave, I can't allow you to change this..."). It could give clear guidance in why it can't change.

1. We could configure which states it would be OK to reload/change the workflow.

This one is a bit more nebulous. Why wouldn't I configure all states to allow changes? If a workflow state allows changes, what happens to items in that workflow state when I delete the workflow state?

I believe, once 3 or 1 are explored, they will result in some very similar logic to protect items in a state that is marked for deletion. And if we are going to guard a state from deletion, I don't believe we need worry about a flag in the database saying the workflow action can change.

[carolyncole]

@jeremyf I was not really thinking about allowing the user to move between workflows. I was just thinking about a cache of pre-existing workflows. The name seems to be the "id" for a workflow so I thought renaming it would also entail a move but I could be wrong.

[jcoyne]

In my ideal world, workflows would be immutable. Workflows could have the same name and an incrementing version number. This is not a high priority for me, so I'm not going to put time into this now.

[jeremyf]

I second @jcoyne's ideal workflow world.

@cam156 I'm not quite following. I wasn't clear about which user would move an object from one workflow to another – I'm assuming an administrative user would move objects from one workflow to another. And yes, the name is the identifier for a workflow (though there is a primary key call :id); Part of my DBA training from ages past was that basing things on primary keys is a bad idea. So the name is a meaningful surrogate for the :id.

[vantuyls]

I like this one:
3) Change the workflow if there is no object in the state being changed or removed.

• Compare the current workflow with the old workflow
• For any states being removed verify that no item is in that state
• Error on the workflow update if the above check fails otherwise change the workflow in place

Though from our conversation on workflow sprint standup today suggests that we need to deal with the json file using names as IDs to identify workflows.

One solution may be to add a 'label' field in the json file. This would allow one to re-label a workflow step without mucking with the name which is tied to the database ID (right?).

@mjgiarlo @cam156

[carolyncole]

I verified that each time the import is run it only adds workflows to the system based on the name. If the name changes then a new workflow is created.

I will move forward with this ticket basing my work on the name being the workflow id, and that we will only delete workflow states if there are not items currently in that workflow.

[carolyncole]

At this point there is a PR in for the work, but since there is a lot of work left on that PR and I am no longer on the sprint I am unassigning myself from this ticket. Hopefully the next mediated deposit person can come in and figure this all out. The code is there and it works, but getting it through the community may be a large task.

[jeremyf]

I'm taking @cam156's branch, incorporating suggestions, and working on it.