search

leastbad / optimism

The missing drop-in solution for realtime remote form validation in Rails.
MIT License
361 stars 42 forks source link

Bug fixes #7

Closed joshleblanc closed 4 years ago

joshleblanc commented 4 years ago

This PR contains a number of fixes for bugs I ran into while implementing Optimism in my application.

object_name is not gauranteed to be a string in the form builder, so we convert it to a string before manipulating it to generate the selectors.

attributes has to be a hash in order to handle nested attributes.

resource needs to be in snake case in order to match the generated ID in the markup. If you had a model like FooBar, then the previous implementation would generate foobar_<rest of the id> rather than foo_bar_<rest of the id>.

head is only defined in a controller, and it's the only line tying broadcast_errors to controllers. I added a check to see if the method is defined before using it so we can use it outside of controllers. (Like in a stimulus reflex)

The errors object is only populated after you validate a model. When used in the controller like the documentation suggests, this will never be a problem (since you're always running it after save fails), however if you're running it outside of the controller, your errors would always be empty, subsequently making calls like model.errors.any? always return false. I added model.invalid? calls in the relevant places to fix this.

leastbad commented 4 years ago

Thanks so much for the excellent PR, @joshleblanc - you made it look easy.

To be perfectly honest, I'm a bit bewildered how the nested attributes ended up so broken. I have to assume that I was tired and committed something incorrect by accident, and nobody has complained.

As a heads up, I'm addressing the complete lack of authentication support in the next version - I have to check semver guides whether this will tip the scales and make it a major release or not. Essentially, configure your connection class to stream_for current_user and revisit your Optimism.channel value in your initializer, as it's been switched from a string to a lambda. This will be a lot more flexible.