Skip to content

Conversation

robbevp
Copy link
Member

@robbevp robbevp commented Feb 15, 2025

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.

A response with errors would then become:

[
  {"attribute" => "title", "type" => "blank", "options" => {}},
  {"attribute" => "normalized_title", "type" => "blank", "options" => {}}
]

Options can be filled with more detail if needed. (For example, if we would have length validations: `{"attribute" => "title", "type" => "too_long", "options" => { "count" => 50 }

@chvp What do you think? Wanted to check this principle first, before I update the controllers, and js-client and web.

  • I've added tests relevant to my changes.

@robbevp robbevp added the enhancement New feature or request label Feb 15, 2025
@robbevp robbevp requested a review from chvp February 15, 2025 14:50
@robbevp robbevp self-assigned this Feb 15, 2025
Copy link
Member

@chvp chvp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member Author

robbevp commented Sep 14, 2025

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
Copy link
Member

chvp commented Sep 14, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants