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

PI Migration - Form updates #1922

Open
madhurrya opened this issue Jul 15, 2024 · 6 comments
Open

PI Migration - Form updates #1922

madhurrya opened this issue Jul 15, 2024 · 6 comments
Assignees

Comments

@madhurrya
Copy link
Contributor

madhurrya commented Jul 15, 2024

In a model, if I update only the things related to a form (json schema etc) it'll not recognize that as an update for the migration.

I changed few things in the json schema for this user task and then I tried to migrate it.

Image

It says 'The process instance is already associated with the most recent process model version.'

Image

Test model : https://timetracking.spiffworkflow.org/process-models/misc:qa:madhu-testing:simple-form-1

json schema forms are not part of the spiff serialization. as a result, if you start a process instance and then change a form json schema, you will not be allowed to migrate the instance to the new form definition (since the system does not see that there is any change that would warrant a migration). to mitigate this, we propose:

consider how to bring existing environments into alignment with the new approach.

we were thinking about adding a table, form_schema, to link up git revisions, form hashes, form filenames, and process model identifiers with their corresponding form contents. we know we need to get something into the serialization in order for the user task form definition to work seamlessly for process instance migration, so we also started writing a Custom converter for user tasks with some guidance from Elizabeth. we got hung up because actually getting the form schema hash turns out to be not at all straightforward. from inside the converter, we could figure out what process model the task spec is in, potentially, and do the filesystem and json operations to determine the hash, but this seems slow and potentially outside the scope of what a to_dict function is generally supposed to do. since this is not something that obviously affects myome, we'll set it aside for now.

@jasquat
Copy link
Contributor

jasquat commented Jul 15, 2024

UPDATED: removed references to dmn files since they are irrelevant.

Forms are an interesting issue.

Right now we are only focused on the bpmn diagram which SpiffWorkflow is able to handle. json files are external to the diagram so in order to update to the new form, we'd have to update the git revision on the process instance so it'll load the new files.

We could probably just allow updating the git revision on the instance if there are no changes but the revision has changed. This would change the form that appears for previous user tasks on the "Tasks" tab on the process instance show page as well. Although this shouldn't really be a problem if the bpmn xml didn't change or even if migration was allowed in the first place.

@madhurrya
Copy link
Contributor Author

I also thought the same issue will be there with the dmn files. But when I updated the dmn values, it recognized the dmn updates in the migration.

@jasquat
Copy link
Contributor

jasquat commented Jul 15, 2024

Oh right, dmn files are added the spiff serialized version of the bpmn xml so they should get picked up in the check and migration like normal.

@jasquat jasquat self-assigned this Jul 15, 2024
@jasquat jasquat moved this from New Issue to In Progress in SpiffWorkflow Jul 17, 2024
@burnettk
Copy link
Contributor

this is a valid issue, and we are thinking we might leave it for now, since some use cases may not emphasize forms. we sort of looked into a robust fix in #1936 and didn't find a clear path. there is also the possibility that we allow some sort of force feature from the UX, even if the system itself doesn't know of any reason why a migration would be necessary. it feels unfortunate to put this on the user (especially if forcing migrations is not something they will commonly do), but it might be easyish to implement.

@madhurrya
Copy link
Contributor Author

@burnettk so are we planning to release the migration work as it is now? Doesn't MyOme need that/use that much? Or are we going to just inform them that currently it is not supported?

@burnettk
Copy link
Contributor

Yeah, I asked and they are good with it.

@calexh-sar calexh-sar moved this from In Progress to Backlog in SpiffWorkflow Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

3 participants