Manual documents #78
Manual documents #78
Conversation
| render_with( | ||
| manual: parent_manual, | ||
| document: current_document, | ||
| ) |
There was a problem hiding this comment.
forgive my possible naivety but what is the benefit of using this render_with over just using instance variables?
There was a problem hiding this comment.
render_with or rather render with the locals option forces you to be explicit about what the view has access to where as use of instance variables makes an implicit demand on the state of the controller.
Instance variables are also much trickier to reason about because the controller (or super class) can set them in many places, such as filter methods which means there's no one place you can see what is set.
There was a problem hiding this comment.
While having them defined in one place could be desirable I am not sure that I see enough of a benefit to deviating from convention. I know I had to look up to see what this was doing when looking at this pull request. It also isn't even consistently used in this pull request.
…xample Remove Orgs allowed_values from Policies finder
Functionality to add documents (with attachments) to manuals.
You may notice the lovely hack to get around CMA Case specific validations on documents that are currently in the content models gem and also there is some serious work to do refactoring the cucumber steps when we've decided how to separate different types of docs.
EDIT: There's also an awful lot of controller and view duplications that we'll need to refactor soon.
https://trello.com/c/CIVqY1uh/63-manuals-add-document-to-a-manual-5