sillsdev / appbuilder-portal

Portal for Scriptoria -- App Publishing Service
MIT License
25 stars 5 forks source link

Add workflow configuration data to WorkflowDefinitions table #1026

Closed FyreByrd closed 2 weeks ago

FyreByrd commented 2 weeks ago

Added two fields to WorkflowDefinitions:

FyreByrd commented 2 weeks ago

I am not 100% sold on the fields being integers. It would be somewhat more readable if they were strings. This would probably require changing the underlying data type of the enums to be strings, which could possibly cause a performance hit for using them as often as they are used.

chrisvire commented 2 weeks ago

@FyreByrd Is this to get these config out of the TypeScript code and into the database? For future Workflow Definitions, we will need UI to edit/add these settings?

FyreByrd commented 2 weeks ago

@FyreByrd Is this to get these config out of the TypeScript code and into the database? For future Workflow Definitions, we will need UI to edit/add these settings?

Yes. I do intend to add UI for that as well, but I wasn't sure whether that should be part of this PR or a different one. I've been trying to keep DB changes as separate minimal PRs.

chrisvire commented 2 weeks ago

@FyreByrd Yes, I think it is good to have separate PRs. You could use interactive rebasing to have the database in one commit and the UI in one comment. It is just a matter of making sure to do a Rebase and merge instead of Squash and merge. Realistically, separate PRs are probably better.

FyreByrd commented 1 week ago

We may want to remove WorkflowScheme and WorkflowBusinessFlow from the WorkflowDefinitions table in the future. Of course, those will remain for now, in keeping with backwards compatibility with S1 during development.