Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-02-16

app/assets/javascripts/ci_variable_list/ajax_variable_list.js

@@ -75,6 +75,7 @@ export default class AjaxVariableList {
 
         if (res.status === statusCodes.OK && res.data) {
           this.updateRowsWithPersistedVariables(res.data.variables);
+          this.variableList.hideValues();
         } else if (res.status === statusCodes.BAD_REQUEST) {
           // Validation failed
           this.errorBox.innerHTML = generateErrorBoxContent(res.data);

app/assets/javascripts/ci_variable_list/ci_variable_list.js

@@ -178,6 +178,10 @@ export default class VariableList {
     this.$container.find('.js-row-remove-button').attr('disabled', !isEnabled);
   }
 
+  hideValues() {
+    this.secretValues.updateDom(false);
+  }
+
   getAllData() {
     // Ignore the last empty row because we don't want to try persist
     // a blank variable and run into validation problems.

app/assets/javascripts/dispatcher.js

@@ -475,11 +475,6 @@ var Dispatcher;
             .then(callDefault)
             .catch(fail);
           break;
-        case 'users:show':
-          import('./pages/users/show')
-            .then(callDefault)
-            .catch(fail);
-          break;
         case 'admin:conversational_development_index:show':
           import('./pages/admin/conversational_development_index/show')
             .then(callDefault)

app/assets/javascripts/importer_status.js

@@ -59,29 +59,36 @@ class ImporterStatus {
     .catch(() => flash(__('An error occurred while importing project')));
   }
 
-  setAutoUpdate() {
-    return setInterval(() => $.get(this.jobsUrl, data => $.each(data, (i, job) => {
-      const jobItem = $(`#project_${job.id}`);
-      const statusField = jobItem.find('.job-status');
+  autoUpdate() {
+    return axios.get(this.jobsUrl)
+      .then(({ data = [] }) => {
+        data.forEach((job) => {
+          const jobItem = $(`#project_${job.id}`);
+          const statusField = jobItem.find('.job-status');
+
+          const spinner = '<i class="fa fa-spinner fa-spin"></i>';
 
-      const spinner = '<i class="fa fa-spinner fa-spin"></i>';
+          switch (job.import_status) {
+            case 'finished':
+              jobItem.removeClass('active').addClass('success');
+              statusField.html('<span><i class="fa fa-check"></i> done</span>');
+              break;
+            case 'scheduled':
+              statusField.html(`${spinner} scheduled`);
+              break;
+            case 'started':
+              statusField.html(`${spinner} started`);
+              break;
+            default:
+              statusField.html(job.import_status);
+              break;
+          }
+        });
+      });
+  }
 
-      switch (job.import_status) {
-        case 'finished':
-          jobItem.removeClass('active').addClass('success');
-          statusField.html('<span><i class="fa fa-check"></i> done</span>');
-          break;
-        case 'scheduled':
-          statusField.html(`${spinner} scheduled`);
-          break;
-        case 'started':
-          statusField.html(`${spinner} started`);
-          break;
-        default:
-          statusField.html(job.import_status);
-          break;
-      }
-    })), 4000);
+  setAutoUpdate() {
+    setInterval(this.autoUpdate.bind(this), 4000);
   }
 }
 

app/assets/javascripts/users/index.js → app/assets/javascripts/pages/users/index.js

+import UserCallout from '~/user_callout';
 import Cookies from 'js-cookie';
 import UserTabs from './user_tabs';
 
@@ -22,4 +23,5 @@ document.addEventListener('DOMContentLoaded', () => {
   const page = $('body').attr('data-page');
   const action = page.split(':')[1];
   initUserProfile(action);
+  new UserCallout(); // eslint-disable-line no-new
 });

app/assets/javascripts/pages/users/show/index.js

-import UserCallout from '~/user_callout';
-
-export default () => new UserCallout();

app/assets/javascripts/users/user_tabs.js → app/assets/javascripts/pages/users/user_tabs.js

-import axios from '../lib/utils/axios_utils';
-import Activities from '../activities';
+import axios from '~/lib/utils/axios_utils';
+import Activities from '~/activities';
+import { localTimeAgo } from '~/lib/utils/datetime_utility';
+import { __ } from '~/locale';
+import flash from '~/flash';
 import ActivityCalendar from './activity_calendar';
-import { localTimeAgo } from '../lib/utils/datetime_utility';
-import { __ } from '../locale';
-import flash from '../flash';
 
 /**
  * UserTabs

app/services/issues/move_service.rb

@@ -19,19 +19,10 @@ module Issues
       # on rewriting notes (unfolding references)
       #
       ActiveRecord::Base.transaction do
-        # New issue tasks
-        #
         @new_issue = create_new_issue
 
-        rewrite_notes
-        rewrite_issue_award_emoji
-        add_note_moved_from
-
-        # Old issue tasks
-        #
-        add_note_moved_to
-        close_issue
-        mark_as_moved
+        update_new_issue
+        update_old_issue
       end
 
       notify_participants
@@ -41,6 +32,18 @@ module Issues
 
     private
 
+    def update_new_issue
+      rewrite_notes
+      rewrite_issue_award_emoji
+      add_note_moved_from
+    end
+
+    def update_old_issue
+      add_note_moved_to
+      close_issue
+      mark_as_moved
+    end
+
     def create_new_issue
       new_params = { id: nil, iid: nil, label_ids: cloneable_label_ids,
                      milestone_id: cloneable_milestone_id,

app/views/users/show.html.haml

@@ -4,9 +4,6 @@
 - page_description @user.bio
 - header_title     @user.name, user_path(@user)
 - @no_container = true
-- content_for :page_specific_javascripts do
-  = webpack_bundle_tag 'common_d3'
-  = webpack_bundle_tag 'users'
 
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, user_url(@user, format: :atom), title: "#{@user.name} activity")

changelogs/unreleased/42929-hide-new-variable-values.yml

+---
+title: Hide CI secret variable values after saving
+merge_request: 17044
+author:
+type: changed

changelogs/unreleased/dm-escape-commit-message.yml

+---
+title: Escape HTML entities in commit messages
+merge_request:
+author:
+type: fixed

config/webpack.config.js

@@ -114,7 +114,6 @@ var config = {
     vue_merge_request_widget: './vue_merge_request_widget/index.js',
     test:                 './test.js',
     two_factor_auth:      './two_factor_auth.js',
-    users:                './users/index.js',
     webpack_runtime:      './webpack.js',
   },
 

doc/development/profiling.md

@@ -27,6 +27,17 @@ Gitlab::Profiler.profile('/my-user')
 # Returns a RubyProf::Profile where 100 seconds is spent in UsersController#show
 ```
 
+For routes that require authorization you will need to provide a user to
+`Gitlab::Profiler`. You can do this like so:
+
+```ruby
+Gitlab::Profiler.profile('/gitlab-org/gitlab-test', user: User.first)
+```
+
+The user you provide will need to have a [personal access
+token](https://docs.gitlab.com/ce/user/profile/personal_access_tokens.html) in
+the GitLab instance.
+
 Passing a `logger:` keyword argument to `Gitlab::Profiler.profile` will send
 ActiveRecord and ActionController log output to that logger. Further options are
 documented with the method source.

lib/banzai/filter/html_entity_filter.rb

@@ -5,7 +5,7 @@ module Banzai
     # Text filter that escapes these HTML entities: & " < >
     class HtmlEntityFilter < HTML::Pipeline::TextFilter
       def call
-        ERB::Util.html_escape_once(text)
+        ERB::Util.html_escape(text)
       end
     end
   end

lib/gitlab/profiler.rb

@@ -45,6 +45,7 @@ module Gitlab
 
       if user
         private_token ||= user.personal_access_tokens.active.pluck(:token).first
+        raise 'Your user must have a personal_access_token' unless private_token
       end
 
       headers['Private-Token'] = private_token if private_token

spec/helpers/events_helper_spec.rb

@@ -20,5 +20,9 @@ describe EventsHelper do
     it 'handles nil values' do
       expect(helper.event_commit_title(nil)).to eq('')
     end
+
+    it 'does not escape HTML entities' do
+      expect(helper.event_commit_title("foo & bar")).to eq("foo & bar")
+    end
   end
 end

spec/javascripts/ci_variable_list/ajax_variable_list_spec.js

+import $ from 'jquery';
 import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
 import AjaxFormVariableList from '~/ci_variable_list/ajax_variable_list';
 
 const VARIABLE_PATCH_ENDPOINT = 'http://test.host/frontend-fixtures/builds-project/variables';
+const HIDE_CLASS = 'hide';
 
 describe('AjaxFormVariableList', () => {
   preloadFixtures('projects/ci_cd_settings.html.raw');
@@ -45,16 +47,16 @@ describe('AjaxFormVariableList', () => {
       const loadingIcon = saveButton.querySelector('.js-secret-variables-save-loading-icon');
 
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(() => {
-        expect(loadingIcon.classList.contains('hide')).toEqual(false);
+        expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(false);
 
         return [200, {}];
       });
 
-      expect(loadingIcon.classList.contains('hide')).toEqual(true);
+      expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true);
 
       ajaxVariableList.onSaveClicked()
         .then(() => {
-          expect(loadingIcon.classList.contains('hide')).toEqual(true);
+          expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true);
         })
         .then(done)
         .catch(done.fail);
@@ -78,11 +80,11 @@ describe('AjaxFormVariableList', () => {
     it('hides any previous error box', (done) => {
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200);
 
-      expect(errorBox.classList.contains('hide')).toEqual(true);
+      expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
 
       ajaxVariableList.onSaveClicked()
         .then(() => {
-          expect(errorBox.classList.contains('hide')).toEqual(true);
+          expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
         })
         .then(done)
         .catch(done.fail);
@@ -103,17 +105,39 @@ describe('AjaxFormVariableList', () => {
         .catch(done.fail);
     });
 
+    it('hides secret values', (done) => {
+      mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200, {});
+
+      const row = container.querySelector('.js-row:first-child');
+      const valueInput = row.querySelector('.js-ci-variable-input-value');
+      const valuePlaceholder = row.querySelector('.js-secret-value-placeholder');
+
+      valueInput.value = 'bar';
+      $(valueInput).trigger('input');
+
+      expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(true);
+      expect(valueInput.classList.contains(HIDE_CLASS)).toBe(false);
+
+      ajaxVariableList.onSaveClicked()
+        .then(() => {
+          expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(false);
+          expect(valueInput.classList.contains(HIDE_CLASS)).toBe(true);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
     it('shows error box with validation errors', (done) => {
       const validationError = 'some validation error';
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(400, [
         validationError,
       ]);
 
-      expect(errorBox.classList.contains('hide')).toEqual(true);
+      expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
 
       ajaxVariableList.onSaveClicked()
         .then(() => {
-          expect(errorBox.classList.contains('hide')).toEqual(false);
+          expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(false);
           expect(errorBox.textContent.trim().replace(/\n+\s+/m, ' ')).toEqual(`Validation failed ${validationError}`);
         })
         .then(done)
@@ -123,11 +147,11 @@ describe('AjaxFormVariableList', () => {
     it('shows flash message when request fails', (done) => {
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(500);
 
-      expect(errorBox.classList.contains('hide')).toEqual(true);
+      expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
 
       ajaxVariableList.onSaveClicked()
         .then(() => {
-          expect(errorBox.classList.contains('hide')).toEqual(true);
+          expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
         })
         .then(done)
         .catch(done.fail);
@@ -170,9 +194,9 @@ describe('AjaxFormVariableList', () => {
       const valueInput = row.querySelector('.js-ci-variable-input-value');
 
       keyInput.value = 'foo';
-      keyInput.dispatchEvent(new Event('input'));
+      $(keyInput).trigger('input');
       valueInput.value = 'bar';
-      valueInput.dispatchEvent(new Event('input'));
+      $(valueInput).trigger('input');
 
       expect(idInput.value).toEqual('');
 

spec/javascripts/ci_variable_list/ci_variable_list_spec.js

 import VariableList from '~/ci_variable_list/ci_variable_list';
 import getSetTimeoutPromise from '../helpers/set_timeout_promise_helper';
 
+const HIDE_CLASS = 'hide';
+
 describe('VariableList', () => {
   preloadFixtures('pipeline_schedules/edit.html.raw');
   preloadFixtures('pipeline_schedules/edit_with_variables.html.raw');
@@ -92,14 +94,14 @@ describe('VariableList', () => {
         const $inputValue = $row.find('.js-ci-variable-input-value');
         const $placeholder = $row.find('.js-secret-value-placeholder');
 
-        expect($placeholder.hasClass('hide')).toBe(false);
-        expect($inputValue.hasClass('hide')).toBe(true);
+        expect($placeholder.hasClass(HIDE_CLASS)).toBe(false);
+        expect($inputValue.hasClass(HIDE_CLASS)).toBe(true);
 
         // Reveal values
         $wrapper.find('.js-secret-value-reveal-button').click();
 
-        expect($placeholder.hasClass('hide')).toBe(true);
-        expect($inputValue.hasClass('hide')).toBe(false);
+        expect($placeholder.hasClass(HIDE_CLASS)).toBe(true);
+        expect($inputValue.hasClass(HIDE_CLASS)).toBe(false);
       });
     });
   });
@@ -179,4 +181,35 @@ describe('VariableList', () => {
       expect($wrapper.find('.js-ci-variable-input-key:not([disabled])').length).toBe(3);
     });
   });
+
+  describe('hideValues', () => {
+    beforeEach(() => {
+      loadFixtures('projects/ci_cd_settings.html.raw');
+      $wrapper = $('.js-ci-variable-list-section');
+
+      variableList = new VariableList({
+        container: $wrapper,
+        formField: 'variables',
+      });
+      variableList.init();
+    });
+
+    it('should hide value input and show placeholder stars', () => {
+      const $row = $wrapper.find('.js-row');
+      const $inputValue = $row.find('.js-ci-variable-input-value');
+      const $placeholder = $row.find('.js-secret-value-placeholder');
+
+      $row.find('.js-ci-variable-input-value')
+        .val('foo')
+        .trigger('input');
+
+      expect($placeholder.hasClass(HIDE_CLASS)).toBe(true);
+      expect($inputValue.hasClass(HIDE_CLASS)).toBe(false);
+
+      variableList.hideValues();
+
+      expect($placeholder.hasClass(HIDE_CLASS)).toBe(false);
+      expect($inputValue.hasClass(HIDE_CLASS)).toBe(true);
+    });
+  });
 });

spec/javascripts/importer_status_spec.js

@@ -3,9 +3,18 @@ import axios from '~/lib/utils/axios_utils';
 import MockAdapter from 'axios-mock-adapter';
 
 describe('Importer Status', () => {
+  let instance;
+  let mock;
+
+  beforeEach(() => {
+    mock = new MockAdapter(axios);
+  });
+
+  afterEach(() => {
+    mock.restore();
+  });
+
   describe('addToImport', () => {
-    let instance;
-    let mock;
     const importUrl = '/import_url';
 
     beforeEach(() => {
@@ -21,11 +30,6 @@ describe('Importer Status', () => {
       spyOn(ImporterStatus.prototype, 'initStatusPage').and.callFake(() => {});
       spyOn(ImporterStatus.prototype, 'setAutoUpdate').and.callFake(() => {});
       instance = new ImporterStatus('', importUrl);
-      mock = new MockAdapter(axios);
-    });
-
-    afterEach(() => {
-      mock.restore();
     });
 
     it('sets table row to active after post request', (done) => {
@@ -44,4 +48,60 @@ describe('Importer Status', () => {
       .catch(done.fail);
     });
   });
+
+  describe('autoUpdate', () => {
+    const jobsUrl = '/jobs_url';
+
+    beforeEach(() => {
+      const div = document.createElement('div');
+      div.innerHTML = `
+        <div id="project_1">
+          <div class="job-status">
+          </div>
+        </div>
+      `;
+
+      document.body.appendChild(div);
+
+      spyOn(ImporterStatus.prototype, 'initStatusPage').and.callFake(() => {});
+      spyOn(ImporterStatus.prototype, 'setAutoUpdate').and.callFake(() => {});
+      instance = new ImporterStatus(jobsUrl);
+    });
+
+    function setupMock(importStatus) {
+      mock.onGet(jobsUrl).reply(200, [{
+        id: 1,
+        import_status: importStatus,
+      }]);
+    }
+
+    function expectJobStatus(done, status) {
+      instance.autoUpdate()
+        .then(() => {
+          expect(document.querySelector('#project_1').innerText.trim()).toEqual(status);
+          done();
+        })
+        .catch(done.fail);
+    }
+
+    it('sets the job status to done', (done) => {
+      setupMock('finished');
+      expectJobStatus(done, 'done');
+    });
+
+    it('sets the job status to scheduled', (done) => {
+      setupMock('scheduled');
+      expectJobStatus(done, 'scheduled');
+    });
+
+    it('sets the job status to started', (done) => {
+      setupMock('started');
+      expectJobStatus(done, 'started');
+    });
+
+    it('sets the job status to custom status', (done) => {
+      setupMock('custom status');
+      expectJobStatus(done, 'custom status');
+    });
+  });
 });

spec/lib/banzai/filter/html_entity_filter_spec.rb

@@ -3,17 +3,12 @@ require 'spec_helper'
 describe Banzai::Filter::HtmlEntityFilter do
   include FilterSpecHelper
 
-  let(:unescaped) { 'foo <strike attr="foo">&&&</strike>' }
-  let(:escaped) { 'foo &lt;strike attr=&quot;foo&quot;&gt;&amp;&amp;&amp;&lt;/strike&gt;' }
+  let(:unescaped) { 'foo <strike attr="foo">&&amp;&</strike>' }
+  let(:escaped) { 'foo &lt;strike attr=&quot;foo&quot;&gt;&amp;&amp;amp;&amp;&lt;/strike&gt;' }
 
   it 'converts common entities to their HTML-escaped equivalents' do
     output = filter(unescaped)
 
     expect(output).to eq(escaped)
   end
-
-  it 'does not double-escape' do
-    escaped = ERB::Util.html_escape("Merge branch 'blabla' into 'master'")
-    expect(filter(escaped)).to eq(escaped)
-  end
 end

spec/lib/gitlab/profiler_spec.rb

@@ -53,6 +53,15 @@ describe Gitlab::Profiler do
       described_class.profile('/', user: user)
     end
 
+    context 'when providing a user without a personal access token' do
+      it 'raises an error' do
+        user = double(:user)
+        allow(user).to receive_message_chain(:personal_access_tokens, :active, :pluck).and_return([])
+
+        expect { described_class.profile('/', user: user) }.to raise_error('Your user must have a personal_access_token')
+      end
+    end
+
     it 'uses the private_token for auth if both it and user are set' do
       user = double(:user)
       user_token = 'user'