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.

Sign up for GitHub

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

Jump to bottom

Added workflow response object to workflow creation call #409

Merged
merged 5 commits into from
Aug 20, 2024
Merged

Added workflow response object to workflow creation call #409

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3db50f9
Added workflow response object to exchanges.yml
PatStLouis Aug 6, 2024
a670049
Update exchanges.yml
PatStLouis Aug 8, 2024
5efdafd
Update exchanges.yml
PatStLouis Aug 8, 2024
3f91229
Remove `stepInformation` property from workflow config
PatStLouis Aug 20, 2024
9ee8ff9
Replace occurences of workflowData with config
PatStLouis Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 51 additions & 6 deletions exchanges.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,17 +217,62 @@ components:
type: string
template:
type: string
workflowId:
id:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id can be optional within the WorkflowConfig object when creating a workflow, indicating either a preferred id (which some servers may honor, others may reject).

type: string
description: the ID that will be used for the created workflow. workflowId is OPTIONAL.
description: The ID that will be used for the created workflow. Passing an ID is OPTIONAL.
CreateWorkflowResponse:
type: object
additionalProperties: false
description: Object containing information about a created workflow.
description: Response containing the created workflowData Object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change workflowData -> config.

Same changes with workflowX -> X (in general).

properties:
workflowId:
type: string
description: The URL that uniquely identifies the created workflow.
workflowData:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think just the workflow config should be returned here and it looks like this is something a little different with some additional metadata is here. Certainly an implementation could do more, but I think we should only say that the workflow config is returned (i.e,. the same thing you'd get if you did a GET on the workflow ID).

Suggested change
workflowData:
config:

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a copy of what is returned by the GET workflowId as you are suggesting with the addition of the workflowId itself under the 'id' property as mentioned in the original issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see we have a couple of mistakes in what's in the spec now then. There should be no workflowData property when creating a workflow, rather, the properties here:

vc-api/exchanges.yml

Lines 190 to 222 in 5efdafd

properties:
steps:
type: object
description: One or more steps required to complete an exchange on the workflow. Passing the steps object is REQUIRED.
properties:
stepName:
type: object
description: The name of the step.
properties:
step:
$ref: "#/components/schemas/WorkflowStep"
initialStep:
type: string
description: The step from the above set that the exchange starts on. Passing intialStep is REQUIRED.
controller:
type: string
description: The controller of the instance. Passing controller is OPTIONAL.
authorization:
type: string
description: Authorization scheme information (e.g., OAuth2 configuration). Passing authorization is OPTIONAL.
credentialTemplates:
type: array
description: One or more VC templates for issuance. Passing credentialTemplates is OPTIONAL.
items:
type: object
properties:
type:
type: string
template:
type: string
id:
type: string
description: The ID that will be used for the created workflow. Passing an ID is OPTIONAL.

Should just be the main top-level properties in the payload, this is the "config" for the workflow.

I also see that the GET workflowId section is inconsistent and adds that extra stepInformation bit -- which we should remove (collapse it down so its nested properties are moved up a level so it matches what should be in the above payload for creating the workflow).

type: object
additionalProperties: false
description: Object containing information about a created workflow.
properties:
id:
type: string
description: The URL that uniquely identifies the created workflow.
stepInformation:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the call: Delete stepInformation

type: object
description: Information about the steps required for the workflow. Returning stepInformation is REQUIRED.
properties:
exchanges:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be a potentially huge number of exchanges for a very popular workflow -- and it isn't part of the config, so we shouldn't do this as a requirement, though some implementation might return this kind of data if that is desirable/not problematic.

type: array
description: The identifiers of the current exchanges associated with the workflow instance.
items:
type: string
PatStLouis marked this conversation as resolved.
Show resolved Hide resolved
steps:
type: object
description: One or more steps required to complete an exchange on the workflow.
properties:
stepName:
type: object
description: The name of the step.
properties:
step:
$ref: "#/components/schemas/WorkflowStep"
initialStep:
type: string
description: The step from the above set that the exchange starts on.
Copy link
Contributor

@dlongley dlongley Aug 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the stepInformation bit should be collapsed to what's in the config, i.e., I think steps and initialStep are nested here when they shouldn't be. See also note on "current exchanges".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a copy of the config of an existing workflow returned by the get workflowId route, could you provide an example of what the config should look like if this is not it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move steps and initialStep up/left one level.

controller:
type: string
description: The controller of the instance. Returning controller is OPTIONAL.
authorization:
type: object
description: Authorization scheme information (e.g., OAuth2 configuration). Returning authorization is OPTIONAL.
credentialTemplates:
type: array
description: One or more VC templates for issuance. Returning credentialTemplates is OPTIONAL.
items:
type: object
properties:
type:
type: string
description: The type of template.
template:
type: string
description: The template itself.
GetWorkflowResponse:
type: object
additionalProperties: false
Expand Down