Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enhancement] Message for 'dependent: :restrict_with_error' #3223

Closed
airled opened this issue Nov 18, 2019 · 2 comments
Closed

[Enhancement] Message for 'dependent: :restrict_with_error' #3223

airled opened this issue Nov 18, 2019 · 2 comments
Milestone

Comments

@airled
Copy link

airled commented Nov 18, 2019

Rails_admin ignores Rails message with key restrict_dependent_destroy (See this)

Example:
Country model:

class Country < ApplicationRecord
  has_many :cities, dependent: :restrict_with_error
end

If you try to destroy country with cities inside irb:

irb(main):001:0> c = Country.first
irb(main):002:0> c.destroy
(1.0ms)  ROLLBACK
irb(main):003:0> c.errors.messages
=> {:base=>["Cannot delete record because dependent cities exist"]}

If we try to delete the country via rails_admin we only got an abstract message:
Country failed to be deleted

Rails_admin should use full error message because an admin have to know what is wrong.

@airled
Copy link
Author

airled commented Nov 18, 2019

The issue is here.
Rails admin uses common error message without full object error messages:

...
else
  flash[:error] = t('admin.flash.error', name: @model_config.label, action: t('admin.actions.delete.done'))
  redirect_path = back_or_index
end

I wanted to make a PR, but there is one more issue. For example, create action does not use redirect when there are any exceptions and so calls handle_save_error method with flash.now.
Delete with errors redirects so it uses just flash message. But iterating over flash messages in view escapes html and you can not just do something like this:

...
else
  flash[:error] = t('admin.flash.error', name: @model_config.label, action: t('admin.actions.delete.done')).html_safe
  flash[:error] += %(<br>- #{@object.errors.full_messages.join('<br>- ')}).html_safe
  redirect_path = back_or_index
end

redirect_to redirect_path

because after redirection your html tags will all be escaped.

It works if you add .html_safe here like this:

- flash && flash.each do |key, value|
  .alert.alert-dismissible{class: flash_alert_class(key)}
    %button.close{type: 'button', :'data-dismiss' => "alert"} &times;
    = value.html_safe

but I don't think it's a good idea.
Does anyone have any suggestions?

@airled airled changed the title Message for 'dependent: :restrict_with_error' [Enhacement] Message for 'dependent: :restrict_with_error' Nov 20, 2019
@airled airled changed the title [Enhacement] Message for 'dependent: :restrict_with_error' [Enhancement] Message for 'dependent: :restrict_with_error' Dec 25, 2019
@jacobjlevine
Copy link

I second my support for this update. And I would generalize it to allow more broadly for displaying all model errors, not just for dependent: :restrict_with_error.

@mshibuya mshibuya added this to the 3.0.0 milestone Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants