Fixes #16025 - Fix ERB & params to import subnets from proxy #3716
Conversation
| <% id = 'subnet_fields_' + subnet.network_address.gsub('/','_').gsub('.','_') %> | ||
| <div class="panel panel-default"> | ||
| <div class="panel-heading" > | ||
| <a class="panel-title" data-toggle="collapse" data-parent='#accordion1' href="<%= '#'+id %>" data-original-title='review' rel='twipsy' > |
There was a problem hiding this comment.
data-original-title should probably be removed here too, it just shows an untranslated tooltip saying "review" and isn't very helpful!
|
Could you also add a functional test for the import action itself, to check the page renders without errors based on some mocked proxy response? |
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
| @@ -55,77 +55,117 @@ def test_destroy | |||
| assert !Subnet.exists?(subnet.id) | |||
| end | |||
|
|
|||
| def test_freeip_fails_no_subnet | |||
There was a problem hiding this comment.
This is just makeup, nothing changed
|
Fixed - basically I added two tests for the import action in the controller & added back the h2 warning. Thanks for reviewing @domcleal ! |
| @@ -1,19 +1,20 @@ | |||
| <% title _("Import IPv4 subnets") %> | |||
|
|
|||
| <h2><%= _("The following IPv4 subnets have been found") %></h2> | |||
|
|
|||
| <h2><%= _("The following IPv4 subnets have been found. Please review the details before creating them.") %></h2> | |||
There was a problem hiding this comment.
Could you drop this to regular paragraph text, or perhaps try h3 etc? The text is very nearly the same size as the main title.
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
Updated to decrease the font-size of the subtitle to h3 , also added col-md-8 so that the h3 tag is aligned with the title and the rest of the form |
| @@ -1,19 +1,20 @@ | |||
| <% title _("Import IPv4 subnets") %> | |||
|
|
|||
| <h2><%= _("The following IPv4 subnets have been found") %></h2> | |||
|
|
|||
| <h4 class='col-md-8'><%= _("The following IPv4 subnets have been found. Please review the details before creating them.") %></h4> | |||
There was a problem hiding this comment.
The col-md-8 adds a float: left to this element which causes it to overlap:
Before this commit, the ERB would just throw a 500 because it tried to show @subnet.name in a template for a @subnet that was still undefined. Aside from that, the create_multiple method in the controller didn't send the right call to filter the params, hence making it impossible to create the imported subnets even with the ERB fix.
|
Updated to remove the col-md-8 - sorry, I was trying it with the web inspector and I guess I disabled something and didn't realize, so the col-md looked fine. |
|
[test] due to failed slave. |
|
[test] |
Before this commit, the ERB would just throw a 500 because it tried to
show
@subnet.namein a template for a@subnetthat was still undefined.Aside from that, the create_multiple method in the controller didn't
send the right call to filter the params, hence making it impossible to
create the imported subnets even with the ERB fix.