Fix wrong status 404 when PATCH request didn't change anything

[Qu4tro]

This is a simple attempt to fix the reported bug described in title.
So, citing #1257 and confirmed by this test in this PR:

Right now in my tests, I'm getting 204 whether or not any rows were updated.

This PR can be tweaked to take care of some of the failing tests, but like in the previous PR, I would argue that some of those failing tests should be tweaked as well.

Let me know what you think.

This resolves #1257 .

[Qu4tro]

Added a few changes:

Using a empty payload no longer touches the db. They keep returning the same status as before.
PATCHs whose conditions result in no records found (and therefore no records updated) return 404.

1º was a necessary condition for 2º, because we need to distinguish between, declaring no changes and not founding records to change, which return different status codes.

What do you think?

[steve-chavez]

we need to distinguish between, declaring no changes and not founding records to change, which return different status codes.

Yes. This is correct 👍

However, the wildcard pattern matchs are a bit hard to follow. I think not hitting the database should not be a concern for now and the code can be a bit more concise(the added variables are good though). It could be something like:

let
  contentRangeHeader = contentRangeH 0 (toInteger $ queryTotal - 1)
        (toInteger <$> if shouldCount then Just queryTotal else Nothing)
  minimalHeaders = [contentRangeHeader]
  fullHeaders = toHeader contentType : minimalHeaders
  updateIsNoOp = S.null $ pjKeys pJson
  status | queryTotal == 0 && not updateIsNoOp = status404
         | iPreferRepresentation apiRequest == Full = status200
         | otherwise = status204
return $ if iPreferRepresentation apiRequest == Full
  then responseLBS status fullHeaders (toS body)
  else responseLBS status minimalHeaders mempty

[Qu4tro]

A few more commits.

• pjKeys, status as suggested.
• Simplified the case statement. I've kept it over the ifs expression, to better handle the CTSingularJSON possibility.
• Made contentRangeH polymorphic, so we can reduce the number of toInteger calls.

[Qu4tro]

Additional commit for some whitespace. I think it reads better.

[Qu4tro]

Further commit that fixes the issue mentioned above. I've also hardcoded the tests, so it doesn't only check if it is an error, but which as well.

Curious thing happened on
https://github.com/PostgREST/postgrest/pull/1272/files#diff-d36f1ad401857a200ca8d27e932681f8L64
Re-running the tests (without destroying db), yields different results (6 or 7).
Kept previous check instead, as the same case it's tested further down.

[steve-chavez]

@Qu4tro Thank you! Sorry the review took so long.