Remove monkey-patching

[artur-intech]

config/initializers/eis_custom_active_model.rb
config/initializers/eis_custom_active_record.rb
config/initializers/eis_custom_flash.rb

[artur-intech]

@ratM1n @teadur What is point of config/initializers/eis_custom_flash.rb?

[teadur]

@artur-beljajev http://api.rubyonrails.org/classes/ActionDispatch/Flash.html

[artur-intech]

What is the reason of monkey-patching the class we don't own? (i.e. the point of having this file, since it is not needed by Rails)

[teadur]

It logs all the flash messages to syslog/standard logger

[artur-intech]

I was actually wondering why that code is present there. If nobody knows the reason, I am about to remove first 3. The latter requires code base change.

[teadur]

They all give some extra logging, i wouldnt remove them just yet.
I think it would make sense to look at logs first and see what value they give to us.
Havent looked at active model/record yet, but flash could be beneficial at some cases.

[artur-intech]

First of all, I am trying to say that monkey patching is very dangerous. There should be unbeatable argument to do it, and currently there is no.
Secondly, I don't understand why do you want to log flash messages? These are the messages that are persisted in session until next request to show them to the user. Please explain me.

[artur-intech]

Example of real-life WTF http://prntscr.com/f1foaz

[artur-intech]

Nothing to test. Should not affect any production code.

[vohmar]

@teadur Is this good by you?

[teadur]

@artur-beljajev why is http://prntscr.com/f1foaz a problem ?
Rather then showing just valid false it shows all the problems with the validation, why is that a problem ?
Or how would you find out the actual problems if the errors arent shown ?
Or how would the behviour in the screenshot change when we remove what we have suggested ?

[artur-intech]

It is unnecessary. active_model_object.errors.full_messages returns what you want.

[teadur]

I think its far easier to use valid? then this long reference to get the error, what is the actual problem with it, does it brake anything ?

[artur-intech]

Why do you need that error at all?

[teadur]

Its really convient when you want to know why some objects validation failed, sadly you cant check object state from any UI you need to use the actual rails console for it.

[artur-intech]

This ticket cannot be closed until config/initializers/eis_custom_active_record.rb is removed.

#231 (comment) remains valid. It creates unnecessary noise with no benefit.

@teadur
9293d09 If you mean model errors, then it is only possible in admin (since other areas use EPP) so there no model objects available. If you're ok with this, I might add errors to UI with #580.

[artur-intech]

@vohmar Any reason why this issue has been closed?

The code is still there

registry/config/initializers/eis_custom_active_record.rb

Line 5 in 4786dbb

Rails.logger.info "USER MSG: ACTIVERECORD: #{m.class} ##{m.id} #{m.errors.full_messages} #{m.errors['epp_errors']}" if m.errors.present?

[artur-intech]

I might also set config.log_level to some higher level to hide all that noise, but I think it opens a possibility that I miss something I need.

[artur-intech]

The information in question may be obtained using active_model_object.errors.full_messages

[artur-intech]

That monkey-patch can lead to problems with Rails update in the future.

[vohmar]

I must have misunderstood that work with config/initializers/eis_custom_active_model.rb, eis_custom_active_record.rb and eis_custom_flash.rb is done.

[artur-intech]

eis_custom_active_model.rb and eis_custom_flash.rb have been removed, that's perfect!
eis_custom_active_record.rb is the only thing in question.

How is that logging in the latter is useful for us?

[teadur]

It's useful when you look at logs, if you think its so big problem and brakes everything remove it.

[vohmar]

in production