diff options
Diffstat (limited to 'app/presenters')
-rw-r--r-- | app/presenters/README.md | 16 | ||||
-rw-r--r-- | app/presenters/ci/build_runner_presenter.rb | 35 | ||||
-rw-r--r-- | app/presenters/clusterable_presenter.rb | 4 | ||||
-rw-r--r-- | app/presenters/commit_status_presenter.rb | 8 | ||||
-rw-r--r-- | app/presenters/instance_clusterable_presenter.rb | 4 | ||||
-rw-r--r-- | app/presenters/pages_domain_presenter.rb | 2 | ||||
-rw-r--r-- | app/presenters/projects/prometheus/alert_presenter.rb | 61 | ||||
-rw-r--r-- | app/presenters/projects/settings/deploy_keys_presenter.rb | 4 | ||||
-rw-r--r-- | app/presenters/release_presenter.rb | 7 | ||||
-rw-r--r-- | app/presenters/snippet_presenter.rb | 10 |
10 files changed, 92 insertions, 59 deletions
diff --git a/app/presenters/README.md b/app/presenters/README.md index dc4173a880e..62aec4fc8a2 100644 --- a/app/presenters/README.md +++ b/app/presenters/README.md @@ -8,7 +8,7 @@ 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 +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 @@ -27,11 +27,11 @@ Presenters should be used for: 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. + 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 +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: @@ -52,22 +52,22 @@ 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 number of CE -> EE merge conflicts since code is in separate files +- 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 as texts, and URLs using URL helpers from - `Gitlab::Routing` but nothing much more fancy. +- 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 Every presenter should inherit from `Gitlab::View::Presenter::Simple`, which -provides a `.presents` method which allows you to define an accessor for the +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`. diff --git a/app/presenters/ci/build_runner_presenter.rb b/app/presenters/ci/build_runner_presenter.rb index 33b7899f912..5e35bfc79ef 100644 --- a/app/presenters/ci/build_runner_presenter.rb +++ b/app/presenters/ci/build_runner_presenter.rb @@ -27,14 +27,13 @@ module Ci def git_depth if git_depth_variable git_depth_variable[:value] - elsif Feature.enabled?(:ci_project_git_depth, default_enabled: true) + else project.ci_default_git_depth end.to_i end def refspecs specs = [] - specs << refspec_for_pipeline_ref if should_expose_merge_request_ref? specs << refspec_for_persistent_ref if persistent_ref_exist? if git_depth > 0 @@ -50,23 +49,10 @@ module Ci private - # We will stop exposing merge request refs when we fully depend on persistent refs - # (i.e. remove `refspec_for_pipeline_ref` when we remove `depend_on_persistent_pipeline_ref` feature flag.) - # `ci_force_exposing_merge_request_refs` is an extra feature flag that allows us to - # forcibly expose MR refs even if the `depend_on_persistent_pipeline_ref` feature flag enabled. - # This is useful when we see an unexpected behaviors/reports from users. - # See https://gitlab.com/gitlab-org/gitlab/issues/35140. - def should_expose_merge_request_ref? - return false unless merge_request_ref? - return true if Feature.enabled?(:ci_force_exposing_merge_request_refs, project) - - Feature.disabled?(:depend_on_persistent_pipeline_ref, project, default_enabled: true) - end - def create_archive(artifacts) return unless artifacts[:untracked] || artifacts[:paths] - { + archive = { artifact_type: :archive, artifact_format: :zip, name: artifacts[:name], @@ -75,6 +61,12 @@ module Ci when: artifacts[:when], expire_in: artifacts[:expire_in] } + + if artifacts.dig(:exclude).present? && ::Gitlab::Ci::Features.artifacts_exclude_enabled? + archive.merge(exclude: artifacts[:exclude]) + else + archive + end end def create_reports(reports, expire_in:) @@ -100,15 +92,18 @@ module Ci "+#{Gitlab::Git::TAG_REF_PREFIX}#{ref}:#{RUNNER_REMOTE_TAG_PREFIX}#{ref}" end - def refspec_for_pipeline_ref - "+#{ref}:#{ref}" - end - def refspec_for_persistent_ref "+#{persistent_ref_path}:#{persistent_ref_path}" end def persistent_ref_exist? + ## + # Persistent refs for pipelines definitely exist from GitLab 12.4, + # hence, we don't need to check the ref existence before passing it to runners. + # Checking refs pressurizes gitaly node and should be avoided. + # Issue: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/2143 + return true if Feature.enabled?(:ci_skip_persistent_ref_existence_check) + pipeline.persistent_ref.exist? end diff --git a/app/presenters/clusterable_presenter.rb b/app/presenters/clusterable_presenter.rb index 6b1d82e7557..5e669ff2e50 100644 --- a/app/presenters/clusterable_presenter.rb +++ b/app/presenters/clusterable_presenter.rb @@ -21,8 +21,8 @@ class ClusterablePresenter < Gitlab::View::Presenter::Delegated can?(current_user, :create_cluster, clusterable) end - def index_path - polymorphic_path([clusterable, :clusters]) + def index_path(options = {}) + polymorphic_path([clusterable, :clusters], options) end def new_path(options = {}) diff --git a/app/presenters/commit_status_presenter.rb b/app/presenters/commit_status_presenter.rb index 23e688e562e..52811e152a6 100644 --- a/app/presenters/commit_status_presenter.rb +++ b/app/presenters/commit_status_presenter.rb @@ -33,14 +33,6 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated def callout_failure_message self.class.callout_failure_messages.fetch(failure_reason.to_sym) end - - def recoverable? - failed? && !unrecoverable? - end - - def unrecoverable? - script_failure? || missing_dependency_failure? || archived_failure? || scheduler_failure? || data_integrity_failure? - end end CommitStatusPresenter.prepend_if_ee('::EE::CommitStatusPresenter') diff --git a/app/presenters/instance_clusterable_presenter.rb b/app/presenters/instance_clusterable_presenter.rb index 0c267fd5735..41071bc7bc7 100644 --- a/app/presenters/instance_clusterable_presenter.rb +++ b/app/presenters/instance_clusterable_presenter.rb @@ -13,8 +13,8 @@ class InstanceClusterablePresenter < ClusterablePresenter end override :index_path - def index_path - admin_clusters_path + def index_path(options = {}) + admin_clusters_path(options) end override :new_path diff --git a/app/presenters/pages_domain_presenter.rb b/app/presenters/pages_domain_presenter.rb index 6b74983d932..6ef89760bec 100644 --- a/app/presenters/pages_domain_presenter.rb +++ b/app/presenters/pages_domain_presenter.rb @@ -8,8 +8,6 @@ class PagesDomainPresenter < Gitlab::View::Presenter::Delegated end def show_auto_ssl_failed_warning? - return false unless Feature.enabled?(:pages_letsencrypt_errors, pages_domain.project) - # validations prevents auto ssl from working, so there is no need to show that warning until return false if needs_verification? diff --git a/app/presenters/projects/prometheus/alert_presenter.rb b/app/presenters/projects/prometheus/alert_presenter.rb index c03925c0871..2114e06a8c5 100644 --- a/app/presenters/projects/prometheus/alert_presenter.rb +++ b/app/presenters/projects/prometheus/alert_presenter.rb @@ -7,6 +7,7 @@ module Projects GENERIC_ALERT_SUMMARY_ANNOTATIONS = %w(monitoring_tool service hosts).freeze MARKDOWN_LINE_BREAK = " \n".freeze INCIDENT_LABEL_NAME = IncidentManagement::CreateIssueService::INCIDENT_LABEL[:title].freeze + METRIC_TIME_WINDOW = 30.minutes def full_title [environment_name, alert_title].compact.join(': ') @@ -119,9 +120,63 @@ module Projects Array(hosts.value).join(' ') end - def metric_embed_for_alert; end + def metric_embed_for_alert + url = embed_url_for_gitlab_alert || embed_url_for_self_managed_alert + + "\n[](#{url})" if url + end + + def embed_url_for_gitlab_alert + return unless gitlab_alert + + metrics_dashboard_project_prometheus_alert_url( + project, + gitlab_alert.prometheus_metric_id, + environment_id: environment.id, + **alert_embed_window_params(embed_time) + ) + end + + def embed_url_for_self_managed_alert + return unless environment && full_query && title + + metrics_dashboard_project_environment_url( + project, + environment, + embed_json: dashboard_for_self_managed_alert.to_json, + **alert_embed_window_params(embed_time) + ) + end + + def embed_time + starts_at ? Time.rfc3339(starts_at) : Time.current + end + + def alert_embed_window_params(time) + { + start: format_embed_timestamp(time - METRIC_TIME_WINDOW), + end: format_embed_timestamp(time + METRIC_TIME_WINDOW) + } + end + + def format_embed_timestamp(time) + time.utc.strftime('%FT%TZ') + end + + def dashboard_for_self_managed_alert + { + panel_groups: [{ + panels: [{ + type: 'line-graph', + title: title, + y_label: y_label, + metrics: [{ + query_range: full_query + }] + }] + }] + } + end end end end - -Projects::Prometheus::AlertPresenter.prepend_if_ee('EE::Projects::Prometheus::AlertPresenter') diff --git a/app/presenters/projects/settings/deploy_keys_presenter.rb b/app/presenters/projects/settings/deploy_keys_presenter.rb index 66211d02696..103c26289bf 100644 --- a/app/presenters/projects/settings/deploy_keys_presenter.rb +++ b/app/presenters/projects/settings/deploy_keys_presenter.rb @@ -60,11 +60,11 @@ module Projects end def to_partial_path - 'projects/deploy_keys/index' + '../../shared/deploy_keys/index' end def form_partial_path - 'projects/deploy_keys/form' + 'shared/deploy_keys/project_group_form' end private diff --git a/app/presenters/release_presenter.rb b/app/presenters/release_presenter.rb index 3db89df1cc8..ea46f0a234b 100644 --- a/app/presenters/release_presenter.rb +++ b/app/presenters/release_presenter.rb @@ -43,13 +43,6 @@ class ReleasePresenter < Gitlab::View::Presenter::Delegated edit_project_release_url(project, release) end - def evidence_file_path - evidence = release.evidences.first - return unless evidence - - project_evidence_url(project, release, evidence, format: :json) - end - private def can_download_code? diff --git a/app/presenters/snippet_presenter.rb b/app/presenters/snippet_presenter.rb index ba0b2b42383..faaf7568c72 100644 --- a/app/presenters/snippet_presenter.rb +++ b/app/presenters/snippet_presenter.rb @@ -12,11 +12,11 @@ class SnippetPresenter < Gitlab::View::Presenter::Delegated end def ssh_url_to_repo - snippet.ssh_url_to_repo if snippet.versioned_enabled_for?(current_user) + snippet.ssh_url_to_repo if snippet.repository_exists? end def http_url_to_repo - snippet.http_url_to_repo if snippet.versioned_enabled_for?(current_user) + snippet.http_url_to_repo if snippet.repository_exists? end def can_read_snippet? @@ -36,10 +36,10 @@ class SnippetPresenter < Gitlab::View::Presenter::Delegated end def blob - if Feature.enabled?(:version_snippets, current_user) && !snippet.repository.empty? - snippet.blobs.first - else + if snippet.empty_repo? snippet.blob + else + snippet.blobs.first end end |