Build problem with servant 0.19

[marinelli]

It's not possible to build sevant-util with servant 0.19:

src/Servant/Util/Combinators/Filtering/Client.hs:47:41: error:
    • Couldn't match type ‘Text’ with ‘ByteString’
      Expected type: Maybe ByteString
        Actual type: Maybe Text
    • In the second argument of ‘appendToQueryString’, namely
        ‘(Just value)’
      In the expression: appendToQueryString key (Just value)
      In the expression:
        let
          filter :: TypeFilter fk a
            = cast sfFilter ?: error "Failed to cast filter"
          (op, value) = typeFilterToReq filter
          keymod = if op == DefFilteringCmd then "" else "[" <> op <> "]"
          ....
        in appendToQueryString key (Just value)
   |
47 |             in appendToQueryString key (Just value)
   |                                         ^^^^^^^^^^

The appendToQueryString type has changed here haskell-servant/servant#1432.

[treeowl]

At a minimum, we want to change the type of typeFilterToReq. Do we also want to change the type of autoFilterEncode?

[marinelli]

I was having a look on how to adapt the code. What it does not sound so much for me is the type of autoFilterEncode. I'm not talking about the Text/ByteString of the expected encoded value, but about the purpose of the autoFilterEncode function by its own. At the end it should evaluate to a pair with the key and the value for a query parameteres, but it does not seem to be really useful to give back the encoded value. The encoding might be moved only in the Servant.Util.Combinators.Filtering.Client module, or for some reason we would like to use the encoded value in other places?
Actually this is not possible due to the manipulatiopn of the encoded value in the Servant.Util.Combinators.Filtering.Filters.General module.

[treeowl]

I'm sorry, but I don't understand what you're saying there.

[marinelli]

I'm sorry, but I don't understand what you're saying there.

Let me rephrase. I was thinking if it would make sense to move the encoding function (toQueryParam and/or the toEncodedUrlPiece) where the appendToQueryString function is used. This would have made suprefluous the use of the encoding function in the autoFilterEncode definitions.
Than I noticed that in the autoFilterEncode definition (inside the Servant.Util.Combinators.Filtering.Filters.General module) the query param value for the FilterItemsIn case is manipulated. It means that it's not possible to do this change.

[marinelli]

it was closed accidentally.

[Martoon-00]

Regarding autoFilterEncode, do I understand your motivation correctly, that if instead of autoFilterEncode we used toQueryParam directly, then the s/Text/ByteString change in servant API would not be even noticed by servant-util as we would never mention Text in function signatures? And we would support servant both prior to and after 0.19?

Really, there is FilterItemsIn case where autoFilterEncode has logic smarter than just delegating to toQueryParam, so we cannot simply replace autoFilterEncode with encodeQueryParam + key.

But I like your idea very much, if my understanding is correct. At least, it would be worth avoiding using specific Text or ByteString as return type of autoFilterEncode, and the current solution in #40 seems best to me here.

[marinelli]

I updated the PR