Support for automation surface and action settings #1160
Conversation
🦋 Changeset detectedLatest commit: 13fd202 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| export type PlatformAppMultiSettingsStructureExport = { | ||
| default: PlatformAppSettingsStructureExport; | ||
| automation?: { | ||
| actions?: { | ||
| [customActionId: string]: PlatformAppSettingsStructureExport; | ||
| }; | ||
| }; | ||
| }; | ||
|
|
||
| export type PlatformAppConfigExport = { | ||
| app: FC; | ||
| settings: PlatformAppSettingsStructureExport; | ||
| settings: PlatformAppMultiSettingsStructureExport | PlatformAppSettingsStructureExport; |
There was a problem hiding this comment.
Looks like a lot of automation specific logic, how about just doing a new Type: "automation"? was there any discussion for that? Then we wouldnt need to adjust the settings structure nor adjust the web-app implementation and could use the default settings in it. Wouldnt the only downside be that then someone would need to have 2 Apps for 2 different functionalities?
There was a problem hiding this comment.
Hej @julianiff
We discussed many approaches to this. The main 3 where:
Add settings path to the action definition in the manifest
- Pro: Everything in one place, no duplicated automation definition (manifest + code)
- Con: Separate, diverging approach from current settings solution (Themes + Apps) -> leads to bad DX
Add new property "automation" to defineApp method
- Pro: No need to touch existing usage of settings in
web-app - Con: Adjustment to bundler needed -> probably resulting in multiple settings bundles
- Con: We would need to extend the
defineAppmethod with every new settings on the horizon -> as we already know, that we will be in need of different settings for apps in multiple places (e.g. project level etc.), this would lead to more complexity in the Brand SDK and the docs.
Extend the settings structure to support multiple PlatformAppSettingsStructureExport
This was the preferred approach in the end and the one we implemented.
- Pro: No changes to DX
- Pro: Future-proof solution, allowing to add further settings (e.g. for project, libraries or brand-level settings) easily without the need to adjust the bundler or add additional parameters to the
defineAppmethod - Pro: Shifts the complexity from Brand SDK to the
web-app-> preferred by Samuel - Con: require changes in the web-app in order to work and provide the right default settings (e.g. in the marketplace).
Moreover, we also discussed the new Type "Automation" quite a few times. While it might seem tempting in the first place (from a code perspective), the downsides are pretty big:
- First and foremost: We see tons of future use cases, where functionality in other surfaces is dependent on data provided by an automation (e.g. Visualize extracted data, trigger automation from a surface, especially to process existing assets etc.). Having them splitted in 2 apps makes app development and maintenance for the app builder a nightmare (version upgrades etc.)
- Having all functionality for a specifc service bundled in one app (e.g. Everything Slack = Slack App), makes it much easier for App Admins to keep the overview as the Marketplace is much less cluttered
- Introducing new Types for every bigger feature would come with repeated adjustments to the Marketplace, making it even more cluttered
- The "one app to rule it all" approach comes with many benefits from a DX perspective, as additional functionality can easily be added on every release, using the same app.
Hope this makes our decision more understandable.
There was a problem hiding this comment.
That makes a lot of sense for sure, thanks for going into details about it.
My follow up question is, how would you write documentation for the automations and would you make the distinctions between what an app can do and what an automation can do?
There was a problem hiding this comment.
Regarding the downsides:
1: If i understand it correctly, what is behind an Automation action is actions that correspond to the IDs in another system and not inside of an App. To enable a new action doesn't depend on the App, but on the third party system. When we then would like to use the output of an automation in an App, we need to define somehow code that we can run in isolation somewhere. This code will not be the same as the visual Code that is used in an App. This would then create another addition to the App structure. If we would add a new type, we could use the settings for settings, and the src folder later on for executable code that we want to run somewhere.
2 & 3 Im not sure about this, we are aiming to have the Marketplace as a lively place, to have more Apps in there sounds like the way to go.
4: Downside of one app to rule all is that for a developer its not clear what he has to do. We mix different concepts and introduce implicit rules on what goes together and what not. For example, when you build an automation you cannot use methods, or when building an App you shouldn't use the automation part in the mainfest.json because thats for the automation usecase. It makes the communication more difficult because we mix different concepts.
brunschgi
changed the title
The changes include:
PlatformAppSettingsStructureExportin the settings object)Manifest validation
Currently the automations surface definition has not been validated (unknowns are just bypassed). With the addition to the
verifyManifest.ts, we make sure that the automation surface definition adheres to the schema.Support for Automation settings
We iterated on the solution together with @getflourish and @olivereisenhut. The current approach is very flexible, allowing to add further settings (e.g. for project, libraries or brand-level settings) easily without the need to adjust the bundler or add additional parameters to the
defineAppmethod. This makes it easier to evolve app settings in the future, which was also a wish from @SamuelAlev.Please note: the changes to the
packages/platform-app/src/platformApps.tsrequire changes in theweb-appin order to work and provide the right default settings (e.g. in the marketplace).