Remove unused notice #14044
Remove unused notice #14044
Conversation
The given notice is never shown because notice are only rendered on the show template
|
Thanks, but we don't feel it's unused. The fact that the flash only happens to be in the show view is because it was only used there, and when the notice was added to the destroy action, we should've updated the index view too. Maybe you'd like to change your PR to add the flash the index view instead? This way the flash will be properly displayed. Let me know if you can/want to do that, otherwise I can do it myself later this week. |
|
Carlos, I thought it have to be removed because a default scaffolding with jbuilder enabled produce a destroy action without any notice. So I proposed that behavior to be consistent with what scaffolding do on a standard new rails app. Please let me know what option you want, I would be very happy to contribute, even on a very minor fix/improvment like this one. |
This is same as we do without JBuilder See this for more rails/rails#14044
|
@f-plante thanks for PR. I think we need this when we are doing things with including JBuilder. I have added a PR here rails/jbuilder#191 |
|
Do we? I checked the index template and we are not showing notice there. |
|
@rafaelfranca we have it here I think https://github.com/rails/rails/blob/master/railties/lib/rails/generators/rails/scaffold_controller/templates/controller.rb#L50 As PR in JBuilder is merged so this can be closed. As we have both consistent on both the places now. Closing this now. thanks @f-plante. |
|
I mean, we don't show the notice in the index page. This is why it is not needed. See https://github.com/rails/rails/blob/master/railties/lib/rails/generators/erb/scaffold/templates/show.html.erb#L1 and https://github.com/rails/rails/blob/master/railties/lib/rails/generators/erb/scaffold/templates/index.html.erb |
|
True there is no flash message when we destroy. Sorry my bad. I made a change in JBuilder as well which needs to revert then. |
|
I think the proper fix is to make index show the notice |
As we are setting notice in destroy action we should display that For more information see rails#14044 And rails/jbuilder#191 closes rails#14044
|
Closed by #16048 |
The given notice is never shown because notice are only rendered on the show view