summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2017-01-09 21:47:32 +0100
committerRémy Coutable <remy@rymai.me>2017-01-18 16:38:34 +0100
commite950830ba6a0efa3b0992e6e55cb5b5842f8573a (patch)
tree4f4a5a9ddb72466e2bdc8f7df4d16e78b4e5f15c
parent78874519db2ca2f18a2fbca5fe070292e521d0c1 (diff)
downloadgitlab-ce-e950830ba6a0efa3b0992e6e55cb5b5842f8573a.tar.gz
Fix typos and improve presenters documentation
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r--app/presenters/README.md67
1 files changed, 28 insertions, 39 deletions
diff --git a/app/presenters/README.md b/app/presenters/README.md
index 38399b5e18b..91c1d2609f5 100644
--- a/app/presenters/README.md
+++ b/app/presenters/README.md
@@ -8,18 +8,15 @@ methods from models to presenters.
### When your view is full of logic
-When your view is full of logic (`if`, `else`, `select` on arrays etc.), it's time
-to create a presenter!
-
-For instance this view is full of logic: https://gitlab.com/gitlab-org/gitlab-ce/blob/d61f8a18e0f7e9d0ed162827f4e8ae2de3756f5c/app/views/projects/builds/_sidebar.html.haml
-can be improved as follows: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7073/diffs
+When your view is full of logic (`if`, `else`, `select` on arrays etc.), it's
+time to create a presenter!
### When your model has a lot of view-related logic/data methods
When your model has a lot of view-related logic/data methods, you can easily
move them to a presenter.
-## Why using presenters instead of helpers?
+## Why are we using presenters instead of helpers?
We don't use presenters to generate complex view output that would rely on helpers.
@@ -28,11 +25,11 @@ Presenters should be used for:
- Data and logic methods that can be pulled & combined into single methods from
view. This can include loops extracted from views too. A good example is
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7073/diffs.
-- Data and logic methods that can be pulled from models
+- Data and logic methods that can be pulled from models.
- Simple text output methods: it's ok if the method returns a string, but not a
whole DOM element for which we'd need HAML, a view context, helpers etc.
-## Why using presenters instead of model concerns?
+## Why use presenters instead of model concerns?
We should strive to follow the single-responsibility principle, and view-related
logic/data methods are definitely not the responsibility of models!
@@ -53,8 +50,8 @@ By moving pure view-related logic/data methods from models & views to presenters
we gain the following benefits:
- rules are more explicit and centralized in the presenter => improves security
-- makes the testing easier & faster as presenters are Plain Old Ruby Object (PORO)
-- makes views much more readable and maintainable
+- testing is easier and faster as presenters are Plain Old Ruby Object (PORO)
+- views are more readable and maintainable
- decreases number of CE -> EE merge conflicts since code is in separate files
- moves the conflicts from views (not always obvious) to presenters (a lot easier to resolve)
@@ -62,26 +59,24 @@ we gain the following benefits:
- Don't use helpers in presenters. Presenters are not aware of the view context.
- Don't generate complex DOM elements, forms etc. with presenters. Presenters
- can return simple data as texts, and URL using URL helpers from
+ can return simple data as texts, and URLs using URL helpers from
`Gitlab::Routing` but nothing much more fancy.
## Implementation
### Presenter definition
-Every presenters should include `Gitlab::View::Presenter`, which provides a
-`.presents` method which allows you to define an accessor for the presented
-object. It also includes common helpers like `Gitlab::Routing` and
+Every presenters should inherit from `Gitlab::View::Presenter::Simple`, which
+provides a `.presents` method which allows you to define an accessor for the
+presented object. It also includes common helpers like `Gitlab::Routing` and
`Gitlab::Allowable`.
```ruby
-class LabelPresenter
- include Gitlab::View::Presenter
-
+class LabelPresenter < Gitlab::View::Presenter::Simple
presents :label
- def blue?
- label.color == :blue
+ def text_color
+ LabelsHelper.text_color_for_bg(label.color)
end
def to_partial_path
@@ -90,19 +85,17 @@ class LabelPresenter
end
```
-In some cases, it can be more practical to transparently delegates all missing
+In some cases, it can be more practical to transparently delegate all missing
method calls to the presented object, in these cases, you can make your
-presenter inherit from `SimpleDelegator`:
+presenter inherit from `Gitlab::View::Presenter::Delegated`:
```ruby
-class LabelPresenter < SimpleDelegator
- include Gitlab::View::Presenter
-
+class LabelPresenter < Gitlab::View::Presenter::Delegated
presents :label
- def blue?
+ def text_color
# color is delegated to label
- color == :blue
+ LabelsHelper.text_color_for_bg(color)
end
def to_partial_path
@@ -113,28 +106,24 @@ end
### Presenter instantiation
-Instantiation must be done via the `Gitlab::View::PresenterFactory` class which
-handles presenters subclassing `SimpleDelegator` as well as those who don't.
+Instantiation must be done via the `Gitlab::View::Presenter::Factory` class which
+detects the presenter based on the presented subject's class.
```ruby
class Projects::LabelsController < Projects::ApplicationController
def edit
- @label = Gitlab::View::PresenterFactory
+ @label = Gitlab::View::Presenter::Factory
.new(@label, user: current_user)
.fabricate!
end
end
```
-You can also define a method on the model:
+You can also include the `Presentable` concern in the model:
```ruby
class Label
- def present(current_user)
- Gitlab::View::PresenterFactory
- .new(self, user: current_user)
- .fabricate!
- end
+ include Presentable
end
```
@@ -151,9 +140,8 @@ end
### Presenter usage
```ruby
-= @label.blue?
-
-= render partial: @label, label: @label
+%div{ class: @label.text_color }
+ = render partial: @label, label: @label
```
You can also present the model in the view:
@@ -161,5 +149,6 @@ You can also present the model in the view:
```ruby
- label = @label.present(current_user)
-= render partial: label, label: label
+%div{ class: label.text_color }
+ = render partial: label, label: label
```