Esri / solution.js

TypeScript wrappers running in Node.js and modern browsers for transferring ArcGIS Online items from one organization to another.
https://esri.github.io/solution.js/
Apache License 2.0
38 stars 11 forks source link

Add support for Survey123 webhooks #1317

Open chris-fox opened 7 months ago

chris-fox commented 7 months ago

It is possible to configure one or more webhooks on a survey123 form and we are starting to see solutions that would leverage them, for example to send a payload to Workflow Manager when a survey is submitted. In looking at the survey api for this it would seem we would need to do the following:

On Creation:

  1. Query the endpoint https://www.arcgis.com/sharing/rest/content/items/e18da2375a18455aa4d2c1d8975c0b1a/info/form.json, where the item id is the id of the survey123 form.
  2. This returns a response that under settings.notificationInfo can have a webhooks property which can contain an array of webhook objects. If the property exists we should store the webhooks array in the template properties object for the form.
  3. We also need to templatize any item id references or portal urls in the webhook definition. For example there may be an item id of an ArcGIS Workflow in the url property of the webhook

On Deployment:

  1. Create the survey form as usual
  2. After the survey is created there is an api to add a webhook: https://survey123.arcgis.com/api/survey/e18da2375a18455aa4d2c1d8975c0b1a/webhook/add where the item id is the id of the new survey123 form.
  3. Then in the payload you can provide the definition of the same detemplatized definition of the webhook stored with the template.

f: json webhook: {"active":true,"name":"Test","url":"https://www.esri.com","includePortalInfo":true,"includeServiceRequest":true,"includeUserInfo":true,"includeServiceResponse":false,"includeSurveyInfo":false,"events":["addData"]} portalUrl: https://www.arcgis.com

Below is an example of a survey123 form with a webhook configured for testing: https://localdeployment.maps.arcgis.com/home/item.html?id=df5239803ee144a09a1f3983d3747f18

MikeTschudi commented 5 months ago

Independent of webhooks and the form.json file, an interesting observation regarding this issue: when creating a Solution, we copy the info section of the Form as a resource and restore it when we deploy. This section is stored in a zip file with the item id of the source item as its name.

The info zip file contains

The four emboldened files contain the source item id. During deployment, neither the zip file name nor the four files is modified to use the deployed item's id.

While the deployed form appears to work, this is fixable. As part of this issue, I'm swizzling the zip file name and the id in the four files during deployment.

chris-fox commented 4 months ago

@MikeTschudi, I tested this and found that the webhooks are coming across with the survey now. However, I found references to the portal url and item ids in the webhook url are not being properly replaced with the variables.

To reproduce create a new solution from the following group: https://localdeployment.maps.arcgis.com/home/group.html?id=2b2465c697cf43ae93582959fd8c60ab

It contains a survey with 2 webhooks and web map. One of the webhooks called 'Swizzle Webhook' has a payload url set to https://localdeployment.maps.arcgis.com/home/item.html?id=ce3dec81bf714d3bb71da9691ab686d. This is the url of the web map that is also included in the solution.

When we create a solution from this group I would expect the payload url to be be updated to '{{portalBaseUrl}}/home/item.html?id={{ce3dec81bf714d3bb71da9691ab686d.itemId}}'.

Then on deployment I would expect this to be swizzled and the resulting survey has a webhook pointing to the correct portal url and the new webmap that was created during deployment.

MikeTschudi commented 4 months ago

Thank you, Chris. The code is only aware of esriinfo/form.* files; I'll fix it to handle general file names. And all swizzling is during deployment; I can change that to templatize during creation and swizzling during deployment.

chris-fox commented 4 months ago

Ahhh that makes sense. One thing I noticed through the browser debugger is that survey123 queries this 'forminfo.json' which has the name of the form. For example:

{
    "name": "Tree Service Request",
    "type": "xform"
}

From there it knows the names of the other files. Might help in discovering the correct files in the zip.

MikeTschudi commented 4 months ago

@chris-fox, the sample *.info file contains webhook URLs

