Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

(#16429) Relpaths #122

Merged
merged 4 commits into from

4 participants

@djmitche

Two of these fixes are simple changes to the CSS.

The third is a code change, but relatively minor.

I added a test for the code change, but I don't see an easy way to test that it gets the right path even at a sub-URI. Still, this shows that the code is executed and produces the correct result in at least one condition.

djmitche added some commits
@djmitche djmitche (#16429) Fix relative URLs of icons in tables.css
These links work when puppet-dashboard is at the top level because browsers treat /../ as equivalent to /.
775ab52
@djmitche djmitche (#16429) fix relative links in application.scss
With corresponding generated changes in application.css
f37359a
@djmitche djmitche (#16429) use a controller reference for the delayed jobs link
This gets the path right even when the app is hosted at a sub-URI.
A basic test of the page looks for the link.
c7560d7
@djmitche djmitche (#16429) use correct links for radiator, too 1b1f0ee
@djmitche

I just added a fourth for the radiator view, which I had missed initially.

@jeffweiss

@djmitche I need to verify that all the tests pass with these changes. In the interim, can you give me a bit more justification on the problem this is actually solving? Thanks!

@djmitche

They fix (partially) the case where you're running the dashboard at a base URL other than /. Most of the dashboard generates links using the Rails way, which handles this case smoothly. The fixes here address a few places where links were generated directly, and incorrectly for this case.

@sodabrew
Owner

Would this allow the dashboard to be one level down, but not two? Or does it work for any dashboard URL that has no more than two levels?

It works for any location. The ../../ was redundant and equivalent to ../ when puppetmaster is in the root, similar to

cd /usr
( cd ../../ && echo $PWD)
( cd ../ && echo $PWD)
@sodabrew
Owner

Ok I re-read the changes. In particular I thought you changed from ../ to ../../ -- but it was the other way around. I do wonder if there are any second-level-with-a-trailing-slash views that would be impacted.

Other than that, I'm comfortable with this PR.

@djmitche

Those that I changed, I observed doing the wrong thing in a running dashboard, so I tend to think not. But I can't prove it beyond this statement.

@sodabrew
Owner

@puppetlabs Do you have any plugins that are extending the Dashboard routes to add third-level deep URL paths? If not, I'd like to merge this in.

@djmitche

This will still work with third-level-deep URL paths. Paths in CSS are relative to the CSS file, not window.url.

All paths generated in Ruby use the controller to generate the path, so they'll be absolute and correct.

@sodabrew
Owner

@djmitche Oh. I'm a doofus. You're right, the images are relative to the css file's url, not to the current page url. Merging now!

@sodabrew sodabrew merged commit 6aa0b98 into sodabrew:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 14, 2012
  1. @djmitche

    (#16429) Fix relative URLs of icons in tables.css

    djmitche authored
    These links work when puppet-dashboard is at the top level because browsers treat /../ as equivalent to /.
  2. @djmitche

    (#16429) fix relative links in application.scss

    djmitche authored
    With corresponding generated changes in application.css
  3. @djmitche

    (#16429) use a controller reference for the delayed jobs link

    djmitche authored
    This gets the path right even when the app is hosted at a sub-URI.
    A basic test of the page looks for the link.
Commits on Sep 18, 2012
  1. @djmitche
This page is out of date. Refresh to see the latest.
View
7 app/views/shared/_node_manager_sidebar.html.haml
@@ -1,11 +1,12 @@
- add_body_class 'with-sidebar'
.group.delayed-job
- %h3= link_to "Background Tasks", "/delayed_job_failures"
+ - failures_node = {:controller => "delayed_job_failures", :action => "index"}
+ %h3= link_to "Background Tasks", failures_node
- if DelayedJobFailure.unread.count > 0
%p.failure
- = link_to "#{ pluralize DelayedJobFailure.unread.count, 'new failed task' }", "/delayed_job_failures"
+ = link_to "#{ pluralize DelayedJobFailure.unread.count, 'new failed task' }", failures_node
- if Delayed::Job.count > 0
%p.warning
%em= "#{ pluralize Delayed::Job.count, 'pending task' }"
@@ -50,7 +51,7 @@
.footer.actionbar
#radiator
- = link_to "Radiator View", '/radiator'
+ = link_to "Radiator View", {:controller => "radiator", :action => "index"}
- unless SETTINGS.enable_read_only_mode || session['ACCESS_CONTROL_ROLE'] == 'READ_ONLY'
= link_to "Add node", new_node_path, :class => 'button'
View
6 public/stylesheets/application.css
@@ -629,11 +629,11 @@ body {
background-position: 0% 50%;
margin-bottom: .5em; }
body .delayed-job p.ok em {
- background: transparent url(/images/icons/delayed-job-ok.png) no-repeat 0% 50%; }
+ background: transparent url(../images/icons/delayed-job-ok.png) no-repeat 0% 50%; }
body .delayed-job p.failure a {
- background-image: url(/images/icons/delayed-job-failure.png); }
+ background-image: url(../images/icons/delayed-job-failure.png); }
body .delayed-job p.warning em {
- background-image: url(/images/icons/delayed-job-warning.png); }
+ background-image: url(../images/icons/delayed-job-warning.png); }
a.autorefresh_link {
color: #aaa; }
View
6 public/stylesheets/sass/application.scss
@@ -808,17 +808,17 @@ body {
}
&.ok {
em {
- background : transparent url( /images/icons/delayed-job-ok.png ) no-repeat 0% 50%;
+ background : transparent url( ../images/icons/delayed-job-ok.png ) no-repeat 0% 50%;
}
}
&.failure {
a {
- background-image : url( /images/icons/delayed-job-failure.png );
+ background-image : url( ../images/icons/delayed-job-failure.png );
}
}
&.warning {
em {
- background-image : url( /images/icons/delayed-job-warning.png );
+ background-image : url( ../images/icons/delayed-job-warning.png );
}
}
}
View
8 public/stylesheets/tables.css
@@ -137,17 +137,17 @@ table td.empty {
display: none;
}
.expandable-list .expandable-link.collapsed-link:before {
- content: url('../../images/icons/bullet_toggle_plus.png');
+ content: url('../images/icons/bullet_toggle_plus.png');
}
.expandable-list .expandable-link:before {
- content: url('../../images/icons/bullet_toggle_minus.png');
+ content: url('../images/icons/bullet_toggle_minus.png');
}
.expandable-list .non-expandable-bullet:before {
- content: url('../../images/icons/bullet_green.png');
+ content: url('../images/icons/bullet_green.png');
}
.inspect-report .expandable-list .non-expandable-bullet:before {
- content: url('../../images/icons/bullet_red.png');
+ content: url('../images/icons/bullet_red.png');
}
.expandable-list .expandable-link, .expandable-list .non-expandable-bullet {
View
30 spec/views/pages/home.html.haml_spec.rb
@@ -0,0 +1,30 @@
+require File.expand_path(File.join(File.dirname(__FILE__), *%w[.. .. spec_helper]))
+
+describe '/pages/home.html.haml' do
+ describe "successful render" do
+ before :each do
+ assigns[:all_nodes] = @all_nodes = [Node.generate!]
+ assigns[:unreported_nodes] = []
+ assigns[:unresponsive_nodes] = []
+ assigns[:failed_nodes] = []
+ assigns[:pending_nodes] = []
+ assigns[:changed_nodes] = []
+ assigns[:unchanged_nodes] = []
+ end
+
+ specify do
+ render
+ response.should be_success
+ end
+
+ it "should have a correct delayed_job_failures link" do
+ render
+ should have_tag('a[href="/delayed_job_failures"]', 'Background Tasks')
+ end
+
+ it "should have a correct radiator link" do
+ render
+ should have_tag('a[href="/radiator"]', 'Radiator View')
+ end
+ end
+end
Something went wrong with that request. Please try again.