TryGhost / Zapier

Ghost <-> Zapier Integration
https://developer.zapier.com/app/1566
MIT License
5 stars 3 forks source link

Unstable fields returned by member triggers and actions #37

Open naz opened 3 years ago

naz commented 3 years ago

Problem

Fields which are not considered "stable" are returned in response body for triggers and actions. This will cause breaking changes in the future if some of these fields are renamed/removed or any other breaking change is introduced to the API.

Current state

We currently return whole member resource in member_created trigger and responses to member_* actions. We are in a pickle here because current version (2.3.2) already returns "unstable" fields like stripe_* and geolocation , example:

Screenshot from 2020-10-22 16-06-57

Solution

Because these fields are already out there and might be relied upon by existing Zaps, we can't just remove unstable fields all of the sudden. They should not have been introduced in the first place, which should be taken into account when connecting the integration with new resources in the future.

A possible solutions for breaking changes in the Members API would be introducing a "shim" for every field which gets broken. The change should be rolled out as minor or patch before a public release of new Ghost version. For example, when the stripe property becomes something else the integration should still maintain this property and hydrate the server response with such field. When a new major version is introduced the hydration logic would be removed. We should also clean up any other "shims" that came up.

Note for future. We should very careful about what we serve in response data, specially when dealing with beta endpoints!

matthanley commented 3 years ago

I agree with the overall sentiment that we should be careful about what we expose via integrations. However - considering the opposing viewpoint: if a user configures an integration with Ghost's beta functionality, imo it's reasonable (but not ideal) for breaking changes in Ghost to break that integration - provided we give them a way to reconfigure an alternative similar integration.

imo 'beta feature' implies all dependencies of that feature should also be considered beta.

naz commented 2 years ago

@royalfig this one could be considered as a "data cleanup" issue similar to the one we have for sample data issue. Similarly to sample data (data comes from webhooks) here it's the other source of variables Zapier integration users bind their Zaps to - the data comes dynamically straight form the API responses.

With Ghost v5 we have introduced email notifications to make Zap owners know when their integrations might break. So this capability could be taken into consideration when deciding if/when we want to limit the data exposed to the user here. Imo, starting v5 it's fairly safe to let the end user use whatever is available and rely on the email notification system (version headers) to keep their Zaps up to date.