Drop the managed Column from team_conv Table in Galley

[mdimjasevic]

(Do not merge before Sept 2022 and/or before most deployments are on chart version >= 4.1.0. In case this is merged and deployed before 4.1.0 was deployed first, a transient period of the time it takes to roll out a deployment (in the order of minutes usually) of 'HTTP status 500 internal server errors' on client API calls to conversations can happen.)

This PR is a follow-up to PR #2125 that was merged on Feb 14, 2022.

The PR also removes the managed key from JSON instances in client API version 2. This makes it a client API breaking change so versioning is used; version 1 is kept intact.

Checklist

• Rebase on top of the latest develop. Make sure to update schemaVersion in Galley and rename services/galley/schema/src/V60_DropManagedConversations.hs accordingly.
• The PR Title explains the impact of the change.
• The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
• If a cassandra schema migration has been added, I ran make git-add-cassandra-schema to update the cassandra schema documentation.
• changelog.d contains the following bits of information (details):
  • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.
  • If a cassandra schema migration is backwards incompatible (see also these docs), measures to be taken by on-premise instance operators are explained.

[pcapriotti]

I'm not sure how the version bounds meant for servant routes ended up in API types, but I would recommend not doing that. As discussed, I don't think we need separate types for older versions, because this feature never actually worked and was never used. We only need to support the interface in V1, not the functionality. So I would only have the types for v2, and just make the JSON instances backward compatible. No messing around with versions should be necessary.

[mdimjasevic]

I'm not sure how the version bounds meant for servant routes ended up in API types, but I would recommend not doing that. As discussed, I don't think we need separate types for older versions, because this feature never actually worked and was never used. We only need to support the interface in V1, not the functionality. So I would only have the types for v2, and just make the JSON instances backward compatible. No messing around with versions should be necessary.

I've reverted the changes to the ConvTeamInfo and NewConv types to incorporate your feedback: they are never used in output/responses, so it doesn't hurt to have managed in their ToJSON instances.

However, TeamConversation and TeamConversationList do end up in output/endpoint responses, so for both of them I've introduced a version boundary type index so we can provide different ToJSON instances depending on the version used.

Can you take a look again?

[pcapriotti]

However, TeamConversation and TeamConversationList do end up in output/endpoint responses, so for both of them I've introduced a version boundary type index so we can provide different ToJSON instances depending on the version used.

I still don't think this is necessary. Just output a managed field with a value of false regardless of the version, and make it deprecated. Ideally, hide it from swagger if that's not too hard to achieve, so we can drop it when V1 becomes unsupported. Also, combinators like From V2 are not intended to be used as type arguments for data types like that. If you ever actually need a type that depends on a version, just use the version itself or simply make a different type with a V2 suffix or similar.

[mdimjasevic]

However, TeamConversation and TeamConversationList do end up in output/endpoint responses, so for both of them I've introduced a version boundary type index so we can provide different ToJSON instances depending on the version used.

I still don't think this is necessary. Just output a managed field with a value of false regardless of the version, and make it deprecated. Ideally, hide it from swagger if that's not too hard to achieve, so we can drop it when V1 becomes unsupported. Also, combinators like From V2 are not intended to be used as type arguments for data types like that. If you ever actually need a type that depends on a version, just use the version itself or simply make a different type with a V2 suffix or similar.

Thanks for this comment and a thorough explanation in a call. I think I've addressed all of your feedback and it should be fine now. Can you take a look?

[pcapriotti]

Looks good to me.