1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
|
# Presenters
This type of class is responsible for giving the view an object which defines
**view-related logic/data methods**. It is usually useful to extract such
methods from models to presenters.
## When to use a presenter?
### 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!
### 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 are we using presenters instead of helpers?
We don't use presenters to generate complex view output that would rely on helpers.
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-foss/merge_requests/7073/diffs.
- 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 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!
Another reason is as follows:
> Avoid using concerns and use presenters instead. Why? After all, concerns seem
to be a core part of Rails and can DRY up code when shared among multiple models.
Nonetheless, the main issue is that concerns don’t make the model object more
cohesive. The code is just better organized. In other words, there’s no real
change to the API of the model.
– https://www.toptal.com/ruby-on-rails/decoupling-rails-components
## Benefits
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
- testing is easier and faster as presenters are Plain Old Ruby Object (PORO)
- views are more readable and maintainable
- decreases the 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)
## What not to do with presenters?
- 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 like texts, and URLs using URL helpers from
`Gitlab::Routing` but nothing much fancier.
## Implementation
### Presenter definition
If you need a presenter class that has only necessary interfaces for the view-related context,
inherit from `Gitlab::View::Presenter::Simple`.
It provides a `.presents` the 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 < Gitlab::View::Presenter::Simple
presents ::Label, as: :label
def text_color
label.color.to_s
end
def to_partial_path
'projects/labels/show'
end
end
```
If you need a presenter class that delegates missing method calls to the presented object,
inherit from `Gitlab::View::Presenter::Delegated`.
This is more like an "extension" in the sense that the produced object is going to have
all of interfaces of the presented object **AND** all of the interfaces in the presenter class:
```ruby
class LabelPresenter < Gitlab::View::Presenter::Delegated
presents ::Label, as: :label
def text_color
# color is delegated to label
color.to_s
end
def to_partial_path
'projects/labels/show'
end
end
```
### Presenter instantiation
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::Presenter::Factory
.new(@label, current_user: current_user)
.fabricate!
end
end
```
You can also include the `Presentable` concern in the model:
```ruby
class Label
include Presentable
end
```
and then in the controller:
```ruby
class Projects::LabelsController < Projects::ApplicationController
def edit
@label = @label.present(current_user: current_user)
end
end
```
### Presenter usage
```ruby
%div{ class: @label.text_color }
= render partial: @label, label: @label
```
You can also present the model in the view:
```ruby
- label = @label.present(current_user: current_user)
%div{ class: label.text_color }
= render partial: label, label: label
```
### Validate accidental overrides
We use presenters in many places, such as Controller, Haml, GraphQL/Rest API,
it's very handy to extend the core/backend logic of Active Record models,
however, there is a risk that it accidentally overrides important logic.
For example, [this production incident](https://gitlab.com/gitlab-com/gl-infra/production/-/issues/5498)
was caused by [including `ActionView::Helpers::UrlHelper` in a presenter](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69537/diffs#4b581cff00ef3cc9780efd23682af383de302e7d_3_3).
The `tag` accesor in `Ci::Build` was accidentally overridden by `ActionView::Helpers::TagHelper#tag`,
and as a conseuqence, a wrong `tag` value was persited into database.
Starting from GitLab 14.4, we validate the presenters (specifically all of the subclasses of `Gitlab::View::Presenter::Delegated`)
that they do not accidentally override core/backend logic. In such case, a pipeline in merge requests fails with an error message,
here is an example:
```plaintext
We've detected that a presetner is overriding a specific method(s) on a subject model.
There is a risk that it accidentally modifies the backend/core logic that leads to production incident.
Please follow https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/presenters/README.md#validate-accidental-overrides
to resolve this error with caution.
Here are the conflict details.
- Ci::PipelinePresenter#tag is overriding Ci::Pipeline#tag. delegator_location: /devkitkat/services/rails/cache/ruby/2.7.0/gems/actionview-6.1.3.2/lib/action_view/helpers/tag_helper.rb:271 original_location: /devkitkat/services/rails/cache/ruby/2.7.0/gems/activemodel-6.1.3.2/lib/active_model/attribute_methods.rb:254
```
Here are the potential solutions:
- If the conflict happens on an instance method in the presenter:
- If you intend to override the core/backend logic, define `delegator_override <method-name>` on top of the conflicted method.
This explicitly adds the method to an allowlist.
- If you do NOT intend to override the core/backend logic, rename the method name in the presenter.
- If the conflict happens on an included module in the presenter, remove the module from the presenter and find a workaround.
### How to use the `Gitlab::Utils::DelegatorOverride` validator
If a presenter class inhertis from `Gitlab::View::Presenter::Delegated`,
you should define what object class is presented:
```ruby
class WebHookLogPresenter < Gitlab::View::Presenter::Delegated
presents ::WebHookLog, as: :web_hook_log # This defines that the presenter presents `WebHookLog` Active Record model.
```
These presenters are validated not to accidentaly override the methods in the presented object.
You can run the validation locally with:
```shell
bundle exec rake lint:static_verification
```
To add a method to an allowlist, use `delegator_override`. For example:
```ruby
class VulnerabilityPresenter < Gitlab::View::Presenter::Delegated
presents ::Vulnerability, as: :vulnerability
delegator_override :description # This adds the `description` method to an allowlist that the override is intentional.
def description
vulnerability.description || finding.description
end
```
To add methods of a module to an allowlist, use `delegator_override_with`. For example:
```ruby
module Ci
class PipelinePresenter < Gitlab::View::Presenter::Delegated
include Gitlab::Utils::StrongMemoize
include ActionView::Helpers::UrlHelper
delegator_override_with Gitlab::Utils::StrongMemoize # TODO: Remove `Gitlab::Utils::StrongMemoize` inclusion as it's duplicate
delegator_override_with ActionView::Helpers::TagHelper # TODO: Remove `ActionView::Helpers::UrlHelper` inclusion as it overrides `Ci::Pipeline#tag`
```
Keep in mind that if you use `delegator_override_with`,
there is a high chance that you're doing **something wrong**.
Read the [Validate Accidental Overrides](#validate-accidental-overrides) for more information.
|