and I changed the code to templatize them to

Does this look correct?

chris-fox commented 4 months ago

Yes, but don’t the item ids need to be the .itemId property like below:

https://workflow.arcgis.com/{{orgId}}/{{661e0e38d4ad4a8f97731b22cb6c95dd.itemId}}/webhooks/createJobFromSurveyResponse/toKxLD9VRi6A8YR9eb7TUw

{{portalBaseUrl}}/home/item.html?id={{ce3dec81bf714d3bb71da9691ab686d1.itemId}}

chris-fox commented 4 months ago

@MikeTschudi, I was testing this today and found when I deploy a survey if the webhook is defined in the form.info file of the zip that we upload for the survey then the webhook is applied to the resulting deployed survey. It looks like this might be all we need to do rather than trying to call the webhook/add api. If we can swizzle the variables in the form.info json of the zip prior to uploading this may be the answer/

chris-fox commented 4 months ago

@MikeTschudi, as part of this issue could you look at a regression I am seeing. If you deploy the item below with a survey it fails on the survey. This same item deploys successfully using the code on production so it appears to be a regression with how we are handling survey123 forms.

https://localdeployment.maps.arcgis.com/home/item.html?id=9b46d19117354182956b13ac5ccadc53

MikeTschudi commented 4 months ago

@MikeTschudi, as part of this issue could you look at a regression I am seeing. If you deploy the item below with a survey it fails on the survey. This same item deploys successfully using the code on production so it appears to be a regression with how we are handling survey123 forms.

https://localdeployment.maps.arcgis.com/home/item.html?id=9b46d19117354182956b13ac5ccadc53

This may be related to the earlier incorrect implementation of webhooks because I can deploy it with the current version on my computer.

MikeTschudi commented 4 months ago

@MikeTschudi, I was testing this today and found when I deploy a survey if the webhook is defined in the form.info file of the zip that we upload for the survey then the webhook is applied to the resulting deployed survey. It looks like this might be all we need to do rather than trying to call the webhook/add api. If we can swizzle the variables in the form.info json of the zip prior to uploading this may be the answer/

The webhooks are definitely re-created via our app without the special call; thank you.

My problem at this time is determining swizzling. For example, consider these webhooks:

A possible rule is to only templatize the protocol+hostname and any AGO-id-matching patterns in a URL where its hostname matches the pattern "*.arcgis.com" but is not "workflow.arcgis.com". In addition, any AGO-id-matching patterns in these URLs have to be added to the Form's dependencies so that they are included in the solution and are available for de-templatizing during deployment.

My concern is that this is looking like a fragile approach that will fail with additional special cases.

@chris-fox, observations?

chris-fox commented 4 months ago

@MikeTschudi, With regard to the protocol and hostname. I think we should only look for the protocol and hostname that match the org that owns the form that is being packaged. So for example if the form is owned by https://mysite.maps.arcgis.com then we would replace references to https://mysite.maps.arcgis.com with {{portalBaseUrl}}. If it is anything else then we can ignore it and not replace. This is the logic we use in other item types as well.

With regards to item ids. Could we do this in a post process after all the items associated with the solution have been found and packaged? In this scenario we would just search for item ids that match an item that is being packaged with the solution. All other 32-character code references can be left unchanged.

I don't think we need to autodiscover dependencies via the webhooks. So for example if a Survey123 form has a webhook that references a workflow item by its item id, if the user doesn't package the workflow item with the survey we won't auto discover it. And as a result we won't swap out the item id reference with the variable. If, however the user includes the workflow and the survey in the group when creating the solution we will swap out the item id reference because we know the id of the workflow being included. We can then add that item as a dependency of the form in the post process.

This logic is consistent with how we are swapping out item ids referenced in other items as well in the post process.

MikeTschudi commented 4 months ago

The best that I can see for testing for owning the form is the isOrgItem property, which is described as "Indicates whether this item and the user whose credential was used to fetch this item belong to the same ArcGIS Enterprise Portal or ArcGIS Online Organization." I'll go with this until I find something better.

