Fs 897 partial success for list users

[lepsa]

Checklist

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

Changes

Adding a new V3 version of the /list-users route that allows for partial success when querying federated servers.

[mdimjasevic]

In general this looks good, but I wonder if you could reuse the existing handler. I'm not 100% sure myself so I'd like to hear what you think.

[mdimjasevic]

My understanding is that the new field is optional. If that is so, then its addition makes the response backwards-compatible. I understand that the new endpoint will not throw for unavailable remote backends, but cannot you bake this into the existing handler? The old endpoint versions wouldn't fail anymore, and they might end up providing an additional field in response, but clients should ignore that field by contract because the field wasn't there when Swaggers for the client API versions were finalised.

[lepsa]

It isn't backwards compatible because it introduces a new layer of structure to the response. Previously it was a top level list with nowhere to add a second list, now it is an object containing at least one list, and optionally a second.

[mdimjasevic]

Oh, right, it's not backwards compatible in terms of JSON. Nevertheless, I believe the handler can be generalised and reused for both endpoint versions. Or at least factoring out the common bits would be good to have.

[lepsa]

I've updated these functions to pull out the common bits

[mdimjasevic]

There's this integration test failing in CI:

    get users by ids on multiple backends:                                                   FAIL

      Exception: Assertions failed:

       2: Just [Qualified {qUnqualified = c62fe3f9-a82d-44bf-89d6-d50ee5817723, qDomain = Domain {_domainText = "federation-test-helper.test-xm78obv8vnze-fed2.svc.cluster.local"}},Qualified {qUnqualified = c6920588-4eba-4143-9a17-fedea6d0806d, qDomain = Domain {_domainText = "federation-test-helper.test-xm78obv8vnze.svc.cluster.local"}}] =/= Nothing

      Response was:

      Response {responseStatus = Status {statusCode = 200, statusMessage = "OK"}, responseVersion = HTTP/1.1, responseHeaders = [("Transfer-Encoding","chunked"),("Date","Tue, 14 Mar 2023 10:40:39 GMT"),("Server","Warp/3.3.23"),("Content-Encoding","gzip"),("Content-Type","application/json;charset=utf-8"),("Vary","Accept-Encoding")], responseBody = Just "{\"found\":[{\"accent_id\":0,\"assets\":[],\"id\":\"c62fe3f9-a82d-44bf-89d6-d50ee5817723\",\"legalhold_status\":\"no_consent\",\"name\":\"\234\182\175\234\184\144\236\191\169\228\181\184\239\182\142\225\147\147\231\138\134\226\145\166\225\187\172\231\135\157\234\184\129\234\145\187\235\171\158\227\134\175\235\166\189\236\138\133\236\154\164\225\140\159\236\133\188\227\187\140\236\153\153\233\136\157\228\131\145\227\173\169\229\184\191\228\145\189\236\167\168\225\131\183\232\187\132\224\168\174\237\147\158\227\185\150\231\163\134\228\128\178\229\140\191\233\168\160\233\131\134\234\133\146\235\166\155\232\160\170\230\186\180\",\"picture\":[],\"qualified_id\":{\"domain\":\"federation-test-helper.test-xm78obv8vnze-fed2.svc.cluster.local\",\"id\":\"c62fe3f9-a82d-44bf-89d6-d50ee5817723\"}},{\"accent_id\":0,\"assets\":[],\"id\":\"c6920588-4eba-4143-9a17-fedea6d0806d\",\"legalhold_status\":\"no_consent\",\"name\":\"\234\131\131\234\140\140\229\144\156\233\155\166\230\170\153\227\162\178\229\133\156\227\177\171\233\180\176\234\133\171\234\150\164\235\162\185\234\156\134\234\156\165\225\160\135\234\150\188\236\176\178\236\158\187\231\150\189\233\171\169\226\175\164\224\184\144\227\162\144\234\139\178\235\146\163\233\130\171\228\185\180\230\164\181\239\169\156\234\136\140\",\"picture\":[],\"qualified_id\":{\"domain\":\"federation-test-helper.test-xm78obv8vnze.svc.cluster.local\",\"id\":\"c6920588-4eba-4143-9a17-fedea6d0806d\"}}]}", responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose}

      CallStack (from HasCallStack):

        error, called at src/Bilge/Assert.hs:91:5 in bilge-0.22.0-1CeiEQekzBEdjo334Kowc:Bilge.Assert

        <!!, called at src/Bilge/Assert.hs:109:19 in bilge-0.22.0-1CeiEQekzBEdjo334Kowc:Bilge.Assert

        !!!, called at test/integration/Federation/End2end.hs:181:5 in main:Federation.End2end

      Use -p '(!/turn/&&!/user.auth.cookies.limit/)&&/get users by ids on multiple backends/' to rerun this test only.

[lepsa]

get users by ids on multiple backends

I've updated that test, setting the API version that it should be using.

[mdimjasevic]

Where does this one and the other two testObject_ListUsersById_user_?.json tests come from? I can't find corresponding Haskell values.

[lepsa]

The haskell values are in module Test.Wire.API.Golden.Manual.ListUsersById.

[mdimjasevic]

I'm sorry, but I can't find Haskell values testObject_ListUsersById_user_1, testObject_ListUsersById_user_2 nor testObject_ListUsersById_user_3 in that module nor in any other module. Note the _user part in the names.

[mdimjasevic]

Looks good! I'd swap the test functions name, but otherwise good to go!

[mdimjasevic]

Shouldn't this one be called testMultipleUsers (as it uses the latest API version, namely v4) and the other one testMultipleUsersV3 (as it uses the old v3)?