Fix select_top_level_html_files ignoring custom http_prefix#438
Fix select_top_level_html_files ignoring custom http_prefix#438inulty-dfe wants to merge 1 commit intoalphagov:mainfrom
select_top_level_html_files ignoring custom http_prefix#438Conversation
The parent URL check was hardcoded to "/" so top-level pages were filtered out when http_prefix was set to a custom path.
| end | ||
|
|
||
| def select_top_level_html_files(resources) | ||
| def select_top_level_html_files(resources, config) |
There was a problem hiding this comment.
Are we confident that no-one will currently be calling this function directly in their own implementation? If we can't be sure, should we provide a default value for the config parameter (even if it is just nil)
| expect(subject.multi_page_table_of_contents(resources, current_page, config, current_page_html).strip).to eq(expected_multi_page_table_of_contents.strip) | ||
| end | ||
|
|
||
| it "selects top-level pages whose parent URL matches the http prefix" do |
There was a problem hiding this comment.
are there any other test cases we could add, such as double nesting the path, or someone uses it weirdly (like adding http_prefix to the config, and setting the value to \)?
|
|
||
| def select_top_level_html_files(resources) | ||
| def select_top_level_html_files(resources, config) | ||
| home_url = config[:http_prefix].end_with?("/") ? config[:http_prefix] : "#{config[:http_prefix]}/" |
There was a problem hiding this comment.
also, what happens if the set http_prefix: "/prefix/" - i.e with a trailing slash or if config[:http_prefix] is nil?
|
This looks really good, nice quick fox for a fiddly problem! All current test pass locally. Before marking as ready for please rebase with our main branch (if not already done) and resolve the outsanding comments. |
2 participants
What’s changed
When we configure a custom
http_prefix(e.g. /docs) to serve our tech docs site under a subpath, Middleman sets the parent URL of top-level pages to/docs/rather than/.select_top_level_html_fileshad the parent URL check hardcoded to "/", so it filtered out all our top-level pages — meaning the multi-page table of contents came back empty.render_page_tree(in the same file) already handled this correctly by computinghome_urlfromconfig[:http_prefix].select_top_level_html_filesjust wasn't updated to match.Here is where we've had to monkey patch the library
Identifying a user need