resolve/refinerycms — pull request #1304 by craigambrose (2012-02-19 07:30:26+00:00)
Title: Content page refactoring

Hi Phil,

Here is the refactoring of the _content_page template which I did at railscamp. Added since then are specs unit testing the SectionPresenter and SectionSetPresenter classes. Although this may duplicate test coverage slightly, I wasn't quite game to remove any of the request specs for pages, as they are still likely to catch potential problems. The new specs however mock out everything else and take less than a second to run, so it shouldn't be a problem to have the slightly duplicated test coverage.

The remaining ruby code in _content_page is mostly just to organise and check the potential parameters passed into the template. Typically these parameters are not used anywhere in the refinerycms git repository, so I presume they are mainly for other engines, such as the blog or whatever. If any of them are genuinely not needed, then removing them would simplify the code a lot.

This line looks particularly worrying:
```allowed_to_use_fallback = !local_assigns[:show_empty_sections] && !local_assigns[:remove_automatic_sections]```

Either of those two local parameters have the same effect, and that effect essentially forces the section to use the override html, or nothing at all. This is a direct refactoring and is how it was before (although it didn't used to be clear that's how it worked before). Perhaps replacing that with a single local assign would be better, as long as other engines conformed to it.

Craig