Commit e1f9113a authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'ce-5949-render_optional' into 'master'

CE: Use render_if_exists to hide EE specific codes

See merge request gitlab-org/gitlab-ce!19041
parents 4a56a6a1 4634e749
...@@ -2,6 +2,11 @@ require 'digest/md5' ...@@ -2,6 +2,11 @@ require 'digest/md5'
require 'uri' require 'uri'
module ApplicationHelper module ApplicationHelper
# See https://docs.gitlab.com/ee/development/ee_features.html#code-in-app-views
def render_if_exists(partial, locals = {})
render(partial, locals) if lookup_context.exists?(partial, [], true)
end
# Check if a particular controller is the current one # Check if a particular controller is the current one
# #
# args - One or more controller names to check # args - One or more controller names to check
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
- breadcrumb_title "Dashboard" - breadcrumb_title "Dashboard"
%div{ class: container_class } %div{ class: container_class }
= render_if_exists "admin/licenses/breakdown", license: @license
.admin-dashboard.prepend-top-default .admin-dashboard.prepend-top-default
.row .row
.col-sm-4 .col-sm-4
...@@ -20,6 +22,7 @@ ...@@ -20,6 +22,7 @@
%h3.text-center %h3.text-center
Users: Users:
= approximate_count_with_delimiters(User) = approximate_count_with_delimiters(User)
= render_if_exists 'users_statistics'
%hr %hr
= link_to 'New user', new_admin_user_path, class: "btn btn-new" = link_to 'New user', new_admin_user_path, class: "btn btn-new"
.col-sm-4 .col-sm-4
...@@ -97,6 +100,9 @@ ...@@ -97,6 +100,9 @@
= reply_email = reply_email
%span.light.pull-right %span.light.pull-right
= boolean_to_icon Gitlab::IncomingEmail.enabled? = boolean_to_icon Gitlab::IncomingEmail.enabled?
= render_if_exists 'elastic_and_geo'
- container_reg = "Container Registry" - container_reg = "Container Registry"
%p{ "aria-label" => "#{container_reg}: status " + (Gitlab.config.registry.enabled ? "on" : "off") } %p{ "aria-label" => "#{container_reg}: status " + (Gitlab.config.registry.enabled ? "on" : "off") }
= container_reg = container_reg
...@@ -144,6 +150,9 @@ ...@@ -144,6 +150,9 @@
GitLab Pages GitLab Pages
%span.pull-right %span.pull-right
= Gitlab::Pages::VERSION = Gitlab::Pages::VERSION
= render_if_exists 'geo'
%p %p
Ruby Ruby
%span.pull-right %span.pull-right
......
...@@ -53,6 +53,9 @@ stub_licensed_features(variable_environment_scope: true) ...@@ -53,6 +53,9 @@ stub_licensed_features(variable_environment_scope: true)
EE-specific comments should not be backported to CE. EE-specific comments should not be backported to CE.
**Note:** This is only meant as a workaround, we should follow up and
resolve this soon.
### Detection of EE-only files ### Detection of EE-only files
For each commit (except on `master`), the `ee-files-location-check` CI job tries For each commit (except on `master`), the `ee-files-location-check` CI job tries
...@@ -105,11 +108,14 @@ is applied not only to models. Here's a list of other examples: ...@@ -105,11 +108,14 @@ is applied not only to models. Here's a list of other examples:
- `ee/app/services/foo/create_service.rb` - `ee/app/services/foo/create_service.rb`
- `ee/app/validators/foo_attr_validator.rb` - `ee/app/validators/foo_attr_validator.rb`
- `ee/app/workers/foo_worker.rb` - `ee/app/workers/foo_worker.rb`
- `ee/app/views/foo.html.haml`
- `ee/app/views/foo/_bar.html.haml`
This works because for every path that are present in CE's eager-load/auto-load This works because for every path that are present in CE's eager-load/auto-load
paths, we add the same `ee/`-prepended path in [`config/application.rb`]. paths, we add the same `ee/`-prepended path in [`config/application.rb`].
This also applies to views.
[`config/application.rb`]: https://gitlab.com/gitlab-org/gitlab-ee/blob/d278b76d6600a0e27d8019a0be27971ba23ab640/config/application.rb#L41-51 [`config/application.rb`]: https://gitlab.com/gitlab-org/gitlab-ee/blob/925d3d4ebc7a2c72964ce97623ae41b8af12538d/config/application.rb#L42-52
### EE features based on CE features ### EE features based on CE features
...@@ -359,9 +365,37 @@ Blocks of code that are EE-specific should be moved to partials. This ...@@ -359,9 +365,37 @@ Blocks of code that are EE-specific should be moved to partials. This
avoids conflicts with big chunks of HAML code that that are not fun to avoids conflicts with big chunks of HAML code that that are not fun to
resolve when you add the indentation to the equation. resolve when you add the indentation to the equation.
EE-specific views should be placed in `ee/app/views/ee/`, using extra EE-specific views should be placed in `ee/app/views/`, using extra
sub-directories if appropriate. sub-directories if appropriate.
Instead of using regular `render`, we should use `render_if_exists`, which
will not render anything if it cannot find the specific partial. We use this
so that we could put `render_if_exists` in CE, keeping code the same between
CE and EE.
Also, it should search for the EE partial first, and then CE partial, and
then if nothing found, render nothing.
This has two uses:
- CE renders nothing, and EE renders its EE partial.
- CE renders its CE partial, and EE renders its EE partial, while the view
file stays the same.
The advantages of this:
- Minimal code difference between CE and EE.
- Very clear hints about where we're extending EE views while reading CE codes.
- Whenever we want to show something different in CE, we could just add CE
partials. Same applies the other way around. If we just use
`render_if_exists`, it would be very easy to change the content in EE.
The disadvantage of this:
- Slightly more work while developing EE features, because now we need to
port `render_if_exists` to CE.
- If we have typos in the partial name, it would be silently ignored.
### Code in `lib/` ### Code in `lib/`
Place EE-specific logic in the top-level `EE` module namespace. Namespace the Place EE-specific logic in the top-level `EE` module namespace. Namespace the
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment