Add a default error summary presenter #311
Conversation
The intention of this presenter is that it provides the current basic behaviour but also provides a convenient way for people with specific requirements to inject them. It currently requires that it's initialized with an error_messages hash but we may yet tweak that so an instance can be passed in and the error messages set on it. Refs #310
|
@jsugarman - having had a bit of a think about it, I think this approach will give you what you need. If we always use a presenter it may as well handle the collection of error messages rather than one-at-a-time. If you need more info, we could always optionally pass the object to the initializer too, something like: def presenter
@presenter.new(error_messages, @builder.object)
endAlternatively, we could pass an instance of the presenter to |
|
Many thanks @peteryates. I just saw this when I pushed up my new branch directly against this repo. I will take a better look when I have time but my initial thoughts is that I don't think this approach will work for our custom error presenter without a major, and rather painful, rewrite on our end. We instantiate our presenter with the model object and a translation file (for lookups) and then have methods to retrieve the summary and field level errors. We pass custom attributes/keys which are then converted to ordinalize nested instances (e.g. attribute : defendant_2_first_name, message I had added some thoughts to this commit while working. However I am not even sure the approach I have taken in my PR would work for us going forward and applying this functionality to other models because I can see that we generate separate line item summary errors for the same attribute with multiple errors. I will take a better look at your PR when I can but do let me know any thoughts, problems, improvements on mine when you can. |
This approach offers a little more flexibility when than just accepting a class. When a class is provided it's instantiated with the object's error messages but when an object is supplied it's used as-is. It needs to implement the method #formatted_error_messages so we check for that, but any other attributes and state can be set up in the application and can contain whatever's necessary.
|
@jsugarman - thanks for the feedback, I kind of had this in mind - I've updated it so it'll accept any object in addition to a presenter class. I believe this will work with your PR now with minimal changes, I think you just need to add a |
|
I just pulled your branch and had a quick play tested against an implementation of the I will have a further look but it I like it's simplicity |
It makes sense to allow a app-wide default to be set and gives us an opportunity to document and test the process better.
There was a problem hiding this comment.
🥇 Have this working as I'd hope for against our app branch.
Couple of minor documentation comments plus do you think this is worth its own entry in the guide to provide an interactive example. You could cherry pick and amend this commit, or am happy to add myself.
To clarify its purpose and usage I have extended the existing error summary documentation template, examples and helpers. I could extend further to add a nested fields type example if deemed worthwhile.
The intention of this presenter is that it provides the current basic behaviour but also provides a convenient way for people with specific requirements to inject them.
It currently requires that it's initialized with an error_messages hash but we may yet tweak that so an instance can be passed in and the error messages set on it.
Refs #310