Return errors as json objects instead of strings

[robbevp]

We currently have a mapping in config/locales/en.yml to convert error messages to keys, which we then look up in web's translations. I realised that by using @object.errors.errors we can just directly return the attributes of each error.

We now always return errors in the shape of:

{ "errors": [{ "attribute": "name", "type": "required" }] }

I opted to always return an object with the key errors and an array, to make have a consistent shape that we can easily distinguish from an unexpected error (or an error that we don't handle ourselves).

For the errors from our validations, we can easily converted these starting from the objects. We map the type (with just the two types of validation errors that we currently have). If we have more types we can grow the map, and maybe add options like: {"attribute": "title", "type": "too_long", "options": { "count": 50 } }
(I'm not sure if I find this map for type all that necessary, but I don't mind it either)

For the few "loose" errors, I converted these to the same shape and tried to keep the keys similar where this made sense. For the errors that start from a form with user input, I added attribute: :base to have an easy way of displaying errors in a form that aren't related to one specific field

• I've added tests relevant to my changes.

I added an extra assertion to all cases where we already tested for a response of unprocessable_content, not_found, forbidden, or unauthorized (unless I missed one)
(There were a few cases where we checked that an update return :unprocessable_content, but didn't actually verify that no changes were made. I added an extra assertion to those)

[chvp]

Did we ever talk about this in person? The one thing I find a bit icky about this is that this basically exposes the rails way of handling errors to the clients. IMO, it should always be theoretically possible to create another API without too much issues.

[robbevp]

Did we ever talk about this in person? The one thing I find a bit icky about this is that this basically exposes the rails way of handling errors to the clients. IMO, it should always be theoretically possible to create another API without too much issues.

IIRC: we said have some way of providing more structured errors to clients, but to add some layer to convert rails' structure to something else
Though when I now think about this, I'm not sure what that would be in practice. We could rename some keys and values but I don't think rails does a bad job of naming them.
Something like the following feels a bit superfluous to me:

ERROR_TYPE_MAP = {
  blank: :required,
  taken: :not_unique
}

def transform_error_for_json(error)
    result = { attribute: error.attribute, type: ERROR_TYPE_MAP[error.type] }
    result[:options] = error.options.except(*ActiveModel::Error::CALLBACKS_OPTIONS, *ActiveModel::Error::MESSAGE_OPTIONS)
    result
  end

Maybe we should just be more explicit about which other options we include? (Rather than including everything except some specific things).

def transform_error_for_json(error)
    result = %i[attribute type].index_with { |it| error.send(it) }
    result[:options] = error.options.slice(:count, :other_option)
    result
  end

[chvp]

Maybe we should just be more explicit about which other options we include? (Rather than including everything except some specific things).

This options seems fine to me.

[robbevp]

@chvp I finally finished this up. I reworked this PR based on the discussion above and updated the description to reflect the actual state of changes

I just noticed that the patch coverage is rather low, since I didn't add new tests if we didn't already check the response. I can still add those (though I think that there are a few cases where we could never actually get this situation - for example I don't think destroying an album can actually fail)

I'll leave this as a draft until we have a matching PRs in api-client-js and web. Do you think this needs any changes in the android app? (it doesn't for the iOS/macOS app)