diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index b920e041135..bd85105ccb4 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -111,15 +111,22 @@ export const fetchDiffFilesBatch = ({ commit, state }) => { commit(types.SET_BATCH_LOADING, true); commit(types.SET_RETRIEVING_BATCHES, true); - const getBatch = page => + const getBatch = (page = 1) => axios .get(state.endpointBatch, { - params: { ...urlParams, page }, + params: { + ...urlParams, + page, + }, }) .then(({ data: { pagination, diff_files } }) => { commit(types.SET_DIFF_DATA_BATCH, { diff_files }); commit(types.SET_BATCH_LOADING, false); - if (!pagination.next_page) commit(types.SET_RETRIEVING_BATCHES, false); + + if (!pagination.next_page) { + commit(types.SET_RETRIEVING_BATCHES, false); + } + return pagination.next_page; }) .then(nextPage => nextPage && getBatch(nextPage)) @@ -132,6 +139,11 @@ export const fetchDiffFilesBatch = ({ commit, state }) => { export const fetchDiffFilesMeta = ({ commit, state }) => { const worker = new TreeWorker(); + const urlParams = {}; + + if (state.useSingleDiffStyle) { + urlParams.view = state.diffViewType; + } commit(types.SET_LOADING, true); @@ -142,16 +154,17 @@ export const fetchDiffFilesMeta = ({ commit, state }) => { }); return axios - .get(state.endpointMetadata) + .get(mergeUrlParams(urlParams, state.endpointMetadata)) .then(({ data }) => { const strippedData = { ...data }; + delete strippedData.diff_files; commit(types.SET_LOADING, false); commit(types.SET_MERGE_REQUEST_DIFFS, data.merge_request_diffs || []); commit(types.SET_DIFF_DATA, strippedData); - prepareDiffData(data); - worker.postMessage(data.diff_files); + worker.postMessage(prepareDiffData(data, state.diffFiles)); + return data; }) .catch(() => worker.terminate()); @@ -226,7 +239,7 @@ export const startRenderDiffsQueue = ({ state, commit }) => { const nextFile = state.diffFiles.find( file => !file.renderIt && - (file.viewer && (!file.viewer.collapsed || !file.viewer.name === diffViewerModes.text)), + (file.viewer && (!file.viewer.collapsed || file.viewer.name !== diffViewerModes.text)), ); if (nextFile) { diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 1505be1a0b2..c26411af5d7 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -148,8 +148,8 @@ export default { }, [types.ADD_COLLAPSED_DIFFS](state, { file, data }) { - prepareDiffData(data); - const [newFileData] = data.diff_files.filter(f => f.file_hash === file.file_hash); + const files = prepareDiffData(data); + const [newFileData] = files.filter(f => f.file_hash === file.file_hash); const selectedFile = state.diffFiles.find(f => f.file_hash === file.file_hash); Object.assign(selectedFile, { ...newFileData }); }, diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index b379f1fabef..80972d2aeb8 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -217,30 +217,19 @@ function diffFileUniqueId(file) { return `${file.content_sha}-${file.file_hash}`; } -function combineDiffFilesWithPriorFiles(files, prior = []) { - files.forEach(file => { - const id = diffFileUniqueId(file); - const oldMatch = prior.find(oldFile => diffFileUniqueId(oldFile) === id); +function mergeTwoFiles(target, source) { + const originalInline = target.highlighted_diff_lines; + const originalParallel = target.parallel_diff_lines; + const missingInline = !originalInline.length; + const missingParallel = !originalParallel.length; - if (oldMatch) { - const missingInline = !file.highlighted_diff_lines; - const missingParallel = !file.parallel_diff_lines; - - if (missingInline) { - Object.assign(file, { - highlighted_diff_lines: oldMatch.highlighted_diff_lines, - }); - } - - if (missingParallel) { - Object.assign(file, { - parallel_diff_lines: oldMatch.parallel_diff_lines, - }); - } - } - }); - - return files; + return { + ...target, + highlighted_diff_lines: missingInline ? source.highlighted_diff_lines : originalInline, + parallel_diff_lines: missingParallel ? source.parallel_diff_lines : originalParallel, + renderIt: source.renderIt, + collapsed: source.collapsed, + }; } function ensureBasicDiffFileLines(file) { @@ -260,13 +249,16 @@ function cleanRichText(text) { } function prepareLine(line) { - return Object.assign(line, { - rich_text: cleanRichText(line.rich_text), - discussionsExpanded: true, - discussions: [], - hasForm: false, - text: undefined, - }); + if (!line.alreadyPrepared) { + Object.assign(line, { + rich_text: cleanRichText(line.rich_text), + discussionsExpanded: true, + discussions: [], + hasForm: false, + text: undefined, + alreadyPrepared: true, + }); + } } function prepareDiffFileLines(file) { @@ -288,11 +280,11 @@ function prepareDiffFileLines(file) { parallelLinesCount += 1; prepareLine(line.right); } + }); - Object.assign(file, { - inlineLinesCount: inlineLines.length, - parallelLinesCount, - }); + Object.assign(file, { + inlineLinesCount: inlineLines.length, + parallelLinesCount, }); return file; @@ -318,11 +310,26 @@ function finalizeDiffFile(file) { return file; } -export function prepareDiffData(diffData, priorFiles) { - return combineDiffFilesWithPriorFiles(diffData.diff_files, priorFiles) +function deduplicateFilesList(files) { + const dedupedFiles = files.reduce((newList, file) => { + const id = diffFileUniqueId(file); + + return { + ...newList, + [id]: newList[id] ? mergeTwoFiles(newList[id], file) : file, + }; + }, {}); + + return Object.values(dedupedFiles); +} + +export function prepareDiffData(diff, priorFiles = []) { + const cleanedFiles = (diff.diff_files || []) .map(ensureBasicDiffFileLines) .map(prepareDiffFileLines) .map(finalizeDiffFile); + + return deduplicateFilesList([...priorFiles, ...cleanedFiles]); } export function getDiffPositionByLineCode(diffFiles, useSingleDiffStyle) { diff --git a/app/assets/javascripts/monitoring/components/charts/single_stat.vue b/app/assets/javascripts/monitoring/components/charts/single_stat.vue index e75ddb05808..3368be4df75 100644 --- a/app/assets/javascripts/monitoring/components/charts/single_stat.vue +++ b/app/assets/javascripts/monitoring/components/charts/single_stat.vue @@ -19,8 +19,21 @@ export default { queryInfo() { return this.graphData.metrics[0]; }, - engineeringNotation() { - return `${roundOffFloat(this.queryInfo.result[0].value[1], 1)}${this.queryInfo.unit}`; + queryResult() { + return this.queryInfo.result[0]?.value[1]; + }, + /** + * This method formats the query result from a promQL expression + * allowing a user to format the data in percentile values + * by using the `max_value` inner property from the graphData prop + * @returns {(String)} + */ + statValue() { + const chartValue = this.graphData?.max_value + ? (this.queryResult / Number(this.graphData.max_value)) * 100 + : this.queryResult; + + return `${roundOffFloat(chartValue, 1)}${this.queryInfo.unit}`; }, graphTitle() { return this.queryInfo.label; @@ -33,6 +46,6 @@ export default {
{{ graphTitle }}
- + diff --git a/app/controllers/projects/settings/operations_controller.rb b/app/controllers/projects/settings/operations_controller.rb index 1571cb8cd34..3817da8596b 100644 --- a/app/controllers/projects/settings/operations_controller.rb +++ b/app/controllers/projects/settings/operations_controller.rb @@ -60,7 +60,7 @@ module Projects # overridden in EE def permitted_project_params - { + project_params = { metrics_setting_attributes: [:external_dashboard_url], error_tracking_setting_attributes: [ @@ -72,6 +72,12 @@ module Projects grafana_integration_attributes: [:token, :grafana_url, :enabled] } + + if Feature.enabled?(:settings_operations_prometheus_service, project) + project_params[:prometheus_integration_attributes] = [:manual_configuration, :api_url] + end + + project_params end end end diff --git a/app/models/project.rb b/app/models/project.rb index b2ac9c99ab6..0ed2510dbf4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -322,6 +322,7 @@ class Project < ApplicationRecord accepts_nested_attributes_for :error_tracking_setting, update_only: true accepts_nested_attributes_for :metrics_setting, update_only: true, allow_destroy: true accepts_nested_attributes_for :grafana_integration, update_only: true, allow_destroy: true + accepts_nested_attributes_for :prometheus_service, update_only: true delegate :feature_available?, :builds_enabled?, :wiki_enabled?, :merge_requests_enabled?, :forking_enabled?, :issues_enabled?, diff --git a/app/services/projects/operations/update_service.rb b/app/services/projects/operations/update_service.rb index 3ee6c2467c2..f0144ad1213 100644 --- a/app/services/projects/operations/update_service.rb +++ b/app/services/projects/operations/update_service.rb @@ -15,6 +15,7 @@ module Projects error_tracking_params .merge(metrics_setting_params) .merge(grafana_integration_params) + .merge(prometheus_integration_params) end def metrics_setting_params @@ -77,6 +78,15 @@ module Projects { grafana_integration_attributes: attrs.merge(_destroy: destroy) } end + + def prometheus_integration_params + return {} unless attrs = params[:prometheus_integration_attributes] + + service = project.find_or_initialize_service(::PrometheusService.to_param) + service.assign_attributes(attrs) + + { prometheus_service_attributes: service.attributes.except(*%w(id project_id created_at updated_at)) } + end end end end diff --git a/changelogs/unreleased/118783-restoring-wikis-fails.yml b/changelogs/unreleased/118783-restoring-wikis-fails.yml new file mode 100644 index 00000000000..f8a5b13ba3a --- /dev/null +++ b/changelogs/unreleased/118783-restoring-wikis-fails.yml @@ -0,0 +1,5 @@ +--- +title: Fix backup restoration with pre-existing wiki +merge_request: 24394 +author: +type: fixed diff --git a/changelogs/unreleased/jivanvl-add-percentile-support-single-stat-charts.yml b/changelogs/unreleased/jivanvl-add-percentile-support-single-stat-charts.yml new file mode 100644 index 00000000000..ddf0ee90a37 --- /dev/null +++ b/changelogs/unreleased/jivanvl-add-percentile-support-single-stat-charts.yml @@ -0,0 +1,5 @@ +--- +title: Add percentile value support to single stat panel types +merge_request: 24813 +author: +type: changed diff --git a/changelogs/unreleased/refactoring-entities-file-17.yml b/changelogs/unreleased/refactoring-entities-file-17.yml new file mode 100644 index 00000000000..fa801c259f0 --- /dev/null +++ b/changelogs/unreleased/refactoring-entities-file-17.yml @@ -0,0 +1,5 @@ +--- +title: Separate label entities into own class files +merge_request: 24938 +author: Rajendra Kadam +type: added diff --git a/changelogs/unreleased/refactoring-entities-file-19.yml b/changelogs/unreleased/refactoring-entities-file-19.yml new file mode 100644 index 00000000000..f5aeac766c3 --- /dev/null +++ b/changelogs/unreleased/refactoring-entities-file-19.yml @@ -0,0 +1,5 @@ +--- +title: Separate entities into own class files +merge_request: 24941 +author: Rajendra Kadam +type: added diff --git a/changelogs/unreleased/refactoring-entities-file-21.yml b/changelogs/unreleased/refactoring-entities-file-21.yml new file mode 100644 index 00000000000..73c4cc4daac --- /dev/null +++ b/changelogs/unreleased/refactoring-entities-file-21.yml @@ -0,0 +1,5 @@ +--- +title: Separate job entities into own class files +merge_request: 24948 +author: Rajendra Kadam +type: added diff --git a/changelogs/unreleased/refactoring-entities-file-24.yml b/changelogs/unreleased/refactoring-entities-file-24.yml new file mode 100644 index 00000000000..f58285aeeb6 --- /dev/null +++ b/changelogs/unreleased/refactoring-entities-file-24.yml @@ -0,0 +1,5 @@ +--- +title: Separate token and template entities into own class files +merge_request: 24955 +author: Rajendra Kadam +type: added diff --git a/changelogs/unreleased/update-handlebarsjs.yml b/changelogs/unreleased/update-handlebarsjs.yml new file mode 100644 index 00000000000..131ef58724b --- /dev/null +++ b/changelogs/unreleased/update-handlebarsjs.yml @@ -0,0 +1,5 @@ +--- +title: Update handlebars to remove issues from dependency dashboard +merge_request: +author: +type: security diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index a38e100303e..5f0f39cd15b 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -143,6 +143,11 @@ their node under pressure. ## How to +### Get access to the GCP Review Apps cluster + +You need to [open an access request (internal link)](https://gitlab.com/gitlab-com/access-requests/issues/new) +for the `gcp-review-apps-sg` GCP group. + ### Log into my Review App The default username is `root` and its password can be found in the 1Password @@ -163,6 +168,7 @@ secure note named `gitlab-{ce,ee} Review App's root password`. ### Run a Rails console +1. Make sure you [have access to the cluster](#get-access-to-the-gcp-review-apps-cluster) first. 1. [Filter Workloads by your Review App slug](https://console.cloud.google.com/kubernetes/workload?project=gitlab-review-apps), e.g. `review-qa-raise-e-12chm0`. 1. Find and open the `task-runner` Deployment, e.g. `review-qa-raise-e-12chm0-task-runner`. @@ -178,6 +184,7 @@ secure note named `gitlab-{ce,ee} Review App's root password`. ### Dig into a Pod's logs +1. Make sure you [have access to the cluster](#get-access-to-the-gcp-review-apps-cluster) first. 1. [Filter Workloads by your Review App slug](https://console.cloud.google.com/kubernetes/workload?project=gitlab-review-apps), e.g. `review-qa-raise-e-12chm0`. 1. Find and open the `migrations` Deployment, e.g. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index aa2cbd3fed3..5d5a61b76f1 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -163,42 +163,6 @@ module API end end - class LabelBasic < Grape::Entity - expose :id, :name, :color, :description, :description_html, :text_color - end - - class Label < LabelBasic - with_options if: lambda { |_, options| options[:with_counts] } do - expose :open_issues_count do |label, options| - label.open_issues_count(options[:current_user]) - end - - expose :closed_issues_count do |label, options| - label.closed_issues_count(options[:current_user]) - end - - expose :open_merge_requests_count do |label, options| - label.open_merge_requests_count(options[:current_user]) - end - end - - expose :subscribed do |label, options| - label.subscribed?(options[:current_user], options[:parent]) - end - end - - class GroupLabel < Label - end - - class ProjectLabel < Label - expose :priority do |label, options| - label.priority(options[:parent]) - end - expose :is_project_label do |label, options| - label.is_a?(::ProjectLabel) - end - end - class List < Grape::Entity expose :id expose :label, using: Entities::LabelBasic @@ -242,130 +206,6 @@ module API expose :message, :starts_at, :ends_at, :color, :font, :target_path, :broadcast_type end - class ApplicationStatistics < Grape::Entity - include ActionView::Helpers::NumberHelper - include CountHelper - - expose :forks do |counts| - approximate_fork_count_with_delimiters(counts) - end - - expose :issues do |counts| - approximate_count_with_delimiters(counts, ::Issue) - end - - expose :merge_requests do |counts| - approximate_count_with_delimiters(counts, ::MergeRequest) - end - - expose :notes do |counts| - approximate_count_with_delimiters(counts, ::Note) - end - - expose :snippets do |counts| - approximate_count_with_delimiters(counts, ::Snippet) - end - - expose :ssh_keys do |counts| - approximate_count_with_delimiters(counts, ::Key) - end - - expose :milestones do |counts| - approximate_count_with_delimiters(counts, ::Milestone) - end - - expose :users do |counts| - approximate_count_with_delimiters(counts, ::User) - end - - expose :projects do |counts| - approximate_count_with_delimiters(counts, ::Project) - end - - expose :groups do |counts| - approximate_count_with_delimiters(counts, ::Group) - end - - expose :active_users do |_| - number_with_delimiter(::User.active.count) - end - end - - class ApplicationSetting < Grape::Entity - def self.exposed_attributes - attributes = ::ApplicationSettingsHelper.visible_attributes - attributes.delete(:performance_bar_allowed_group_path) - attributes.delete(:performance_bar_enabled) - attributes.delete(:allow_local_requests_from_hooks_and_services) - - # let's not expose the secret key in a response - attributes.delete(:asset_proxy_secret_key) - attributes.delete(:eks_secret_access_key) - - attributes - end - - expose :id, :performance_bar_allowed_group_id - expose(*exposed_attributes) - expose(:restricted_visibility_levels) do |setting, _options| - setting.restricted_visibility_levels.map { |level| Gitlab::VisibilityLevel.string_level(level) } - end - expose(:default_project_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_project_visibility) } - expose(:default_snippet_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_snippet_visibility) } - expose(:default_group_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_group_visibility) } - - expose(*::ApplicationSettingsHelper.external_authorization_service_attributes) - - # support legacy names, can be removed in v5 - expose :password_authentication_enabled_for_web, as: :password_authentication_enabled - expose :password_authentication_enabled_for_web, as: :signin_enabled - expose :allow_local_requests_from_web_hooks_and_services, as: :allow_local_requests_from_hooks_and_services - end - - class Appearance < Grape::Entity - expose :title - expose :description - - expose :logo do |appearance, options| - appearance.logo.url - end - - expose :header_logo do |appearance, options| - appearance.header_logo.url - end - - expose :favicon do |appearance, options| - appearance.favicon.url - end - - expose :new_project_guidelines - expose :header_message - expose :footer_message - expose :message_background_color - expose :message_font_color - expose :email_header_and_footer_enabled - end - - # deprecated old Release representation - class TagRelease < Grape::Entity - expose :tag, as: :tag_name - expose :description - end - - module Releases - class Link < Grape::Entity - expose :id - expose :name - expose :url - expose :external?, as: :external - end - - class Source < Grape::Entity - expose :format - expose :url - end - end - class Release < Grape::Entity include ::API::Helpers::Presentable @@ -477,40 +317,6 @@ module API expose :id, :token end - class JobArtifactFile < Grape::Entity - expose :filename - expose :cached_size, as: :size - end - - class JobArtifact < Grape::Entity - expose :file_type, :size, :filename, :file_format - end - - class JobBasic < Grape::Entity - expose :id, :status, :stage, :name, :ref, :tag, :coverage, :allow_failure - expose :created_at, :started_at, :finished_at - expose :duration - expose :user, with: Entities::User - expose :commit, with: Entities::Commit - expose :pipeline, with: Entities::PipelineBasic - - expose :web_url do |job, _options| - Gitlab::Routing.url_helpers.project_job_url(job.project, job) - end - end - - class Job < JobBasic - # artifacts_file is included in job_artifacts, but kept for backward compatibility (remove in api/v5) - expose :artifacts_file, using: JobArtifactFile, if: -> (job, opts) { job.artifacts? } - expose :job_artifacts, as: :artifacts, using: JobArtifact - expose :runner, with: Runner - expose :artifacts_expire_at - end - - class JobBasicWithProject < JobBasic - expose :project, with: Entities::ProjectIdentity - end - class Trigger < Grape::Entity include ::API::Helpers::Presentable @@ -585,32 +391,6 @@ module API expose :content end - class TemplatesList < Grape::Entity - expose :key - expose :name - end - - class Template < Grape::Entity - expose :name, :content - end - - class BroadcastMessage < Grape::Entity - expose :id, :message, :starts_at, :ends_at, :color, :font - expose :active?, as: :active - end - - class PersonalAccessToken < Grape::Entity - expose :id, :name, :revoked, :created_at, :scopes - expose :active?, as: :active - expose :expires_at do |personal_access_token| - personal_access_token.expires_at ? personal_access_token.expires_at.strftime("%Y-%m-%d") : nil - end - end - - class PersonalAccessTokenWithToken < PersonalAccessToken - expose :token - end - class ImpersonationToken < PersonalAccessToken expose :impersonation end diff --git a/lib/api/entities/appearance.rb b/lib/api/entities/appearance.rb new file mode 100644 index 00000000000..c3cffc8d05c --- /dev/null +++ b/lib/api/entities/appearance.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module API + module Entities + class Appearance < Grape::Entity + expose :title + expose :description + + expose :logo do |appearance, options| + appearance.logo.url + end + + expose :header_logo do |appearance, options| + appearance.header_logo.url + end + + expose :favicon do |appearance, options| + appearance.favicon.url + end + + expose :new_project_guidelines + expose :header_message + expose :footer_message + expose :message_background_color + expose :message_font_color + expose :email_header_and_footer_enabled + end + end +end diff --git a/lib/api/entities/application_setting.rb b/lib/api/entities/application_setting.rb new file mode 100644 index 00000000000..6ca2d1e6da4 --- /dev/null +++ b/lib/api/entities/application_setting.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module API + module Entities + class ApplicationSetting < Grape::Entity + def self.exposed_attributes + attributes = ::ApplicationSettingsHelper.visible_attributes + attributes.delete(:performance_bar_allowed_group_path) + attributes.delete(:performance_bar_enabled) + attributes.delete(:allow_local_requests_from_hooks_and_services) + + # let's not expose the secret key in a response + attributes.delete(:asset_proxy_secret_key) + attributes.delete(:eks_secret_access_key) + + attributes + end + + expose :id, :performance_bar_allowed_group_id + expose(*exposed_attributes) + expose(:restricted_visibility_levels) do |setting, _options| + setting.restricted_visibility_levels.map { |level| Gitlab::VisibilityLevel.string_level(level) } + end + expose(:default_project_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_project_visibility) } + expose(:default_snippet_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_snippet_visibility) } + expose(:default_group_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_group_visibility) } + + expose(*::ApplicationSettingsHelper.external_authorization_service_attributes) + + # support legacy names, can be removed in v5 + expose :password_authentication_enabled_for_web, as: :password_authentication_enabled + expose :password_authentication_enabled_for_web, as: :signin_enabled + expose :allow_local_requests_from_web_hooks_and_services, as: :allow_local_requests_from_hooks_and_services + end + end +end diff --git a/lib/api/entities/application_statistics.rb b/lib/api/entities/application_statistics.rb new file mode 100644 index 00000000000..4bcba1da464 --- /dev/null +++ b/lib/api/entities/application_statistics.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module API + module Entities + class ApplicationStatistics < Grape::Entity + include ActionView::Helpers::NumberHelper + include CountHelper + + expose :forks do |counts| + approximate_fork_count_with_delimiters(counts) + end + + expose :issues do |counts| + approximate_count_with_delimiters(counts, ::Issue) + end + + expose :merge_requests do |counts| + approximate_count_with_delimiters(counts, ::MergeRequest) + end + + expose :notes do |counts| + approximate_count_with_delimiters(counts, ::Note) + end + + expose :snippets do |counts| + approximate_count_with_delimiters(counts, ::Snippet) + end + + expose :ssh_keys do |counts| + approximate_count_with_delimiters(counts, ::Key) + end + + expose :milestones do |counts| + approximate_count_with_delimiters(counts, ::Milestone) + end + + expose :users do |counts| + approximate_count_with_delimiters(counts, ::User) + end + + expose :projects do |counts| + approximate_count_with_delimiters(counts, ::Project) + end + + expose :groups do |counts| + approximate_count_with_delimiters(counts, ::Group) + end + + expose :active_users do |_| + number_with_delimiter(::User.active.count) + end + end + end +end diff --git a/lib/api/entities/broadcast_message.rb b/lib/api/entities/broadcast_message.rb new file mode 100644 index 00000000000..31d4cc50526 --- /dev/null +++ b/lib/api/entities/broadcast_message.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module API + module Entities + class BroadcastMessage < Grape::Entity + expose :id, :message, :starts_at, :ends_at, :color, :font + expose :active?, as: :active + end + end +end diff --git a/lib/api/entities/group_label.rb b/lib/api/entities/group_label.rb new file mode 100644 index 00000000000..4e1b9226e6d --- /dev/null +++ b/lib/api/entities/group_label.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module API + module Entities + class GroupLabel < Entities::Label + end + end +end diff --git a/lib/api/entities/job.rb b/lib/api/entities/job.rb new file mode 100644 index 00000000000..cbee8794007 --- /dev/null +++ b/lib/api/entities/job.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module API + module Entities + class Job < Entities::JobBasic + # artifacts_file is included in job_artifacts, but kept for backward compatibility (remove in api/v5) + expose :artifacts_file, using: Entities::JobArtifactFile, if: -> (job, opts) { job.artifacts? } + expose :job_artifacts, as: :artifacts, using: Entities::JobArtifact + expose :runner, with: Entities::Runner + expose :artifacts_expire_at + end + end +end diff --git a/lib/api/entities/job_artifact.rb b/lib/api/entities/job_artifact.rb new file mode 100644 index 00000000000..94dbdb38fee --- /dev/null +++ b/lib/api/entities/job_artifact.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module API + module Entities + class JobArtifact < Grape::Entity + expose :file_type, :size, :filename, :file_format + end + end +end diff --git a/lib/api/entities/job_artifact_file.rb b/lib/api/entities/job_artifact_file.rb new file mode 100644 index 00000000000..fa2851a7f0e --- /dev/null +++ b/lib/api/entities/job_artifact_file.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module API + module Entities + class JobArtifactFile < Grape::Entity + expose :filename + expose :cached_size, as: :size + end + end +end diff --git a/lib/api/entities/job_basic.rb b/lib/api/entities/job_basic.rb new file mode 100644 index 00000000000..a8541039934 --- /dev/null +++ b/lib/api/entities/job_basic.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module API + module Entities + class JobBasic < Grape::Entity + expose :id, :status, :stage, :name, :ref, :tag, :coverage, :allow_failure + expose :created_at, :started_at, :finished_at + expose :duration + expose :user, with: Entities::User + expose :commit, with: Entities::Commit + expose :pipeline, with: Entities::PipelineBasic + + expose :web_url do |job, _options| + Gitlab::Routing.url_helpers.project_job_url(job.project, job) + end + end + end +end diff --git a/lib/api/entities/job_basic_with_project.rb b/lib/api/entities/job_basic_with_project.rb new file mode 100644 index 00000000000..09387e045ec --- /dev/null +++ b/lib/api/entities/job_basic_with_project.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module API + module Entities + class JobBasicWithProject < Entities::JobBasic + expose :project, with: Entities::ProjectIdentity + end + end +end diff --git a/lib/api/entities/label.rb b/lib/api/entities/label.rb new file mode 100644 index 00000000000..ca9a0912331 --- /dev/null +++ b/lib/api/entities/label.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module API + module Entities + class Label < Entities::LabelBasic + with_options if: lambda { |_, options| options[:with_counts] } do + expose :open_issues_count do |label, options| + label.open_issues_count(options[:current_user]) + end + + expose :closed_issues_count do |label, options| + label.closed_issues_count(options[:current_user]) + end + + expose :open_merge_requests_count do |label, options| + label.open_merge_requests_count(options[:current_user]) + end + end + + expose :subscribed do |label, options| + label.subscribed?(options[:current_user], options[:parent]) + end + end + end +end diff --git a/lib/api/entities/label_basic.rb b/lib/api/entities/label_basic.rb new file mode 100644 index 00000000000..ed52688638e --- /dev/null +++ b/lib/api/entities/label_basic.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module API + module Entities + class LabelBasic < Grape::Entity + expose :id, :name, :color, :description, :description_html, :text_color + end + end +end diff --git a/lib/api/entities/personal_access_token.rb b/lib/api/entities/personal_access_token.rb new file mode 100644 index 00000000000..d6fb9af6ab3 --- /dev/null +++ b/lib/api/entities/personal_access_token.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module API + module Entities + class PersonalAccessToken < Grape::Entity + expose :id, :name, :revoked, :created_at, :scopes + expose :active?, as: :active + expose :expires_at do |personal_access_token| + personal_access_token.expires_at ? personal_access_token.expires_at.strftime("%Y-%m-%d") : nil + end + end + end +end diff --git a/lib/api/entities/personal_access_token_with_token.rb b/lib/api/entities/personal_access_token_with_token.rb new file mode 100644 index 00000000000..84dcd3bc8d8 --- /dev/null +++ b/lib/api/entities/personal_access_token_with_token.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module API + module Entities + class PersonalAccessTokenWithToken < Entities::PersonalAccessToken + expose :token + end + end +end diff --git a/lib/api/entities/project_label.rb b/lib/api/entities/project_label.rb new file mode 100644 index 00000000000..b47a9414ddb --- /dev/null +++ b/lib/api/entities/project_label.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module API + module Entities + class ProjectLabel < Entities::Label + expose :priority do |label, options| + label.priority(options[:parent]) + end + expose :is_project_label do |label, options| + label.is_a?(::ProjectLabel) + end + end + end +end diff --git a/lib/api/entities/releases/link.rb b/lib/api/entities/releases/link.rb new file mode 100644 index 00000000000..6cc01e0e981 --- /dev/null +++ b/lib/api/entities/releases/link.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module API + module Entities + module Releases + class Link < Grape::Entity + expose :id + expose :name + expose :url + expose :external?, as: :external + end + end + end +end diff --git a/lib/api/entities/releases/source.rb b/lib/api/entities/releases/source.rb new file mode 100644 index 00000000000..2b0c8038ddf --- /dev/null +++ b/lib/api/entities/releases/source.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module API + module Entities + module Releases + class Source < Grape::Entity + expose :format + expose :url + end + end + end +end diff --git a/lib/api/entities/tag_release.rb b/lib/api/entities/tag_release.rb new file mode 100644 index 00000000000..d5f73d60332 --- /dev/null +++ b/lib/api/entities/tag_release.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module API + module Entities + # deprecated old Release representation + class TagRelease < Grape::Entity + expose :tag, as: :tag_name + expose :description + end + end +end diff --git a/lib/api/entities/template.rb b/lib/api/entities/template.rb new file mode 100644 index 00000000000..ef364d971bf --- /dev/null +++ b/lib/api/entities/template.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module API + module Entities + class Template < Grape::Entity + expose :name, :content + end + end +end diff --git a/lib/api/entities/templates_list.rb b/lib/api/entities/templates_list.rb new file mode 100644 index 00000000000..8e8aa1bd285 --- /dev/null +++ b/lib/api/entities/templates_list.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module API + module Entities + class TemplatesList < Grape::Entity + expose :key + expose :name + end + end +end diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb index 974e32ce17c..123a695be13 100644 --- a/lib/backup/repository.rb +++ b/lib/backup/repository.rb @@ -96,6 +96,7 @@ module Backup end wiki = ProjectWiki.new(project) + wiki.repository.remove rescue nil path_to_wiki_bundle = path_to_bundle(wiki) if File.exist?(path_to_wiki_bundle) diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 667a6336952..0a2cc6f6aa5 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -196,6 +196,39 @@ describe Projects::Settings::OperationsController do end end + context 'prometheus integration' do + describe 'PATCH #update' do + let(:params) do + { + prometheus_integration_attributes: { + manual_configuration: '0', + api_url: 'https://gitlab.prometheus.rocks' + } + } + end + + context 'feature flag :settings_operations_prometheus_service is enabled' do + before do + stub_feature_flags(settings_operations_prometheus_service: true) + end + + it_behaves_like 'PATCHable' + end + + context 'feature flag :settings_operations_prometheus_service is disabled' do + before do + stub_feature_flags(settings_operations_prometheus_service: false) + end + + it_behaves_like 'PATCHable' do + let(:permitted_params) do + ActionController::Parameters.new(params.except(:prometheus_integration_attributes)).permit! + end + end + end + end + end + private def project_params(project, params = {}) diff --git a/spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb b/spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb new file mode 100644 index 00000000000..92d90926c0a --- /dev/null +++ b/spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Batch diffs', :js do + include MergeRequestDiffHelpers + include RepoHelpers + + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'empty-branch') } + + before do + stub_feature_flags(single_mr_diff_view: true) + stub_feature_flags(diffs_batch_load: true) + + sign_in(project.owner) + + visit diffs_project_merge_request_path(merge_request.project, merge_request) + wait_for_requests + + # Add discussion to first line of first file + click_diff_line(find('.diff-file.file-holder:first-of-type tr.line_holder.new:first-of-type')) + page.within('.js-discussion-note-form') do + fill_in('note_note', with: 'First Line Comment') + click_button('Comment') + end + + # Add discussion to first line of last file + click_diff_line(find('.diff-file.file-holder:last-of-type tr.line_holder.new:first-of-type')) + page.within('.js-discussion-note-form') do + fill_in('note_note', with: 'Last Line Comment') + click_button('Comment') + end + + wait_for_requests + end + + it 'assigns discussions to diff files across multiple batch pages' do + # Reload so we know the discussions are persisting across batch loads + visit page.current_url + + # Wait for JS to settle + wait_for_requests + + expect(page).to have_selector('.diff-files-holder .file-holder', count: 39) + + # Confirm discussions are applied to appropriate files (should be contained in multiple diff pages) + page.within('.diff-file.file-holder:first-of-type .notes .timeline-entry .note .note-text') do + expect(page).to have_content('First Line Comment') + end + + page.within('.diff-file.file-holder:last-of-type .notes .timeline-entry .note .note-text') do + expect(page).to have_content('Last Line Comment') + end + end + + context 'when user visits a URL with a link directly to to a discussion' do + context 'which is in the first batched page of diffs' do + it 'scrolls to the correct discussion' do + page.within('.diff-file.file-holder:first-of-type') do + click_link('just now') + end + + visit page.current_url + + wait_for_requests + + # Confirm scrolled to correct UI element + expect(page.find('.diff-file.file-holder:first-of-type .discussion-notes .timeline-entry li.note[id]').obscured?).to be_falsey + expect(page.find('.diff-file.file-holder:last-of-type .discussion-notes .timeline-entry li.note[id]').obscured?).to be_truthy + end + end + + context 'which is in at least page 2 of the batched pages of diffs' do + it 'scrolls to the correct discussion' do + page.within('.diff-file.file-holder:last-of-type') do + click_link('just now') + end + + visit page.current_url + + wait_for_requests + + # Confirm scrolled to correct UI element + expect(page.find('.diff-file.file-holder:first-of-type .discussion-notes .timeline-entry li.note[id]').obscured?).to be_truthy + expect(page.find('.diff-file.file-holder:last-of-type .discussion-notes .timeline-entry li.note[id]').obscured?).to be_falsey + end + end + end + + context 'when user switches view styles' do + before do + find('.js-show-diff-settings').click + click_button 'Side-by-side' + + wait_for_requests + end + + it 'has the correct discussions applied to files across batched pages' do + expect(page).to have_selector('.diff-files-holder .file-holder', count: 39) + + page.within('.diff-file.file-holder:first-of-type .notes .timeline-entry .note .note-text') do + expect(page).to have_content('First Line Comment') + end + + page.within('.diff-file.file-holder:last-of-type .notes .timeline-entry .note .note-text') do + expect(page).to have_content('Last Line Comment') + end + end + end +end diff --git a/spec/frontend/monitoring/components/charts/single_stat_spec.js b/spec/frontend/monitoring/components/charts/single_stat_spec.js index 2410dae112b..1aa7ba867b4 100644 --- a/spec/frontend/monitoring/components/charts/single_stat_spec.js +++ b/spec/frontend/monitoring/components/charts/single_stat_spec.js @@ -18,9 +18,53 @@ describe('Single Stat Chart component', () => { }); describe('computed', () => { - describe('engineeringNotation', () => { + describe('statValue', () => { it('should interpolate the value and unit props', () => { - expect(singleStatChart.vm.engineeringNotation).toBe('91MB'); + expect(singleStatChart.vm.statValue).toBe('91MB'); + }); + + it('should change the value representation to a percentile one', () => { + singleStatChart.setProps({ + graphData: { + ...graphDataPrometheusQuery, + max_value: 120, + }, + }); + + expect(singleStatChart.vm.statValue).toContain('75.8'); + }); + + it('should display NaN for non numeric max_value values', () => { + singleStatChart.setProps({ + graphData: { + ...graphDataPrometheusQuery, + max_value: 'not a number', + }, + }); + + expect(singleStatChart.vm.statValue).toContain('NaN'); + }); + + it('should display NaN for missing query values', () => { + singleStatChart.setProps({ + graphData: { + ...graphDataPrometheusQuery, + metrics: [ + { + ...graphDataPrometheusQuery.metrics[0], + result: [ + { + ...graphDataPrometheusQuery.metrics[0].result[0], + value: [''], + }, + ], + }, + ], + max_value: 120, + }, + }); + + expect(singleStatChart.vm.statValue).toContain('NaN'); }); }); }); diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index af2dd7b4f93..ff17d8ec158 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -158,16 +158,19 @@ describe('DiffsStoreActions', () => { const res1 = { diff_files: [], pagination: { next_page: 2 } }; const res2 = { diff_files: [], pagination: {} }; mock - .onGet(endpointBatch, { params: { page: undefined, per_page: DIFFS_PER_PAGE, w: '1' } }) - .reply(200, res1); - mock - .onGet(endpointBatch, { params: { page: 2, per_page: DIFFS_PER_PAGE, w: '1' } }) + .onGet(endpointBatch, { + params: { page: 1, per_page: DIFFS_PER_PAGE, w: '1', view: 'inline' }, + }) + .reply(200, res1) + .onGet(endpointBatch, { + params: { page: 2, per_page: DIFFS_PER_PAGE, w: '1', view: 'inline' }, + }) .reply(200, res2); testAction( fetchDiffFilesBatch, {}, - { endpointBatch }, + { endpointBatch, useSingleDiffStyle: true, diffViewType: 'inline' }, [ { type: types.SET_BATCH_LOADING, payload: true }, { type: types.SET_RETRIEVING_BATCHES, payload: true }, @@ -188,7 +191,7 @@ describe('DiffsStoreActions', () => { describe('fetchDiffFilesMeta', () => { it('should fetch diff meta information', done => { - const endpointMetadata = '/fetch/diffs_meta'; + const endpointMetadata = '/fetch/diffs_meta?view=inline'; const mock = new MockAdapter(axios); const data = { diff_files: [] }; const res = { data }; @@ -213,6 +216,108 @@ describe('DiffsStoreActions', () => { }); }); + describe('when the single diff view feature flag is off', () => { + describe('fetchDiffFiles', () => { + it('should fetch diff files', done => { + const endpoint = '/fetch/diff/files?w=1'; + const mock = new MockAdapter(axios); + const res = { diff_files: 1, merge_request_diffs: [] }; + mock.onGet(endpoint).reply(200, res); + + testAction( + fetchDiffFiles, + {}, + { + endpoint, + diffFiles: [], + showWhitespace: false, + diffViewType: 'inline', + useSingleDiffStyle: false, + }, + [ + { type: types.SET_LOADING, payload: true }, + { type: types.SET_LOADING, payload: false }, + { type: types.SET_MERGE_REQUEST_DIFFS, payload: res.merge_request_diffs }, + { type: types.SET_DIFF_DATA, payload: res }, + ], + [], + () => { + mock.restore(); + done(); + }, + ); + + fetchDiffFiles({ state: { endpoint }, commit: () => null }) + .then(data => { + expect(data).toEqual(res); + done(); + }) + .catch(done.fail); + }); + }); + + describe('fetchDiffFilesBatch', () => { + it('should fetch batch diff files', done => { + const endpointBatch = '/fetch/diffs_batch'; + const mock = new MockAdapter(axios); + const res1 = { diff_files: [], pagination: { next_page: 2 } }; + const res2 = { diff_files: [], pagination: {} }; + mock + .onGet(endpointBatch, { params: { page: 1, per_page: DIFFS_PER_PAGE, w: '1' } }) + .reply(200, res1) + .onGet(endpointBatch, { params: { page: 2, per_page: DIFFS_PER_PAGE, w: '1' } }) + .reply(200, res2); + + testAction( + fetchDiffFilesBatch, + {}, + { endpointBatch, useSingleDiffStyle: false }, + [ + { type: types.SET_BATCH_LOADING, payload: true }, + { type: types.SET_RETRIEVING_BATCHES, payload: true }, + { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res1.diff_files } }, + { type: types.SET_BATCH_LOADING, payload: false }, + { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: [] } }, + { type: types.SET_BATCH_LOADING, payload: false }, + { type: types.SET_RETRIEVING_BATCHES, payload: false }, + ], + [], + () => { + mock.restore(); + done(); + }, + ); + }); + }); + + describe('fetchDiffFilesMeta', () => { + it('should fetch diff meta information', done => { + const endpointMetadata = '/fetch/diffs_meta?'; + const mock = new MockAdapter(axios); + const data = { diff_files: [] }; + const res = { data }; + mock.onGet(endpointMetadata).reply(200, res); + + testAction( + fetchDiffFilesMeta, + {}, + { endpointMetadata, useSingleDiffStyle: false }, + [ + { type: types.SET_LOADING, payload: true }, + { type: types.SET_LOADING, payload: false }, + { type: types.SET_MERGE_REQUEST_DIFFS, payload: [] }, + { type: types.SET_DIFF_DATA, payload: { data } }, + ], + [], + () => { + mock.restore(); + done(); + }, + ); + }); + }); + }); + describe('setHighlightedRow', () => { it('should mark currently selected diff and set lineHash and fileHash of highlightedRow', () => { testAction(setHighlightedRow, 'ABC_123', {}, [ diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 24405dcc796..cb89a89e216 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -55,8 +55,8 @@ describe('DiffsStoreMutations', () => { const state = { diffFiles: [ { - content_sha: diffFileMockData.content_sha, - file_hash: diffFileMockData.file_hash, + ...diffFileMockData, + parallel_diff_lines: [], }, ], }; diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index 638b4510221..051820cedfa 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -333,10 +333,10 @@ describe('DiffsStoreUtils', () => { diff_files: [Object.assign({}, mock, { highlighted_diff_lines: undefined })], }; - utils.prepareDiffData(preparedDiff); - utils.prepareDiffData(splitInlineDiff); - utils.prepareDiffData(splitParallelDiff); - utils.prepareDiffData(completedDiff, [mock]); + preparedDiff.diff_files = utils.prepareDiffData(preparedDiff); + splitInlineDiff.diff_files = utils.prepareDiffData(splitInlineDiff); + splitParallelDiff.diff_files = utils.prepareDiffData(splitParallelDiff); + completedDiff.diff_files = utils.prepareDiffData(completedDiff, [mock]); }); it('sets the renderIt and collapsed attribute on files', () => { @@ -390,6 +390,37 @@ describe('DiffsStoreUtils', () => { expect(completedDiff.diff_files[0].parallel_diff_lines.length).toBeGreaterThan(0); expect(completedDiff.diff_files[0].highlighted_diff_lines.length).toBeGreaterThan(0); }); + + it('leaves files in the existing state', () => { + const priorFiles = [mock]; + const fakeNewFile = { + ...mock, + content_sha: 'ABC', + file_hash: 'DEF', + }; + const updatedFilesList = utils.prepareDiffData({ diff_files: [fakeNewFile] }, priorFiles); + + expect(updatedFilesList).toEqual([mock, fakeNewFile]); + }); + + it('completes an existing split diff without overwriting existing diffs', () => { + // The current state has a file that has only loaded inline lines + const priorFiles = [{ ...mock, parallel_diff_lines: [] }]; + // The next (batch) load loads two files: the other half of that file, and a new file + const fakeBatch = [ + { ...mock, highlighted_diff_lines: undefined }, + { ...mock, highlighted_diff_lines: undefined, content_sha: 'ABC', file_hash: 'DEF' }, + ]; + const updatedFilesList = utils.prepareDiffData({ diff_files: fakeBatch }, priorFiles); + + expect(updatedFilesList).toEqual([ + mock, + jasmine.objectContaining({ + content_sha: 'ABC', + file_hash: 'DEF', + }), + ]); + }); }); describe('isDiscussionApplicableToLine', () => { diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repository_spec.rb index 32e718d4b3b..2ac1b0d2583 100644 --- a/spec/lib/backup/repository_spec.rb +++ b/spec/lib/backup/repository_spec.rb @@ -86,6 +86,22 @@ describe Backup::Repository do expect(pool_repository.object_pool.exists?).to be(true) end end + + it 'cleans existing repositories' do + wiki_repository_spy = spy(:wiki) + + allow_next_instance_of(ProjectWiki) do |project_wiki| + allow(project_wiki).to receive(:repository).and_return(wiki_repository_spy) + end + + expect_next_instance_of(Repository) do |repo| + expect(repo).to receive(:remove) + end + + subject.restore + + expect(wiki_repository_spy).to have_received(:remove) + end end describe '#empty_repo?' do diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb index dce0ee05b1f..792036273d6 100644 --- a/spec/services/projects/operations/update_service_spec.rb +++ b/spec/services/projects/operations/update_service_spec.rb @@ -289,5 +289,86 @@ describe Projects::Operations::UpdateService do end end end + + context 'prometheus integration' do + context 'prometheus params were passed into service' do + let(:prometheus_service) do + build_stubbed(:prometheus_service, project: project, properties: { + api_url: "http://example.prometheus.com", + manual_configuration: "0" + }) + end + let(:prometheus_params) do + { + "type" => "PrometheusService", + "title" => nil, + "active" => true, + "properties" => { "api_url" => "http://example.prometheus.com", "manual_configuration" => "0" }, + "instance" => false, + "push_events" => true, + "issues_events" => true, + "merge_requests_events" => true, + "tag_push_events" => true, + "note_events" => true, + "category" => "monitoring", + "default" => false, + "wiki_page_events" => true, + "pipeline_events" => true, + "confidential_issues_events" => true, + "commit_events" => true, + "job_events" => true, + "confidential_note_events" => true, + "deployment_events" => false, + "description" => nil, + "comment_on_event_enabled" => true + } + end + let(:params) do + { + prometheus_integration_attributes: { + api_url: 'http://new.prometheus.com', + manual_configuration: '1' + } + } + end + + it 'uses Project#find_or_initialize_service to include instance defined defaults and pass them to Projects::UpdateService', :aggregate_failures do + project_update_service = double(Projects::UpdateService) + prometheus_update_params = prometheus_params.merge('properties' => { + 'api_url' => 'http://new.prometheus.com', + 'manual_configuration' => '1' + }) + + expect(project) + .to receive(:find_or_initialize_service) + .with('prometheus') + .and_return(prometheus_service) + expect(Projects::UpdateService) + .to receive(:new) + .with(project, user, { prometheus_service_attributes: prometheus_update_params }) + .and_return(project_update_service) + expect(project_update_service).to receive(:execute) + + subject.execute + end + end + + context 'prometheus params were not passed into service' do + let(:params) { { something: :else } } + + it 'does not pass any prometheus params into Projects::UpdateService', :aggregate_failures do + project_update_service = double(Projects::UpdateService) + + expect(project).not_to receive(:find_or_initialize_service) + expect(Projects::UpdateService) + .to receive(:new) + .with(project, user, {}) + .and_return(project_update_service) + expect(project_update_service).to receive(:execute) + + subject.execute + end + end + end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 3092fb7116a..90fb6b932ee 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -497,6 +497,63 @@ describe Projects::UpdateService do update_project(project, user, { name: 'New name' }) end end + + context 'when updating nested attributes for prometheus service' do + context 'prometheus service exists' do + let(:prometheus_service_attributes) do + attributes_for(:prometheus_service, + project: project, + properties: { api_url: "http://new.prometheus.com", manual_configuration: "0" } + ) + end + + let!(:prometheus_service) do + create(:prometheus_service, + project: project, + properties: { api_url: "http://old.prometheus.com", manual_configuration: "0" } + ) + end + + it 'updates existing record' do + expect { update_project(project, user, prometheus_service_attributes: prometheus_service_attributes) } + .to change { prometheus_service.reload.api_url } + .from("http://old.prometheus.com") + .to("http://new.prometheus.com") + end + end + + context 'prometheus service does not exist' do + context 'valid parameters' do + let(:prometheus_service_attributes) do + attributes_for(:prometheus_service, + project: project, + properties: { api_url: "http://example.prometheus.com", manual_configuration: "0" } + ) + end + + it 'creates new record' do + expect { update_project(project, user, prometheus_service_attributes: prometheus_service_attributes) } + .to change { ::PrometheusService.where(project: project).count } + .from(0) + .to(1) + end + end + + context 'invalid parameters' do + let(:prometheus_service_attributes) do + attributes_for(:prometheus_service, + project: project, + properties: { api_url: nil, manual_configuration: "1" } + ) + end + + it 'does not create new record' do + expect { update_project(project, user, prometheus_service_attributes: prometheus_service_attributes) } + .not_to change { ::PrometheusService.where(project: project).count } + end + end + end + end end describe '#run_auto_devops_pipeline?' do diff --git a/yarn.lock b/yarn.lock index cffb5809a2b..ce6a0e94fc1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5413,9 +5413,9 @@ handle-thing@^2.0.0: integrity sha512-d4sze1JNC454Wdo2fkuyzCr6aHcbL6PGGuFAz0Li/NcOm1tCHGnWDRmJP85dh9IhQErTc2svWFEX5xHIOo//kQ== handlebars@^4.1.2: - version "4.1.2" - resolved "https://registry.yarnpkg.com/handlebars/-/handlebars-4.1.2.tgz#b6b37c1ced0306b221e094fc7aca3ec23b131b67" - integrity sha512-nvfrjqvt9xQ8Z/w0ijewdD/vvWDTOweBUm96NTr66Wfvo1mJenBLwcYmPs3TIBP5ruzYGD7Hx/DaM9RmhroGPw== + version "4.7.2" + resolved "https://registry.yarnpkg.com/handlebars/-/handlebars-4.7.2.tgz#01127b3840156a0927058779482031afe0e730d7" + integrity sha512-4PwqDL2laXtTWZghzzCtunQUTLbo31pcCJrd/B/9JP8XbhVzpS5ZXuKqlOzsd1rtcaLo4KqAn8nl8mkknS4MHw== dependencies: neo-async "^2.6.0" optimist "^0.6.1"