Handle zero plural resource names #1407
Conversation
| end | ||
|
|
||
| context 'with zero plural noun model name' do | ||
| let!(:peter) { create(:series, name: 'Peter')} |
There was a problem hiding this comment.
Layout/SpaceInsideBlockBraces: Space missing inside }.
| post :update, params: {id: peter.id, event: {name: "Hans"}}.merge(params) | ||
| expect(response.redirect_url).to eq("http://test.host/admin/events?page=6&q%5Bname_or_hidden_name_or_description_or_location_name_cont%5D=some_query") | ||
| context 'with regular noun model name' do | ||
| let(:peter) { create(:event, name: 'Peter') } |
There was a problem hiding this comment.
Layout/ExtraSpacing: Unnecessary spacing detected.
dbwinger
force-pushed
the
zero-plural-resources
branch
from
ef0afc3 to
3a10fb6
Compare
| @@ -0,0 +1,3 @@ | |||
| class Admin::SeriesController < Alchemy::Admin::ResourcesController | |||
|
|
|||
| end | |||
There was a problem hiding this comment.
Could you please add a new line at the end of the file and remove the empty line inside the class body? I updated the Rubocop settings to also include the dummy app, so would have been catched by HoundCI. Thanks
| @@ -0,0 +1,2 @@ | |||
| class Series < ActiveRecord::Base | |||
| end | |||
There was a problem hiding this comment.
Could you please add a new line at the end of the file? I updated the Rubocop settings to also include the dummy app, so would have been catched by HoundCI. Thanks
| expect(controller).to receive(:controller_path) { 'admin/series' } | ||
| post :create, params: {series: {name: "Hans"}}.merge(params) | ||
| expect(response.redirect_url).to eq("http://test.host/admin/series?page=6&q%5Bname_cont%5D=some_query") | ||
| end |
There was a problem hiding this comment.
I would like to see some newlines between let, it and context. Thanks
dbwinger
force-pushed
the
zero-plural-resources
branch
from
3a10fb6 to
5db149b
Compare
| expect(response.redirect_url).to eq("http://test.host/admin/events?page=6&q%5Bname_or_hidden_name_or_description_or_location_name_cont%5D=some_query") | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
There was a problem hiding this comment.
@tvdeyen did I misunderstand your request, add the newline incorrectly, or is Robocop not set up correctly?
There was a problem hiding this comment.
@dbwinger Now all looks fine, besides the trailing whitespace that's left in this line
There was a problem hiding this comment.
@tvdeyen Sorry, I was thinking this was reporting a problem with the newline I added at your request to the end of the file, but now I see that's not it. I think I've got it all straightened out now.
| expect(response.redirect_url).to eq("http://test.host/admin/events?page=6&q%5Bname_or_hidden_name_or_description_or_location_name_cont%5D=some_query") | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
dbwinger
force-pushed
the
zero-plural-resources
branch
from
5db149b to
de75144
Compare
| @@ -76,4 +102,4 @@ | |||
| expect(response.redirect_url).to eq("http://test.host/admin/events?page=6&q%5Bname_or_hidden_name_or_description_or_location_name_cont%5D=some_query") | |||
| end | |||
| end | |||
| end | |||
| end | |||
There was a problem hiding this comment.
Layout/TrailingBlankLines: Final newline missing.
dbwinger
force-pushed
the
zero-plural-resources
branch
from
de75144 to
922c5c1
Compare
|
@tvdeyen Can this be merged into the stable branches? I'm currently on |
|
@dbwinger sure. A separate PR for 3.6-stable and 4.0-stable would be great. Thanks |
Fixes issue where 'zero plural' (same word for singular and plural) resource model names can't be routed correctly by
ActionDispatch::Routing::PolymorphicRoutes.polymorphic_urlusing justResource#namespaced_resources_namename, because the plural and singular are the same. In this case, sendingResource#namespaced_resources_nametopolymorphic_urlroutes to the singular#showpath instead of the expected#index. This changes it to use the resource model's class instead, which will reliably route to the#indexpath in all cases.