From d0630ed38d9b7afa9f7099016507756ea5e77edf Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 11 Sep 2017 14:13:16 -0400 Subject: [PATCH 1/4] notes.js: also return the original content when getting form data --- app/assets/javascripts/notes.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 9c008da1a5d..0e70999326a 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1280,10 +1280,12 @@ export default class Notes { * Get data from Form attributes to use for saving/submitting comment. */ getFormData($form) { + const content = $form.find('.js-note-text').val(); return { formData: $form.serialize(), - formContent: _.escape($form.find('.js-note-text').val()), + formContent: _.escape(content), formAction: $form.attr('action'), + formContentOriginal: content, }; } From 1d30e5e96434a0b9506baeda29680f347be11556 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 11 Sep 2017 14:13:54 -0400 Subject: [PATCH 2/4] notes.js: use the original content when resetting the form Otherwise, when an error occurred, the content was escaped and re-escaped on every error. Fixes #37724 --- app/assets/javascripts/notes.js | 4 ++-- spec/javascripts/notes_spec.js | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 0e70999326a..5a6868be444 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1417,7 +1417,7 @@ export default class Notes { const isMainForm = $form.hasClass('js-main-target-form'); const isDiscussionForm = $form.hasClass('js-discussion-note-form'); const isDiscussionResolve = $submitBtn.hasClass('js-comment-resolve-button'); - const { formData, formContent, formAction } = this.getFormData($form); + const { formData, formContent, formAction, formContentOriginal } = this.getFormData($form); let noteUniqueId; let systemNoteUniqueId; let hasQuickActions = false; @@ -1576,7 +1576,7 @@ export default class Notes { $form = $notesContainer.parent().find('form'); } - $form.find('.js-note-text').val(formContent); + $form.find('.js-note-text').val(formContentOriginal); this.reenableTargetFormSubmitButton(e); this.addNoteError($form); }); diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 66c52611614..4546b88e44d 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -103,6 +103,16 @@ import '~/notes'; $('.js-comment-button').click(); expect(this.autoSizeSpy).toHaveBeenTriggered(); }); + + it('should not place escaped text in the comment box in case of error', function() { + const deferred = $.Deferred(); + spyOn($, 'ajax').and.returnValue(deferred.promise()); + $(textarea).text('A comment with `markup`.'); + + deferred.reject(); + $('.js-comment-button').click(); + expect($(textarea).val()).toEqual('A comment with `markup`.'); + }); }); describe('updateNote', () => { From 5e2a748bb71f09cd662f10e5b3de6155c9c742ac Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 18 Oct 2017 13:26:00 +0200 Subject: [PATCH 3/4] Add Gitaly data to the Peek performance bar --- app/views/peek/views/_gitaly.html.haml | 7 +++++ changelogs/unreleased/zj-peek-gitaly.yml | 5 ++++ config/initializers/peek.rb | 1 + lib/gitlab/gitaly_client.rb | 9 +++++++ lib/peek/views/gitaly.rb | 34 ++++++++++++++++++++++++ 5 files changed, 56 insertions(+) create mode 100644 app/views/peek/views/_gitaly.html.haml create mode 100644 changelogs/unreleased/zj-peek-gitaly.yml create mode 100644 lib/peek/views/gitaly.rb diff --git a/app/views/peek/views/_gitaly.html.haml b/app/views/peek/views/_gitaly.html.haml new file mode 100644 index 00000000000..a7d040d6821 --- /dev/null +++ b/app/views/peek/views/_gitaly.html.haml @@ -0,0 +1,7 @@ +- local_assigns.fetch(:view) + +%strong + %span{ data: { defer_to: "#{view.defer_key}-duration" } } ... + \/ + %span{ data: { defer_to: "#{view.defer_key}-calls" } } ... + Gitaly diff --git a/changelogs/unreleased/zj-peek-gitaly.yml b/changelogs/unreleased/zj-peek-gitaly.yml new file mode 100644 index 00000000000..bd2f2a07540 --- /dev/null +++ b/changelogs/unreleased/zj-peek-gitaly.yml @@ -0,0 +1,5 @@ +--- +title: Add Gitaly metrics to the performance bar +merge_request: +author: +type: other diff --git a/config/initializers/peek.rb b/config/initializers/peek.rb index a54d53cbbe2..1cff355346c 100644 --- a/config/initializers/peek.rb +++ b/config/initializers/peek.rb @@ -16,6 +16,7 @@ Peek.into Peek::Views::Redis Peek.into Peek::Views::Sidekiq Peek.into Peek::Views::Rblineprof Peek.into Peek::Views::GC +Peek.into Peek::Views::Gitaly # rubocop:disable Style/ClassAndModuleCamelCase class PEEK_DB_CLIENT diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 6c1ae19ff11..6868be26758 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -33,6 +33,12 @@ module Gitlab MUTEX = Mutex.new private_constant :MUTEX + class << self + attr_accessor :query_time + end + + self.query_time = 0 + def self.stub(name, storage) MUTEX.synchronize do @stubs ||= {} @@ -83,11 +89,14 @@ module Gitlab # end # def self.call(storage, service, rpc, request) + start = Process.clock_gettime(Process::CLOCK_MONOTONIC) enforce_gitaly_request_limits(:call) kwargs = request_kwargs(storage) kwargs = yield(kwargs) if block_given? stub(service, storage).__send__(rpc, request, kwargs) # rubocop:disable GitlabSecurity/PublicSend + ensure + self.query_time += Process.clock_gettime(Process::CLOCK_MONOTONIC) - start end def self.request_kwargs(storage) diff --git a/lib/peek/views/gitaly.rb b/lib/peek/views/gitaly.rb new file mode 100644 index 00000000000..d519d8e86fa --- /dev/null +++ b/lib/peek/views/gitaly.rb @@ -0,0 +1,34 @@ +module Peek + module Views + class Gitaly < View + def duration + ::Gitlab::GitalyClient.query_time + end + + def calls + ::Gitlab::GitalyClient.get_request_count + end + + def results + { duration: formatted_duration, calls: calls } + end + + private + + def formatted_duration + ms = duration * 1000 + if ms >= 1000 + "%.2fms" % ms + else + "%.0fms" % ms + end + end + + def setup_subscribers + subscribe 'start_processing.action_controller' do + ::Gitlab::GitalyClient.query_time = 0 + end + end + end + end +end From 07a8c54355d3b5d51319edcef397fd998d9d5999 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Mon, 23 Oct 2017 10:57:23 +0000 Subject: [PATCH 4/4] Create new branch from dropdown. --- .../repo/components/new_branch_form.vue | 115 +++++++++++++++++ .../javascripts/repo/components/repo.vue | 32 ++++- app/assets/javascripts/repo/index.js | 22 ++++ .../javascripts/repo/services/repo_service.js | 10 ++ .../javascripts/repo/stores/repo_store.js | 1 + .../projects/tree/_tree_header.html.haml | 2 +- app/views/shared/_ref_switcher.html.haml | 23 +++- spec/features/projects/ref_switcher_spec.rb | 35 +++++ .../repo/components/new_branch_form_spec.js | 122 ++++++++++++++++++ spec/javascripts/repo/components/repo_spec.js | 96 ++++++++++++++ 10 files changed, 449 insertions(+), 9 deletions(-) create mode 100644 app/assets/javascripts/repo/components/new_branch_form.vue create mode 100644 spec/javascripts/repo/components/new_branch_form_spec.js create mode 100644 spec/javascripts/repo/components/repo_spec.js diff --git a/app/assets/javascripts/repo/components/new_branch_form.vue b/app/assets/javascripts/repo/components/new_branch_form.vue new file mode 100644 index 00000000000..eac43e692b0 --- /dev/null +++ b/app/assets/javascripts/repo/components/new_branch_form.vue @@ -0,0 +1,115 @@ + + + diff --git a/app/assets/javascripts/repo/components/repo.vue b/app/assets/javascripts/repo/components/repo.vue index 0a89a9f16cb..6310bdb3270 100644 --- a/app/assets/javascripts/repo/components/repo.vue +++ b/app/assets/javascripts/repo/components/repo.vue @@ -8,7 +8,9 @@ import RepoMixin from '../mixins/repo_mixin'; import PopupDialog from '../../vue_shared/components/popup_dialog.vue'; import Store from '../stores/repo_store'; import Helper from '../helpers/repo_helper'; +import Service from '../services/repo_service'; import MonacoLoaderHelper from '../helpers/monaco_loader_helper'; +import eventHub from '../event_hub'; export default { data() { @@ -24,12 +26,19 @@ export default { PopupDialog, RepoPreview, }, - + created() { + eventHub.$on('createNewBranch', this.createNewBranch); + }, mounted() { Helper.getContent().catch(Helper.loadingError); }, - + destroyed() { + eventHub.$off('createNewBranch', this.createNewBranch); + }, methods: { + getCurrentLocation() { + return location.href; + }, toggleDialogOpen(toggle) { this.dialog.open = toggle; }, @@ -38,8 +47,25 @@ export default { this.toggleDialogOpen(false); this.dialog.status = status; }, - toggleBlobView: Store.toggleBlobView, + createNewBranch(branch) { + Service.createBranch({ + branch, + ref: Store.currentBranch, + }).then((res) => { + const newBranchName = res.data.name; + const newUrl = this.getCurrentLocation().replace(Store.currentBranch, newBranchName); + + Store.currentBranch = newBranchName; + + history.pushState({ key: Helper.key }, '', newUrl); + + eventHub.$emit('createNewBranchSuccess', newBranchName); + eventHub.$emit('toggleNewBranchDropdown'); + }).catch((err) => { + eventHub.$emit('createNewBranchError', err.response.data.message); + }); + }, }, }; diff --git a/app/assets/javascripts/repo/index.js b/app/assets/javascripts/repo/index.js index 65dee7d5fd1..85e960df497 100644 --- a/app/assets/javascripts/repo/index.js +++ b/app/assets/javascripts/repo/index.js @@ -5,6 +5,7 @@ import Service from './services/repo_service'; import Store from './stores/repo_store'; import Repo from './components/repo.vue'; import RepoEditButton from './components/repo_edit_button.vue'; +import newBranchForm from './components/new_branch_form.vue'; import Translate from '../vue_shared/translate'; function initDropdowns() { @@ -62,6 +63,26 @@ function initRepoEditButton(el) { }); } +function initNewBranchForm() { + const el = document.querySelector('.js-new-branch-dropdown'); + + if (!el) return null; + + return new Vue({ + el, + components: { + newBranchForm, + }, + render(createElement) { + return createElement('new-branch-form', { + props: { + currentBranch: Store.currentBranch, + }, + }); + }, + }); +} + function initRepoBundle() { const repo = document.getElementById('repo'); const editButton = document.querySelector('.editable-mode'); @@ -73,6 +94,7 @@ function initRepoBundle() { initRepo(repo); initRepoEditButton(editButton); + initNewBranchForm(); } $(initRepoBundle); diff --git a/app/assets/javascripts/repo/services/repo_service.js b/app/assets/javascripts/repo/services/repo_service.js index d68d71a4629..786b5637493 100644 --- a/app/assets/javascripts/repo/services/repo_service.js +++ b/app/assets/javascripts/repo/services/repo_service.js @@ -1,8 +1,11 @@ import axios from 'axios'; +import csrf from '../../lib/utils/csrf'; import Store from '../stores/repo_store'; import Api from '../../api'; import Helper from '../helpers/repo_helper'; +axios.defaults.headers.common[csrf.headerKey] = csrf.token; + const RepoService = { url: '', options: { @@ -10,6 +13,7 @@ const RepoService = { format: 'json', }, }, + createBranchPath: '/api/:version/projects/:id/repository/branches', richExtensionRegExp: /md/, getRaw(url) { @@ -73,6 +77,12 @@ const RepoService = { .then(this.commitFlash); }, + createBranch(payload) { + const url = Api.buildUrl(this.createBranchPath) + .replace(':id', Store.projectId); + return axios.post(url, payload); + }, + commitFlash(data) { if (data.short_id && data.stats) { window.Flash(`Your changes have been committed. Commit ${data.short_id} with ${data.stats.additions} additions, ${data.stats.deletions} deletions.`, 'notice'); diff --git a/app/assets/javascripts/repo/stores/repo_store.js b/app/assets/javascripts/repo/stores/repo_store.js index 49d7317a17e..39e1b4e5849 100644 --- a/app/assets/javascripts/repo/stores/repo_store.js +++ b/app/assets/javascripts/repo/stores/repo_store.js @@ -13,6 +13,7 @@ const RepoStore = { projectId: '', projectName: '', projectUrl: '', + branchUrl: '', blobRaw: '', currentBlobView: 'repo-preview', openedFiles: [], diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml index 6cddc38d11a..47f3f2b459a 100644 --- a/app/views/projects/tree/_tree_header.html.haml +++ b/app/views/projects/tree/_tree_header.html.haml @@ -1,6 +1,6 @@ .tree-ref-container .tree-ref-holder - = render 'shared/ref_switcher', destination: 'tree', path: @path + = render 'shared/ref_switcher', destination: 'tree', path: @path, show_create: true - unless show_new_repo? = render 'projects/tree/old_tree_header' diff --git a/app/views/shared/_ref_switcher.html.haml b/app/views/shared/_ref_switcher.html.haml index 7ad743b3b81..6d7c9633913 100644 --- a/app/views/shared/_ref_switcher.html.haml +++ b/app/views/shared/_ref_switcher.html.haml @@ -1,3 +1,4 @@ +- show_new_branch_form = show_new_repo? && show_create && can?(current_user, :push_code, @project) - dropdown_toggle_text = @ref || @project.default_branch = form_tag switch_project_refs_path(@project), method: :get, class: "project-refs-form" do = hidden_field_tag :destination, destination @@ -7,8 +8,20 @@ = hidden_field_tag key, value, id: nil .dropdown = dropdown_toggle dropdown_toggle_text, { toggle: "dropdown", selected: dropdown_toggle_text, ref: @ref, refs_url: refs_project_path(@project), field_name: 'ref', submit_form_on_click: true, visit: true }, { toggle_class: "js-project-refs-dropdown" } - .dropdown-menu.dropdown-menu-selectable.git-revision-dropdown{ class: ("dropdown-menu-align-right" if local_assigns[:align_right]) } - = dropdown_title _("Switch branch/tag") - = dropdown_filter _("Search branches and tags") - = dropdown_content - = dropdown_loading + .dropdown-menu.dropdown-menu-selectable.git-revision-dropdown.dropdown-menu-paging{ class: ("dropdown-menu-align-right" if local_assigns[:align_right]) } + .dropdown-page-one + = dropdown_title _("Switch branch/tag") + = dropdown_filter _("Search branches and tags") + = dropdown_content + = dropdown_loading + - if show_new_branch_form + = dropdown_footer do + %ul.dropdown-footer-list + %li + %a.dropdown-toggle-page{ href: "#" } + Create new branch + - if show_new_branch_form + .dropdown-page-two + = dropdown_title("Create new branch", options: { back: true }) + = dropdown_content do + .js-new-branch-dropdown diff --git a/spec/features/projects/ref_switcher_spec.rb b/spec/features/projects/ref_switcher_spec.rb index f8695403857..50c0bfd580d 100644 --- a/spec/features/projects/ref_switcher_spec.rb +++ b/spec/features/projects/ref_switcher_spec.rb @@ -6,6 +6,7 @@ feature 'Ref switcher', :js do before do project.team << [user, :master] + page.driver.set_cookie('new_repo', 'true') sign_in(user) visit project_tree_path(project, 'master') end @@ -40,4 +41,38 @@ feature 'Ref switcher', :js do expect(page).to have_title "'test'" end + + context "create branch" do + let(:input) { find('.js-new-branch-name') } + + before do + click_button 'master' + wait_for_requests + + page.within '.project-refs-form' do + find(".dropdown-footer-list a").click + end + end + + it "shows error message for the invalid branch name" do + input.set 'foo bar' + click_button('Create') + wait_for_requests + expect(page).to have_content 'Branch name is invalid' + end + + it "should create new branch properly" do + input.set 'new-branch-name' + click_button('Create') + wait_for_requests + expect(find('.js-project-refs-dropdown')).to have_content 'new-branch-name' + end + + it "should create new branch by Enter key" do + input.set 'new-branch-name-2' + input.native.send_keys :enter + wait_for_requests + expect(find('.js-project-refs-dropdown')).to have_content 'new-branch-name-2' + end + end end diff --git a/spec/javascripts/repo/components/new_branch_form_spec.js b/spec/javascripts/repo/components/new_branch_form_spec.js new file mode 100644 index 00000000000..c9c5ce096fc --- /dev/null +++ b/spec/javascripts/repo/components/new_branch_form_spec.js @@ -0,0 +1,122 @@ +import Vue from 'vue'; +import newBranchForm from '~/repo/components/new_branch_form.vue'; +import eventHub from '~/repo/event_hub'; +import RepoStore from '~/repo/stores/repo_store'; +import createComponent from '../../helpers/vue_mount_component_helper'; + +describe('Multi-file editor new branch form', () => { + let vm; + + beforeEach(() => { + const Component = Vue.extend(newBranchForm); + + RepoStore.currentBranch = 'master'; + + vm = createComponent(Component, { + currentBranch: RepoStore.currentBranch, + }); + }); + + afterEach(() => { + vm.$destroy(); + + RepoStore.currentBranch = ''; + }); + + describe('template', () => { + it('renders submit as disabled', () => { + expect(vm.$el.querySelector('.btn').getAttribute('disabled')).toBe('disabled'); + }); + + it('enables the submit button when branch is not empty', (done) => { + vm.branchName = 'testing'; + + Vue.nextTick(() => { + expect(vm.$el.querySelector('.btn').getAttribute('disabled')).toBeNull(); + + done(); + }); + }); + + it('displays current branch creating from', (done) => { + Vue.nextTick(() => { + expect(vm.$el.querySelector('p').textContent.replace(/\s+/g, ' ').trim()).toBe('Create from: master'); + + done(); + }); + }); + }); + + describe('submitNewBranch', () => { + it('sets to loading', () => { + vm.submitNewBranch(); + + expect(vm.loading).toBeTruthy(); + }); + + it('hides current flash element', (done) => { + vm.$refs.flashContainer.innerHTML = '
'; + + vm.submitNewBranch(); + + Vue.nextTick(() => { + expect(vm.$el.querySelector('.flash-alert')).toBeNull(); + + done(); + }); + }); + + it('emits an event with branchName', () => { + spyOn(eventHub, '$emit'); + + vm.branchName = 'testing'; + + vm.submitNewBranch(); + + expect(eventHub.$emit).toHaveBeenCalledWith('createNewBranch', 'testing'); + }); + }); + + describe('showErrorMessage', () => { + it('sets loading to false', () => { + vm.loading = true; + + vm.showErrorMessage(); + + expect(vm.loading).toBeFalsy(); + }); + + it('creates flash element', () => { + vm.showErrorMessage('error message'); + + expect(vm.$el.querySelector('.flash-alert')).not.toBeNull(); + expect(vm.$el.querySelector('.flash-alert').textContent.trim()).toBe('error message'); + }); + }); + + describe('createdNewBranch', () => { + it('set loading to false', () => { + vm.loading = true; + + vm.createdNewBranch(); + + expect(vm.loading).toBeFalsy(); + }); + + it('resets branch name', () => { + vm.branchName = 'testing'; + + vm.createdNewBranch(); + + expect(vm.branchName).toBe(''); + }); + + it('sets the dropdown toggle text', () => { + vm.dropdownText = document.createElement('span'); + + vm.createdNewBranch('branch name'); + + expect(vm.dropdownText.textContent).toBe('branch name'); + }); + }); +}); diff --git a/spec/javascripts/repo/components/repo_spec.js b/spec/javascripts/repo/components/repo_spec.js new file mode 100644 index 00000000000..3558a155728 --- /dev/null +++ b/spec/javascripts/repo/components/repo_spec.js @@ -0,0 +1,96 @@ +import Vue from 'vue'; +import repo from '~/repo/components/repo.vue'; +import RepoStore from '~/repo/stores/repo_store'; +import Service from '~/repo/services/repo_service'; +import eventHub from '~/repo/event_hub'; +import createComponent from '../../helpers/vue_mount_component_helper'; + +describe('repo component', () => { + let vm; + + beforeEach(() => { + const Component = Vue.extend(repo); + + RepoStore.currentBranch = 'master'; + + vm = createComponent(Component); + }); + + afterEach(() => { + vm.$destroy(); + + RepoStore.currentBranch = ''; + }); + + describe('createNewBranch', () => { + beforeEach(() => { + spyOn(history, 'pushState'); + }); + + describe('success', () => { + beforeEach(() => { + spyOn(Service, 'createBranch').and.returnValue(Promise.resolve({ + data: { + name: 'test', + }, + })); + }); + + it('calls createBranch with branchName', () => { + eventHub.$emit('createNewBranch', 'test'); + + expect(Service.createBranch).toHaveBeenCalledWith({ + branch: 'test', + ref: RepoStore.currentBranch, + }); + }); + + it('pushes new history state', (done) => { + RepoStore.currentBranch = 'master'; + + spyOn(vm, 'getCurrentLocation').and.returnValue('http://test.com/master'); + + eventHub.$emit('createNewBranch', 'test'); + + setTimeout(() => { + expect(history.pushState).toHaveBeenCalledWith(jasmine.anything(), '', 'http://test.com/test'); + done(); + }); + }); + + it('updates stores currentBranch', (done) => { + eventHub.$emit('createNewBranch', 'test'); + + setTimeout(() => { + expect(RepoStore.currentBranch).toBe('test'); + + done(); + }); + }); + }); + + describe('failure', () => { + beforeEach(() => { + spyOn(Service, 'createBranch').and.returnValue(Promise.reject({ + response: { + data: { + message: 'test', + }, + }, + })); + }); + + it('emits createNewBranchError event', (done) => { + spyOn(eventHub, '$emit').and.callThrough(); + + eventHub.$emit('createNewBranch', 'test'); + + setTimeout(() => { + expect(eventHub.$emit).toHaveBeenCalledWith('createNewBranchError', 'test'); + + done(); + }); + }); + }); + }); +});