Fix setting team feature status in Stern/backoffice

[mheinzel]

See https://github.com/zinfra/backend-issues/issues/1529

[akshaymankar]

Looks good!
Have you run this locally? If you have, let's document how to do that.

[mheinzel]

It worked as expected when I curled it, but I'm still trying to run the backoffice Swagger app.

I'll keep at it, might just fix https://github.com/zinfra/backend-issues/issues/1496 and https://github.com/wireapp/wire-server/tree/develop/tools/stern#how-to-run-stern-together-with-the-rest-of-wire-server as well. 🤷

[mheinzel]

Umhh, so I managed to get this working locally with some trickery, but there is a bit of a usability issue:

Normally, we want JSON requests and responses to be objects, but for the backoffice this doesn't really make sense, as it's much more cumbersome now to send requests.

So I guess we should accept a plain JSON string instead (only in Stern)? To do that, we will need to split out a separate type again...

[akshaymankar]

Yeah, it will be nicer to accept a plain string in the body, that way we get the dropdown. But this is also fine as few other endpoints in the same swagger-ui require much more complex JSONs and so the users clearly understand this.
So, what I want to say is that it is upto you if you want to make it nicer or avoid spending time on this.

[fisx]

i'm in favor of making this more usable, if it's not more than 3 hours (i don't think it should be.)

[fisx]

all we need to do is move the type which was there before from the body to the query, right?

[mheinzel]

all we need to do is move the type which was there before from the body to the query, right?

That's actually a good idea, requiring fewer changes. Unfortunately, I already started doing this by splitting the type into one with is just a JSON string and one with the object wrapper around it. Almost done with that...

Might be slightly nicer to have that separation, because then also the types defined for the interface correspond more closely to their JSON structure (which will also be helpful if we want to start automatically deriving JSON stuff). I'll push the update soon, so you can have a look and shout if you think the query parameter is the better solution.

[mheinzel]

@fisx @akshaymankar This should work now.

(and I verified it's a dropdown in the backoffice again)

[mheinzel]

Have you run this locally? If you have, let's document how to do that.

I'll do that as part of #1148.

Where do you think would be the best place to document it? I was first thinking about the stern README, but now I'm not sure anymore.

[akshaymankar]

Where do you think would be the best place to document it? I was first thinking about the stern README, but now I'm not sure anymore.

It would be nice if it started when I run services/start-services-only.sh and then we probably don't even need to document it like rest of the services.

[mheinzel]

Okay, I'll look into it as part of the other PR. This one should be ready to be merged, though. Do you think the approach with splitting into TeamFeatureStatus and TeamFeatureStatusValue makes sense?

[fisx]

sigh

This is rolling back the change we did in tag image/2.83.22 (#1122). The reason we changed it was that it made the code simpler all over the backend, as you can tell from all the awkwardness you had to add. :-P

So, good thing you asked for a second review!

[fisx]

we discussed this, and came to the conclusion that this code is more beaurocratic, but less surprising. so we'll keep it.

[fisx]

as discussed: all good! pleas take a look at my commit and merge if you agree.

[mheinzel]

I rebase this and tested the backoffice. Ran into an issue with the query param deserialization, which I fixed now.

I will merge once CI passed.