summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2018-05-23 12:56:11 +0000
committerRémy Coutable <remy@rymai.me>2018-05-23 12:56:11 +0000
commite1f9113a3751d6ba228d2c7b0d99245625a98ba5 (patch)
treed01f428446e0810f13f899e219211d7b6a00f6bf
parent4a56a6a1b743b4adb5f4de7833efc8caa97a6132 (diff)
parent4634e749ee743a8d5123a23d63c18cb0b9350711 (diff)
downloadgitlab-ce-e1f9113a3751d6ba228d2c7b0d99245625a98ba5.tar.gz
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
-rw-r--r--app/helpers/application_helper.rb5
-rw-r--r--app/views/admin/dashboard/index.html.haml9
-rw-r--r--doc/development/ee_features.md38
3 files changed, 50 insertions, 2 deletions
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index aa4569500b8..f5d94ad96a1 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -2,6 +2,11 @@ require 'digest/md5'
require 'uri'
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
#
# args - One or more controller names to check
diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml
index 41ef646fc0e..cfa1d9d0f0c 100644
--- a/app/views/admin/dashboard/index.html.haml
+++ b/app/views/admin/dashboard/index.html.haml
@@ -2,6 +2,8 @@
- breadcrumb_title "Dashboard"
%div{ class: container_class }
+ = render_if_exists "admin/licenses/breakdown", license: @license
+
.admin-dashboard.prepend-top-default
.row
.col-sm-4
@@ -20,6 +22,7 @@
%h3.text-center
Users:
= approximate_count_with_delimiters(User)
+ = render_if_exists 'users_statistics'
%hr
= link_to 'New user', new_admin_user_path, class: "btn btn-new"
.col-sm-4
@@ -97,6 +100,9 @@
= reply_email
%span.light.pull-right
= boolean_to_icon Gitlab::IncomingEmail.enabled?
+
+ = render_if_exists 'elastic_and_geo'
+
- container_reg = "Container Registry"
%p{ "aria-label" => "#{container_reg}: status " + (Gitlab.config.registry.enabled ? "on" : "off") }
= container_reg
@@ -144,6 +150,9 @@
GitLab Pages
%span.pull-right
= Gitlab::Pages::VERSION
+
+ = render_if_exists 'geo'
+
%p
Ruby
%span.pull-right
diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md
index 4873090a2d4..057a4094aed 100644
--- a/doc/development/ee_features.md
+++ b/doc/development/ee_features.md
@@ -53,6 +53,9 @@ stub_licensed_features(variable_environment_scope: true)
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
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:
- `ee/app/services/foo/create_service.rb`
- `ee/app/validators/foo_attr_validator.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
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
@@ -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
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.
+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/`
Place EE-specific logic in the top-level `EE` module namespace. Namespace the