From 1ea2a85cbe61222765dc3ef50e8c0e4c1b2f5bfa Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 25 Sep 2017 23:38:34 +0200 Subject: [PATCH 01/89] Make Prometheus metrics endpoint return empty response when metrics are disabled --- app/controllers/metrics_controller.rb | 16 +++++++++------- spec/controllers/metrics_controller_spec.rb | 3 ++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/app/controllers/metrics_controller.rb b/app/controllers/metrics_controller.rb index 37587a52eaf..d81ad135198 100644 --- a/app/controllers/metrics_controller.rb +++ b/app/controllers/metrics_controller.rb @@ -3,10 +3,16 @@ class MetricsController < ActionController::Base protect_from_forgery with: :exception - before_action :validate_prometheus_metrics - def index - render text: metrics_service.metrics_text, content_type: 'text/plain; version=0.0.4' + response = if Gitlab::Metrics.prometheus_metrics_enabled? + metrics_service.metrics_text + else + help_page = help_page_url('administration/monitoring/prometheus/gitlab_metrics', + anchor: 'gitlab-prometheus-metrics' + ) + "# Metrics are disabled, see: #{help_page}\n" + end + render text: response, content_type: 'text/plain; version=0.0.4' end private @@ -14,8 +20,4 @@ class MetricsController < ActionController::Base def metrics_service @metrics_service ||= MetricsService.new end - - def validate_prometheus_metrics - render_404 unless Gitlab::Metrics.prometheus_metrics_enabled? - end end diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index 7b0976e3e67..d6f68b73428 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -78,7 +78,8 @@ describe MetricsController do it 'returns proper response' do get :index - expect(response.status).to eq(404) + expect(response.status).to eq(200) + expect(response.body).to eq("# Metrics are disabled, see: http://test.host/help/administration/monitoring/prometheus/gitlab_metrics#gitlab-prometheus-metrics\n") end end end From af13ffbcedd8952433d4d3f6693aac3450c5fa6f Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Wed, 1 Nov 2017 01:16:29 +0000 Subject: [PATCH 02/89] Dont emit toggle-markdown event if current target is already active and add specs --- .../vue_shared/components/markdown/header.vue | 16 ++++++--- .../components/markdown/field_spec.js | 36 +++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/vue_shared/components/markdown/header.vue b/app/assets/javascripts/vue_shared/components/markdown/header.vue index 5bf2a90cc3b..ef1c2dcf638 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/header.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/header.vue @@ -16,12 +16,18 @@ toolbarButton, }, methods: { + isMarkdownForm(form) { + return form && !form.find('.js-vue-markdown-field').length; + }, + + isActiveTarget(target) { + return target.closest('li').classList.contains('active'); + }, + toggleMarkdownPreview(e, form) { - if (form && !form.find('.js-vue-markdown-field').length) { - return; - } else if (e.target.blur) { - e.target.blur(); - } + if (e.target.blur) e.target.blur(); + + if (this.isMarkdownForm(form) || this.isActiveTarget(e.target)) return; this.$emit('toggle-markdown'); }, diff --git a/spec/javascripts/vue_shared/components/markdown/field_spec.js b/spec/javascripts/vue_shared/components/markdown/field_spec.js index 60a5c2ae74e..734266d0ebb 100644 --- a/spec/javascripts/vue_shared/components/markdown/field_spec.js +++ b/spec/javascripts/vue_shared/components/markdown/field_spec.js @@ -39,6 +39,7 @@ describe('Markdown field component', () => { describe('markdown preview', () => { let previewLink; + let writeLink; beforeEach(() => { spyOn(Vue.http, 'post').and.callFake(() => new Promise((resolve) => { @@ -52,6 +53,7 @@ describe('Markdown field component', () => { })); previewLink = vm.$el.querySelector('.nav-links li:nth-child(2) a'); + writeLink = vm.$el.querySelector('.nav-links li:nth-child(1) a'); }); it('sets preview link as active', (done) => { @@ -103,6 +105,40 @@ describe('Markdown field component', () => { done(); }, 0); }); + + function assertLinks(isWrite) { + expect(writeLink.parentNode.classList.contains('active')).toEqual(isWrite); + expect(previewLink.parentNode.classList.contains('active')).toEqual(!isWrite); + expect(vm.$el.querySelector('.md-preview').style.display).toEqual(isWrite ? 'none' : ''); + } + + it('clicking already active write or preview link does nothing', (done) => { + writeLink.click(); + + setTimeout(() => { + assertLinks(true); + + writeLink.click(); + + setTimeout(() => { + assertLinks(true); + + previewLink.click(); + + setTimeout(() => { + assertLinks(false); + + previewLink.click(); + + setTimeout(() => { + assertLinks(false); + + done(); + }); + }); + }); + }); + }); }); describe('markdown buttons', () => { From ce22977fb6a70e75ada07b9a670cfc079bb4fd26 Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Wed, 1 Nov 2017 12:46:05 +0000 Subject: [PATCH 03/89] Use settimeoutpromise in field_spec --- .../components/markdown/field_spec.js | 37 +++++++------------ 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/spec/javascripts/vue_shared/components/markdown/field_spec.js b/spec/javascripts/vue_shared/components/markdown/field_spec.js index 734266d0ebb..40cfafdf792 100644 --- a/spec/javascripts/vue_shared/components/markdown/field_spec.js +++ b/spec/javascripts/vue_shared/components/markdown/field_spec.js @@ -1,5 +1,6 @@ import Vue from 'vue'; import fieldComponent from '~/vue_shared/components/markdown/field.vue'; +import setTimeoutPromise from '../../../helpers/set_timeout_promise_helper'; describe('Markdown field component', () => { let vm; @@ -115,29 +116,19 @@ describe('Markdown field component', () => { it('clicking already active write or preview link does nothing', (done) => { writeLink.click(); - setTimeout(() => { - assertLinks(true); - - writeLink.click(); - - setTimeout(() => { - assertLinks(true); - - previewLink.click(); - - setTimeout(() => { - assertLinks(false); - - previewLink.click(); - - setTimeout(() => { - assertLinks(false); - - done(); - }); - }); - }); - }); + setTimeoutPromise() + .then(() => assertLinks(true)) + .then(() => writeLink.click()) + .then(() => setTimeoutPromise()) + .then(() => assertLinks(true)) + .then(() => previewLink.click()) + .then(() => setTimeoutPromise()) + .then(() => assertLinks(false)) + .then(() => previewLink.click()) + .then(() => setTimeoutPromise()) + .then(() => assertLinks(false)) + .then(done) + .catch(done.fail); }); }); From 3fc05267cac2d4a2e83a482d80afab958b1d2474 Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Wed, 1 Nov 2017 13:17:06 +0000 Subject: [PATCH 04/89] Let field.vue handle the changing of preview --- .../vue_shared/components/markdown/field.vue | 6 ++++-- .../vue_shared/components/markdown/header.vue | 16 +++++++--------- .../vue_shared/components/markdown/field_spec.js | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/vue_shared/components/markdown/field.vue b/app/assets/javascripts/vue_shared/components/markdown/field.vue index 8c0d9b9cda8..62c4e014e73 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/field.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/field.vue @@ -45,8 +45,10 @@ }, }, methods: { - toggleMarkdownPreview() { - this.previewMarkdown = !this.previewMarkdown; + toggleMarkdownPreview(isPreview) { + if (isPreview === this.previewMarkdown) return; + + this.previewMarkdown = isPreview; /* Can't use `$refs` as the component is technically in the parent component diff --git a/app/assets/javascripts/vue_shared/components/markdown/header.vue b/app/assets/javascripts/vue_shared/components/markdown/header.vue index ef1c2dcf638..d4a3f532a09 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/header.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/header.vue @@ -20,16 +20,12 @@ return form && !form.find('.js-vue-markdown-field').length; }, - isActiveTarget(target) { - return target.closest('li').classList.contains('active'); - }, - - toggleMarkdownPreview(e, form) { + toggleMarkdownPreview(e, isPreview, form) { if (e.target.blur) e.target.blur(); - if (this.isMarkdownForm(form) || this.isActiveTarget(e.target)) return; + if (this.isMarkdownForm(form)) return; - this.$emit('toggle-markdown'); + this.$emit('toggle-markdown', isPreview); }, }, mounted() { @@ -48,17 +44,19 @@