chris-fox commented 4 months ago

@MikeTschudi, sorry we don't need to do that. Can we just look for the org url that the user is logged into when creating the solution? We just want to match that. I know we have similar logic elsewhere in the app for replacing the org url with {{portalBaseUrl}}

MikeTschudi commented 4 months ago

I can definitely do that--thanks. It was the org ownership of the form itself that I was trying to solve.

chris-fox commented 4 months ago

I think generally our requirement for creating a solution is that you are an owner of the item or an admin of the org you are creating the solution from (This is definitely the case for feature services). So your logged in user can be assumed to apart of the same org as the items we are building solutions for.

MikeTschudi commented 4 months ago

It appears that we need the webhook dependency check. Using the "Survey123 Webhook" group as an example, it has two items

Because the two items don't have a dependency on each other, they're deployed in parallel. The Form has a dependency on the Web Map via a webhook, but without having the Web Map in the Form's dependency list, it's just luck if the deployed Web Map exists when the Form needs it for a post-process swizzle of the webhook AGO id.

chris-fox commented 4 months ago

@MikeTschudi, could we:

  1. In the original templatization logic during creation if we discover an item id from the solution referenced via a webhook we add the that item as a dependency of the survey form.
  2. We may still have cases of circular dependencies. Can't we treat the survey like we do with other items that if we have unresolved variables during the creation of the survey than we run it again at the end after all the items have been created to try to replace the variables again and then update the survey zip again?
MikeTschudi commented 4 months ago

With 1, I can put that dependency query back. How about I add the same restriction as for templatizing with {{portalBaseUrl}}: the hostname needs to be part of the same org?

2 sounds good; if the item is not yet available, it will come back with an unresolved Promise, which is recognizable and can be used to indicate that we need to revisit the swizzle.

chris-fox commented 4 months ago

With 1 I am not sure the item id url will always have the hostname in the url. Is it possible to do this:

  1. When we search through the webhook definition we look for every item id that is being packaged with the solution
  2. If we find a reference to one of the item ids, we replace it with the variable
  3. We then add that item id as a dependency of the survey form.

We do this logic for every item id that is found in the webhook definitions.

MikeTschudi commented 4 months ago

