Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move outcome templates into their own sub-directory #2023

Merged
merged 49 commits into from
Oct 20, 2015

Conversation

floehopper
Copy link
Contributor

Motivation

We're planning on storing question page content in the flow directory alongside
landing page and outcome page content. It's already a bit confusing that the
landing page content is stored alongside the question page content and it will
become more so when question page content is there too.

So this pull request moves all outcome templates into an outcomes sub-directory c.f. smartdown flows.

User-visible changes

None.

Technical details

This was all pretty straightforward, if a little tedious!

The first commit breaks the regression tests for all flows by requiring the outcome templates to be in an outcomes sub-directory. Each subsequent commit fixes the regression test for an individual flow, updating the checksums accordingly. No changes to the regression test inputs or artefacts were required.

The sub-directories in marriage-abroad were moved into the new outcomes sub-directory.

The rendering of shared & data partials in various flows uses relative paths outside of the flow directory and so with the outcome templates another level down, these had to be changed from e.g.:

<%= render partial: '../shared/partial-name.govspeak.erb' -%>

to:

<%= render partial: '../../shared/partial-name.govspeak.erb' -%>

I did think about allowing both the flow directory and the new outcomes sub-directory, but it made the code quite a bit more complicated and in the end I decided that it's better to be stricter.

@erkde
Copy link
Contributor

erkde commented Oct 20, 2015

Seems to be a few places where we prefix paths to shared partials with a '../..' path, which is the kind of thing I forget to do and maybe we can do away with by appending it to the Rails view_path?

Otherwise, looks good to me 👍

@floehopper
Copy link
Contributor Author

I think adding the shared and data_partials paths to the list of view paths set on the instance of ActionView::Base instance used in SmartAnswer::ErbRenderer is a good idea.

However, I think it's slightly orthogonal to this PR, although I accept this PR does make things slightly worse. I'll address this concern in a separate PR - I've added a Trello card to remind me.

I suspect we might also be able to combine the shared and data_partial directories since I'm not sure there's much (if any) difference between them now.

@floehopper floehopper force-pushed the move-outcome-templates-into-their-own-sub-directory branch from 32d10de to 26047b1 Compare October 20, 2015 13:02
@floehopper
Copy link
Contributor Author

I've rebased this branch against master and force-pushed in preparation for merging.

floehopper added a commit that referenced this pull request Oct 20, 2015
…eir-own-sub-directory

Move outcome templates into their own sub-directory
@floehopper floehopper merged commit 594d368 into master Oct 20, 2015
@floehopper floehopper deleted the move-outcome-templates-into-their-own-sub-directory branch October 20, 2015 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants