wire-server icon indicating copy to clipboard operation
wire-server copied to clipboard

Fs 1008 event schema docs

Open battermann opened this issue 2 years ago • 3 comments

https://wearezeta.atlassian.net/browse/FS-1008

Todos:

  • [x] ToSchema instances of team notifications are wrong. To do this right, search schema-profunctor tutorial for bind/dispatch, and consult conversation events for examples.
  • [x] swagger2 doesn't handle types with the same name from different models well; it silently drops the second definition, which is what you want only if there are no name clashes as in our case with three types called Event and three types called EventType. We have solved this by rendering the three event types seperately and returning each declarations list in a super-list. For a better work-around, check https://github.com/GetShopTV/swagger2/issues/14.
  • [ ] The different EventData constructors all have different json schema that partially overlap. Our schemas only represent the union of all those constructors, rather than a list of cases. There may be a better way even in swagger v2: https://swagger.io/specification/v2/ (look for "polymorphism")
  • [x] (wire cloud) expose end-point via nginz (only on staging).
  • [ ] Document how this works in https://docs.wire.com/understand/api-client-perspective/swagger.html

Checklist

  • [x] Add a new entry in an appropriate subdirectory of changelog.d
  • [x] Read and follow the PR guidelines

Ready for review, 2 out of 5 todos are still open.

Swagger models are exposed under http://localhost:8082/api/event-notification-schemas/swagger-ui/ locally and should be under https://staging-nginz-https.zinfra.io/api/event-notification-schemas/swagger-ui/ after this is deployed to staging.

battermann avatar Nov 28 '22 09:11 battermann

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar May 10 '23 07:05 CLAassistant

@battermann would it make sense to merge this and resolve the remaining points in a separate PR?

fisx avatar Oct 26 '23 07:10 fisx

@battermann would it make sense to merge this and resolve the remaining points in a separate PR?

Yes! However, we need to resolve the merge conflicts first. I can put it on my todo list...

battermann avatar Oct 26 '23 10:10 battermann