I can put that handling back. There's one special case that I bumped into: the default map id, which is an item that we don't include in a Solution. Currently, I leave this alone.

    "displayInfo": {
        "map": {
            "defaultType": {
                "name": "id:8b18b6ff9f39438fab763b888bab4b2f||Community Map"
            },
chris-fox commented 4 months ago

Yes, that is a living atlas basemap, per the logic because we don't package that with the solution we should not templatize it.

chris-fox commented 4 months ago

I can put that handling back. There's one special case that I bumped into: the default map id, which is an item that we don't include in a Solution. Currently, I leave this alone.

    "displayInfo": {
        "map": {
            "defaultType": {
                "name": "id:8b18b6ff9f39438fab763b888bab4b2f||Community Map"
            },

I just want to be clear with this logic above, I don't want to add special cases to the logic for specific properties. If item id "8b18b6ff9f39438fab763b888bab4b2f" is one of the items being packaged with the solution we should replace it with the variable. If it is not we should not replace it with a variable.

We should let which items are packaged with the solution determine which item ids are replaced with variables. Not auto discover item ids.

MikeTschudi commented 3 months ago

@chris-fox , do we need to support creating solutions in Enterprise at this time?

chris-fox commented 3 months ago

We do have some cases where users will author their solutions in enterprise for use in enterprise, so yes. Does this impact this issue?

MikeTschudi commented 3 months ago

Only a little bit. It is a bit awkward to recognize the user's portal for replacement in the webhook. I found an AGO property and an Enterprise property that I think will work.

chris-fox commented 3 months ago

Thanks, i will say that on deployment {{portalBaseUrl}} is being replaced properly for enterprise, so whatever we are using to get that value on deployment should work.

chris-fox commented 3 months ago

Verified this with the solution.js demo app. Was able to create a solution with a couple of webhooks and it handled swizzling the url that included the org url and item in the solution and a workflow manager project. Looks great, thanks!

chris-fox commented 1 month ago

@MikeTschudi, we found an issue where a webhook was not being properly replaced for workflow manager and as a result this needs a little additional work. This is not needed for R2, but we will want to fix early for R3 so the team can test and implement their solution.

Steps:

  1. Create a solution using the following group: https://localdeployment.maps.arcgis.com/home/group.html?id=99f096ee4183483ab4fcf2e5604eca31
  2. Download the zip from the survey after the solution is created. You will see that the webhook defined with in the form.info file has had the org id replaced with the variable. However there is another file called form.json that also defines webhooks and it hasn't been replaced with a variable.

Are we looking for specific files with this replacement logic? I am wondering if we could make it more generic and look through all files, with the exception of the XLSX per previous issues?

Related, rather than replacing: https://workflow.arcgis.com/piPfTFmrV9d1DIvN/f0dc11d2a53640e3be3af6360062ead5/webhooks/createJobFromSurveyResponse/FGVtlVN1RO2_bhjsHOHXCA

with:

https://workflow.arcgis.com/{{user.orgId}}/f0dc11d2a53640e3be3af6360062ead5/webhooks/createJobFromSurveyResponse/FGVtlVN1RO2_bhjsHOHXCA

Could we replace it with something like:

{{workflowBaseUrl}}/f0dc11d2a53640e3be3af6360062ead5/webhooks/createJobFromSurveyResponse/FGVtlVN1RO2_bhjsHOHXCA

This would allow it to work for Enterprise as well when the workflow base url will be different then workflow.arcgis.com.

Lastly, we are swapping out the workflow item id on deployment properly (f0dc11d2a53640e3be3af6360062ead5), but it seems weird to me that we are not replacing it with the variable like we do in all other places. So could we do this too creating a url like:

{{workflowBaseUrl}}/{{f0dc11d2a53640e3be3af6360062ead5.itemId}}/webhooks/createJobFromSurveyResponse/FGVtlVN1RO2_bhjsHOHXCA

MikeTschudi commented 1 month ago

Currently, we only look in *.info files. I'll generalize it to all but .xlsx.

MikeTschudi commented 1 month ago

<name>.info has

{
  "notificationsInfo": {
    "webhooks": [
      "url": "..."
    ]
  }
}

<name>.json has

{
  "serviceInfo": {
    "url": "..."
  },
  "settings": {
    "notificationsInfo": {
      "webhooks": [
        "url": "..."
      ]
    }
  }
}

<name>.itemInfo has a null url property, and I'm not templatizing it.

where "<name>" is provided by the forminfo.json file under the property "name".

The other files don't have properties that appear to be templatizable.

<name>.info's serviceInfo property looks similar to "https://services7.arcgis.com/&lt;orgId&gt;/arcgis/rest/services/survey123_da927ccfb40b4f15b390814b26079dc6_form/FeatureServer"; I'm templatizing it to {{<serviceInfo.itemId>.url}}

chris-fox commented 1 month ago

@MikeTschudi, can we not add custom logic for each file and property in the zip? I think it is too difficult to stay on top of as they change their spec. We have more generic searching for item id references, feature service urls in things like ExB that I think work really well and is flexible as the spec changes.

What I would propose is during creation at the very end after all the dependencies have been discovered for all templates included in the solution we loop through each file in the zip with the exception of the .xlsx and do the following:

Let's not search and replace the orgID specifically anymore, as this causes problems like in your example above for the feature service. The serviceInfo property you have above should be handled with the appropriate {{f0dc11d2a53640e3be3af6360062ead5.url}} property, because this is a feature service that is included in the solution.

When we deploy the solution we would should be able to replace/resolve all these references. If they are not handled in the first pass of adding the item we can handle them in the post process once all the items have been created in the deployment.

MikeTschudi commented 3 weeks ago

Implemented via PR #1468