Environment banner #72
Conversation
…ls.env except production, also added response header X-Environment which responses with the current Rails.env to be included in all v0 json requests
| @@ -1,3 +1,10 @@ | |||
| class ApplicationController < ActionController::Base | |||
| # pass | |||
| before_action :set_environment | |||
There was a problem hiding this comment.
Since there’s actual code in this class now, you should get rid of the # pass comment :)
| @@ -11,10 +11,18 @@ | |||
| </head> | |||
|
|
|||
| <body> | |||
| <% unless @env == "production" %> | |||
| <header class="page-header"> | |||
There was a problem hiding this comment.
This should really be inside the pre-existing header element (you’re only meant to have one per HTML “section” — this is, article, aside, nav, section elements). Otherwise we should make it a plain old div.
| @@ -164,4 +164,8 @@ class Api::V0::PagesControllerTest < ActionDispatch::IntegrationTest | |||
| results = body['data'] | |||
| assert_not results.any? {|p| p.key? 'versions'}, 'Some pages have a "versions" property' | |||
| end | |||
| test 'includes environment in header of response' do | |||
There was a problem hiding this comment.
Would you add a blank line between tests, please?
|
Looks awesome, thanks! |
|
Really glad I could help. I checked out the goals of the overall project and you folks are doing some terrific work. Keep up the good work! |
|
Thanks! I just realized we don’t have a “Contributors” section in the README, so I’m about to add one. Would you like to be listed there? |
|
That would be great thanks! |
|
Feel free to update #75 if you’d like your link to point somewhere else. |
fixes, #67, I haven't setup a test yet for the environment banner, if you need a test setup for it I'd recommend doing an integration test with capybara which I could setup. I didn't want to add the dependency to your project without your input first since it would be bringing in a library just to test one thing. I also added some color to the banner and made it center, hopefully it sticks out more.
I added a test for the X-Environment header, its not very expansive, but I think with a little guidance I could add more tests.