Skip to content
This repository has been archived by the owner. It is now read-only.

Allow customisation of app's home path #56

Merged
merged 1 commit into from Dec 30, 2014
Merged

Allow customisation of app's home path #56

merged 1 commit into from Dec 30, 2014

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Dec 30, 2014

HMRC contacts app puts its homepage within an admin namespace, so the crown link in the head needs to be modifiable.

  • Introduce an app_home_path variable
  • Default it to root_path for backwards compatibility
HMRC contacts app puts its homepage within an admin namespace, so the
crown link in the head needs to be modifiable.

* Introduce an `app_home_path` variable
* Default it to `root_path` for backwards compatibility
@@ -34,6 +34,11 @@
end
end

it 'renders a link to a custom home path' do
visit '/'
expect(page).to have_selector('a[href="/style-guide"]', text: 'app_title')

This comment has been minimized.

@vinayvinay

vinayvinay Dec 30, 2014
Contributor

where does '/style-guide' come from?

This comment has been minimized.

@fofr

fofr Dec 30, 2014
Author Contributor

This comment has been minimized.

@vinayvinay

vinayvinay Dec 30, 2014
Contributor

I figured that bit out. My concern is that the test looks incomplete if there's not setup data corresponding to the asserted value. Where does govuk_admin_template_engine_path get set?

This comment has been minimized.

@fofr

fofr Dec 30, 2014
Author Contributor

It's setup in spec/dummy/app/views/layouts/application.html.erb, a dummy app.
If I remember correctly, modifying a content_for on the fly then re-rendering the page to test for it is non-trivial.

vinayvinay added a commit that referenced this pull request Dec 30, 2014
Allow customisation of app's home path
@vinayvinay vinayvinay merged commit c5a2a76 into master Dec 30, 2014
1 check passed
1 check passed
default "Build #87 succeeded on Jenkins"
Details
@vinayvinay vinayvinay deleted the custom-root-path branch Dec 30, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.