diff options
author | Z.J. van de Weg <zegerjan@gitlab.com> | 2016-10-04 16:03:13 +0200 |
---|---|---|
committer | Z.J. van de Weg <zegerjan@gitlab.com> | 2016-10-14 11:07:00 +0200 |
commit | 6a4f71008390752e6b5574a9e9bdf277732d852d (patch) | |
tree | cedc380844ee38ec20b89916a6038d3c4aeaa1c1 | |
parent | e4c74ffe3ed93815b131445796e63e2127eb8c3e (diff) | |
download | gitlab-ce-6a4f71008390752e6b5574a9e9bdf277732d852d.tar.gz |
Show what time ago a MR was deployed
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/assets/javascripts/merge_request_widget.js.es6 (renamed from app/assets/javascripts/merge_request_widget.js) | 55 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/merge_requests.scss | 4 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 31 | ||||
-rw-r--r-- | app/models/environment.rb | 8 | ||||
-rw-r--r-- | app/models/merge_request.rb | 9 | ||||
-rw-r--r-- | app/models/repository.rb | 8 | ||||
-rw-r--r-- | app/views/projects/merge_requests/widget/_heading.html.haml | 16 | ||||
-rw-r--r-- | app/views/projects/merge_requests/widget/_show.html.haml | 2 | ||||
-rw-r--r-- | spec/features/merge_requests/widget_deployments_spec.rb | 26 | ||||
-rw-r--r-- | spec/javascripts/merge_request_widget_spec.js | 27 | ||||
-rw-r--r-- | spec/models/environment_spec.rb | 17 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 23 | ||||
-rw-r--r-- | spec/views/projects/merge_requests/_heading.html.haml_spec.rb | 28 |
14 files changed, 202 insertions, 53 deletions
diff --git a/CHANGELOG b/CHANGELOG index 99bbd99726d..348ac41a534 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -50,6 +50,7 @@ v 8.13.0 (unreleased) - Add new issue button to each list on Issues Board - Added soft wrap button to repository file/blob editor - Update namespace validation to forbid reserved names (.git and .atom) (Will Starms) + - Show the time ago a merge request was deployed to an environment - Add word-wrap to issue title on issue and milestone boards (ClemMakesApps) - Fix todos page mobile viewport layout (ClemMakesApps) - Fix inconsistent highlighting of already selected activity nav-links (ClemMakesApps) diff --git a/app/assets/javascripts/merge_request_widget.js b/app/assets/javascripts/merge_request_widget.js.es6 index 7bbcdf59838..7c727cf214f 100644 --- a/app/assets/javascripts/merge_request_widget.js +++ b/app/assets/javascripts/merge_request_widget.js.es6 @@ -1,7 +1,26 @@ -(function() { + ((global) => { var indexOf = [].indexOf || function(item) { for (var i = 0, l = this.length; i < l; i++) { if (i in this && this[i] === item) return i; } return -1; }; - this.MergeRequestWidget = (function() { + const DEPLOYMENT_TEMPLATE = `<div class="mr-widget-heading" id="<%- id %>"> + <div class="ci_widget ci-success"> + <%= ci_success_icon %> + <span> + Deployed to + <a href="<%- url %>" target="_blank" class="environment"> + <%- name %> + </a> + <span class="js-environment-timeago" data-toggle="tooltip" data-placement="top" data-title="<%- deployed_at_formatted %>"> + <%- deployed_at %> + </span> + <a class="js-environment-link" href="<%- external_url %>" target="_blank"> + <i class="fa fa-external-link"></i> + View on <%- external_url_formatted %> + </a> + </span> + </div> + </div>`; + + global.MergeRequestWidget = (function() { function MergeRequestWidget(opts) { // Initialize MergeRequestWidget behavior // @@ -10,6 +29,7 @@ // ci_status_url - String, URL to use to check CI status // this.opts = opts; + this.$widgetBody = $('.mr-widget-body'); $('#modal_merge_info').modal({ show: false }); @@ -20,6 +40,7 @@ this.clearEventListeners(); this.addEventListeners(); this.getCIStatus(false); + this.retrieveSuccessIcon(); this.pollCIStatus(); notifyPermissions(); } @@ -48,6 +69,12 @@ })(this)); }; + MergeRequestWidget.prototype.retrieveSuccessIcon = function() { + const $ciSuccessIcon = $('.js-success-icon'); + this.$ciSuccessIcon = $ciSuccessIcon.html(); + $ciSuccessIcon.remove(); + } + MergeRequestWidget.prototype.mergeInProgress = function(deleteSourceBranch) { if (deleteSourceBranch == null) { deleteSourceBranch = false; @@ -62,7 +89,7 @@ urlSuffix = deleteSourceBranch ? '?deleted_source_branch=true' : ''; return window.location.href = window.location.pathname + urlSuffix; } else if (data.merge_error) { - return $('.mr-widget-body').html("<h4>" + data.merge_error + "</h4>"); + return this.$widgetBody.html("<h4>" + data.merge_error + "</h4>"); } else { callback = function() { return merge_request_widget.mergeInProgress(deleteSourceBranch); @@ -118,6 +145,7 @@ if (data.status === '') { return; } + if (data.environments && data.environments.length) _this.renderEnvironments(data.environments); if (_this.firstCICheck || data.status !== _this.opts.ci_status && (data.status != null)) { _this.opts.ci_status = data.status; _this.showCIStatus(data.status); @@ -150,6 +178,25 @@ })(this)); }; + MergeRequestWidget.prototype.renderEnvironments = function(environments) { + for (let i = 0; i < environments.length; i++) { + const environment = environments[i]; + if ($(`.mr-state-widget #${ environment.id }`).length) return; + const $template = $(DEPLOYMENT_TEMPLATE); + if (!environment.external_url) $('.js-environment-link', $template).remove(); + if (environment.deployed_at) { + environment.deployed_at = $.timeago(environment.deployed_at) + '.'; + } else { + $('.js-environment-timeago', $template).remove(); + environment.name += '.'; + } + environment.ci_success_icon = this.$ciSuccessIcon; + const templateString = _.unescape($template[0].outerHTML); + const template = _.template(templateString)(environment) + this.$widgetBody.before(template); + } + }; + MergeRequestWidget.prototype.showCIStatus = function(state) { var allowed_states; if (state == null) { @@ -190,4 +237,4 @@ })(); -}).call(this); + })(window.gl || (window.gl = {})); diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 7cf69c56d15..96d5547154d 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -121,6 +121,10 @@ color: #5c5d5e; } + .js-deployment-link { + display: inline-block; + } + .mr-widget-body { h4 { font-weight: 600; diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 869d96b86f4..28225fbb762 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -393,11 +393,40 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end + environments = @merge_request.environments + deployments = @merge_request.deployments + + if environments.present? + environments = environments.select { |e| can?(current_user, :read_environment, e) }.map do |environment| + project = environment.project + deployment = deployments.find { |d| d.environment == environment } + + environment = { + name: environment.name, + id: environment.id, + url: namespace_project_environment_path(project.namespace, project, environment), + external_url: environment.external_url, + deployed_at: deployment ? deployment.created_at : nil + } + + if environment[:external_url] + environment[:external_url_formatted] = environment[:external_url].gsub(/\A.*?:\/\//, '') + end + + if environment[:deployed_at] + environment[:deployed_at_formatted] = environment[:deployed_at].to_time.in_time_zone.to_s(:medium) + end + + environment + end + end + response = { title: merge_request.title, sha: merge_request.diff_head_commit.short_id, status: status, - coverage: coverage + coverage: coverage, + environments: environments } render json: response diff --git a/app/models/environment.rb b/app/models/environment.rb index f0f3ee23223..1c7d06906f3 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -48,6 +48,14 @@ class Environment < ActiveRecord::Base self.name == "production" end + def deployment_id_for(commit) + ref = project.repository.ref_name_for_sha(ref_path, commit.sha) + + return nil unless ref + + ref.split('/').last.to_i + end + def ref_path "refs/environments/#{Shellwords.shellescape(name)}" end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a743bf313ae..ec8c09f83db 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -685,6 +685,15 @@ class MergeRequest < ActiveRecord::Base !pipeline || pipeline.success? end + def deployments + deployment_ids = + environments.map do |environment| + environment.deployment_id_for(diff_head_commit) + end.compact + + Deployments.find(deployment_ids) + end + def environments return [] unless diff_head_commit diff --git a/app/models/repository.rb b/app/models/repository.rb index 608c99eed46..37833cf004f 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -719,6 +719,14 @@ class Repository end end + def ref_name_for_sha(environment_ref_path, sha) + args = %W(#{Gitlab.config.git.bin_path} for-each-ref --count=1 #{environment_ref_path} --contains #{sha}) + + # Not found -> ["", 0] + # Found -> ["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0] + Gitlab::Popen.popen(args, path_to_repo).first.split.last + end + def refs_contains_sha(ref_type, sha) args = %W(#{Gitlab.config.git.bin_path} #{ref_type} --contains #{sha}) names = Gitlab::Popen.popen(args, path_to_repo).first diff --git a/app/views/projects/merge_requests/widget/_heading.html.haml b/app/views/projects/merge_requests/widget/_heading.html.haml index 5b7f83c344f..cda8f0b7de6 100644 --- a/app/views/projects/merge_requests/widget/_heading.html.haml +++ b/app/views/projects/merge_requests/widget/_heading.html.haml @@ -44,17 +44,5 @@ = icon("times-circle") Could not connect to the CI server. Please check your settings and try again. -- @merge_request.environments.sort_by(&:name).each do |environment| - - if can?(current_user, :read_environment, environment) - .mr-widget-heading - .ci_widget.ci-success - = ci_icon_for_status("success") - %span - Deployed to - = succeed '.' do - = link_to environment.name, environment_path(environment), class: 'environment' - - external_url = environment.external_url - - if external_url - = link_to external_url, target: '_blank' do - %span.hidden-xs View on #{external_url.gsub(/\A.*?:\/\//, '')} - = icon('external-link', right: true) + .js-success-icon.hidden + = ci_icon_for_status('success') diff --git a/app/views/projects/merge_requests/widget/_show.html.haml b/app/views/projects/merge_requests/widget/_show.html.haml index ea618263a4a..856ec1e0bee 100644 --- a/app/views/projects/merge_requests/widget/_show.html.haml +++ b/app/views/projects/merge_requests/widget/_show.html.haml @@ -33,4 +33,4 @@ merge_request_widget.clearEventListeners(); } - merge_request_widget = new MergeRequestWidget(opts); + merge_request_widget = new window.gl.MergeRequestWidget(opts); diff --git a/spec/features/merge_requests/widget_deployments_spec.rb b/spec/features/merge_requests/widget_deployments_spec.rb new file mode 100644 index 00000000000..8e23ec50d4a --- /dev/null +++ b/spec/features/merge_requests/widget_deployments_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +feature 'Widget Deployments Header', feature: true, js: true do + include WaitForAjax + + describe 'when deployed to an environment' do + let(:project) { merge_request.target_project } + let(:merge_request) { create(:merge_request, :merged) } + let(:environment) { create(:environment, project: project) } + let!(:deployment) do + create(:deployment, environment: environment, sha: project.commit('master').id) + end + + before do + login_as :admin + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'displays that the environment is deployed' do + wait_for_ajax + + expect(page).to have_content("Deployed to #{environment.name}") + expect(find('.ci_widget > span > span')['data-title']).to eq(deployment.created_at.to_time.in_time_zone.to_s(:medium)) + end + end +end diff --git a/spec/javascripts/merge_request_widget_spec.js b/spec/javascripts/merge_request_widget_spec.js index 17b32914ec3..75ef10939de 100644 --- a/spec/javascripts/merge_request_widget_spec.js +++ b/spec/javascripts/merge_request_widget_spec.js @@ -1,5 +1,5 @@ - /*= require merge_request_widget */ +/*= require lib/utils/jquery.timeago.js */ (function() { describe('MergeRequestWidget', function() { @@ -20,7 +20,7 @@ gitlab_icon: "gitlab_logo.png", builds_path: "http://sampledomain.local/sampleBuildsPath" }; - this["class"] = new MergeRequestWidget(this.opts); + this["class"] = new window.gl.MergeRequestWidget(this.opts); return this.ciStatusData = { "title": "Sample MR title", "sha": "12a34bc5", @@ -30,7 +30,7 @@ }); return describe('getCIStatus', function() { beforeEach(function() { - return spyOn(jQuery, 'getJSON').and.callFake((function(_this) { + spyOn(jQuery, 'getJSON').and.callFake((function(_this) { return function(req, cb) { return cb(_this.ciStatusData); }; @@ -61,13 +61,30 @@ this["class"].getCIStatus(false); return expect(spy).not.toHaveBeenCalled(); }); - return it('should not display a notification on the first check after the widget has been created', function() { + it('should not display a notification on the first check after the widget has been created', function() { var spy; spy = spyOn(window, 'notify'); - this["class"] = new MergeRequestWidget(this.opts); + this["class"] = new window.gl.MergeRequestWidget(this.opts); this["class"].getCIStatus(true); return expect(spy).not.toHaveBeenCalled(); }); + it('should call renderEnvironments when the environments property is set', function() { + this.ciStatusData.environments = [{ + created_at: '2016-09-12T13:38:30.636Z', + environment_id: 1, + environment_name: 'env1', + external_url: 'https://test-url.com', + external_url_formatted: 'test-url.com' + }]; + var spy = spyOn(this['class'], 'renderEnvironments').and.stub(); + this['class'].getCIStatus(false); + expect(spy).toHaveBeenCalledWith(this.ciStatusData.environments); + }); + it('should not call renderEnvironments when the environments property is not set', function() { + var spy = spyOn(this['class'], 'renderEnvironments').and.stub(); + this['class'].getCIStatus(false); + expect(spy).not.toHaveBeenCalled(); + }); }); }); diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 6b1867a44e1..fb9629ac47a 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -64,6 +64,23 @@ describe Environment, models: true do end end + describe '#deployment_id_for' do + let(:project) { create(:project) } + let!(:environment) { create(:environment, project: project) } + let!(:deployment) { create(:deployment, environment: environment, ref: commit.parent.id) } + let!(:deployment1) { create(:deployment, environment: environment, ref: commit.id) } + let(:head_commit) { project.commit } + let(:commit) { project.commit.parent } + + it 'returns deployment id for the environment' do + expect(environment.deployment_id_for(commit)).to eq deployment1.id + end + + it 'return nil when no deployment is found' do + expect(environment.deployment_id_for(head_commit)).to eq nil + end + end + describe '#environment_type' do subject { environment.environment_type } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 4b80efbe12b..f977cf73673 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -7,15 +7,18 @@ describe Repository, models: true do let(:project) { create(:project) } let(:repository) { project.repository } let(:user) { create(:user) } + let(:commit_options) do author = repository.user_to_committer(user) { message: 'Test message', committer: author, author: author } end + let(:merge_commit) do merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project) merge_commit_id = repository.merge(user, merge_request, commit_options) repository.commit(merge_commit_id) end + let(:author_email) { FFaker::Internet.email } # I have to remove periods from the end of the name @@ -90,6 +93,26 @@ describe Repository, models: true do end end + describe '#ref_name_for_sha' do + context 'ref found' do + it 'returns the ref' do + allow_any_instance_of(Gitlab::Popen).to receive(:popen). + and_return(["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0]) + + expect(repository.ref_name_for_sha('bla', '0' * 40)).to eq 'refs/environments/production/77' + end + end + + context 'ref not found' do + it 'returns nil' do + allow_any_instance_of(Gitlab::Popen).to receive(:popen). + and_return(["", 0]) + + expect(repository.ref_name_for_sha('bla', '0' * 40)).to eq nil + end + end + end + describe '#last_commit_for_path' do subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id } diff --git a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb b/spec/views/projects/merge_requests/_heading.html.haml_spec.rb deleted file mode 100644 index 86980f59cd8..00000000000 --- a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -require 'spec_helper' - -describe 'projects/merge_requests/widget/_heading' do - include Devise::Test::ControllerHelpers - - context 'when released to an environment' do - let(:project) { merge_request.target_project } - let(:merge_request) { create(:merge_request, :merged) } - let(:environment) { create(:environment, project: project) } - let!(:deployment) do - create(:deployment, environment: environment, sha: project.commit('master').id) - end - - before do - assign(:merge_request, merge_request) - assign(:project, project) - - allow(view).to receive(:can?).and_return(true) - - render - end - - it 'displays that the environment is deployed' do - expect(rendered).to match("Deployed to") - expect(rendered).to match("#{environment.name}") - end - end -end |