search

SenseNet / sn-workflow

Workflow component for the sensenet platform, based on Windows Workflow Foundation 4.5
GNU General Public License v2.0
3 stars 2 forks source link

WorkflowNotificationObservers shouldn't have reference to PortalContext #4

Open pietervanh opened 6 years ago

pietervanh commented 6 years ago

https://github.com/SenseNet/sn-workflow/blob/dd6b0840504ac8bfb0049b68f9cb1ec34352e8bc/src/Workflow/WorkflowNotificationObserver.cs#L164

It's bad to presume you have a portalcontext here. I can start a save operation on a content in a contentlist with workflows that are started on new items, from a workflowactivity. Where you don't have a portalcontext.

tusmester commented 6 years ago

It is true that this should not fail if there is no portalcontext present. The problem is that if there is none (meaning we are on a background thread), we do not know which is the "owner site", we cannot fill this field - which may cause a problem if the workflow expects it to be there (e.g. wants to send emails, with urls in it).

If you start a workflow from another workflow, you could pass on this info to the new workflow. But this way we make this the caller's responsibility.

pietervanh commented 6 years ago

I think it's more logical it reads "OwnerSiteUrl" from /Root/System/Settings/Workflow.settings

tusmester commented 6 years ago

There is an issue with having a global setting for this: usually we use this base url for constructing urls in emails sent from the workflow. But these workflows can be started under different sites, and we do not know this inside the workflow. And even if we knew the site (for example because the workflow content is under one of the sites), we wouldn't know which url of that site should we use (as sites may have multiple urls). This is why we added the OwnerSiteUrl property to the workflow itself, because that is filled at the time the workflow is started.

I do not know how could we construct a proper configuration for this. Of course we could add a simple global setting for a url as a fallback if the value is not available on the wf. This could work in case of smaller projects, where there is only a single public url for your site. But I think this could be only a fallback, because workflows should be able to work in a multi-site environment.