From e8c723543cfc4c1d905a5794a2da1bef7689d784 Mon Sep 17 00:00:00 2001 From: Baldinof Date: Wed, 9 Mar 2016 15:25:48 +0100 Subject: [PATCH 001/261] Close merge requests when removing fork relation --- CHANGELOG | 1 + app/controllers/projects_controller.rb | 2 +- app/models/merge_request.rb | 1 + app/models/project.rb | 12 +++++++++++- spec/models/project_spec.rb | 19 +++++++++++++++++++ 5 files changed, 33 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index c1c90903d5..84739fab82 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -26,6 +26,7 @@ v 8.6.0 (unreleased) - Show labels in dashboard and group milestone views - Add main language of a project in the list of projects (Tiago Botelho) - Add ability to show archived projects on dashboard, explore and group pages + - Remove fork link closes all merge requests opened on source project (Florent Baldino) v 8.5.5 - Ensure removing a project removes associated Todo entries diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index aea08ecce3..a26d11459f 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -72,7 +72,7 @@ class ProjectsController < ApplicationController def remove_fork return access_denied! unless can?(current_user, :remove_fork_project, @project) - if @project.unlink_fork + if @project.unlink_fork(current_user) flash[:notice] = 'The fork relationship has been removed.' end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index c1e18bb3cc..18ec48b57f 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -137,6 +137,7 @@ class MergeRequest < ActiveRecord::Base scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } scope :in_projects, ->(project_ids) { where("source_project_id in (:project_ids) OR target_project_id in (:project_ids)", project_ids: project_ids) } scope :of_projects, ->(ids) { where(target_project_id: ids) } + scope :from_project, ->(project) { where(source_project_id: project.id) } scope :merged, -> { with_state(:merged) } scope :closed_and_merged, -> { with_states(:closed, :merged) } diff --git a/app/models/project.rb b/app/models/project.rb index 65829bec77..859758293e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -890,12 +890,22 @@ class Project < ActiveRecord::Base self.builds_enabled = true end - def unlink_fork + def unlink_fork(user) if forked? forked_from_project.lfs_objects.find_each do |lfs_object| lfs_object.projects << self end + merge_requests = forked_from_project.merge_requests.opened.from_project(self) + + unless merge_requests.empty? + close_service = MergeRequests::CloseService.new(self, user) + + merge_requests.each do |mr| + close_service.execute(mr) + end + end + forked_project_link.destroy end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2fa38a5d3d..ba4fb2f822 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -647,4 +647,23 @@ describe Project, models: true do project.expire_caches_before_rename('foo') end end + + describe '#unlink_fork' do + let(:fork_link) { create(:forked_project_link) } + let(:fork_project) { fork_link.forked_to_project } + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request, source_project: fork_project, target_project: fork_link.forked_from_project) } + let!(:close_service) { MergeRequests::CloseService.new(fork_project, user) } + + it 'remove fork relation and close all pending merge requests' do + allow(MergeRequests::CloseService).to receive(:new). + with(fork_project, user). + and_return(close_service) + + expect(close_service).to receive(:execute).with(merge_request) + expect(fork_project.forked_project_link).to receive(:destroy) + + fork_project.unlink_fork(user) + end + end end From fa4126acffdfe13741e05a60ad5ed7fd407b4f16 Mon Sep 17 00:00:00 2001 From: Baldinof Date: Tue, 22 Mar 2016 15:34:35 +0100 Subject: [PATCH 002/261] Move unlink fork logic to a service --- app/controllers/projects_controller.rb | 2 +- app/models/project.rb | 20 ------------ app/services/projects/unlink_fork_service.rb | 19 +++++++++++ spec/models/project_spec.rb | 19 ----------- .../projects/unlink_fork_service_spec.rb | 32 +++++++++++++++++++ 5 files changed, 52 insertions(+), 40 deletions(-) create mode 100644 app/services/projects/unlink_fork_service.rb create mode 100644 spec/services/projects/unlink_fork_service_spec.rb diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 94789702d6..87657e4e3d 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -71,7 +71,7 @@ class ProjectsController < ApplicationController def remove_fork return access_denied! unless can?(current_user, :remove_fork_project, @project) - if @project.unlink_fork(current_user) + if ::Projects::UnlinkForkService.new(@project, current_user).execute flash[:notice] = 'The fork relationship has been removed.' end end diff --git a/app/models/project.rb b/app/models/project.rb index 8d9908128e..691b706ea4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -917,26 +917,6 @@ class Project < ActiveRecord::Base self.builds_enabled = true end - def unlink_fork(user) - if forked? - forked_from_project.lfs_objects.find_each do |lfs_object| - lfs_object.projects << self - end - - merge_requests = forked_from_project.merge_requests.opened.from_project(self) - - unless merge_requests.empty? - close_service = MergeRequests::CloseService.new(self, user) - - merge_requests.each do |mr| - close_service.execute(mr) - end - end - - forked_project_link.destroy - end - end - def any_runners?(&block) if runners.active.any?(&block) return true diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb new file mode 100644 index 0000000000..d0703effa1 --- /dev/null +++ b/app/services/projects/unlink_fork_service.rb @@ -0,0 +1,19 @@ +module Projects + class UnlinkForkService < BaseService + def execute + return unless @project.forked? + + @project.forked_from_project.lfs_objects.find_each do |lfs_object| + lfs_object.projects << self + end + + merge_requests = @project.forked_from_project.merge_requests.opened.from_project(@project) + + merge_requests.each do |mr| + MergeRequests::CloseService.new(@project, @current_user).execute(mr) + end + + @project.forked_project_link.destroy + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1ca78daa5b..59c5ffa6b9 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -699,25 +699,6 @@ describe Project, models: true do end end - describe '#unlink_fork' do - let(:fork_link) { create(:forked_project_link) } - let(:fork_project) { fork_link.forked_to_project } - let(:user) { create(:user) } - let(:merge_request) { create(:merge_request, source_project: fork_project, target_project: fork_link.forked_from_project) } - let!(:close_service) { MergeRequests::CloseService.new(fork_project, user) } - - it 'remove fork relation and close all pending merge requests' do - allow(MergeRequests::CloseService).to receive(:new). - with(fork_project, user). - and_return(close_service) - - expect(close_service).to receive(:execute).with(merge_request) - expect(fork_project.forked_project_link).to receive(:destroy) - - fork_project.unlink_fork(user) - end - end - describe '.search_by_title' do let(:project) { create(:project, name: 'kittens') } diff --git a/spec/services/projects/unlink_fork_service_spec.rb b/spec/services/projects/unlink_fork_service_spec.rb new file mode 100644 index 0000000000..f287b0a59b --- /dev/null +++ b/spec/services/projects/unlink_fork_service_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Projects::UnlinkForkService, services: true do + subject { Projects::UnlinkForkService.new(fork_project, user) } + + let(:fork_link) { create(:forked_project_link) } + let(:fork_project) { fork_link.forked_to_project } + let(:user) { create(:user) } + + context 'with opened merge request on the source project' do + let(:merge_request) { create(:merge_request, source_project: fork_project, target_project: fork_link.forked_from_project) } + let(:mr_close_service) { MergeRequests::CloseService.new(fork_project, user) } + + before do + allow(MergeRequests::CloseService).to receive(:new). + with(fork_project, user). + and_return(mr_close_service) + end + + it 'close all pending merge requests' do + expect(mr_close_service).to receive(:execute).with(merge_request) + + subject.execute + end + end + + it 'remove fork relation' do + expect(fork_project.forked_project_link).to receive(:destroy) + + subject.execute + end +end From aa97720a939dedb2197eedb93fcecf194d78ea96 Mon Sep 17 00:00:00 2001 From: connorshea Date: Mon, 28 Mar 2016 16:43:42 -0600 Subject: [PATCH 003/261] Upgrade bundler-audit from 0.4.0 to 0.5.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundler Audit can now run check with the `—update` flag to update the Ruby CVE repository before checking. This removes the need for two separate commands in GitLab CI. See the Changelog for more information: https://github.com/rubysec/bundler-audit/blob/master/ChangeLog.md#050--2 015-02-28 --- .gitlab-ci.yml | 3 +-- Gemfile.lock | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 53f115c92c..336ceb3102 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -158,8 +158,7 @@ bundler:audit: only: - master script: - - "bundle exec bundle-audit update" - - "bundle exec bundle-audit check --ignore OSVDB-115941" + - "bundle exec bundle-audit check --update --ignore OSVDB-115941" tags: - ruby - mysql diff --git a/Gemfile.lock b/Gemfile.lock index da27c62acb..e613b5c0c3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -99,7 +99,7 @@ GEM bullet (5.0.0) activesupport (>= 3.0.0) uniform_notifier (~> 1.9.0) - bundler-audit (0.4.0) + bundler-audit (0.5.0) bundler (~> 1.2) thor (~> 0.18) byebug (8.2.1) From a1597d22aecfb65dc1a3f063f574622cf6768d1d Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Tue, 29 Mar 2016 12:41:54 +0200 Subject: [PATCH 004/261] Remove 2FA status on enable page --- app/views/profiles/two_factor_auths/new.html.haml | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/views/profiles/two_factor_auths/new.html.haml b/app/views/profiles/two_factor_auths/new.html.haml index 5d342ef58e..69fc81cb45 100644 --- a/app/views/profiles/two_factor_auths/new.html.haml +++ b/app/views/profiles/two_factor_auths/new.html.haml @@ -7,8 +7,6 @@ %p Increase your account's security by enabling two-factor authentication (2FA). .col-lg-9 - %p - Status: #{current_user.two_factor_enabled? ? 'enabled' : 'disabled'} %p Download the Google Authenticator application from App Store for iOS or Google Play for Android and scan this code. More information is available in the #{link_to('documentation', help_page_path('profile', 'two_factor_authentication'))}. From b69f8a62b2078fbd43413c670dea76871b74d0d5 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 30 Mar 2016 15:04:58 -0300 Subject: [PATCH 005/261] Add specific markdown_preview route for Wikis --- app/controllers/projects/wikis_controller.rb | 14 ++++++++++++++ app/views/layouts/project.html.haml | 6 +++++- config/routes.rb | 1 + 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index 02ceb8f433..9f3a4a6972 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -88,6 +88,20 @@ class Projects::WikisController < Projects::ApplicationController ) end + def markdown_preview + text = params[:text] + + ext = Gitlab::ReferenceExtractor.new(@project, current_user, current_user) + ext.analyze(text) + + render json: { + body: view_context.markdown(text, pipeline: :wiki, project_wiki: @project_wiki), + references: { + users: ext.users.map(&:username) + } + } + end + def git_access end diff --git a/app/views/layouts/project.html.haml b/app/views/layouts/project.html.haml index ab527e8e43..a7ef31acd3 100644 --- a/app/views/layouts/project.html.haml +++ b/app/views/layouts/project.html.haml @@ -5,10 +5,14 @@ - content_for :scripts_body_top do - project = @target_project || @project + - if @project_wiki + - markdown_preview_path = namespace_project_wikis_markdown_preview_path(project.namespace, project) + - else + - markdown_preview_path = markdown_preview_namespace_project_path(project.namespace, project) - if current_user :javascript window.project_uploads_path = "#{namespace_project_uploads_path project.namespace,project}"; - window.markdown_preview_path = "#{markdown_preview_namespace_project_path(project.namespace, project)}"; + window.markdown_preview_path = "#{markdown_preview_path}"; - content_for :scripts_body do = render "layouts/init_auto_complete" if current_user diff --git a/config/routes.rb b/config/routes.rb index 6bf22fb445..e57c04595f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -575,6 +575,7 @@ Rails.application.routes.draw do # Order matters to give priority to these matches get '/wikis/git_access', to: 'wikis#git_access' get '/wikis/pages', to: 'wikis#pages', as: 'wiki_pages' + post '/wikis/markdown_preview', to:'wikis#markdown_preview' post '/wikis', to: 'wikis#create' get '/wikis/*id/history', to: 'wikis#history', as: 'wiki_history', constraints: WIKI_SLUG_ID From 7d56d592cd7d1df718d59981af1ced9d0be8eaac Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 30 Mar 2016 15:30:36 -0300 Subject: [PATCH 006/261] Fixed Gollum pages link url expansion to render correctly in preview --- lib/banzai/filter/gollum_tags_filter.rb | 12 +++++++++--- spec/lib/banzai/filter/gollum_tags_filter_spec.rb | 8 +++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/banzai/filter/gollum_tags_filter.rb b/lib/banzai/filter/gollum_tags_filter.rb index 7ce26db1b9..cfd3e28ed0 100644 --- a/lib/banzai/filter/gollum_tags_filter.rb +++ b/lib/banzai/filter/gollum_tags_filter.rb @@ -144,12 +144,18 @@ module Banzai # if it is not. def process_page_link_tag(parts) if parts.size == 1 - url = parts[0].strip + reference = parts[0].strip else - name, url = *parts.compact.map(&:strip) + name, reference = *parts.compact.map(&:strip) end - content_tag(:a, name || url, href: url) + if url?(reference) + href = reference + else + href = ::File.join(project_wiki_base_path, reference) + end + + content_tag(:a, name || reference, href: href) end def project_wiki diff --git a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb index 5e23c5c319..ad777fe4b9 100644 --- a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb +++ b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb @@ -70,20 +70,22 @@ describe Banzai::Filter::GollumTagsFilter, lib: true do end context 'linking internal resources' do - it "the created link's text will be equal to the resource's text" do + it "the created link's text will include the resource's text and project_wiki_base_path" do tag = '[[wiki-slug]]' doc = filter("See #{tag}", project_wiki: project_wiki) + expected_path = ::File.join(project_wiki.wiki_base_path, 'wiki-slug') expect(doc.at_css('a').text).to eq 'wiki-slug' - expect(doc.at_css('a')['href']).to eq 'wiki-slug' + expect(doc.at_css('a')['href']).to eq expected_path end it "the created link's text will be link-text" do tag = '[[link-text|wiki-slug]]' doc = filter("See #{tag}", project_wiki: project_wiki) + expected_path = ::File.join(project_wiki.wiki_base_path, 'wiki-slug') expect(doc.at_css('a').text).to eq 'link-text' - expect(doc.at_css('a')['href']).to eq 'wiki-slug' + expect(doc.at_css('a')['href']).to eq expected_path end end From 0e290a84012ed9d2138cb6bc31b5db9c62e330c9 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 23 Mar 2016 15:23:45 +0000 Subject: [PATCH 007/261] Started arrow key movement on dropdowns --- app/assets/javascripts/gl_dropdown.js.coffee | 57 +++++++++++++++++-- .../stylesheets/framework/dropdowns.scss | 11 +++- 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 0fea2a69cb..8686cb9984 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -96,6 +96,7 @@ class GitLabDropdown LOADING_CLASS = "is-loading" PAGE_TWO_CLASS = "is-page-two" ACTIVE_CLASS = "is-active" + CURRENT_INDEX = 0 FILTER_INPUT = '.dropdown-input .dropdown-input-field' @@ -218,6 +219,8 @@ class GitLabDropdown return true opened: => + @addArrowKeyEvent() + contentHtml = $('.dropdown-content', @dropdown).html() if @remote && contentHtml is "" @remote.execute() @@ -228,6 +231,7 @@ class GitLabDropdown @dropdown.trigger('shown.gl.dropdown') hidden: (e) => + @removeArrayKeyEvent() if @options.filterable @dropdown .find(".dropdown-input-field") @@ -322,8 +326,8 @@ class GitLabDropdown ).join('') noResults: -> - html = "
  • " - html += "" + html = "
  • " @@ -343,7 +347,7 @@ class GitLabDropdown selectedObject = @renderedData[selectedIndex] value = if @options.id then @options.id(selectedObject, el) else selectedObject.id field = @dropdown.parent().find("input[name='#{fieldName}'][value='#{value}']") - + if el.hasClass(ACTIVE_CLASS) el.removeClass(ACTIVE_CLASS) field.remove() @@ -384,8 +388,53 @@ class GitLabDropdown # simulate a click on the first link $(selector).trigger "click" + addArrowKeyEvent: -> + ARROW_KEY_CODES = [38, 40] + $input = @dropdown.find(".dropdown-input-field") + + selector = '.dropdown-content li:not(.divider)' + if @dropdown.find(".dropdown-toggle-page").length + selector = ".dropdown-page-one #{selector}" + + $('body').on 'keydown', (e) => + currentKeyCode = e.keyCode + + if ARROW_KEY_CODES.indexOf(currentKeyCode) >= 0 + e.preventDefault() + e.stopPropagation() + + $listItems = $(selector, @dropdown) + + if @options.filterable + $input.blur() + + if currentKeyCode is 40 + # Move down + CURRENT_INDEX += 1 if CURRENT_INDEX < $listItems.length + else if currentKeyCode is 38 + # Move up + CURRENT_INDEX -= 1 if CURRENT_INDEX > 0 + + @highlightRowAtIndex(CURRENT_INDEX) + + return false + + removeArrayKeyEvent: -> + $('body').off 'keydown' + + highlightRowAtIndex: (index, prevIndex) -> + # Remove the class for the previously focused row + $('.is-focused', @dropdown).removeClass 'is-focused' + + # Update the class for the row at the specific index + selector = ".dropdown-content li:not(.divider):eq(#{index})" + if @dropdown.find(".dropdown-toggle-page").length + selector = ".dropdown-page-one #{selector}" + + $listItem = $(selector, @dropdown) + $listItem.addClass "is-focused" + $.fn.glDropdown = (opts) -> return @.each -> if (!$.data @, 'glDropdown') $.data(@, 'glDropdown', new GitLabDropdown @, opts) - diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 82dc1acbd0..fe03c040e6 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -104,6 +104,14 @@ padding: 0 10px; } + .is-focused { + a { + background-color: $dropdown-link-hover-bg; + text-decoration: none; + outline: 0; + } + } + .divider { height: 1px; margin: 8px 10px; @@ -132,8 +140,7 @@ overflow: hidden; &:hover, - &:focus, - &.is-focused { + &:focus { background-color: $dropdown-link-hover-bg; text-decoration: none; outline: 0; From b244317a38f80740bd1508362316c8e70424f14c Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 24 Mar 2016 10:26:15 +0000 Subject: [PATCH 008/261] Scrolls the dropdown content down --- app/assets/javascripts/gl_dropdown.js.coffee | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 8686cb9984..094b1a1240 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -96,7 +96,7 @@ class GitLabDropdown LOADING_CLASS = "is-loading" PAGE_TWO_CLASS = "is-page-two" ACTIVE_CLASS = "is-active" - CURRENT_INDEX = 0 + CURRENT_INDEX = -1 FILTER_INPUT = '.dropdown-input .dropdown-input-field' @@ -410,7 +410,7 @@ class GitLabDropdown if currentKeyCode is 40 # Move down - CURRENT_INDEX += 1 if CURRENT_INDEX < $listItems.length + CURRENT_INDEX += 1 if CURRENT_INDEX < ($listItems.length - 1) else if currentKeyCode is 38 # Move up CURRENT_INDEX -= 1 if CURRENT_INDEX > 0 @@ -434,6 +434,18 @@ class GitLabDropdown $listItem = $(selector, @dropdown) $listItem.addClass "is-focused" + # Dropdown content scroll area + $dropdownContent = $listItem.closest('.dropdown-content') + dropdownContentBottom = $dropdownContent.prop('offsetTop') + $dropdownContent.prop('offsetHeight') + + # Get the offset bottom of the list item + listItemBottom = $listItem.prop('offsetTop') + $listItem.prop('offsetHeight') + console.log listItemBottom, dropdownContentBottom + + if listItemBottom > dropdownContentBottom + # Scroll the dropdown content down + $dropdownContent.scrollTop(listItemBottom - dropdownContentBottom) + $.fn.glDropdown = (opts) -> return @.each -> if (!$.data @, 'glDropdown') From 858eee9ee851e6346e38582ab4b102fc038490c0 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 24 Mar 2016 11:01:23 +0000 Subject: [PATCH 009/261] Correctly scrolls the dropdown up & down with arrow keys --- app/assets/javascripts/gl_dropdown.js.coffee | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 094b1a1240..e7572e4eda 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -436,15 +436,22 @@ class GitLabDropdown # Dropdown content scroll area $dropdownContent = $listItem.closest('.dropdown-content') - dropdownContentBottom = $dropdownContent.prop('offsetTop') + $dropdownContent.prop('offsetHeight') + dropdownScrollTop = $dropdownContent.prop('scrollTop') + dropdownContentHeight = $dropdownContent.prop('offsetHeight') + dropdownContentTop = $dropdownContent.prop('offsetTop') + dropdownContentBottom = dropdownContentTop + dropdownContentHeight # Get the offset bottom of the list item - listItemBottom = $listItem.prop('offsetTop') + $listItem.prop('offsetHeight') - console.log listItemBottom, dropdownContentBottom + listItemHeight = $listItem.prop('offsetHeight') + listItemTop = $listItem.prop('offsetTop') + listItemBottom = listItemTop + listItemHeight - if listItemBottom > dropdownContentBottom + if listItemBottom > dropdownContentBottom + dropdownScrollTop # Scroll the dropdown content down $dropdownContent.scrollTop(listItemBottom - dropdownContentBottom) + else if listItemTop < dropdownContentTop + dropdownScrollTop + # Scroll the dropdown content up + $dropdownContent.scrollTop(listItemTop - dropdownContentTop) $.fn.glDropdown = (opts) -> return @.each -> From 29872c39055d55118cc8b9249caeb659041fea97 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 24 Mar 2016 12:12:08 +0000 Subject: [PATCH 010/261] Enter triggers the currently highlighted element click --- app/assets/javascripts/gl_dropdown.js.coffee | 25 ++++++++++---------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index e7572e4eda..1937e5367d 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -146,11 +146,11 @@ class GitLabDropdown data: => return @fullData callback: (data) => + CURRENT_INDEX = -1 @parseData data - @highlightRow 1 enterCallback: => if @enterCallback - @selectFirstRow() + @selectRowAtIndex 0 # Event listeners @@ -380,10 +380,11 @@ class GitLabDropdown return selectedObject - selectFirstRow: -> - selector = '.dropdown-content li:first-child a' + selectRowAtIndex: (index) -> + selector = ".dropdown-content li:not(.divider):eq(#{index}) a" + if @dropdown.find(".dropdown-toggle-page").length - selector = ".dropdown-page-one .dropdown-content li:first-child a" + selector = ".dropdown-page-one #{selector}" # simulate a click on the first link $(selector).trigger "click" @@ -403,6 +404,7 @@ class GitLabDropdown e.preventDefault() e.stopPropagation() + PREV_INDEX = CURRENT_INDEX $listItems = $(selector, @dropdown) if @options.filterable @@ -415,23 +417,22 @@ class GitLabDropdown # Move up CURRENT_INDEX -= 1 if CURRENT_INDEX > 0 - @highlightRowAtIndex(CURRENT_INDEX) + @highlightRowAtIndex($listItems, CURRENT_INDEX) if CURRENT_INDEX isnt PREV_INDEX return false + if currentKeyCode is 13 + @selectRowAtIndex CURRENT_INDEX + removeArrayKeyEvent: -> $('body').off 'keydown' - highlightRowAtIndex: (index, prevIndex) -> + highlightRowAtIndex: ($listItems, index) -> # Remove the class for the previously focused row $('.is-focused', @dropdown).removeClass 'is-focused' # Update the class for the row at the specific index - selector = ".dropdown-content li:not(.divider):eq(#{index})" - if @dropdown.find(".dropdown-toggle-page").length - selector = ".dropdown-page-one #{selector}" - - $listItem = $(selector, @dropdown) + $listItem = $listItems.eq(index) $listItem.addClass "is-focused" # Dropdown content scroll area From a103fcb7cb14b3bf9f7af44409c79331274e9adc Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 29 Mar 2016 12:57:10 +0100 Subject: [PATCH 011/261] Moved back some css classes --- app/assets/javascripts/gl_dropdown.js.coffee | 6 +++--- app/assets/stylesheets/framework/dropdowns.scss | 11 ++--------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 1937e5367d..5a55517529 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -326,8 +326,8 @@ class GitLabDropdown ).join('') noResults: -> - html = "" @@ -433,7 +433,7 @@ class GitLabDropdown # Update the class for the row at the specific index $listItem = $listItems.eq(index) - $listItem.addClass "is-focused" + $listItem.find('a:first-child').addClass "is-focused" # Dropdown content scroll area $dropdownContent = $listItem.closest('.dropdown-content') diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index fe03c040e6..82dc1acbd0 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -104,14 +104,6 @@ padding: 0 10px; } - .is-focused { - a { - background-color: $dropdown-link-hover-bg; - text-decoration: none; - outline: 0; - } - } - .divider { height: 1px; margin: 8px 10px; @@ -140,7 +132,8 @@ overflow: hidden; &:hover, - &:focus { + &:focus, + &.is-focused { background-color: $dropdown-link-hover-bg; text-decoration: none; outline: 0; From f76066b9fb17cd0aace49e353d57ce05976f27e8 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 31 Mar 2016 08:55:12 +0100 Subject: [PATCH 012/261] Fixed issue based on feedback --- app/assets/javascripts/gl_dropdown.js.coffee | 47 +++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 5a55517529..466213496e 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -1,5 +1,6 @@ class GitLabDropdownFilter BLUR_KEYCODES = [27, 40] + ARROW_KEY_CODES = [38, 40] HAS_VALUE_CLASS = "has-value" constructor: (@input, @options) -> @@ -22,19 +23,23 @@ class GitLabDropdownFilter # Key events timeout = "" @input.on "keyup", (e) => + keyCode = e.which + + return if ARROW_KEY_CODES.indexOf(keyCode) >= 0 + if @input.val() isnt "" and !$inputContainer.hasClass HAS_VALUE_CLASS $inputContainer.addClass HAS_VALUE_CLASS else if @input.val() is "" and $inputContainer.hasClass HAS_VALUE_CLASS $inputContainer.removeClass HAS_VALUE_CLASS - if e.keyCode is 13 and @input.val() isnt "" + if keyCode is 13 and @input.val() isnt "" if @options.enterCallback @options.enterCallback() return clearTimeout timeout timeout = setTimeout => - blur_field = @shouldBlur e.keyCode + blur_field = @shouldBlur keyCode search_text = @input.val() if blur_field and @filterInputBlur @@ -96,7 +101,7 @@ class GitLabDropdown LOADING_CLASS = "is-loading" PAGE_TWO_CLASS = "is-page-two" ACTIVE_CLASS = "is-active" - CURRENT_INDEX = -1 + currentIndex = -1 FILTER_INPUT = '.dropdown-input .dropdown-input-field' @@ -146,7 +151,7 @@ class GitLabDropdown data: => return @fullData callback: (data) => - CURRENT_INDEX = -1 + currentIndex = -1 @parseData data enterCallback: => if @enterCallback @@ -311,11 +316,11 @@ class GitLabDropdown if @highlight text = @highlightTextMatches(text, @filterInput.val()) - html = "
  • " - html += "" - html += text - html += "" - html += "
  • " + html = "
  • + + #{text} + +
  • " return html @@ -398,31 +403,31 @@ class GitLabDropdown selector = ".dropdown-page-one #{selector}" $('body').on 'keydown', (e) => - currentKeyCode = e.keyCode + currentKeyCode = e.which if ARROW_KEY_CODES.indexOf(currentKeyCode) >= 0 e.preventDefault() - e.stopPropagation() + e.stopImmediatePropagation() - PREV_INDEX = CURRENT_INDEX + PREV_INDEX = currentIndex $listItems = $(selector, @dropdown) - if @options.filterable - $input.blur() + # if @options.filterable + # $input.blur() if currentKeyCode is 40 # Move down - CURRENT_INDEX += 1 if CURRENT_INDEX < ($listItems.length - 1) + currentIndex += 1 if currentIndex < ($listItems.length - 1) else if currentKeyCode is 38 # Move up - CURRENT_INDEX -= 1 if CURRENT_INDEX > 0 + currentIndex -= 1 if currentIndex > 0 - @highlightRowAtIndex($listItems, CURRENT_INDEX) if CURRENT_INDEX isnt PREV_INDEX + @highlightRowAtIndex($listItems, currentIndex) if currentIndex isnt PREV_INDEX return false if currentKeyCode is 13 - @selectRowAtIndex CURRENT_INDEX + @selectRowAtIndex currentIndex removeArrayKeyEvent: -> $('body').off 'keydown' @@ -437,13 +442,13 @@ class GitLabDropdown # Dropdown content scroll area $dropdownContent = $listItem.closest('.dropdown-content') - dropdownScrollTop = $dropdownContent.prop('scrollTop') - dropdownContentHeight = $dropdownContent.prop('offsetHeight') + dropdownScrollTop = $dropdownContent.scrollTop() + dropdownContentHeight = $dropdownContent.outerHeight() dropdownContentTop = $dropdownContent.prop('offsetTop') dropdownContentBottom = dropdownContentTop + dropdownContentHeight # Get the offset bottom of the list item - listItemHeight = $listItem.prop('offsetHeight') + listItemHeight = $listItem.outerHeight() listItemTop = $listItem.prop('offsetTop') listItemBottom = listItemTop + listItemHeight From ccc929d75d1f34accc4c7a6604c26cac6113a18c Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 31 Mar 2016 11:50:00 -0500 Subject: [PATCH 013/261] CSS tweaks - Use colors according to design - Fix search input' width --- app/assets/stylesheets/framework/variables.scss | 8 ++++---- app/assets/stylesheets/pages/search.scss | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index bb49ae396c..b86dbcb04b 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -199,11 +199,11 @@ $award-emoji-new-btn-icon-color: #dcdcdc; /* * Search Box */ -$search-input-border-color: $dropdown-input-focus-border; +$search-input-border-color: rgba(#4688f1, .8); $search-input-focus-shadow-color: $dropdown-input-focus-shadow; -$search-input-width: $dropdown-width; +$search-input-width: 244px; $location-badge-color: #aaa; $location-badge-bg: $gray-normal; +$location-badge-active-bg: #4f91f8; $location-icon-color: #e7e9ed; -$location-active-color: $gl-text-color; -$location-active-bg: $search-input-border-color; +$location-icon-active-color: #807e7e; diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index 3c74d25beb..9274796233 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -135,13 +135,13 @@ .location-badge { @include transition(all .15s); - background-color: $location-active-bg; + background-color: $location-badge-active-bg; color: $white-light; } .search-input-wrap { i { - color: $location-active-color; + color: $location-icon-active-color; } } From e3a983050714aa54a3c7911666cdd0abbc36e57a Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 31 Mar 2016 15:28:45 -0500 Subject: [PATCH 014/261] Skip default behaviour if we are clicking a result for the same location --- app/assets/javascripts/gl_dropdown.js.coffee | 2 +- app/assets/javascripts/search_autocomplete.js.coffee | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 0fea2a69cb..e4df7d46d5 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -174,7 +174,7 @@ class GitLabDropdown selected = self.rowClicked $(@) if self.options.clicked - self.options.clicked(selected) + self.options.clicked(selected, e) # Finds an element inside wrapper element getElement: (selector) -> diff --git a/app/assets/javascripts/search_autocomplete.js.coffee b/app/assets/javascripts/search_autocomplete.js.coffee index 030655491b..2656c6e30a 100644 --- a/app/assets/javascripts/search_autocomplete.js.coffee +++ b/app/assets/javascripts/search_autocomplete.js.coffee @@ -62,6 +62,8 @@ class @SearchAutocomplete search: fields: ['text'] data: @getData.bind(@) + selectable: true + clicked: @onClick.bind(@) getData: (term, callback) -> _this = @ @@ -268,3 +270,9 @@ class @SearchAutocomplete
  • Loading...
  • " @dropdownContent.html(html) + + onClick: (item, e) -> + if location.pathname.indexOf(item.url) isnt -1 + e.preventDefault() + @disableAutocomplete() + @searchInput.val('') From 932c2f59cbf8c7a393b5e23fc8b79ce9a7db76e4 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Tue, 29 Mar 2016 19:05:02 +0100 Subject: [PATCH 015/261] hide user profile activity graph on mobile and enable horizontal scroll for medium screens --- app/assets/stylesheets/framework/calendar.scss | 6 ++++++ app/views/users/show.html.haml | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/framework/calendar.scss b/app/assets/stylesheets/framework/calendar.scss index e3192823a1..0b3af592d4 100644 --- a/app/assets/stylesheets/framework/calendar.scss +++ b/app/assets/stylesheets/framework/calendar.scss @@ -1,3 +1,9 @@ +.calender-block { + @media (min-width: $screen-sm-min) and (max-width: $screen-lg-min) { + overflow-x: scroll; + } +} + .user-calendar-activities { .calendar_onclick_hr { padding: 0; diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index bca816f22c..0c4b6a5618 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -87,7 +87,7 @@ %div{ class: container_class } .tab-content #activity.tab-pane - .gray-content-block.white.second-block + .gray-content-block.calender-block.white.second-block.hidden-xs %div{ class: container_class } .user-calendar{data: {href: user_calendar_path}} %h4.center.light From ff676333d0d257869a917a5e48e9717d219b8311 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 31 Mar 2016 19:28:17 -0500 Subject: [PATCH 016/261] Add current element and event as params to clicked callback --- app/assets/javascripts/gl_dropdown.js.coffee | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index e4df7d46d5..d447013f63 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -171,10 +171,11 @@ class GitLabDropdown selector = ".dropdown-page-one .dropdown-content a" @dropdown.on "click", selector, (e) -> - selected = self.rowClicked $(@) + $el = $(@) + selected = self.rowClicked $el if self.options.clicked - self.options.clicked(selected, e) + self.options.clicked(selected, $el, e) # Finds an element inside wrapper element getElement: (selector) -> From ab42560ea6c2fade09b8436a893bf4a269e67c39 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 31 Mar 2016 19:28:41 -0500 Subject: [PATCH 017/261] Return selected object if toggleLabel option is not defined --- app/assets/javascripts/gl_dropdown.js.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index d447013f63..ca8d9a1694 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -352,6 +352,8 @@ class GitLabDropdown # Toggle the dropdown label if @options.toggleLabel $(@el).find(".dropdown-toggle-text").text @options.toggleLabel + else + selectedObject else if !value? field.remove() From 824fecb728ae7534b8ef56aaa6679814f3924cd5 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 31 Mar 2016 19:31:01 -0500 Subject: [PATCH 018/261] Bring back search context when chosing the same project/group --- .../javascripts/search_autocomplete.js.coffee | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/search_autocomplete.js.coffee b/app/assets/javascripts/search_autocomplete.js.coffee index 2656c6e30a..564fb265b9 100644 --- a/app/assets/javascripts/search_autocomplete.js.coffee +++ b/app/assets/javascripts/search_autocomplete.js.coffee @@ -104,6 +104,8 @@ class @SearchAutocomplete lastCategory = suggestion.category data.push + id: "#{suggestion.category.toLowerCase()}-#{suggestion.id}" + category: suggestion.category text: suggestion.label url: suggestion.url @@ -271,8 +273,27 @@ class @SearchAutocomplete " @dropdownContent.html(html) - onClick: (item, e) -> + onClick: (item, $el, e) -> if location.pathname.indexOf(item.url) isnt -1 e.preventDefault() + if not @badgePresent + if item.category is 'Projects' + @projectInputEl.val(item.id) + @addLocationBadge( + value: 'This project' + ) + + if item.category is 'Groups' + @groupInputEl.val(item.id) + @addLocationBadge( + value: 'This group' + ) + + $el.removeClass('is-active') @disableAutocomplete() - @searchInput.val('') + @searchInput.val('').focus() + + # We need to wait because of @skipBlurEvent + setTimeout( => + @onSearchInputFocus() + , 200) From 28266d9d610123d20333c6b05fcf83d1fb3e336a Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 1 Apr 2016 00:17:37 -0300 Subject: [PATCH 019/261] Added WikiLinkFilter --- lib/banzai/filter/wiki_link_filter.rb | 52 +++++++++++++++++++++++++++ lib/banzai/pipeline/wiki_pipeline.rb | 6 ++-- 2 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 lib/banzai/filter/wiki_link_filter.rb diff --git a/lib/banzai/filter/wiki_link_filter.rb b/lib/banzai/filter/wiki_link_filter.rb new file mode 100644 index 0000000000..7a585eff65 --- /dev/null +++ b/lib/banzai/filter/wiki_link_filter.rb @@ -0,0 +1,52 @@ +require 'uri' + +module Banzai + module Filter + # HTML filter that "fixes" relative links to files in a repository. + # + # Context options: + # :project_wiki + class WikiLinkFilter < HTML::Pipeline::Filter + + def call + return doc unless project_wiki? + + doc.search('a:not(.gfm)').each do |el| + process_link_attr el.attribute('href') + end + + doc + end + + protected + + def project_wiki? + !context[:project_wiki].nil? + end + + def process_link_attr(html_attr) + return if html_attr.blank? + + uri = URI(html_attr.value) + if uri.relative? && uri.path.present? && uri.path + html_attr.value = rebuild_wiki_uri(uri).to_s + end + rescue URI::Error + # noop + end + + def rebuild_wiki_uri(uri) + uri.path = ::File.join(project_wiki_base_path, uri.path) + uri + end + + def project_wiki + context[:project_wiki] + end + + def project_wiki_base_path + project_wiki && project_wiki.wiki_base_path + end + end + end +end diff --git a/lib/banzai/pipeline/wiki_pipeline.rb b/lib/banzai/pipeline/wiki_pipeline.rb index 0b5a9e0b2b..1cdb380896 100644 --- a/lib/banzai/pipeline/wiki_pipeline.rb +++ b/lib/banzai/pipeline/wiki_pipeline.rb @@ -2,8 +2,10 @@ module Banzai module Pipeline class WikiPipeline < FullPipeline def self.filters - @filters ||= super.insert_after(Filter::TableOfContentsFilter, - Filter::GollumTagsFilter) + @filters ||= begin + super.insert_after(Filter::TableOfContentsFilter, Filter::GollumTagsFilter) + .insert_after(Filter::GollumTagsFilter, Filter::WikiLinkFilter) + end end end end From cd4f3da750a172f14eac914f3f648cf0a803192a Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Fri, 1 Apr 2016 09:01:44 -0500 Subject: [PATCH 020/261] Add labels to block element --- app/views/projects/ci/builds/_build.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index d22d1da840..2cf9115e4d 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -39,7 +39,7 @@ %td = build.name - .pull-right + .label-container - if build.tags.any? - build.tags.each do |tag| %span.label.label-primary From bb2214bd64b621e3d0813696535ce4c960612ca1 Mon Sep 17 00:00:00 2001 From: Sandra Freihofer Date: Fri, 1 Apr 2016 16:20:29 +0200 Subject: [PATCH 021/261] Fix award emoji picker for relative_url MR !2888 caused a regression where the emoji picker does not load when using relative_url, because the path for the controller was hardcoded. --- app/assets/javascripts/awards_handler.coffee | 4 ++-- app/views/votes/_votes_block.html.haml | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 47b080406d..6a670d5e88 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -1,5 +1,5 @@ class @AwardsHandler - constructor: (@post_emoji_url, @noteable_type, @noteable_id, @aliases) -> + constructor: (@get_emojis_url, @post_emoji_url, @noteable_type, @noteable_id, @aliases) -> $(".js-add-award").on "click", (event) => event.stopPropagation() event.preventDefault() @@ -34,7 +34,7 @@ class @AwardsHandler $("#emoji_search").focus() else $('.js-add-award').addClass "is-loading" - $.get "/emojis", (response) => + $.get @get_emojis_url, (response) => $('.js-add-award').removeClass "is-loading" $(".js-award-holder").append response setTimeout => diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index 0264722977..8ffcdc4a32 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -15,12 +15,14 @@ - if current_user :javascript + var get_emojis_url = "#{emojis_path}"; var post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}"; var noteable_type = "#{votable.class.name.underscore}"; var noteable_id = "#{votable.id}"; var aliases = #{AwardEmoji.aliases.to_json}; window.awards_handler = new AwardsHandler( + get_emojis_url, post_emoji_url, noteable_type, noteable_id, From fb8b0041d86080f0f8d53f13be1016b0b199a47f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 30 Mar 2016 18:53:30 -0400 Subject: [PATCH 022/261] First pass at a Testing styleguide [ci skip] --- CONTRIBUTING.md | 2 +- doc/development/README.md | 1 + doc/development/testing.md | 105 +++++++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 doc/development/testing.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 511336f384..1f26a5d7ea 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -448,7 +448,7 @@ merge request: - multi-line method chaining style **Option B**: dot `.` on previous line - string literal quoting style **Option A**: single quoted by default 1. [Rails](https://github.com/bbatsov/rails-style-guide) -1. [Testing](https://github.com/thoughtbot/guides/tree/master/style/testing) +1. [Testing](doc/development/testing.md) 1. [CoffeeScript](https://github.com/thoughtbot/guides/tree/master/style/coffeescript) 1. [SCSS styleguide][scss-styleguide] 1. [Shell commands](doc/development/shell_commands.md) created by GitLab diff --git a/doc/development/README.md b/doc/development/README.md index 1b281809af..a8bc4fe5ab 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -9,4 +9,5 @@ - [Shell commands](shell_commands.md) in the GitLab codebase - [Sidekiq debugging](sidekiq_debugging.md) - [SQL guidelines](sql.md) for SQL guidelines +- [Testing standards and style guidelines](testing.md) - [UI guide](ui_guide.md) for building GitLab with existing css styles and elements diff --git a/doc/development/testing.md b/doc/development/testing.md new file mode 100644 index 0000000000..d37ef5d878 --- /dev/null +++ b/doc/development/testing.md @@ -0,0 +1,105 @@ +# Testing Standards and Style Guidelines + +This guide outlines standards and best practices for automated testing of GitLab +CE and EE. + +It is meant to be an _extension_ of the [thoughtbot testing +styleguide](https://github.com/thoughtbot/guides/tree/master/style/testing). If +this guide defines a rule that contradicts the thoughtbot guide, this guide +takes precedence. Some guidelines may be repeated verbatim to stress their +importance. + +## Factories + +GitLab uses [factory_girl] as a test +fixture replacement. + +- Factory definitions live in `spec/factories/`, named using the pluralization + of their corresponding model (`User` factories are defined in `users.rb`). +- There should be only one top-level factory definition per file. +- Make use of [Traits] to clean up definitions and usages. +- When defining a factory, don't define attributes that are not required for the + resulting record to pass validation. +- When instantiating from a factory, don't supply extraneous attributes that + aren't required by the test. + +[factory_girl]: https://github.com/thoughtbot/factory_girl +[Traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits + +## JavaScript + +GitLab uses [Teaspoon] to run its [Jasmine] JavaScript specs. They can be run on +the command line via `bundle exec teaspoon`, or via a web browser at +`http://localhost:3000/teaspoon` when the Rails server is running. + +- JavaScript tests live in `spec/javascripts/`, matching the folder structure of + `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js.coffee` has a corresponding + `spec/javascripts/behaviors/autosize_spec.js.coffee` file. +- Haml fixtures required for JavaScript tests live in + `spec/javascripts/fixtures`. They should contain the bare minimum amount of + markup necessary for the test. + + > **Warning:** Keep in mind that a Rails view may change and + invalidate your test, but everything will still pass because your fixture + doesn't reflect the latest view. + +- Keep in mind that in a CI environment, these tests are run in a headless + browser and you will not have access to certain APIs, such as + [`Notification`](https://developer.mozilla.org/en-US/docs/Web/API/notification), + which will have to be stubbed. + +[Teaspoon]: https://github.com/modeset/teaspoon +[Jasmine]: https://github.com/jasmine/jasmine + +## RSpec + +### General Guidelines + +- Use a single, top-level `describe ClassName` block. +- Use `described_class` instead of repeating the class name being described. +- Use `.method` to describe class methods and `#method` to describe instance + methods. +- Use `context` to test branching logic. +- Don't `describe` symbols (see [Gotchas](gotchas.md#dont-describe-symbols)). +- Prefer `not_to` to `to_not`. +- Try to match the ordering of tests to the ordering within the class. + +### Test speed + +GitLab has a massive test suite that, without parallelization, can take more +than an hour to run. It's important that we make an effort to write tests that +are accurate and effective _as well as_ fast. + +Here are some things to keep in mind regarding test performance: + +- `double` and `spy` are faster than `FactoryGirl.build(...)` +- `FactoryGirl.build(...)` and `.build_stubbed` are faster than `.create`. +- Don't `create` an object when `build`, `build_stubbed`, `attributes_for`, + `spy`, or `double` will do. Database persistence is slow! +- Use `create(:empty_project)` instead of `create(:project)` when you don't need + the underlying repository. Filesystem operations are slow! +- Don't mark a feature as requiring JavaScript (through `@javascript` in + Spinach or `js: true` in RSpec) unless it's _actually_ required for the test + to be valid. Headless browser testing is slow! + +### Features / Integration + +- Feature specs live in `spec/features/` and should be named + `ROLE_ACTION_spec.rb`, such as `user_changes_password_spec.rb`. +- Use only one `feature` block per feature spec file. +- Use scenario titles that describe the success and failure paths. +- Avoid scenario titles that add no information, such as "successfully." +- Avoid scenario titles that repeat the feature title. + +## Spinach (feature) tests + +GitLab [moved from Cucumber to Spinach](https://github.com/gitlabhq/gitlabhq/pull/1426) +for its feature/integration tests in September 2012. + +As of March 2016, we are [trying to avoid adding new Spinach +tests](https://gitlab.com/gitlab-org/gitlab-ce/issues/14121) going forward, +opting for [RSpec feature](#features-integration) specs. + +Adding new Spinach scenarios is acceptable _only if_ the new scenario requires +no more than one new `step` definition. If more than that is required, the +test should be re-implemented using RSpec instead. From 0b9d9816f8ef1d871a050cea5d8bc3d9203c3d18 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 30 Mar 2016 19:11:02 -0400 Subject: [PATCH 023/261] Add a note about Four-Phase Test to Testing guide [ci skip] --- doc/development/testing.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/development/testing.md b/doc/development/testing.md index d37ef5d878..a7e85ecebe 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -63,6 +63,8 @@ the command line via `bundle exec teaspoon`, or via a web browser at - Don't `describe` symbols (see [Gotchas](gotchas.md#dont-describe-symbols)). - Prefer `not_to` to `to_not`. - Try to match the ordering of tests to the ordering within the class. +- Try to follow the [Four-Phase Test](https://robots.thoughtbot.com/four-phase-test) + pattern, using newlines to separate phases. ### Test speed From 60f4081e135bdcd893d60192e652c4b829c656dd Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 1 Apr 2016 20:29:48 -0400 Subject: [PATCH 024/261] Factories don't have to be limited to `ActiveRecord` objects [ci skip] --- doc/development/testing.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/development/testing.md b/doc/development/testing.md index a7e85ecebe..3d1c4ccab4 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -22,6 +22,8 @@ fixture replacement. resulting record to pass validation. - When instantiating from a factory, don't supply extraneous attributes that aren't required by the test. +- Factories don't have to be limited to `ActiveRecord` objects. + [See example](https://gitlab.com/gitlab-org/gitlab-ce/commit/0b8cefd3b2385a21cfed779bd659978c0402766d). [factory_girl]: https://github.com/thoughtbot/factory_girl [Traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits From 2e83b562030e078ce6d37f81915590426a57c820 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 1 Apr 2016 20:30:24 -0400 Subject: [PATCH 025/261] Add a section about `let` to the Testing guide [ci skip] --- doc/development/testing.md | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/doc/development/testing.md b/doc/development/testing.md index 3d1c4ccab4..187d852e53 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -11,8 +11,7 @@ importance. ## Factories -GitLab uses [factory_girl] as a test -fixture replacement. +GitLab uses [factory_girl] as a test fixture replacement. - Factory definitions live in `spec/factories/`, named using the pluralization of their corresponding model (`User` factories are defined in `users.rb`). @@ -65,8 +64,30 @@ the command line via `bundle exec teaspoon`, or via a web browser at - Don't `describe` symbols (see [Gotchas](gotchas.md#dont-describe-symbols)). - Prefer `not_to` to `to_not`. - Try to match the ordering of tests to the ordering within the class. -- Try to follow the [Four-Phase Test](https://robots.thoughtbot.com/four-phase-test) - pattern, using newlines to separate phases. +- Try to follow the [Four-Phase Test][four-phase-test] pattern, using newlines + to separate phases. + +[four-phase-test]: https://robots.thoughtbot.com/four-phase-test + +### `let` variables + +GitLab's RSpec suite has made extensive use of `let` variables to reduce +duplication. However, this sometimes [comes at the cost of clarity][lets-not], +so we need to set some guidelines for their use going forward: + +- `let` variables are preferable to instance variables. Local variables are + preferable to `let` variables. +- Use `let` to reduce duplication throughout an entire spec file. +- Don't use `let` to define variables used by a single test; define them as + local variables inside the test's `it` block. +- Don't define a `let` variable inside the top-level `describe` block that's + only used in a more deeply-nested `context` or `describe` block. Keep the + definition as close as possible to where it's used. +- Try to avoid overriding the definition of one `let` variable with another. +- Don't define a `let` variable that's only used by the definition of another. + Use a helper method instead. + +[lets-not]: https://robots.thoughtbot.com/lets-not ### Test speed From 6ffda88c973ce9be263d0e9142a3105f2cccf000 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 1 Apr 2016 20:36:44 -0400 Subject: [PATCH 026/261] Add a link back to Development documentation to Testing guide [ci skip] --- doc/development/testing.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/development/testing.md b/doc/development/testing.md index 187d852e53..23417845f1 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -128,3 +128,7 @@ opting for [RSpec feature](#features-integration) specs. Adding new Spinach scenarios is acceptable _only if_ the new scenario requires no more than one new `step` definition. If more than that is required, the test should be re-implemented using RSpec instead. + +--- + +[Return to Development documentation](README.md) From deca9fc6266dfd5588b23f64157ddb05e5713412 Mon Sep 17 00:00:00 2001 From: Alessio Biancalana Date: Sat, 2 Apr 2016 14:44:50 +0200 Subject: [PATCH 027/261] Added bottom margin to CLI instructions --- app/assets/stylesheets/pages/projects.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 4e6aa8cd1a..61150746c7 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -315,7 +315,7 @@ pre.light-well { } .git-empty { - margin: 0 7px; + margin: 0 7px 7px; h5 { color: #5c5d5e; From 261c8e765f5b13dd627fcda5ca1ae263ecfad0c8 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Thu, 31 Mar 2016 23:33:32 +0100 Subject: [PATCH 028/261] fix missing filters on status tab when user swithches to another state --- app/assets/javascripts/issues.js.coffee | 15 +++++++++++ app/assets/javascripts/lib/url_utility.js | 33 +++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 app/assets/javascripts/lib/url_utility.js diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index b1479bfb44..8d069a209d 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -26,6 +26,20 @@ $(".selected_issue").bind "change", Issues.checkChanged + # Update state filters if present in page + updateStateFilters: -> + stateFilters = $('.issues-state-filters') + newParams = {} + paramKeys = ['author_id', 'label_name', 'milestone_title', 'assignee_id', 'issue_search'] + + for paramKey in paramKeys + newParams[paramKey] = getUrlParameter(paramKey) or '' + + if stateFilters.length + stateFilters.find('a').each -> + initialUrl = $(this).attr 'href' + $(this).attr 'href', mergeUrlParams(newParams, initialUrl) + # Make sure we trigger ajax request only after user stop typing initSearch: -> @timer = null @@ -54,6 +68,7 @@ # Change url so if user reload a page - search results are saved history.replaceState {page: issuesUrl}, document.title, issuesUrl Issues.reload() + Issues.updateStateFilters() dataType: "json" checkChanged: -> diff --git a/app/assets/javascripts/lib/url_utility.js b/app/assets/javascripts/lib/url_utility.js new file mode 100644 index 0000000000..5fa3a4c69d --- /dev/null +++ b/app/assets/javascripts/lib/url_utility.js @@ -0,0 +1,33 @@ +function getUrlParameter(sParam) { + var sPageURL = decodeURIComponent(window.location.search.substring(1)), + sURLVariables = sPageURL.split('&'), + sParameterName, + i; + + for (i = 0; i < sURLVariables.length; i++) { + sParameterName = sURLVariables[i].split('='); + + if (sParameterName[0] === sParam) { + return sParameterName[1] === undefined ? true : sParameterName[1]; + } + } +} + +/** + * @param {Object} params - url keys and value to merge + * @param {String} url + */ +function mergeUrlParams(params, url){ + var newUrl = decodeURIComponent(url); + + Object.keys(params).forEach(function(paramName) { + var pattern = new RegExp('\\b('+paramName+'=).*?(&|$)') + if (url.search(pattern) >= 0){ + newUrl = newUrl.replace(pattern,'$1' + params[paramName] + '$2'); + } else { + newUrl = newUrl + (newUrl.indexOf('?') > 0 ? '&' : '?') + paramName + '=' + params[paramName] + } + }); + + return newUrl; +} \ No newline at end of file From db8836ca84ab86dc39b4d8b3282603c724a78e67 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Sat, 2 Apr 2016 16:56:31 +0100 Subject: [PATCH 029/261] attach the utitlity function to the global scope with some changes --- app/assets/javascripts/issues.js.coffee | 4 +-- app/assets/javascripts/lib/url_utility.js | 33 ------------------- .../javascripts/lib/url_utility.js.coffee | 31 +++++++++++++++++ 3 files changed, 33 insertions(+), 35 deletions(-) delete mode 100644 app/assets/javascripts/lib/url_utility.js create mode 100644 app/assets/javascripts/lib/url_utility.js.coffee diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index 8d069a209d..0d9f2094c2 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -33,12 +33,12 @@ paramKeys = ['author_id', 'label_name', 'milestone_title', 'assignee_id', 'issue_search'] for paramKey in paramKeys - newParams[paramKey] = getUrlParameter(paramKey) or '' + newParams[paramKey] = gl.utils.getUrlParameter(paramKey) or '' if stateFilters.length stateFilters.find('a').each -> initialUrl = $(this).attr 'href' - $(this).attr 'href', mergeUrlParams(newParams, initialUrl) + $(this).attr 'href', gl.utils.mergeUrlParams(newParams, initialUrl) # Make sure we trigger ajax request only after user stop typing initSearch: -> diff --git a/app/assets/javascripts/lib/url_utility.js b/app/assets/javascripts/lib/url_utility.js deleted file mode 100644 index 5fa3a4c69d..0000000000 --- a/app/assets/javascripts/lib/url_utility.js +++ /dev/null @@ -1,33 +0,0 @@ -function getUrlParameter(sParam) { - var sPageURL = decodeURIComponent(window.location.search.substring(1)), - sURLVariables = sPageURL.split('&'), - sParameterName, - i; - - for (i = 0; i < sURLVariables.length; i++) { - sParameterName = sURLVariables[i].split('='); - - if (sParameterName[0] === sParam) { - return sParameterName[1] === undefined ? true : sParameterName[1]; - } - } -} - -/** - * @param {Object} params - url keys and value to merge - * @param {String} url - */ -function mergeUrlParams(params, url){ - var newUrl = decodeURIComponent(url); - - Object.keys(params).forEach(function(paramName) { - var pattern = new RegExp('\\b('+paramName+'=).*?(&|$)') - if (url.search(pattern) >= 0){ - newUrl = newUrl.replace(pattern,'$1' + params[paramName] + '$2'); - } else { - newUrl = newUrl + (newUrl.indexOf('?') > 0 ? '&' : '?') + paramName + '=' + params[paramName] - } - }); - - return newUrl; -} \ No newline at end of file diff --git a/app/assets/javascripts/lib/url_utility.js.coffee b/app/assets/javascripts/lib/url_utility.js.coffee new file mode 100644 index 0000000000..abd556e0b4 --- /dev/null +++ b/app/assets/javascripts/lib/url_utility.js.coffee @@ -0,0 +1,31 @@ +((w) -> + + w.gl ?= {} + w.gl.utils ?= {} + + w.gl.utils.getUrlParameter = (sParam) -> + sPageURL = decodeURIComponent(window.location.search.substring(1)) + sURLVariables = sPageURL.split('&') + sParameterName = undefined + i = 0 + while i < sURLVariables.length + sParameterName = sURLVariables[i].split('=') + if sParameterName[0] is sParam + return if sParameterName[1] is undefined then true else sParameterName[1] + i++ + + # # + # @param {Object} params - url keys and value to merge + # @param {String} url + # # + w.gl.utils.mergeUrlParams = (params, url) -> + newUrl = decodeURIComponent(url) + for paramName, paramValue of params + pattern = new RegExp "\\b(#{paramName}=).*?(&|$)" + if url.search(pattern) >= 0 + newUrl = newUrl.replace pattern, "$1#{paramValue}$2" + else + newUrl = "#{newUrl}#{(if newUrl.indexOf('?') > 0 then '&' else '?')}#{paramName}=#{paramValue}" + newUrl + +) window From 65fa7161736f6688719b6e66952c611770a64473 Mon Sep 17 00:00:00 2001 From: Baldinof Date: Sun, 3 Apr 2016 15:17:14 +0000 Subject: [PATCH 030/261] Fix rubocop in unlink fork service specs --- spec/services/projects/unlink_fork_service_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/services/projects/unlink_fork_service_spec.rb b/spec/services/projects/unlink_fork_service_spec.rb index f287b0a59b..23f5555d3e 100644 --- a/spec/services/projects/unlink_fork_service_spec.rb +++ b/spec/services/projects/unlink_fork_service_spec.rb @@ -25,8 +25,8 @@ describe Projects::UnlinkForkService, services: true do end it 'remove fork relation' do - expect(fork_project.forked_project_link).to receive(:destroy) + expect(fork_project.forked_project_link).to receive(:destroy) - subject.execute + subject.execute end end From b7f0b22b9fe76f634d9e8cbce03cfaac41f333e6 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 29 Mar 2016 18:17:31 +0100 Subject: [PATCH 031/261] Started refactoring of note form --- app/assets/javascripts/notes.js.coffee | 9 ----- app/assets/stylesheets/framework/common.scss | 7 ---- app/assets/stylesheets/pages/note_form.scss | 7 ++-- app/views/projects/_zen.html.haml | 7 ++-- app/views/projects/notes/_form.html.haml | 2 +- .../projects/notes/_notes_with_form.html.haml | 34 +++++++++++-------- 6 files changed, 27 insertions(+), 39 deletions(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index ff06c57f2b..9963299988 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -251,14 +251,9 @@ class @Notes Sets some hidden fields in the form. ### setupMainTargetNoteForm: -> - # find the form form = $(".js-new-note-form") - # insert the form after the button - form.clone().replaceAll $(".js-main-target-form") - form = form.prev("form") - # show the form @setupNoteForm(form) @@ -266,10 +261,6 @@ class @Notes form.removeClass "js-new-note-form" form.addClass "js-main-target-form" - # remove unnecessary fields and buttons - form.find("#note_line_code").remove() - form.find(".js-close-discussion-note-form").remove() - ### General note form setup. diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 9b676d759e..91ac5af3c9 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -125,13 +125,6 @@ p.time { height: 150px; } -// Fixes alignment on notes. -.new_note { - label { - text-align: left; - } -} - // Fix issue with notes & lists creating a bunch of bottom borders. li.note { img { max-width: 100% } diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 655f88b0c2..91b5216a22 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -17,16 +17,17 @@ } .diff-file, .discussion { - .new_note { + .new-note { margin: 0; border: none; } } -.new_note { + +.new-note { display: none; } -.new_note, .note-edit-form { +.new-note, .note-edit-form { .note-form-actions { margin-top: $gl-padding; } diff --git a/app/views/projects/_zen.html.haml b/app/views/projects/_zen.html.haml index e701253d7d..93a7e1cfeb 100644 --- a/app/views/projects/_zen.html.haml +++ b/app/views/projects/_zen.html.haml @@ -5,8 +5,7 @@ = f.text_area attr, class: classes - else = text_area_tag attr, nil, class: classes - %a.js-zen-enter(tabindex="-1" href="#") - = icon('expand') - Edit in fullscreen - %a.js-zen-leave(tabindex="-1" href="#") + %a.js-zen-enter{ tabindex: "-1", href: "#" } + Go full screen + %a.js-zen-leave{ tabindex: "-1", href: "#" } = icon('compress') diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index f675f092da..8517e5a765 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -1,4 +1,4 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new_note js-new-note-form js-quick-submit common-note-form gfm-form" }, authenticity_token: true do |f| += form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new-note js-new-note-form js-quick-submit common-note-form gfm-form" }, authenticity_token: true do |f| = hidden_field_tag :view, diff_view = hidden_field_tag :line_type = note_target_fields(@note) diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml index 910eb6cf66..003f42db0b 100644 --- a/app/views/projects/notes/_notes_with_form.html.haml +++ b/app/views/projects/notes/_notes_with_form.html.haml @@ -1,20 +1,24 @@ %ul#notes-list.notes.main-notes-list.timeline = render "projects/notes/notes" -.js-notes-busy - -.js-main-target-form -- if can? current_user, :create_note, @project - = render "projects/notes/form", view: diff_view -- else - .disabled-comment-area - .disabled-profile - .disabled-comment - %span - Please - = link_to "register",new_user_session_path - or - = link_to "login",new_user_session_path - to post a comment + %li.timeline-entry + .timeline-icon + - if can? current_user, :create_note, @project + %a.author_link{ href: user_path(current_user) } + = image_tag avatar_icon(current_user), alt: current_user.to_reference, class: 'avatar s40' + .timeline-content + .js-main-target-form + - if can? current_user, :create_note, @project + = render "projects/notes/form", view: diff_view + - else + .disabled-comment-area + .disabled-profile + .disabled-comment + %span + Please + = link_to "register",new_user_session_path + or + = link_to "login",new_user_session_path + to post a comment :javascript var notes = new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{diff_view}") From 0331fa3f3d27dbffdd2144073cf9c62fe7837aa1 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 30 Mar 2016 08:28:06 +0100 Subject: [PATCH 032/261] Restyling on elements in comment form --- app/assets/javascripts/notes.js.coffee | 5 -- app/assets/javascripts/zen_mode.js.coffee | 2 +- .../stylesheets/framework/markdown_area.scss | 15 ++++++ .../stylesheets/framework/typography.scss | 2 +- app/assets/stylesheets/framework/zen.scss | 46 ++++++++----------- app/assets/stylesheets/pages/note_form.scss | 28 ++++++----- app/views/projects/_md_preview.html.haml | 18 ++++---- app/views/projects/_zen.html.haml | 8 ++-- app/views/projects/notes/_edit_form.html.haml | 2 +- app/views/projects/notes/_form.html.haml | 2 +- .../projects/notes/_notes_with_form.html.haml | 24 +++++----- 11 files changed, 76 insertions(+), 76 deletions(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 9963299988..24569c4f97 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -1,5 +1,4 @@ #= require autosave -#= require autosize #= require dropzone #= require dropzone_input #= require gfm_auto_complete @@ -288,7 +287,6 @@ class @Notes else previewButton.removeClass("turn-on").addClass "turn-off" - autosize(textarea) new Autosave textarea, [ "Note" form.find("#note_commit_id").val() @@ -370,9 +368,6 @@ class @Notes textarea = form.find("textarea") textarea.focus() - if isNewForm - autosize(textarea) - # HACK (rspeicher/DouweM): Work around a Chrome 43 bug(?). # The textarea has the correct value, Chrome just won't show it unless we # modify it, so let's clear it and re-set it! diff --git a/app/assets/javascripts/zen_mode.js.coffee b/app/assets/javascripts/zen_mode.js.coffee index e1c5446eaa..99f35ecfb0 100644 --- a/app/assets/javascripts/zen_mode.js.coffee +++ b/app/assets/javascripts/zen_mode.js.coffee @@ -42,7 +42,7 @@ class @ZenMode $(e.currentTarget).trigger('zen_mode:leave') $(document).on 'zen_mode:enter', (e) => - @enter(e.target.parentNode) + @enter($(e.target).closest('.md-area').find('.zen-backdrop')) $(document).on 'zen_mode:leave', (e) => @exit() diff --git a/app/assets/stylesheets/framework/markdown_area.scss b/app/assets/stylesheets/framework/markdown_area.scss index 8328aac4e7..9e2240bd21 100644 --- a/app/assets/stylesheets/framework/markdown_area.scss +++ b/app/assets/stylesheets/framework/markdown_area.scss @@ -65,6 +65,21 @@ position: relative; } +.md-header { + .nav-links { + .active { + a { + border-bottom-color: #000; + } + } + + a { + padding-top: 0; + line-height: 1; + } + } +} + .referenced-users { color: #4c4e54; padding-top: 10px; diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index b1886fbe67..2ed0f82f91 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -244,7 +244,7 @@ a > code { * Textareas intended for GFM * */ -textarea.js-gfm-input { +.js-gfm-input { font-family: $monospace_font; color: $gl-text-color; } diff --git a/app/assets/stylesheets/framework/zen.scss b/app/assets/stylesheets/framework/zen.scss index 02e24ec7c4..e75f4471e6 100644 --- a/app/assets/stylesheets/framework/zen.scss +++ b/app/assets/stylesheets/framework/zen.scss @@ -1,26 +1,4 @@ .zennable { - a.js-zen-enter { - color: $gl-gray; - position: absolute; - top: 0; - right: 4px; - line-height: 56px; - } - - a.js-zen-leave { - display: none; - color: $gl-text-color; - position: absolute; - top: 10px; - right: 10px; - padding: 5px; - font-size: 36px; - - &:hover { - color: #111; - } - } - .zen-backdrop { &.fullscreen { background-color: white; @@ -47,11 +25,7 @@ margin: 0 auto; } - a.js-zen-enter { - display: none; - } - - a.js-zen-leave { + .zen-control-leave { display: block; position: absolute; top: 0; @@ -59,3 +33,21 @@ } } } + +.zen-cotrol { + color: #555; + line-height: 31px; +} + +.zen-control-leave { + display: none; + color: $gl-text-color; + position: absolute; + right: 10px; + padding: 5px; + font-size: 36px; + + &:hover { + color: #111; + } +} diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 91b5216a22..554f87c05a 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -1,10 +1,6 @@ /** * Note Form */ - -.comment-btn { - @extend .btn-create; -} .reply-btn { @extend .btn-primary; margin: 10px $gl-padding; @@ -42,18 +38,22 @@ max-width: 100%; } - .note_text { - width: 100%; - } - .comment-hints { margin-top: -12px; } } -/* loading indicator */ -.notes-busy { - margin: 18px; +.note-textarea { + padding-left: 0; + padding-right: 0; + font-family: $regular_font; + border-left: 0; + border-right: 0; + resize: none!important; // TODO: Find a way to remove this !important + + &:focus { + outline: 0; + } } .note-image-attach { @@ -64,11 +64,9 @@ .common-note-form { margin: 0; - background: #fff; padding: $gl-padding; - margin-left: -$gl-padding; - margin-right: -$gl-padding; - margin-bottom: -$gl-padding; + border: 1px solid #E5E5E5; + border-radius: $border-radius-base; } .note-form-actions { diff --git a/app/views/projects/_md_preview.html.haml b/app/views/projects/_md_preview.html.haml index 1fb37ef662..32c4433861 100644 --- a/app/views/projects/_md_preview.html.haml +++ b/app/views/projects/_md_preview.html.haml @@ -1,18 +1,20 @@ .md-area .md-header.clearfix - %ul.nav-links + %ul.nav-links.pull-left %li.active - %a.js-md-write-button(href="#md-write-holder" tabindex="-1") + %a.js-md-write-button{ href: "#md-write-holder" } Write %li - %a.js-md-preview-button(href="#md-preview-holder" tabindex="-1") + %a.js-md-preview-button{ href: "#md-preview-holder" } Preview + .pull-right + %a.zen-cotrol.js-zen-enter{ href: "#" } + Go full screen - %div - .md-write-holder - = yield - .md.md-preview-holder.hide - .js-md-preview{class: (preview_class if defined?(preview_class))} + .md-write-holder + = yield + .md.md-preview-holder.hide + .js-md-preview{class: (preview_class if defined?(preview_class))} - if defined?(referenced_users) && referenced_users %div.referenced-users.hide diff --git a/app/views/projects/_zen.html.haml b/app/views/projects/_zen.html.haml index 93a7e1cfeb..efa60c88b1 100644 --- a/app/views/projects/_zen.html.haml +++ b/app/views/projects/_zen.html.haml @@ -2,10 +2,8 @@ .zen-backdrop - classes << ' js-gfm-input js-autosize markdown-area' - if defined?(f) && f - = f.text_area attr, class: classes + = f.text_area attr, class: classes, placeholder: "Write a comment or drag your files here..." - else - = text_area_tag attr, nil, class: classes - %a.js-zen-enter{ tabindex: "-1", href: "#" } - Go full screen - %a.js-zen-leave{ tabindex: "-1", href: "#" } + = text_area_tag attr, nil, class: classes, placeholder: "Write a comment or drag your files here..." + %a.zen-cotrol.zen-control-leave.js-zen-leave{ href: "#" } = icon('compress') diff --git a/app/views/projects/notes/_edit_form.html.haml b/app/views/projects/notes/_edit_form.html.haml index 2999befffc..3a5551b08c 100644 --- a/app/views/projects/notes/_edit_form.html.haml +++ b/app/views/projects/notes/_edit_form.html.haml @@ -2,7 +2,7 @@ = form_for note, url: namespace_project_note_path(@project.namespace, @project, note), method: :put, remote: true, authenticity_token: true, html: { class: 'edit-note js-quick-submit' } do |f| = note_target_fields(note) = render layout: 'projects/md_preview', locals: { preview_class: 'md-preview' } do - = render 'projects/zen', f: f, attr: :note, classes: 'note_text js-note-text js-task-list-field' + = render 'projects/zen', f: f, attr: :note, classes: 'note-textarea js-note-text js-task-list-field' = render 'projects/notes/hints' .note-form-actions.clearfix diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index 8517e5a765..c446ecec2c 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -8,7 +8,7 @@ = f.hidden_field :noteable_type = render layout: 'projects/md_preview', locals: { preview_class: "md-preview", referenced_users: true } do - = render 'projects/zen', f: f, attr: :note, classes: 'note_text js-note-text' + = render 'projects/zen', f: f, attr: :note, classes: 'note-textarea js-note-text' = render 'projects/notes/hints' .error-alert diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml index 003f42db0b..8a5a319995 100644 --- a/app/views/projects/notes/_notes_with_form.html.haml +++ b/app/views/projects/notes/_notes_with_form.html.haml @@ -7,18 +7,18 @@ = image_tag avatar_icon(current_user), alt: current_user.to_reference, class: 'avatar s40' .timeline-content .js-main-target-form - - if can? current_user, :create_note, @project - = render "projects/notes/form", view: diff_view - - else - .disabled-comment-area - .disabled-profile - .disabled-comment - %span - Please - = link_to "register",new_user_session_path - or - = link_to "login",new_user_session_path - to post a comment + - if can? current_user, :create_note, @project + = render "projects/notes/form", view: diff_view + - else + .disabled-comment-area + .disabled-profile + .disabled-comment + %span + Please + = link_to "register",new_user_session_path + or + = link_to "login",new_user_session_path + to post a comment :javascript var notes = new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{diff_view}") From 3c2b0e7572c11d24b96f2762a03c8cc47f11f510 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 30 Mar 2016 10:00:43 +0100 Subject: [PATCH 033/261] Added toolbar to comment form --- app/assets/javascripts/issue.js.coffee | 15 ---- .../javascripts/merge_request.js.coffee | 16 ---- app/assets/javascripts/notes.js.coffee | 6 ++ .../stylesheets/framework/markdown_area.scss | 12 +-- app/assets/stylesheets/framework/zen.scss | 73 +++++++++++-------- app/assets/stylesheets/pages/note_form.scss | 59 +++++++++++---- app/views/projects/_md_preview.html.haml | 13 ++-- app/views/projects/_zen.html.haml | 17 ++--- app/views/projects/notes/_hints.html.haml | 28 +++++-- 9 files changed, 130 insertions(+), 109 deletions(-) diff --git a/app/assets/javascripts/issue.js.coffee b/app/assets/javascripts/issue.js.coffee index d663e34871..44a8aa6883 100644 --- a/app/assets/javascripts/issue.js.coffee +++ b/app/assets/javascripts/issue.js.coffee @@ -6,25 +6,10 @@ class @Issue constructor: -> # Prevent duplicate event bindings @disableTaskList() - @fixAffixScroll() if $('a.btn-close').length @initTaskList() @initIssueBtnEventListeners() - fixAffixScroll: -> - fixAffix = -> - $discussion = $('.issuable-discussion') - $sidebar = $('.issuable-sidebar') - if $sidebar.hasClass('no-affix') - $sidebar.removeClass(['affix-top','affix']) - discussionHeight = $discussion.height() - sidebarHeight = $sidebar.height() - if sidebarHeight > discussionHeight - $discussion.height(sidebarHeight + 50) - $sidebar.addClass('no-affix') - $(window).on('resize', fixAffix) - fixAffix() - initTaskList: -> $('.detail-page-description .js-task-list-container').taskList('enable') $(document).on 'tasklist:changed', '.detail-page-description .js-task-list-container', @updateTaskList diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee index 6af5a48a0b..1f46e33142 100644 --- a/app/assets/javascripts/merge_request.js.coffee +++ b/app/assets/javascripts/merge_request.js.coffee @@ -15,8 +15,6 @@ class @MergeRequest this.$('.show-all-commits').on 'click', => this.showAllCommits() - @fixAffixScroll(); - @initTabs() # Prevent duplicate event bindings @@ -30,20 +28,6 @@ class @MergeRequest $: (selector) -> this.$el.find(selector) - fixAffixScroll: -> - fixAffix = -> - $discussion = $('.issuable-discussion') - $sidebar = $('.issuable-sidebar') - if $sidebar.hasClass('no-affix') - $sidebar.removeClass(['affix-top','affix']) - discussionHeight = $discussion.height() - sidebarHeight = $sidebar.height() - if sidebarHeight > discussionHeight - $discussion.height(sidebarHeight + 50) - $sidebar.addClass('no-affix') - $(window).on('resize', fixAffix) - fixAffix() - initTabs: -> if @opts.action != 'new' # `MergeRequests#new` has no tab-persisting or lazy-loading behavior diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 24569c4f97..864156cb71 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -1,4 +1,5 @@ #= require autosave +#= require autosize #= require dropzone #= require dropzone_input #= require gfm_auto_complete @@ -287,6 +288,8 @@ class @Notes else previewButton.removeClass("turn-on").addClass "turn-off" + autosize(textarea) + new Autosave textarea, [ "Note" form.find("#note_commit_id").val() @@ -368,6 +371,9 @@ class @Notes textarea = form.find("textarea") textarea.focus() + if isNewForm + autosize(textarea) + # HACK (rspeicher/DouweM): Work around a Chrome 43 bug(?). # The textarea has the correct value, Chrome just won't show it unless we # modify it, so let's clear it and re-set it! diff --git a/app/assets/stylesheets/framework/markdown_area.scss b/app/assets/stylesheets/framework/markdown_area.scss index 9e2240bd21..ae23e19172 100644 --- a/app/assets/stylesheets/framework/markdown_area.scss +++ b/app/assets/stylesheets/framework/markdown_area.scss @@ -1,9 +1,7 @@ .div-dropzone-wrapper { .div-dropzone { position: relative; - padding: 0; - border: 0; - margin-bottom: 5px; + margin-bottom: -5px; .div-dropzone-focus { border-color: #66afe9 !important; @@ -25,12 +23,10 @@ .div-dropzone-spinner { position: absolute; - top: 100%; - left: 100%; - margin-top: -1.1em; - margin-left: -1.1em; + bottom: 10px; + right: 5px; opacity: 0; - font-size: 30px; + font-size: 20px; transition: opacity 200ms ease-in-out; } diff --git a/app/assets/stylesheets/framework/zen.scss b/app/assets/stylesheets/framework/zen.scss index e75f4471e6..951d794916 100644 --- a/app/assets/stylesheets/framework/zen.scss +++ b/app/assets/stylesheets/framework/zen.scss @@ -1,42 +1,51 @@ -.zennable { - .zen-backdrop { - &.fullscreen { - background-color: white; - position: fixed; +.zen-backdrop { + &.fullscreen { + background-color: white; + position: fixed; + top: 0; + bottom: 0; + left: 0; + right: 0; + z-index: 1031; + + textarea { + border: none; + box-shadow: none; + border-radius: 0; + color: #000; + font-size: 20px; + line-height: 26px; + padding: 30px; + display: block; + outline: none; + resize: none; + height: 100vh; + max-width: 900px; + margin: 0 auto; + } + + .zen-control-leave { + display: block; + position: absolute; top: 0; - bottom: 0; - left: 0; - right: 0; - z-index: 1031; - - textarea { - border: none; - box-shadow: none; - border-radius: 0; - color: #000; - font-size: 20px; - line-height: 26px; - padding: 30px; - display: block; - outline: none; - resize: none; - height: 100vh; - max-width: 900px; - margin: 0 auto; - } - - .zen-control-leave { - display: block; - position: absolute; - top: 0; - } } } } .zen-cotrol { + padding: 0; color: #555; - line-height: 31px; + background: none; + border: 0; +} + +.zen-control-full { + color: #959494; + + &:hover { + color: $gl-link-color; + text-decoration: none; + } } .zen-control-leave { diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 554f87c05a..561b37e582 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -47,9 +47,7 @@ padding-left: 0; padding-right: 0; font-family: $regular_font; - border-left: 0; - border-right: 0; - resize: none!important; // TODO: Find a way to remove this !important + border: 0; &:focus { outline: 0; @@ -63,10 +61,11 @@ } .common-note-form { - margin: 0; - padding: $gl-padding; - border: 1px solid #E5E5E5; - border-radius: $border-radius-base; + .md-area { + padding: $gl-padding-top $gl-padding; + border: 1px solid #E5E5E5; + border-radius: $border-radius-base; + } } .note-form-actions { @@ -151,11 +150,43 @@ } } -.comment-hints { - color: #999; - background: #fff; - padding: 7px; - margin-top: -7px; - border: 1px solid $border-color; - font-size: 13px; +.comment-toolbar { + padding-top: $gl-padding-top; + border-top: 1px solid $border-color; +} + +.toolbar-button { + float: left; + margin-right: $gl-padding; + padding: 0; + background: none; + border: 0; + color: #959494; + font-size: 14px; + line-height: 16px; + + &:hover, + &:focus { + color: $gl-link-color; + outline: 0; + } + + &:last-child { + margin-right: 0; + } +} + +.toolbar-button-icon { + position: relative; + top: 1px; + margin-right: 3px; + color: inherit; + font-size: 16px; +} + +.toolbar-text { + float: left; + color: #959494; + font-size: 14px; + line-height: 16px; } diff --git a/app/views/projects/_md_preview.html.haml b/app/views/projects/_md_preview.html.haml index 32c4433861..4920910fee 100644 --- a/app/views/projects/_md_preview.html.haml +++ b/app/views/projects/_md_preview.html.haml @@ -1,20 +1,19 @@ .md-area - .md-header.clearfix - %ul.nav-links.pull-left + .md-header + %ul.nav-links %li.active %a.js-md-write-button{ href: "#md-write-holder" } Write %li %a.js-md-preview-button{ href: "#md-preview-holder" } Preview - .pull-right - %a.zen-cotrol.js-zen-enter{ href: "#" } - Go full screen + %li.pull-right + %button.zen-cotrol.zen-control-full.js-zen-enter{ type: 'button' } + Go full screen .md-write-holder = yield - .md.md-preview-holder.hide - .js-md-preview{class: (preview_class if defined?(preview_class))} + .md.md-preview-holder.js-md-preview.hide{class: (preview_class if defined?(preview_class))} - if defined?(referenced_users) && referenced_users %div.referenced-users.hide diff --git a/app/views/projects/_zen.html.haml b/app/views/projects/_zen.html.haml index efa60c88b1..bddff5cdcb 100644 --- a/app/views/projects/_zen.html.haml +++ b/app/views/projects/_zen.html.haml @@ -1,9 +1,8 @@ -.zennable - .zen-backdrop - - classes << ' js-gfm-input js-autosize markdown-area' - - if defined?(f) && f - = f.text_area attr, class: classes, placeholder: "Write a comment or drag your files here..." - - else - = text_area_tag attr, nil, class: classes, placeholder: "Write a comment or drag your files here..." - %a.zen-cotrol.zen-control-leave.js-zen-leave{ href: "#" } - = icon('compress') +.zen-backdrop + - classes << ' js-gfm-input js-autosize markdown-area' + - if defined?(f) && f + = f.text_area attr, class: classes, placeholder: "Write a comment or drag your files here..." + - else + = text_area_tag attr, nil, class: classes, placeholder: "Write a comment or drag your files here..." + %a.zen-cotrol.zen-control-leave.js-zen-leave{ href: "#" } + = icon('compress') diff --git a/app/views/projects/notes/_hints.html.haml b/app/views/projects/notes/_hints.html.haml index 6e7929bdab..7f83656146 100644 --- a/app/views/projects/notes/_hints.html.haml +++ b/app/views/projects/notes/_hints.html.haml @@ -1,9 +1,21 @@ -.comment-hints.clearfix - .pull-left +.comment-toolbar.clearfix + %button.toolbar-button.js-toolbar-button{ type: 'button', data: { prefix: ':' }, tabindex: '-1' } + = icon('smile-o', class: 'toolbar-button-icon') + Emoji + .toolbar-text + Styling with = link_to 'Markdown', help_page_path('markdown', 'markdown'), target: '_blank', tabindex: -1 - tip: - = random_markdown_tip - .pull-right - = link_to '#', class: 'markdown-selector', tabindex: -1 do - = icon('paperclip') - Attach a file + is supported + %button.toolbar-button.markdown-selector.pull-right{ type: 'button', tabindex: '-1' } + = icon('file-image-o', class: 'toolbar-button-icon') + Attach a file + +-# .comment-hints.clearfix +-# .pull-left +-# = link_to 'Markdown', help_page_path('markdown', 'markdown'), target: '_blank', tabindex: -1 +-# tip: +-# = random_markdown_tip +-# .pull-right +-# = link_to '#', class: 'markdown-selector', tabindex: -1 do +-# = icon('paperclip') +-# Attach a file From 38e32780a85463e862e5847a436be6819e28c81c Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 30 Mar 2016 10:04:21 +0100 Subject: [PATCH 034/261] Hides current user icon on mobile --- app/assets/stylesheets/pages/notes.scss | 6 ++++++ app/views/projects/notes/_notes_with_form.html.haml | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 072de68c8e..0f45c1fe33 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -20,6 +20,12 @@ ul.notes { .timeline-content { margin-left: 55px; + + &.timeline-content-form { + @media (max-width: $screen-sm-max) { + margin-left: 0; + } + } } .note-created-ago, .note-updated-at { diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml index 8a5a319995..51e33dd774 100644 --- a/app/views/projects/notes/_notes_with_form.html.haml +++ b/app/views/projects/notes/_notes_with_form.html.haml @@ -1,11 +1,11 @@ %ul#notes-list.notes.main-notes-list.timeline = render "projects/notes/notes" %li.timeline-entry - .timeline-icon + .timeline-icon.hidden-xs.hidden-sm - if can? current_user, :create_note, @project %a.author_link{ href: user_path(current_user) } = image_tag avatar_icon(current_user), alt: current_user.to_reference, class: 'avatar s40' - .timeline-content + .timeline-content.timeline-content-form .js-main-target-form - if can? current_user, :create_note, @project = render "projects/notes/form", view: diff_view From af3284d98b68dedbdab3821fa99ee53437ee3d32 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 30 Mar 2016 10:19:58 +0100 Subject: [PATCH 035/261] Focus style for comment form --- app/assets/javascripts/notes.js.coffee | 6 ++++ .../stylesheets/framework/markdown_area.scss | 7 ++-- app/assets/stylesheets/pages/note_form.scss | 32 ++++++++++++++----- app/views/projects/notes/_hints.html.haml | 2 +- 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 864156cb71..40326ec96a 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -288,6 +288,12 @@ class @Notes else previewButton.removeClass("turn-on").addClass "turn-off" + textarea.on 'focus', -> + $(this).closest('.md-area').addClass 'is-focused' + + textarea.on 'blur', -> + $(this).closest('.md-area').removeClass 'is-focused' + autosize(textarea) new Autosave textarea, [ diff --git a/app/assets/stylesheets/framework/markdown_area.scss b/app/assets/stylesheets/framework/markdown_area.scss index ae23e19172..ea8e1c902c 100644 --- a/app/assets/stylesheets/framework/markdown_area.scss +++ b/app/assets/stylesheets/framework/markdown_area.scss @@ -82,11 +82,8 @@ } .md-preview-holder { - background: #fff; - border: 1px solid #ddd; - min-height: 169px; - padding: 5px; - box-shadow: none; + min-height: 167px; + padding: 10px 0; } .markdown-area { diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 561b37e582..7bd666d1c6 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -44,8 +44,7 @@ } .note-textarea { - padding-left: 0; - padding-right: 0; + padding: 10px 0; font-family: $regular_font; border: 0; @@ -63,8 +62,18 @@ .common-note-form { .md-area { padding: $gl-padding-top $gl-padding; - border: 1px solid #E5E5E5; + border: 1px solid #e5e5e5; border-radius: $border-radius-base; + + &.is-focused { + border-color: #3b99fc; + box-shadow: 0 0 4px rgba(#3b99fc, .7); + + .comment-toolbar, + .nav-links { + border-color: #3b99fc; + } + } } } @@ -156,8 +165,6 @@ } .toolbar-button { - float: left; - margin-right: $gl-padding; padding: 0; background: none; border: 0; @@ -171,8 +178,14 @@ outline: 0; } - &:last-child { - margin-right: 0; + @media (min-width: $screen-md-min) { + float: left; + margin-right: $gl-padding; + + &:last-child { + float: right; + margin-right: 0; + } } } @@ -185,8 +198,11 @@ } .toolbar-text { - float: left; color: #959494; font-size: 14px; line-height: 16px; + + @media (min-width: $screen-md-min) { + float: left; + } } diff --git a/app/views/projects/notes/_hints.html.haml b/app/views/projects/notes/_hints.html.haml index 7f83656146..19783c8a81 100644 --- a/app/views/projects/notes/_hints.html.haml +++ b/app/views/projects/notes/_hints.html.haml @@ -6,7 +6,7 @@ Styling with = link_to 'Markdown', help_page_path('markdown', 'markdown'), target: '_blank', tabindex: -1 is supported - %button.toolbar-button.markdown-selector.pull-right{ type: 'button', tabindex: '-1' } + %button.toolbar-button.markdown-selector{ type: 'button', tabindex: '-1' } = icon('file-image-o', class: 'toolbar-button-icon') Attach a file From f0d2f370ccef7fd1fca4477111e6ded8133c4b36 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 30 Mar 2016 10:24:50 +0100 Subject: [PATCH 036/261] Removed css code that isnt used --- app/assets/javascripts/notes.js.coffee | 1 - app/assets/stylesheets/pages/note_form.scss | 28 --------------------- app/views/projects/notes/_hints.html.haml | 10 -------- 3 files changed, 39 deletions(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 40326ec96a..ae16769ad4 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -305,7 +305,6 @@ class @Notes ] # remove notify commit author checkbox for non-commit notes - form.find(".js-notify-commit-author").remove() if form.find("#note_noteable_type").val() isnt "Commit" GitLab.GfmAutoComplete.setup() new DropzoneInput(form) form.show() diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 7bd666d1c6..5691279121 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -37,10 +37,6 @@ img { max-width: 100%; } - - .comment-hints { - margin-top: -12px; - } } .note-textarea { @@ -77,30 +73,6 @@ } } -.note-form-actions { - .note-form-option { - margin-top: 8px; - margin-left: 30px; - @extend .pull-left; - } - - .js-notify-commit-author { - float: left; - } - - .write-preview-btn { - // makes the "absolute" position for links relative to this - position: relative; - - // preview/edit buttons - > a { - position: absolute; - right: 5px; - top: 8px; - } - } -} - .note-edit-form { display: none; font-size: 15px; diff --git a/app/views/projects/notes/_hints.html.haml b/app/views/projects/notes/_hints.html.haml index 19783c8a81..0c6758210b 100644 --- a/app/views/projects/notes/_hints.html.haml +++ b/app/views/projects/notes/_hints.html.haml @@ -9,13 +9,3 @@ %button.toolbar-button.markdown-selector{ type: 'button', tabindex: '-1' } = icon('file-image-o', class: 'toolbar-button-icon') Attach a file - --# .comment-hints.clearfix --# .pull-left --# = link_to 'Markdown', help_page_path('markdown', 'markdown'), target: '_blank', tabindex: -1 --# tip: --# = random_markdown_tip --# .pull-right --# = link_to '#', class: 'markdown-selector', tabindex: -1 do --# = icon('paperclip') --# Attach a file From 0581df2971d63bcb7966637b6a3da2b4d7feb62d Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 30 Mar 2016 13:39:41 +0100 Subject: [PATCH 037/261] Discussion form --- app/assets/javascripts/notes.js.coffee | 13 +++++++--- app/assets/stylesheets/pages/note_form.scss | 5 ++++ .../projects/notes/_notes_with_form.html.haml | 26 +++++++++---------- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index ae16769ad4..22cad1d612 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -254,6 +254,9 @@ class @Notes # find the form form = $(".js-new-note-form") + # Set a global clone of the form for later cloning + @formClone = form.clone() + # show the form @setupNoteForm(form) @@ -452,15 +455,15 @@ class @Notes Shows the note form below the notes. ### replyToDiscussionNote: (e) => - form = $(".js-new-note-form") + form = @formClone.clone() replyLink = $(e.target).closest(".js-discussion-reply-button") replyLink.hide() # insert the form after the button - form.clone().insertAfter replyLink + replyLink.after form # show the form - @setupDiscussionNoteForm(replyLink, replyLink.next("form")) + @setupDiscussionNoteForm(replyLink, form) ### Shows the diff or discussion form and does some setup on it. @@ -485,7 +488,9 @@ class @Notes .text(form.find('.js-close-discussion-note-form').data('cancel-text')) @setupNoteForm form form.find(".js-note-text").focus() - form.addClass "js-discussion-note-form" + form + .removeClass('js-main-target-form') + .addClass("discussion-form js-discussion-note-form") ### Called when clicking on the "add a comment" button on the side of a diff line. diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 5691279121..85b323e2ca 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -73,6 +73,11 @@ } } +.discussion-form { + padding: $gl-padding-top $gl-padding; + background-color: #fff; +} + .note-edit-form { display: none; font-size: 15px; diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml index 51e33dd774..2f9903e26f 100644 --- a/app/views/projects/notes/_notes_with_form.html.haml +++ b/app/views/projects/notes/_notes_with_form.html.haml @@ -1,24 +1,24 @@ %ul#notes-list.notes.main-notes-list.timeline = render "projects/notes/notes" +%ul.notes.timeline %li.timeline-entry .timeline-icon.hidden-xs.hidden-sm - if can? current_user, :create_note, @project %a.author_link{ href: user_path(current_user) } = image_tag avatar_icon(current_user), alt: current_user.to_reference, class: 'avatar s40' .timeline-content.timeline-content-form - .js-main-target-form - - if can? current_user, :create_note, @project - = render "projects/notes/form", view: diff_view - - else - .disabled-comment-area - .disabled-profile - .disabled-comment - %span - Please - = link_to "register",new_user_session_path - or - = link_to "login",new_user_session_path - to post a comment + - if can? current_user, :create_note, @project + = render "projects/notes/form", view: diff_view + - else + .disabled-comment-area + .disabled-profile + .disabled-comment + %span + Please + = link_to "register",new_user_session_path + or + = link_to "login",new_user_session_path + to post a comment :javascript var notes = new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{diff_view}") From 3b4c4dd7b35b662755fed58c6d1ed6f9ba82b575 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 30 Mar 2016 13:54:35 +0100 Subject: [PATCH 038/261] Logged out box --- app/assets/javascripts/notes.js.coffee | 3 +- .../stylesheets/pages/merge_requests.scss | 42 ++++++------------- app/views/projects/notes/_edit_form.html.haml | 5 ++- .../projects/notes/_notes_with_form.html.haml | 25 +++++------ 4 files changed, 27 insertions(+), 48 deletions(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 22cad1d612..0ee1e70da2 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -501,7 +501,6 @@ class @Notes addDiffNote: (e) => e.preventDefault() link = e.currentTarget - form = $(".js-new-note-form") row = $(link).closest("tr") nextRow = row.next() hasNotes = nextRow.is(".notes_holder") @@ -533,7 +532,7 @@ class @Notes addForm = true if addForm - newForm = form.clone() + newForm = @formClone.clone() newForm.appendTo row.next().find(targetContent) # show the form diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 7ff63ca20b..b080490f75 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -195,39 +195,21 @@ line-height: 31px; } -.disabled-comment-area { - padding: 16px 0; +.disabled-comment { + margin-left: -$gl-padding-top; + margin-right: -$gl-padding-top; + background-color: $gray-light; + border-radius: $border-radius-base; + border: 1px solid $border-gray-normal; + color: #b2b2b2; + line-height: 200px; - .disabled-profile { - width: 40px; - height: 40px; - background: $border-gray-dark; - border-radius: 20px; - display: inline-block; - margin-right: 10px; + .disabled-comment-text { + line-height: normal; } - .disabled-comment { - background: $gray-light; - display: inline-block; - vertical-align: top; - height: 200px; - border-radius: 4px; - border: 1px solid $border-gray-normal; - padding-top: 90px; - text-align: center; - right: 20px; - position: absolute; - left: 70px; - margin-bottom: 20px; - - span { - color: #b2b2b2; - - a { - color: $md-link-color; - } - } + a { + color: $gl-link-color; } } diff --git a/app/views/projects/notes/_edit_form.html.haml b/app/views/projects/notes/_edit_form.html.haml index 3a5551b08c..23e4f93eab 100644 --- a/app/views/projects/notes/_edit_form.html.haml +++ b/app/views/projects/notes/_edit_form.html.haml @@ -1,5 +1,5 @@ .note-edit-form - = form_for note, url: namespace_project_note_path(@project.namespace, @project, note), method: :put, remote: true, authenticity_token: true, html: { class: 'edit-note js-quick-submit' } do |f| + = form_for note, url: namespace_project_note_path(@project.namespace, @project, note), method: :put, remote: true, authenticity_token: true, html: { class: 'edit-note common-note-form js-quick-submit' } do |f| = note_target_fields(note) = render layout: 'projects/md_preview', locals: { preview_class: 'md-preview' } do = render 'projects/zen', f: f, attr: :note, classes: 'note-textarea js-note-text js-task-list-field' @@ -7,4 +7,5 @@ .note-form-actions.clearfix = f.submit 'Save Comment', class: 'btn btn-nr btn-save btn-grouped js-comment-button' - = link_to 'Cancel', '#', class: 'btn btn-nr btn-cancel note-edit-cancel' + %button.btn.btn-nr.btn-cancel.note-edit-cancel{ type: 'button' } + Cancel diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml index 2f9903e26f..cc42aab5c5 100644 --- a/app/views/projects/notes/_notes_with_form.html.haml +++ b/app/views/projects/notes/_notes_with_form.html.haml @@ -2,23 +2,20 @@ = render "projects/notes/notes" %ul.notes.timeline %li.timeline-entry - .timeline-icon.hidden-xs.hidden-sm - - if can? current_user, :create_note, @project + - if can? current_user, :create_note, @project + .timeline-icon.hidden-xs.hidden-sm %a.author_link{ href: user_path(current_user) } = image_tag avatar_icon(current_user), alt: current_user.to_reference, class: 'avatar s40' - .timeline-content.timeline-content-form - - if can? current_user, :create_note, @project + .timeline-content.timeline-content-form = render "projects/notes/form", view: diff_view - - else - .disabled-comment-area - .disabled-profile - .disabled-comment - %span - Please - = link_to "register",new_user_session_path - or - = link_to "login",new_user_session_path - to post a comment + - else + .disabled-comment.text-center + .disabled-comment-text.inline + Please + = link_to "register",new_user_session_path + or + = link_to "login",new_user_session_path + to post a comment :javascript var notes = new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{diff_view}") From 40297b8f118742fc710f2ae0364d7bbcd82c40e6 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 30 Mar 2016 14:03:14 +0100 Subject: [PATCH 039/261] Reduced focus shadow --- app/assets/stylesheets/pages/note_form.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 85b323e2ca..c829ffa330 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -63,7 +63,8 @@ &.is-focused { border-color: #3b99fc; - box-shadow: 0 0 4px rgba(#3b99fc, .7); + box-shadow: 0 0 2px rgba(#000, .2), + 0 0 4px rgba(#3b99fc, .5); .comment-toolbar, .nav-links { From 29f414aa5a4d63d5676d5c0e4d89584f45ef8c35 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 30 Mar 2016 16:20:43 +0100 Subject: [PATCH 040/261] Tests update --- app/assets/stylesheets/pages/notes.scss | 2 +- features/steps/shared/diff_note.rb | 4 ++-- spec/features/issues_spec.rb | 2 +- spec/features/notes_on_merge_requests_spec.rb | 2 +- spec/javascripts/fixtures/zen_mode.html.haml | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 0f45c1fe33..46a0724603 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -155,7 +155,7 @@ ul.notes { &.notes_content { background-color: #fff; border-width: 1px 0; - padding-top: 0; + padding: 0; vertical-align: top; &.parallel{ border-width: 1px; diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index 906b66a4a6..32c3e99f45 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -125,7 +125,7 @@ module SharedDiffNote step 'I should only see one diff form' do page.within(diff_file_selector) do - expect(page).to have_css("form.new_note", count: 1) + expect(page).to have_css("form.new-note", count: 1) end end @@ -161,7 +161,7 @@ module SharedDiffNote step 'I should see a temporary diff comment form' do page.within(diff_file_selector) do - expect(page).to have_css(".js-temp-notes-holder form.new_note") + expect(page).to have_css(".js-temp-notes-holder form.new-note") end end diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index db46657c36..79000666cc 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -22,7 +22,7 @@ describe 'Issues', feature: true do before do visit edit_namespace_project_issue_path(project.namespace, project, issue) - click_link "Edit" + click_button "Go full screen" end it 'should open new issue popup' do diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index d9a8058efd..133fca6923 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -152,7 +152,7 @@ describe 'Comments', feature: true do it 'has .new_note css class' do page.within('.js-temp-notes-holder') do - expect(subject).to have_css('.new_note') + expect(subject).to have_css('.new-note') end end end diff --git a/spec/javascripts/fixtures/zen_mode.html.haml b/spec/javascripts/fixtures/zen_mode.html.haml index 1701652c61..cb906a7fea 100644 --- a/spec/javascripts/fixtures/zen_mode.html.haml +++ b/spec/javascripts/fixtures/zen_mode.html.haml @@ -1,4 +1,4 @@ -.zennable +.md-area .zen-backdrop %textarea#note_note.js-gfm-input.markdown-area %a.js-zen-enter(tabindex="-1" href="#") From ccc64676a97d251658190bfb62e97b166cee4db1 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 30 Mar 2016 17:13:18 +0100 Subject: [PATCH 041/261] Commits comment test update --- features/steps/shared/note.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb index fb0462d6e0..7949d252f0 100644 --- a/features/steps/shared/note.rb +++ b/features/steps/shared/note.rb @@ -2,7 +2,7 @@ module SharedNote include Spinach::DSL step 'I delete a comment' do - page.within('.notes') do + page.within('.main-notes-list') do find('.note').hover find(".js-note-delete").click end From 1f5083343081adb6e4a9a438600163844d2e9875 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 30 Mar 2016 20:11:41 +0100 Subject: [PATCH 042/261] Updated tests --- app/assets/javascripts/notes.js.coffee | 14 +++++++++----- features/steps/shared/note.rb | 2 +- spec/features/notes_on_merge_requests_spec.rb | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 0ee1e70da2..35547adf8a 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -65,6 +65,8 @@ class @Notes # add diff note $(document).on "click", ".js-add-diff-note-button", @addDiffNote + $(document).on "mouseover", ".js-add-diff-note-button", -> + console.log $(this).data('line-code') # hide diff note form $(document).on "click", ".js-close-discussion-note-form", @cancelDiscussionForm @@ -264,6 +266,8 @@ class @Notes form.removeClass "js-new-note-form" form.addClass "js-main-target-form" + form.find("#note_line_code").remove() + ### General note form setup. @@ -500,8 +504,9 @@ class @Notes ### addDiffNote: (e) => e.preventDefault() - link = e.currentTarget - row = $(link).closest("tr") + $link = $(e.currentTarget) + console.log $link.data('line-code') + row = $link.closest("tr") nextRow = row.next() hasNotes = nextRow.is(".notes_holder") addForm = false @@ -510,7 +515,7 @@ class @Notes # In parallel view, look inside the correct left/right pane if @isParallelView() - lineType = $(link).data("lineType") + lineType = $link.data("lineType") targetContent += "." + lineType rowCssToAdd = "" @@ -536,7 +541,7 @@ class @Notes newForm.appendTo row.next().find(targetContent) # show the form - @setupDiscussionNoteForm $(link), newForm + @setupDiscussionNoteForm $link, newForm ### Called in response to "cancel" on a diff note form. @@ -561,7 +566,6 @@ class @Notes cancelDiscussionForm: (e) => e.preventDefault() - form = $(".js-new-note-form") form = $(e.target).closest(".js-discussion-note-form") @removeDiscussionNoteForm(form) diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb index 7949d252f0..a3c3887ab4 100644 --- a/features/steps/shared/note.rb +++ b/features/steps/shared/note.rb @@ -128,7 +128,7 @@ module SharedNote end step 'I edit the last comment with a +1' do - page.within(".notes") do + page.within(".main-notes-list") do find(".note").hover find('.js-note-edit').click end diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 133fca6923..70d0864783 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -225,6 +225,6 @@ describe 'Comments', feature: true do end def click_diff_line(data = line_code) - page.find(%Q{button[data-line-code="#{data}"]}, visible: false).click + execute_script("$('button[data-line-code=\"#{data}\"]').click()") end end From f62d7d261b6d321400a68fa23daf7eceaba66e05 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 4 Apr 2016 10:49:08 +0100 Subject: [PATCH 043/261] Removed console.log from notes --- app/assets/javascripts/notes.js.coffee | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 35547adf8a..86e3b860fc 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -65,8 +65,6 @@ class @Notes # add diff note $(document).on "click", ".js-add-diff-note-button", @addDiffNote - $(document).on "mouseover", ".js-add-diff-note-button", -> - console.log $(this).data('line-code') # hide diff note form $(document).on "click", ".js-close-discussion-note-form", @cancelDiscussionForm @@ -505,7 +503,6 @@ class @Notes addDiffNote: (e) => e.preventDefault() $link = $(e.currentTarget) - console.log $link.data('line-code') row = $link.closest("tr") nextRow = row.next() hasNotes = nextRow.is(".notes_holder") From 51df93607fc5c4ef2cac13a90aaf7450c38fdf4f Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 4 Apr 2016 11:00:29 +0100 Subject: [PATCH 044/261] SCSS colors into variables --- .../stylesheets/framework/variables.scss | 6 ++++++ app/assets/stylesheets/framework/zen.scss | 4 ++-- .../stylesheets/pages/merge_requests.scss | 18 ------------------ app/assets/stylesheets/pages/note_form.scss | 11 +++++------ app/assets/stylesheets/pages/notes.scss | 18 ++++++++++++++++++ 5 files changed, 31 insertions(+), 26 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 98fe794d36..c2defd3188 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -217,3 +217,9 @@ $notes-light-color: #8e8e8e; $notes-action-color: #c3c3c3; $notes-role-color: #8e8e8e; $notes-role-border-color: #e4e4e4; + +$note-disabled-comment-color: #b2b2b2; +$note-form-border-color: #e5e5e5; +$note-toolbar-color: #959494; + +$zen-control-hover-color: #111; diff --git a/app/assets/stylesheets/framework/zen.scss b/app/assets/stylesheets/framework/zen.scss index 951d794916..f870ea0d87 100644 --- a/app/assets/stylesheets/framework/zen.scss +++ b/app/assets/stylesheets/framework/zen.scss @@ -40,7 +40,7 @@ } .zen-control-full { - color: #959494; + color: $note-toolbar-color; &:hover { color: $gl-link-color; @@ -57,6 +57,6 @@ font-size: 36px; &:hover { - color: #111; + color: $zen-control-hover-color; } } diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index b080490f75..1c6a420897 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -195,24 +195,6 @@ line-height: 31px; } -.disabled-comment { - margin-left: -$gl-padding-top; - margin-right: -$gl-padding-top; - background-color: $gray-light; - border-radius: $border-radius-base; - border: 1px solid $border-gray-normal; - color: #b2b2b2; - line-height: 200px; - - .disabled-comment-text { - line-height: normal; - } - - a { - color: $gl-link-color; - } -} - .builds { .table-holder { overflow-x: scroll; diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index c829ffa330..a909776b43 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -58,17 +58,17 @@ .common-note-form { .md-area { padding: $gl-padding-top $gl-padding; - border: 1px solid #e5e5e5; + border: 1px solid $note-form-border-color; border-radius: $border-radius-base; &.is-focused { - border-color: #3b99fc; + border-color: $focus-border-color; box-shadow: 0 0 2px rgba(#000, .2), - 0 0 4px rgba(#3b99fc, .5); + 0 0 4px rgba($focus-border-color, .4); .comment-toolbar, .nav-links { - border-color: #3b99fc; + border-color: $focus-border-color; } } } @@ -139,6 +139,7 @@ .comment-toolbar { padding-top: $gl-padding-top; + color: $note-toolbar-color; border-top: 1px solid $border-color; } @@ -146,7 +147,6 @@ padding: 0; background: none; border: 0; - color: #959494; font-size: 14px; line-height: 16px; @@ -176,7 +176,6 @@ } .toolbar-text { - color: #959494; font-size: 14px; line-height: 16px; diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 46a0724603..a9f88d2ccf 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -287,3 +287,21 @@ ul.notes { } } } + +.disabled-comment { + margin-left: -$gl-padding-top; + margin-right: -$gl-padding-top; + background-color: $gray-light; + border-radius: $border-radius-base; + border: 1px solid $border-gray-normal; + color: $note-disabled-comment-color; + line-height: 200px; + + .disabled-comment-text { + line-height: normal; + } + + a { + color: $gl-link-color; + } +} From a6b5b50e14885a82530794c6ea35c940305244dd Mon Sep 17 00:00:00 2001 From: Baldinof Date: Mon, 4 Apr 2016 14:41:01 +0000 Subject: [PATCH 045/261] Fix incorrect variable name --- app/services/projects/unlink_fork_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb index d0703effa1..315c3e1629 100644 --- a/app/services/projects/unlink_fork_service.rb +++ b/app/services/projects/unlink_fork_service.rb @@ -4,7 +4,7 @@ module Projects return unless @project.forked? @project.forked_from_project.lfs_objects.find_each do |lfs_object| - lfs_object.projects << self + lfs_object.projects << @project end merge_requests = @project.forked_from_project.merge_requests.opened.from_project(@project) From 1be3c634c8372a4d10edc1ddf16fb50021624a57 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 4 Apr 2016 16:58:35 +0200 Subject: [PATCH 046/261] Fix transparent bg for expand sidebar button Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/framework/sidebar.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index c741c826ae..1d49249dd8 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -200,7 +200,7 @@ top: 0; left: 0; font-size: 20px; - background: transparent; + background: #fff; height: 59px; text-align: center; line-height: 59px; From 11c1dda35fdb3a058dda739270b1dac8f3516f16 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 4 Apr 2016 18:59:04 +0200 Subject: [PATCH 047/261] Fix event rendering when create project Signed-off-by: Dmitriy Zaporozhets --- app/helpers/events_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index a36b13a7db..592bad8ba2 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -216,7 +216,7 @@ module EventsHelper end def event_row_class(event) - if event.body? || event.created_project? + if event.body? "event-block" else "event-inline" From d5fadfe0f5ebc03d33d1e61bba89d054025870e8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 4 Apr 2016 19:23:50 +0200 Subject: [PATCH 048/261] Improve UI for admin/groups page Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/framework/nav.scss | 11 ++++++ app/views/admin/groups/_group.html.haml | 28 ++++++++++++++ app/views/admin/groups/index.html.haml | 45 +++++------------------ 3 files changed, 49 insertions(+), 35 deletions(-) create mode 100644 app/views/admin/groups/_group.html.haml diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index fc3b0a422a..94f5a12ff6 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -56,6 +56,17 @@ } } + .nav-search { + display: inline-block; + width: 50%; + padding: 11px 0; + + /* Small devices (phones, tablets, 768px and lower) */ + @media (max-width: $screen-sm-min) { + width: 100%; + } + } + .nav-links { display: inline-block; width: 50%; diff --git a/app/views/admin/groups/_group.html.haml b/app/views/admin/groups/_group.html.haml new file mode 100644 index 0000000000..166c0e9afe --- /dev/null +++ b/app/views/admin/groups/_group.html.haml @@ -0,0 +1,28 @@ +- css_class = '' unless local_assigns[:css_class] +- css_class += " no-description" if group.description.blank? + +%li.group-row{ class: css_class } + .controls.hidden-xs + = link_to 'Edit', edit_admin_group_path(group), id: "edit_#{dom_id(group)}", class: "btn btn-grouped btn-sm" + = link_to 'Destroy', [:admin, group], data: {confirm: "REMOVE #{group.name}? Are you sure?"}, method: :delete, class: "btn btn-grouped btn-sm btn-remove" + + .stats + %span + = icon('bookmark') + = number_with_delimiter(group.projects.count) + + %span + = icon('users') + = number_with_delimiter(group.users.count) + + %span.visibility-icon.has-tooltip{data: { container: 'body', placement: 'left' }, title: visibility_icon_description(group)} + = visibility_level_icon(group.visibility_level, fw: false) + + = image_tag group_icon(group), class: "avatar s40 hidden-xs" + .title + = link_to [:admin, group], class: 'group-name' do + = group.name + + - if group.description.present? + .description + = markdown(group.description, pipeline: :description) diff --git a/app/views/admin/groups/index.html.haml b/app/views/admin/groups/index.html.haml index 6bdc885a31..775072a744 100644 --- a/app/views/admin/groups/index.html.haml +++ b/app/views/admin/groups/index.html.haml @@ -1,20 +1,19 @@ - page_title "Groups" %h3.page-title Groups (#{number_with_delimiter(@groups.total_count)}) - = link_to 'New Group', new_admin_group_path, class: "btn btn-new pull-right" %p.light Group allows you to keep projects organized. Use groups for uniting related projects. -%hr -= form_tag admin_groups_path, method: :get, class: 'form-inline' do - = hidden_field_tag :sort, @sort - .form-group - = text_field_tag :name, params[:name], class: "form-control" - = button_tag "Search", class: "btn submit btn-primary" +.top-area + .nav-search + = form_tag admin_groups_path, method: :get, class: 'form-inline' do + = hidden_field_tag :sort, @sort + = text_field_tag :name, params[:name], class: "form-control" + = button_tag "Search", class: "btn submit btn-primary" - .pull-right + .nav-controls .dropdown.inline %a.dropdown-toggle.btn{href: '#', "data-toggle" => "dropdown"} %span.light @@ -33,34 +32,10 @@ = sort_title_recently_updated = link_to admin_groups_path(sort: sort_value_oldest_updated) do = sort_title_oldest_updated + = link_to 'New Group', new_admin_group_path, class: "btn btn-new" -%hr - -%ul.bordered-list +%ul.content-list - @groups.each do |group| - %li - .clearfix - .pull-right.prepend-top-10 - = link_to 'Edit', edit_admin_group_path(group), id: "edit_#{dom_id(group)}", class: "btn btn-sm" - = link_to 'Destroy', [:admin, group], data: {confirm: "REMOVE #{group.name}? Are you sure?"}, method: :delete, class: "btn btn-sm btn-remove" - - %h4 - = link_to [:admin, group] do - %span{ class: visibility_level_color(group.visibility_level) } - = visibility_level_icon(group.visibility_level) - - %i.fa.fa-folder - = group.name - - → - %span.monospace - %strong #{group.path}/ - .clearfix - %p - = truncate group.description, length: 150 - .clearfix - %p.light - #{pluralize(group.members.size, 'member')}, #{pluralize(group.projects.count, 'project')} - + = render 'group', group: group = paginate @groups, theme: "gitlab" From 3e6d27ee59d5750cc6c6140cda4fa08100dbd58a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 4 Apr 2016 19:36:21 +0200 Subject: [PATCH 049/261] Dont use monospace font for names Signed-off-by: Dmitriy Zaporozhets --- app/views/admin/builds/_build.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/builds/_build.html.haml b/app/views/admin/builds/_build.html.haml index 588ad76742..3571eefd57 100644 --- a/app/views/admin/builds/_build.html.haml +++ b/app/views/admin/builds/_build.html.haml @@ -15,7 +15,7 @@ %td - if project - = link_to project.name_with_namespace, admin_namespace_project_path(project.namespace, project), class: "monospace" + = link_to project.name_with_namespace, admin_namespace_project_path(project.namespace, project) %td = link_to build.short_sha, namespace_project_commit_path(build.project.namespace, build.project, build.sha), class: "monospace" From f9fd0031595be404c439899f5b68dfa753223fdd Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 4 Apr 2016 19:41:31 +0200 Subject: [PATCH 050/261] Fix missing paddings in admin area Signed-off-by: Dmitriy Zaporozhets --- app/views/admin/dashboard/index.html.haml | 2 +- app/views/admin/deploy_keys/index.html.haml | 2 +- app/views/admin/labels/index.html.haml | 12 +++++++----- app/views/admin/runners/index.html.haml | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 3274ba5377..6dd2fef395 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -1,4 +1,4 @@ -.admin-dashboard +.admin-dashboard.prepend-top-default .row .col-md-4 %h4 Statistics diff --git a/app/views/admin/deploy_keys/index.html.haml b/app/views/admin/deploy_keys/index.html.haml index 41c4389997..149593e7f4 100644 --- a/app/views/admin/deploy_keys/index.html.haml +++ b/app/views/admin/deploy_keys/index.html.haml @@ -1,5 +1,5 @@ - page_title "Deploy Keys" -.panel.panel-default +.panel.panel-default.prepend-top-default .panel-heading Public deploy keys (#{@deploy_keys.count}) .controls diff --git a/app/views/admin/labels/index.html.haml b/app/views/admin/labels/index.html.haml index 3c57e3dc17..05d6b9ed23 100644 --- a/app/views/admin/labels/index.html.haml +++ b/app/views/admin/labels/index.html.haml @@ -1,8 +1,10 @@ - page_title "Labels" -= link_to new_admin_label_path, class: "pull-right btn btn-nr btn-new" do - New label -%h3.page-title - Labels + +%div + = link_to new_admin_label_path, class: "pull-right btn btn-nr btn-new" do + New label + %h3.page-title + Labels %hr .labels @@ -13,4 +15,4 @@ - else .light-well .nothing-here-block There are no labels yet - + diff --git a/app/views/admin/runners/index.html.haml b/app/views/admin/runners/index.html.haml index c407972cd0..2dad64b8d0 100644 --- a/app/views/admin/runners/index.html.haml +++ b/app/views/admin/runners/index.html.haml @@ -1,4 +1,4 @@ -%p.lead +%p.lead.prepend-top-default %span To register a new runner you should enter the following registration token. With this token the runner will request a unique runner token and use that for future communication. From 6e0f8453c0c1bef3dc4d9c1200aaf144011ce42d Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 4 Apr 2016 21:05:51 +0200 Subject: [PATCH 051/261] Reload forked_project_link projects This fixes ./spec/services/projects/unlink_fork_service_spec.rb which somehow started failing on the master branch. It certainly isn't a very elegant solution but seems to be the easiest/best way of solving this problem for the time being. --- spec/factories/forked_project_links.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/factories/forked_project_links.rb b/spec/factories/forked_project_links.rb index 252bf2747e..19a54946fe 100644 --- a/spec/factories/forked_project_links.rb +++ b/spec/factories/forked_project_links.rb @@ -13,5 +13,10 @@ FactoryGirl.define do factory :forked_project_link do association :forked_to_project, factory: :project association :forked_from_project, factory: :project + + after(:create) do |link| + link.forked_from_project.reload + link.forked_to_project.reload + end end end From be6f1c74278519d49244a5c29b016725b8eec1e3 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 4 Apr 2016 23:52:27 +0200 Subject: [PATCH 052/261] Single quotes paradise Signed-off-by: Dmitriy Zaporozhets --- app/views/admin/groups/_group.html.haml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/admin/groups/_group.html.haml b/app/views/admin/groups/_group.html.haml index 166c0e9afe..9025aaac09 100644 --- a/app/views/admin/groups/_group.html.haml +++ b/app/views/admin/groups/_group.html.haml @@ -1,10 +1,10 @@ - css_class = '' unless local_assigns[:css_class] -- css_class += " no-description" if group.description.blank? +- css_class += ' no-description' if group.description.blank? %li.group-row{ class: css_class } .controls.hidden-xs - = link_to 'Edit', edit_admin_group_path(group), id: "edit_#{dom_id(group)}", class: "btn btn-grouped btn-sm" - = link_to 'Destroy', [:admin, group], data: {confirm: "REMOVE #{group.name}? Are you sure?"}, method: :delete, class: "btn btn-grouped btn-sm btn-remove" + = link_to 'Edit', edit_admin_group_path(group), id: "edit_#{dom_id(group)}", class: 'btn btn-grouped btn-sm' + = link_to 'Destroy', [:admin, group], data: {confirm: "REMOVE #{group.name}? Are you sure?"}, method: :delete, class: 'btn btn-grouped btn-sm btn-remove' .stats %span @@ -18,7 +18,7 @@ %span.visibility-icon.has-tooltip{data: { container: 'body', placement: 'left' }, title: visibility_icon_description(group)} = visibility_level_icon(group.visibility_level, fw: false) - = image_tag group_icon(group), class: "avatar s40 hidden-xs" + = image_tag group_icon(group), class: 'avatar s40 hidden-xs' .title = link_to [:admin, group], class: 'group-name' do = group.name From 4b3747b2cf01e7878407649edacfa7c825537ca9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 4 Apr 2016 23:56:15 +0200 Subject: [PATCH 053/261] Add missing changelog item about improving navigation sidebar Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index f72bb670ec..68ea96aaaa 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -18,6 +18,7 @@ v 8.7.0 (unreleased) - Fix creation of merge requests for orphaned branches (Stan Hu) - Fall back to `In-Reply-To` and `References` headers when sub-addressing is not available (David Padilla) - Remove "Congratulations!" tweet button on newly-created project. (Connor Shea) + - Improved UX of the navigation sidebar v 8.6.4 - Don't attempt to fetch any tags from a forked repo (Stan Hu) From f2005fa56682a5dc3b57d610cb733a34b2e08520 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 4 Apr 2016 19:35:39 -0300 Subject: [PATCH 054/261] Flush repository cache before import project data GitHub Pull Requests importer handle with the repository while importing data, we need to make sure that the cached values are valid. --- app/models/repository.rb | 5 +++++ app/services/projects/import_service.rb | 2 ++ spec/models/repository_spec.rb | 14 ++++++++++++++ spec/services/projects/import_service_spec.rb | 17 +++++++++++++++++ 4 files changed, 38 insertions(+) diff --git a/app/models/repository.rb b/app/models/repository.rb index e80c223840..a8e826c9cb 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -364,6 +364,11 @@ class Repository expire_tag_count_cache end + def before_import + expire_emptiness_caches + expire_exists_cache + end + # Runs code after a repository has been forked/imported. def after_import expire_emptiness_caches diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index 2015897dd1..ef15ef6a47 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -46,6 +46,8 @@ module Projects def import_data return unless has_importer? + project.repository.before_import + unless importer.execute raise Error, 'The remote data could not be imported.' end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index c5d5a1c249..f517f325c0 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -612,6 +612,20 @@ describe Repository, models: true do end end + describe '#before_import' do + it 'flushes the emptiness cachess' do + expect(repository).to receive(:expire_emptiness_caches) + + repository.before_import + end + + it 'flushes the exists cache' do + expect(repository).to receive(:expire_exists_cache) + + repository.before_import + end + end + describe '#after_import' do it 'flushes the emptiness cachess' do expect(repository).to receive(:expire_emptiness_caches) diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index 04f474c736..32bf3acf48 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -72,6 +72,23 @@ describe Projects::ImportService, services: true do expect(result[:status]).to eq :success end + it 'flushes various caches' do + expect_any_instance_of(Gitlab::Shell).to receive(:import_repository). + with(project.path_with_namespace, project.import_url). + and_return(true) + + expect_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute). + and_return(true) + + expect_any_instance_of(Repository).to receive(:expire_emptiness_caches). + and_call_original + + expect_any_instance_of(Repository).to receive(:expire_exists_cache). + and_call_original + + subject.execute + end + it 'fails if importer fails' do expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).with(project.path_with_namespace, project.import_url).and_return(true) expect_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute).and_return(false) From 21837bfb203d2d3ec85df7cb1d94b3b1818d4ab5 Mon Sep 17 00:00:00 2001 From: connorshea Date: Sat, 2 Apr 2016 01:07:22 -0600 Subject: [PATCH 055/261] Add comments to the SCSS Lint config file [ci skip] Also add some previously missing linters. --- .scss-lint.yml | 105 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/.scss-lint.yml b/.scss-lint.yml index 3ce0c4901b..835a4a88c4 100644 --- a/.scss-lint.yml +++ b/.scss-lint.yml @@ -7,21 +7,44 @@ exclude: - 'app/assets/stylesheets/pages/emojis.scss' linters: + # Reports when you use improper spacing around ! (the "bang") in !default, + # !global, !important, and !optional flags. BangFormat: enabled: false + # Whether or not to prefer `border: 0` over `border: none`. BorderZero: enabled: false + # Reports when you define a rule set using a selector with chained classes + # (a.k.a. adjoining classes). + ChainedClasses: + enabled: false + + # Prefer hexadecimal color codes over color keywords. + # (e.g. `color: green` is a color keyword) ColorKeyword: enabled: false + # Prefer color literals (keywords or hexadecimal codes) to be used only in + # variable declarations. They should be referred to via variables everywhere + # else. ColorVariable: enabled: false + # Which form of comments to prefer in CSS. Comment: enabled: false + + # Reports @debug statements (which you probably left behind accidentally). + DebugStatement: + enabled: false + # Rule sets should be ordered as follows: + # - @extend declarations + # - @include declarations without inner @content + # - properties, @include declarations with inner @content + # - nested rule sets. DeclarationOrder: enabled: false @@ -32,15 +55,25 @@ linters: DisableLinterReason: enabled: true + # Reports when you define the same property twice in a single rule set. DuplicateProperty: enabled: false + # Separate rule, function, and mixin declarations with empty lines. EmptyLineBetweenBlocks: enabled: false + # Reports when you have an empty rule set. EmptyRule: enabled: false + # Reports when you have an @extend directive. + ExtendDirective: + enabled: false + + # Files should always have a final newline. This results in better diffs + # when adding lines to the file, since SCM systems such as git won't + # think that you touched the last line. FinalNewline: enabled: false @@ -53,12 +86,17 @@ linters: HexNotation: enabled: true + # Avoid using ID selectors. IdSelector: enabled: false + # The basenames of @imported SCSS partials should not begin with an + # underscore and should not include the filename extension. ImportPath: enabled: false + # Avoid using !important in properties. It is usually indicative of a + # misunderstanding of CSS specificity and can lead to brittle code. ImportantRule: enabled: false @@ -67,33 +105,51 @@ linters: enabled: true width: 2 + # Don't write leading zeros for numeric values with a decimal point. LeadingZero: enabled: false + # Reports when you define the same selector twice in a single sheet. MergeableSelector: enabled: false + # Functions, mixins, variables, and placeholders should be declared + # with all lowercase letters and hyphens instead of underscores. NameFormat: enabled: false + # Avoid nesting selectors too deeply. NestingDepth: enabled: false + # Always use placeholder selectors in @extend. PlaceholderInExtend: enabled: false + # Sort properties in a strict order. PropertySortOrder: enabled: false + # Reports when you use an unknown or disabled CSS property + # (ignoring vendor-prefixed properties). PropertySpelling: enabled: false + # Configure which units are allowed for property values. + PropertyUnits: + enabled: false + + # Pseudo-elements, like ::before, and ::first-letter, should be declared + # with two colons. Pseudo-classes, like :hover and :first-child, should + # be declared with one colon. PseudoElement: enabled: false + # Avoid qualifying elements in selectors (also known as "tag-qualifying"). QualifyingElement: enabled: false + # Don't write selectors with a depth of applicability greater than 3. SelectorDepth: enabled: false @@ -113,9 +169,12 @@ linters: enabled: true allow_single_line_rule_sets: true + # Split selectors onto separate lines after each comma, and have each + # individual selector occupy a single line. SingleLinePerSelector: enabled: false + # Commas in lists should be followed by a space. SpaceAfterComma: enabled: false @@ -128,29 +187,75 @@ linters: # colon. SpaceAfterPropertyName: enabled: true + + # Variables should be formatted with a single space separating the colon + # from the variable's value. + SpaceAfterVariableColon: + enabled: false + + # Variables should be formatted with no space between the name and the + # colon. + SpaceAfterVariableName: + enabled: false + # Operators should be formatted with a single space on both sides of an + # infix operator. SpaceAroundOperator: enabled: false # Opening braces should be preceded by a single space. SpaceBeforeBrace: enabled: true + + # Parentheses should not be padded with spaces. + SpaceBetweenParens: + enabled: false + # Enforces that string literals should be written with a consistent form + # of quotes (single or double). StringQuotes: enabled: false + # Property values, @extend, @include, and @import directives, and variable + # declarations should always end with a semicolon. TrailingSemicolon: enabled: false + # Reports lines containing trailing whitespace. TrailingWhitespace: enabled: false + # Don't write trailing zeros for numeric values with a decimal point. + TrailingZero: + enabled: false + + # Don't use the `all` keyword to specify transition properties. + TransitionAll: + enabled: false + + # Numeric values should not contain unnecessary fractional portions. UnnecessaryMantissa: enabled: false + # Do not use parent selector references (&) when they would otherwise + # be unnecessary. UnnecessaryParentReference: enabled: false + + # URLs should be valid and not contain protocols or domain names. + UrlFormat: + enabled: false + # URLs should always be enclosed within quotes. + UrlQuotes: + enabled: false + + # Properties, like color and font, are easier to read and maintain + # when defined using variables rather than literals. + VariableForProperty: + enabled: false + + # Avoid vendor prefixes. Or rather: don't write them yourself. VendorPrefix: enabled: false From 1a168279fa3eb87c2061917707397af21e7b26ea Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Mon, 4 Apr 2016 19:09:12 -0500 Subject: [PATCH 056/261] Prepare SAML for group retrieval --- lib/gitlab/saml/auth_hash.rb | 17 ++++++++++++++ lib/gitlab/saml/config.rb | 22 ++++++++++++++++++ lib/gitlab/saml/user.rb | 43 ++++++++++++++++++++++++++++++++++-- 3 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 lib/gitlab/saml/auth_hash.rb create mode 100644 lib/gitlab/saml/config.rb diff --git a/lib/gitlab/saml/auth_hash.rb b/lib/gitlab/saml/auth_hash.rb new file mode 100644 index 0000000000..5ffccc0e10 --- /dev/null +++ b/lib/gitlab/saml/auth_hash.rb @@ -0,0 +1,17 @@ +module Gitlab + module Saml + class AuthHash < Gitlab::OAuth::AuthHash + + def groups + get_raw(Gitlab::Saml::Config.groups) + end + + private + + def get_raw(key) + auth_hash.extra[:raw_info][key] + end + + end + end +end diff --git a/lib/gitlab/saml/config.rb b/lib/gitlab/saml/config.rb new file mode 100644 index 0000000000..dade4c0fa6 --- /dev/null +++ b/lib/gitlab/saml/config.rb @@ -0,0 +1,22 @@ +# Load a specific server configuration +module Gitlab + module Saml + class Config + + class << self + def options + Gitlab.config.omniauth.providers.find { |provider| provider.name == 'saml' } + end + + def groups + options['groups_attribute'] + end + + def external_groups + options['external_groups'] + end + end + + end + end +end diff --git a/lib/gitlab/saml/user.rb b/lib/gitlab/saml/user.rb index b1e30110ef..14eda337d9 100644 --- a/lib/gitlab/saml/user.rb +++ b/lib/gitlab/saml/user.rb @@ -7,6 +7,11 @@ module Gitlab module Saml class User < Gitlab::OAuth::User + def initialize(auth_hash) + super + update_user_attributes + end + def save super('SAML') end @@ -18,7 +23,7 @@ module Gitlab @user ||= find_or_create_ldap_user end - if auto_link_saml_enabled? + if auto_link_saml_user? @user ||= find_by_email end @@ -37,11 +42,45 @@ module Gitlab end end + def changed? + gl_user.changed? || gl_user.identities.any?(&:changed?) + end + protected - def auto_link_saml_enabled? + def build_new_user + user = super + if external_users_enabled? + unless (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty? + user.external = true + end + end + user + end + + def auto_link_saml_user? Gitlab.config.omniauth.auto_link_saml_user end + + def external_users_enabled? + !Gitlab::Saml::Config.external_groups.nil? + end + + def auth_hash=(auth_hash) + @auth_hash = Gitlab::Saml::AuthHash.new(auth_hash) + end + + def update_user_attributes + if persisted? + if external_users_enabled? + if (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty? + gl_user.external = false + else + gl_user.external = true + end + end + end + end end end end From d14f080c0a7e4cfb2893a9fb2462a650f02a6ef9 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Mon, 4 Apr 2016 19:09:46 -0500 Subject: [PATCH 057/261] Remove unnecessary LDAP tests from SAML tests --- spec/lib/gitlab/saml/user_spec.rb | 66 ++++--------------------------- 1 file changed, 7 insertions(+), 59 deletions(-) diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb index de7cd99d49..6f5cf3a1cf 100644 --- a/spec/lib/gitlab/saml/user_spec.rb +++ b/spec/lib/gitlab/saml/user_spec.rb @@ -23,6 +23,12 @@ describe Gitlab::Saml::User, lib: true do allow(Gitlab::LDAP::Config).to receive_messages(messages) end + def stub_saml_config(messages) + allow(Gitlab::Saml::Config).to receive_messages(messages) + end + + before { stub_saml_config({ options: { name: 'saml', args: {} } }) } + describe 'account exists on server' do before { stub_omniauth_config({ allow_single_sign_on: ['saml'], auto_link_saml_user: true }) } context 'and should bind with SAML' do @@ -138,7 +144,7 @@ describe Gitlab::Saml::User, lib: true do end describe 'blocking' do - before { stub_omniauth_config({ allow_saml_sign_up: true, auto_link_saml_user: true }) } + before { stub_omniauth_config({ allow_single_sign_on: ['saml'], auto_link_saml_user: true }) } context 'signup with SAML only' do context 'dont block on create' do @@ -162,64 +168,6 @@ describe Gitlab::Saml::User, lib: true do end end - context 'signup with linked omniauth and LDAP account' do - before do - stub_omniauth_config(auto_link_ldap_user: true) - allow(ldap_user).to receive(:uid) { uid } - allow(ldap_user).to receive(:username) { uid } - allow(ldap_user).to receive(:email) { ['johndoe@example.com','john2@example.com'] } - allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } - allow(saml_user).to receive(:ldap_person).and_return(ldap_user) - end - - context "and no account for the LDAP user" do - context 'dont block on create (LDAP)' do - before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: false) } - - it do - saml_user.save - expect(gl_user).to be_valid - expect(gl_user).not_to be_blocked - end - end - - context 'block on create (LDAP)' do - before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: true) } - - it do - saml_user.save - expect(gl_user).to be_valid - expect(gl_user).to be_blocked - end - end - end - - context 'and LDAP user has an account already' do - let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') } - - context 'dont block on create (LDAP)' do - before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: false) } - - it do - saml_user.save - expect(gl_user).to be_valid - expect(gl_user).not_to be_blocked - end - end - - context 'block on create (LDAP)' do - before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: true) } - - it do - saml_user.save - expect(gl_user).to be_valid - expect(gl_user).not_to be_blocked - end - end - end - end - - context 'sign-in' do before do saml_user.save From 943d8d4b90fbdc4f68d548c0566343903f895138 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Mon, 4 Apr 2016 19:10:17 -0500 Subject: [PATCH 058/261] Config examples --- config/gitlab.yml.example | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index fb1c3476f6..c607e32a05 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -345,6 +345,8 @@ production: &base # # - { name: 'saml', # label: 'Our SAML Provider', + # groups_attribute: 'Groups', + # external_groups: ['Contractors', 'Freelancers'], # args: { # assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', # idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', From e99855bfe4b4741d33d5575fdf1f0bc2edd85844 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Mon, 4 Apr 2016 19:10:59 -0500 Subject: [PATCH 059/261] Avoid saving again if the user attributes haven't changed --- app/controllers/omniauth_callbacks_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 21135f7d60..d28e96c3f1 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -55,7 +55,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController end else saml_user = Gitlab::Saml::User.new(oauth) - saml_user.save + saml_user.save if saml_user.changed? @user = saml_user.gl_user continue_login_process From 7a2370f74060b2f065e3602700fe1b33fda4685c Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 4 Apr 2016 21:25:38 -0400 Subject: [PATCH 060/261] Standardize the way we check for and display form errors - Some views had a "Close" button. We've removed this, because we don't want users accidentally hiding the validation errors and not knowing what needs to be fixed. - Some views used `li`, some used `p`, some used `span`. We've standardized on `li`. - Some views only showed the first error. We've standardized on showing all of them. - Some views added an `#error_explanation` div, which we've made standard. --- app/helpers/form_helper.rb | 18 ++++++++ app/views/abuse_reports/new.html.haml | 6 +-- app/views/admin/appearances/_form.html.haml | 5 +- .../application_settings/_form.html.haml | 6 +-- app/views/admin/applications/_form.html.haml | 7 +-- .../admin/broadcast_messages/_form.html.haml | 6 +-- app/views/admin/deploy_keys/new.html.haml | 6 +-- app/views/admin/groups/_form.html.haml | 5 +- app/views/admin/hooks/index.html.haml | 6 +-- app/views/admin/identities/_form.html.haml | 6 +-- app/views/admin/labels/_form.html.haml | 8 +--- app/views/admin/users/_form.html.haml | 6 +-- .../doorkeeper/applications/_form.html.haml | 6 +-- app/views/groups/edit.html.haml | 4 +- app/views/groups/new.html.haml | 5 +- app/views/profiles/keys/_form.html.haml | 6 +-- .../profiles/notifications/show.html.haml | 6 +-- app/views/profiles/passwords/edit.html.haml | 7 +-- app/views/profiles/passwords/new.html.haml | 7 +-- app/views/profiles/show.html.haml | 7 +-- app/views/projects/_errors.html.haml | 5 +- .../projects/deploy_keys/_form.html.haml | 6 +-- app/views/projects/hooks/index.html.haml | 6 +-- app/views/projects/labels/_form.html.haml | 8 +--- .../merge_requests/_new_compare.html.haml | 5 +- app/views/projects/milestones/_form.html.haml | 7 +-- .../protected_branches/index.html.haml | 6 +-- app/views/projects/variables/show.html.haml | 8 +--- app/views/projects/wikis/_form.html.haml | 6 +-- app/views/shared/_service_settings.html.haml | 7 +-- app/views/shared/issuable/_form.html.haml | 9 +--- app/views/shared/snippets/_form.html.haml | 6 +-- spec/helpers/form_helper_spec.rb | 46 +++++++++++++++++++ 33 files changed, 105 insertions(+), 153 deletions(-) create mode 100644 app/helpers/form_helper.rb create mode 100644 spec/helpers/form_helper_spec.rb diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb new file mode 100644 index 0000000000..6a43be2cf3 --- /dev/null +++ b/app/helpers/form_helper.rb @@ -0,0 +1,18 @@ +module FormHelper + def form_errors(model) + return unless model.errors.any? + + pluralized = 'error'.pluralize(model.errors.count) + headline = "The form contains the following #{pluralized}:" + + content_tag(:div, class: 'alert alert-danger', id: 'error_explanation') do + content_tag(:h4, headline) << + content_tag(:ul) do + model.errors.full_messages. + map { |msg| content_tag(:li, msg) }. + join. + html_safe + end + end + end +end diff --git a/app/views/abuse_reports/new.html.haml b/app/views/abuse_reports/new.html.haml index 3bc1b24b5e..06be1a5331 100644 --- a/app/views/abuse_reports/new.html.haml +++ b/app/views/abuse_reports/new.html.haml @@ -3,11 +3,9 @@ %p Please use this form to report users who create spam issues, comments or behave inappropriately. %hr = form_for @abuse_report, html: { class: 'form-horizontal js-quick-submit js-requires-input'} do |f| + = form_errors(@abuse_report) + = f.hidden_field :user_id - - if @abuse_report.errors.any? - .alert.alert-danger - - @abuse_report.errors.full_messages.each do |msg| - %p= msg .form-group = f.label :user_id, class: 'control-label' .col-sm-10 diff --git a/app/views/admin/appearances/_form.html.haml b/app/views/admin/appearances/_form.html.haml index 6f325914d1..d88f3ad314 100644 --- a/app/views/admin/appearances/_form.html.haml +++ b/app/views/admin/appearances/_form.html.haml @@ -1,8 +1,5 @@ = form_for @appearance, url: admin_appearances_path, html: { class: 'form-horizontal'} do |f| - - if @appearance.errors.any? - .alert.alert-danger - - @appearance.errors.full_messages.each do |msg| - %p= msg + = form_errors(@appearance) %fieldset.sign-in %legend diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index de86dacbb1..a8cca1a81c 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -1,9 +1,5 @@ = form_for @application_setting, url: admin_application_settings_path, html: { class: 'form-horizontal fieldset-form' } do |f| - - if @application_setting.errors.any? - #error_explanation - .alert.alert-danger - - @application_setting.errors.full_messages.each do |msg| - %p= msg + = form_errors(@application_setting) %fieldset %legend Visibility and Access Controls diff --git a/app/views/admin/applications/_form.html.haml b/app/views/admin/applications/_form.html.haml index e18f7b499d..4aacbb8cd7 100644 --- a/app/views/admin/applications/_form.html.haml +++ b/app/views/admin/applications/_form.html.haml @@ -1,9 +1,6 @@ = form_for [:admin, @application], url: @url, html: {class: 'form-horizontal', role: 'form'} do |f| - - if application.errors.any? - .alert.alert-danger - %button{ type: "button", class: "close", "data-dismiss" => "alert"} × - - application.errors.full_messages.each do |msg| - %p= msg + = form_errors(application) + = content_tag :div, class: 'form-group' do = f.label :name, class: 'col-sm-2 control-label' .col-sm-10 diff --git a/app/views/admin/broadcast_messages/_form.html.haml b/app/views/admin/broadcast_messages/_form.html.haml index b748460a9f..6b157abf84 100644 --- a/app/views/admin/broadcast_messages/_form.html.haml +++ b/app/views/admin/broadcast_messages/_form.html.haml @@ -4,10 +4,8 @@ = render_broadcast_message(@broadcast_message.message.presence || "Your message here") = form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form form-horizontal js-quick-submit js-requires-input'} do |f| - -if @broadcast_message.errors.any? - .alert.alert-danger - - @broadcast_message.errors.full_messages.each do |msg| - %p= msg + = form_errors(@broadcast_message) + .form-group = f.label :message, class: 'control-label' .col-sm-10 diff --git a/app/views/admin/deploy_keys/new.html.haml b/app/views/admin/deploy_keys/new.html.haml index 5b46b3222a..15aa059c93 100644 --- a/app/views/admin/deploy_keys/new.html.haml +++ b/app/views/admin/deploy_keys/new.html.haml @@ -4,11 +4,7 @@ %div = form_for [:admin, @deploy_key], html: { class: 'deploy-key-form form-horizontal' } do |f| - -if @deploy_key.errors.any? - .alert.alert-danger - %ul - - @deploy_key.errors.full_messages.each do |msg| - %li= msg + = form_errors(@deploy_key) .form-group = f.label :title, class: "control-label" diff --git a/app/views/admin/groups/_form.html.haml b/app/views/admin/groups/_form.html.haml index 7f2b1cd235..0cc405401c 100644 --- a/app/views/admin/groups/_form.html.haml +++ b/app/views/admin/groups/_form.html.haml @@ -1,8 +1,5 @@ = form_for [:admin, @group], html: { class: "form-horizontal" } do |f| - - if @group.errors.any? - .alert.alert-danger - %span= @group.errors.full_messages.first - + = form_errors(@group) = render 'shared/group_form', f: f .form-group.group-description-holder diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml index 53b3cd04c6..ad952052f2 100644 --- a/app/views/admin/hooks/index.html.haml +++ b/app/views/admin/hooks/index.html.haml @@ -10,10 +10,8 @@ = form_for @hook, as: :hook, url: admin_hooks_path, html: { class: 'form-horizontal' } do |f| - -if @hook.errors.any? - .alert.alert-danger - - @hook.errors.full_messages.each do |msg| - %p= msg + = form_errors(@hook) + .form-group = f.label :url, "URL:", class: 'control-label' .col-sm-10 diff --git a/app/views/admin/identities/_form.html.haml b/app/views/admin/identities/_form.html.haml index 3a78855822..112a201faf 100644 --- a/app/views/admin/identities/_form.html.haml +++ b/app/views/admin/identities/_form.html.haml @@ -1,9 +1,5 @@ = form_for [:admin, @user, @identity], html: { class: 'form-horizontal fieldset-form' } do |f| - - if @identity.errors.any? - #error_explanation - .alert.alert-danger - - @identity.errors.full_messages.each do |msg| - %p= msg + = form_errors(@identity) .form-group = f.label :provider, class: 'control-label' diff --git a/app/views/admin/labels/_form.html.haml b/app/views/admin/labels/_form.html.haml index 8c6b389bf1..448aa95354 100644 --- a/app/views/admin/labels/_form.html.haml +++ b/app/views/admin/labels/_form.html.haml @@ -1,11 +1,5 @@ = form_for [:admin, @label], html: { class: 'form-horizontal label-form js-requires-input' } do |f| - -if @label.errors.any? - .row - .col-sm-offset-2.col-sm-10 - .alert.alert-danger - - @label.errors.full_messages.each do |msg| - %span= msg - %br + = form_errors(@label) .form-group = f.label :title, class: 'control-label' diff --git a/app/views/admin/users/_form.html.haml b/app/views/admin/users/_form.html.haml index d2527ede99..b05fdbd555 100644 --- a/app/views/admin/users/_form.html.haml +++ b/app/views/admin/users/_form.html.haml @@ -1,10 +1,6 @@ .user_new = form_for [:admin, @user], html: { class: 'form-horizontal fieldset-form' } do |f| - -if @user.errors.any? - #error_explanation - .alert.alert-danger - - @user.errors.full_messages.each do |msg| - %p= msg + = form_errors(@user) %fieldset %legend Account diff --git a/app/views/doorkeeper/applications/_form.html.haml b/app/views/doorkeeper/applications/_form.html.haml index 906b067615..5c98265727 100644 --- a/app/views/doorkeeper/applications/_form.html.haml +++ b/app/views/doorkeeper/applications/_form.html.haml @@ -1,9 +1,5 @@ = form_for application, url: doorkeeper_submit_path(application), html: {role: 'form'} do |f| - - if application.errors.any? - .alert.alert-danger - %ul - - application.errors.full_messages.each do |msg| - %li= msg + = form_errors(application) .form-group = f.label :name, class: 'label-light' diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml index ea5a035839..a698cbbe9d 100644 --- a/app/views/groups/edit.html.haml +++ b/app/views/groups/edit.html.haml @@ -5,9 +5,7 @@ Group settings .panel-body = form_for @group, html: { multipart: true, class: "form-horizontal" }, authenticity_token: true do |f| - - if @group.errors.any? - .alert.alert-danger - %span= @group.errors.full_messages.first + = form_errors(@group) = render 'shared/group_form', f: f .form-group diff --git a/app/views/groups/new.html.haml b/app/views/groups/new.html.haml index 30ab8aeba1..2b8bc269e6 100644 --- a/app/views/groups/new.html.haml +++ b/app/views/groups/new.html.haml @@ -6,10 +6,7 @@ %hr = form_for @group, html: { class: 'group-form form-horizontal' } do |f| - - if @group.errors.any? - .alert.alert-danger - %span= @group.errors.full_messages.first - + = form_errors(@group) = render 'shared/group_form', f: f, autofocus: true .form-group.group-description-holder diff --git a/app/views/profiles/keys/_form.html.haml b/app/views/profiles/keys/_form.html.haml index 4d78215ed3..b3ed59a1a4 100644 --- a/app/views/profiles/keys/_form.html.haml +++ b/app/views/profiles/keys/_form.html.haml @@ -1,10 +1,6 @@ %div = form_for [:profile, @key], html: { class: 'js-requires-input' } do |f| - - if @key.errors.any? - .alert.alert-danger - %ul - - @key.errors.full_messages.each do |msg| - %li= msg + = form_errors(@key) .form-group = f.label :key, class: 'label-light' diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml index 3d15c0d932..6609295a2a 100644 --- a/app/views/profiles/notifications/show.html.haml +++ b/app/views/profiles/notifications/show.html.haml @@ -2,11 +2,7 @@ - header_title page_title, profile_notifications_path = form_for @user, url: profile_notifications_path, method: :put, html: { class: 'update-notifications prepend-top-default' } do |f| - -if @user.errors.any? - %div.alert.alert-danger - %ul - - @user.errors.full_messages.each do |msg| - %li= msg + = form_errors(@user) = hidden_field_tag :notification_type, 'global' .row diff --git a/app/views/profiles/passwords/edit.html.haml b/app/views/profiles/passwords/edit.html.haml index 44d758dceb..5ac8a8b9d0 100644 --- a/app/views/profiles/passwords/edit.html.haml +++ b/app/views/profiles/passwords/edit.html.haml @@ -13,11 +13,8 @@ - unless @user.password_automatically_set? or recover your current one = form_for @user, url: profile_password_path, method: :put, html: {class: "update-password"} do |f| - -if @user.errors.any? - .alert.alert-danger - %ul - - @user.errors.full_messages.each do |msg| - %li= msg + = form_errors(@user) + - unless @user.password_automatically_set? .form-group = f.label :current_password, class: 'label-light' diff --git a/app/views/profiles/passwords/new.html.haml b/app/views/profiles/passwords/new.html.haml index d165f758c8..2eb9fac57c 100644 --- a/app/views/profiles/passwords/new.html.haml +++ b/app/views/profiles/passwords/new.html.haml @@ -7,11 +7,8 @@ Please set a new password before proceeding. %br After a successful password update you will be redirected to login screen. - -if @user.errors.any? - .alert.alert-danger - %ul - - @user.errors.full_messages.each do |msg| - %li= msg + + = form_errors(@user) - unless @user.password_automatically_set? .form-group diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index dcb3be9585..f59d27f7ed 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -1,9 +1,6 @@ = form_for @user, url: profile_path, method: :put, html: { multipart: true, class: "edit-user prepend-top-default" }, authenticity_token: true do |f| - -if @user.errors.any? - %div.alert.alert-danger - %ul - - @user.errors.full_messages.each do |msg| - %li= msg + = form_errors(@user) + .row .col-lg-3.profile-settings-sidebar %h4.prepend-top-0 diff --git a/app/views/projects/_errors.html.haml b/app/views/projects/_errors.html.haml index 7c8bb33ed7..2dba22d3be 100644 --- a/app/views/projects/_errors.html.haml +++ b/app/views/projects/_errors.html.haml @@ -1,4 +1 @@ -- if @project.errors.any? - .alert.alert-danger - %button{ type: "button", class: "close", "data-dismiss" => "alert"} × - = @project.errors.full_messages.first += form_errors(@project) diff --git a/app/views/projects/deploy_keys/_form.html.haml b/app/views/projects/deploy_keys/_form.html.haml index 5e182af266..f6565f8583 100644 --- a/app/views/projects/deploy_keys/_form.html.haml +++ b/app/views/projects/deploy_keys/_form.html.haml @@ -1,10 +1,6 @@ %div = form_for [@project.namespace.becomes(Namespace), @project, @key], url: namespace_project_deploy_keys_path, html: { class: 'deploy-key-form form-horizontal js-requires-input' } do |f| - -if @key.errors.any? - .alert.alert-danger - %ul - - @key.errors.full_messages.each do |msg| - %li= msg + = form_errors(@key) .form-group = f.label :title, class: "control-label" diff --git a/app/views/projects/hooks/index.html.haml b/app/views/projects/hooks/index.html.haml index 67d016bd87..e39224d86c 100644 --- a/app/views/projects/hooks/index.html.haml +++ b/app/views/projects/hooks/index.html.haml @@ -9,10 +9,8 @@ %hr.clearfix = form_for [@project.namespace.becomes(Namespace), @project, @hook], as: :hook, url: namespace_project_hooks_path(@project.namespace, @project), html: { class: 'form-horizontal' } do |f| - -if @hook.errors.any? - .alert.alert-danger - - @hook.errors.full_messages.each do |msg| - %p= msg + = form_errors(@hook) + .form-group = f.label :url, "URL", class: 'control-label' .col-sm-10 diff --git a/app/views/projects/labels/_form.html.haml b/app/views/projects/labels/_form.html.haml index be7a0bb562..aa143e54ff 100644 --- a/app/views/projects/labels/_form.html.haml +++ b/app/views/projects/labels/_form.html.haml @@ -1,11 +1,5 @@ = form_for [@project.namespace.becomes(Namespace), @project, @label], html: { class: 'form-horizontal label-form js-quick-submit js-requires-input' } do |f| - -if @label.errors.any? - .row - .col-sm-offset-2.col-sm-10 - .alert.alert-danger - - @label.errors.full_messages.each do |msg| - %span= msg - %br + = form_errors(@label) .form-group = f.label :title, class: 'control-label' diff --git a/app/views/projects/merge_requests/_new_compare.html.haml b/app/views/projects/merge_requests/_new_compare.html.haml index 01dc7519be..0931f743a3 100644 --- a/app/views/projects/merge_requests/_new_compare.html.haml +++ b/app/views/projects/merge_requests/_new_compare.html.haml @@ -28,10 +28,7 @@ .mr_target_commit - if @merge_request.errors.any? - .alert.alert-danger - - @merge_request.errors.full_messages.each do |msg| - %div= msg - + = form_errors(@merge_request) - elsif @merge_request.source_branch.present? && @merge_request.target_branch.present? .light-well.append-bottom-default .center diff --git a/app/views/projects/milestones/_form.html.haml b/app/views/projects/milestones/_form.html.haml index 23f2bca7ba..b2dae1c70e 100644 --- a/app/views/projects/milestones/_form.html.haml +++ b/app/views/projects/milestones/_form.html.haml @@ -1,9 +1,6 @@ = form_for [@project.namespace.becomes(Namespace), @project, @milestone], html: {class: 'form-horizontal milestone-form gfm-form js-quick-submit js-requires-input'} do |f| - -if @milestone.errors.any? - .alert.alert-danger - %ul - - @milestone.errors.full_messages.each do |msg| - %li= msg + = form_errors(@milestone) + .row .col-md-6 .form-group diff --git a/app/views/projects/protected_branches/index.html.haml b/app/views/projects/protected_branches/index.html.haml index cfd7e1534c..653b02da4d 100644 --- a/app/views/projects/protected_branches/index.html.haml +++ b/app/views/projects/protected_branches/index.html.haml @@ -13,11 +13,7 @@ - if can? current_user, :admin_project, @project = form_for [@project.namespace.becomes(Namespace), @project, @protected_branch], html: { class: 'form-horizontal' } do |f| - -if @protected_branch.errors.any? - .alert.alert-danger - %ul - - @protected_branch.errors.full_messages.each do |msg| - %li= msg + = form_errors(@protected_branch) .form-group = f.label :name, "Branch", class: 'control-label' diff --git a/app/views/projects/variables/show.html.haml b/app/views/projects/variables/show.html.haml index efe1e6f24c..ca284b84d3 100644 --- a/app/views/projects/variables/show.html.haml +++ b/app/views/projects/variables/show.html.haml @@ -13,13 +13,7 @@ = nested_form_for @project, url: url_for(controller: 'projects/variables', action: 'update'), html: { class: 'form-horizontal' } do |f| - - if @project.errors.any? - #error_explanation - %p.lead= "#{pluralize(@project.errors.count, "error")} prohibited this project from being saved:" - .alert.alert-error - %ul - - @project.errors.full_messages.each do |msg| - %li= msg + = form_errors(@project) = f.fields_for :variables do |variable_form| .form-group diff --git a/app/views/projects/wikis/_form.html.haml b/app/views/projects/wikis/_form.html.haml index f0d1932e23..812876e283 100644 --- a/app/views/projects/wikis/_form.html.haml +++ b/app/views/projects/wikis/_form.html.haml @@ -1,9 +1,5 @@ = form_for [@project.namespace.becomes(Namespace), @project, @page], method: @page.persisted? ? :put : :post, html: { class: 'form-horizontal wiki-form gfm-form prepend-top-default js-quick-submit' } do |f| - -if @page.errors.any? - #error_explanation - .alert.alert-danger - - @page.errors.full_messages.each do |msg| - %p= msg + = form_errors(@page) = f.hidden_field :title, value: @page.title .form-group diff --git a/app/views/shared/_service_settings.html.haml b/app/views/shared/_service_settings.html.haml index 5a60ff5a5d..fc935166bf 100644 --- a/app/views/shared/_service_settings.html.haml +++ b/app/views/shared/_service_settings.html.haml @@ -1,9 +1,4 @@ -- if @service.errors.any? - #error_explanation - .alert.alert-danger - %ul - - @service.errors.full_messages.each do |msg| - %li= msg += form_errors(@service) - if @service.help.present? .well diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index e2a9e5bfb9..0cda785f91 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -1,10 +1,5 @@ -- if issuable.errors.any? - .row - .col-sm-offset-2.col-sm-10 - .alert.alert-danger - - issuable.errors.full_messages.each do |msg| - %span= msg - %br += form_errors(issuable) + .form-group = f.label :title, class: 'control-label' .col-sm-10 diff --git a/app/views/shared/snippets/_form.html.haml b/app/views/shared/snippets/_form.html.haml index 1041eccd1d..47ec09f62c 100644 --- a/app/views/shared/snippets/_form.html.haml +++ b/app/views/shared/snippets/_form.html.haml @@ -1,10 +1,6 @@ .snippet-form-holder = form_for @snippet, url: url, html: { class: "form-horizontal snippet-form js-requires-input" } do |f| - - if @snippet.errors.any? - .alert.alert-danger - %ul - - @snippet.errors.full_messages.each do |msg| - %li= msg + = form_errors(@snippet) .form-group = f.label :title, class: 'control-label' diff --git a/spec/helpers/form_helper_spec.rb b/spec/helpers/form_helper_spec.rb new file mode 100644 index 0000000000..b20373a96f --- /dev/null +++ b/spec/helpers/form_helper_spec.rb @@ -0,0 +1,46 @@ +require 'rails_helper' + +describe FormHelper do + describe 'form_errors' do + it 'returns nil when model has no errors' do + model = double(errors: []) + + expect(helper.form_errors(model)).to be_nil + end + + it 'renders an alert div' do + model = double(errors: errors_stub('Error 1')) + + expect(helper.form_errors(model)). + to include('
    ') + end + + it 'contains a summary message' do + single_error = double(errors: errors_stub('A')) + multi_errors = double(errors: errors_stub('A', 'B', 'C')) + + expect(helper.form_errors(single_error)). + to include('

    The form contains the following error:') + expect(helper.form_errors(multi_errors)). + to include('

    The form contains the following errors:') + end + + it 'renders each message' do + model = double(errors: errors_stub('Error 1', 'Error 2', 'Error 3')) + + errors = helper.form_errors(model) + + aggregate_failures do + expect(errors).to include('
  • Error 1
  • ') + expect(errors).to include('
  • Error 2
  • ') + expect(errors).to include('
  • Error 3
  • ') + end + end + + def errors_stub(*messages) + ActiveModel::Errors.new(double).tap do |errors| + messages.each { |msg| errors.add(:base, msg) } + end + end + end +end From b9abf938edf52e762d320b1eb8732155e23d7b72 Mon Sep 17 00:00:00 2001 From: connorshea Date: Wed, 30 Mar 2016 16:03:49 -0600 Subject: [PATCH 061/261] Wrap images in discussions and wikis with a link to the image source using ImageLinkFilter. Resolves #14411. See merge request !3464 --- CHANGELOG | 1 + .../stylesheets/framework/typography.scss | 6 +++++ features/steps/project/wiki.rb | 2 +- lib/banzai/filter/image_link_filter.rb | 27 +++++++++++++++++++ lib/banzai/pipeline/gfm_pipeline.rb | 1 + spec/features/atom/users_spec.rb | 2 +- .../banzai/filter/image_link_filter_spec.rb | 24 +++++++++++++++++ 7 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 lib/banzai/filter/image_link_filter.rb create mode 100644 spec/lib/banzai/filter/image_link_filter_spec.rb diff --git a/CHANGELOG b/CHANGELOG index f72bb670ec..39239bebcf 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.7.0 (unreleased) + - All images in discussions and wikis now link to their source files !3464 (Connor Shea). - Improved Markdown rendering performance !3389 (Yorick Peterse) - Don't attempt to look up an avatar in repo if repo directory does not exist (Stan hu) - Preserve time notes/comments have been updated at when moving issue diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index b1886fbe67..c3c7bc9fdb 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -138,6 +138,12 @@ } } + a.no-attachment-icon { + &:before { + display: none; + } + } + /* Link to current header. */ h1, h2, h3, h4, h5, h6 { position: relative; diff --git a/features/steps/project/wiki.rb b/features/steps/project/wiki.rb index 223b7277b5..9f6aed1c5b 100644 --- a/features/steps/project/wiki.rb +++ b/features/steps/project/wiki.rb @@ -85,7 +85,7 @@ class Spinach::Features::ProjectWiki < Spinach::FeatureSteps end step 'I have an existing Wiki page with images linked on page' do - wiki.create_page("pictures", "Look at this [image](image.jpg)\n\n ![image](image.jpg)", :markdown, "first commit") + wiki.create_page("pictures", "Look at this [image](image.jpg)\n\n ![alt text](image.jpg)", :markdown, "first commit") @wiki_page = wiki.find_page("pictures") end diff --git a/lib/banzai/filter/image_link_filter.rb b/lib/banzai/filter/image_link_filter.rb new file mode 100644 index 0000000000..ccd106860b --- /dev/null +++ b/lib/banzai/filter/image_link_filter.rb @@ -0,0 +1,27 @@ +module Banzai + module Filter + # HTML filter that wraps links around inline images. + class ImageLinkFilter < HTML::Pipeline::Filter + + # Find every image that isn't already wrapped in an `a` tag, create + # a new node (a link to the image source), copy the image as a child + # of the anchor, and then replace the img with the link-wrapped version. + def call + doc.xpath('descendant-or-self::img[not(ancestor::a)]').each do |img| + + link = doc.document.create_element( + 'a', + class: 'no-attachment-icon', + href: img['src'], + target: '_blank' + ) + + link.children = img.clone + img.replace(link) + end + + doc + end + end + end +end diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb index 8cd4b50e65..ed3cfd6b02 100644 --- a/lib/banzai/pipeline/gfm_pipeline.rb +++ b/lib/banzai/pipeline/gfm_pipeline.rb @@ -7,6 +7,7 @@ module Banzai Filter::SanitizationFilter, Filter::UploadLinkFilter, + Filter::ImageLinkFilter, Filter::EmojiFilter, Filter::TableOfContentsFilter, Filter::AutolinkFilter, diff --git a/spec/features/atom/users_spec.rb b/spec/features/atom/users_spec.rb index dc41be8246..de6aed74fb 100644 --- a/spec/features/atom/users_spec.rb +++ b/spec/features/atom/users_spec.rb @@ -61,7 +61,7 @@ describe "User Feed", feature: true do end it 'should have XHTML summaries in merge request descriptions' do - expect(body).to match /Here is the fix: ]*\/>/ + expect(body).to match /Here is the fix: ]*>]*\/><\/a>/ end end end diff --git a/spec/lib/banzai/filter/image_link_filter_spec.rb b/spec/lib/banzai/filter/image_link_filter_spec.rb new file mode 100644 index 0000000000..dd5594750c --- /dev/null +++ b/spec/lib/banzai/filter/image_link_filter_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe Banzai::Filter::ImageLinkFilter, lib: true do + include FilterSpecHelper + + def image(path) + %() + end + + it 'wraps the image with a link to the image src' do + doc = filter(image('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg')) + expect(doc.at_css('img')['src']).to eq doc.at_css('a')['href'] + end + + it 'does not wrap a duplicate link' do + exp = act = %q(#{image('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg')}) + expect(filter(act).to_html).to eq exp + end + + it 'works with external images' do + doc = filter(image('https://i.imgur.com/DfssX9C.jpg')) + expect(doc.at_css('img')['src']).to eq doc.at_css('a')['href'] + end +end From 8e3f5ebaa080bec65b35c21b8a70f84c7ed9fa63 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 5 Apr 2016 09:07:40 +0100 Subject: [PATCH 062/261] CS multiline --- app/assets/javascripts/gl_dropdown.js.coffee | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 466213496e..6a825a67a1 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -331,11 +331,11 @@ class GitLabDropdown ).join('') noResults: -> - html = "" + html = "" highlightRow: (index) -> if @filterInput.val() isnt "" From 4d9d06ec5057c512bd05ab5883ca842c7a3a3c9a Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 5 Apr 2016 09:12:26 +0100 Subject: [PATCH 063/261] Removed emoji button from notes form --- app/views/projects/notes/_hints.html.haml | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/views/projects/notes/_hints.html.haml b/app/views/projects/notes/_hints.html.haml index 0c6758210b..0b00204340 100644 --- a/app/views/projects/notes/_hints.html.haml +++ b/app/views/projects/notes/_hints.html.haml @@ -1,7 +1,4 @@ .comment-toolbar.clearfix - %button.toolbar-button.js-toolbar-button{ type: 'button', data: { prefix: ':' }, tabindex: '-1' } - = icon('smile-o', class: 'toolbar-button-icon') - Emoji .toolbar-text Styling with = link_to 'Markdown', help_page_path('markdown', 'markdown'), target: '_blank', tabindex: -1 From 831d1807df6d1d9f8213898614a24bea78208114 Mon Sep 17 00:00:00 2001 From: "P.S.V.R" Date: Tue, 5 Apr 2016 16:20:08 +0800 Subject: [PATCH 064/261] Eliminate ugly scroll bars shown in modal boxes --- app/assets/stylesheets/pages/help.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/assets/stylesheets/pages/help.scss b/app/assets/stylesheets/pages/help.scss index bd224705f0..604f1700cf 100644 --- a/app/assets/stylesheets/pages/help.scss +++ b/app/assets/stylesheets/pages/help.scss @@ -59,6 +59,9 @@ position: relative; overflow-y: auto; padding: 15px; + .form-actions { + margin: -$gl-padding+1; + } } body.modal-open { From b8d1545bf18e672a68b9095e4c9c6cd6c018aad7 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 5 Apr 2016 10:54:34 +0200 Subject: [PATCH 065/261] Update language after doing all other operations --- app/services/git_push_service.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index c007d648dd..c76c118df1 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -43,13 +43,14 @@ class GitPushService < BaseService @push_commits = @project.repository.commits_between(params[:oldrev], params[:newrev]) process_commit_messages end - # Checks if the main language has changed in the project and if so - # it updates it accordingly - update_main_language # Update merge requests that may be affected by this push. A new branch # could cause the last commit of a merge request to change. update_merge_requests + # Checks if the main language has changed in the project and if so + # it updates it accordingly + update_main_language + perform_housekeeping end From aad3b6ddf88e31072602af7d1d06f64e1823673b Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 5 Apr 2016 11:08:41 +0200 Subject: [PATCH 066/261] Update language only on HEAD of the repository --- app/services/git_push_service.rb | 3 +++ spec/services/git_push_service_spec.rb | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index c76c118df1..36c9ee92da 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -55,6 +55,9 @@ class GitPushService < BaseService end def update_main_language + return unless is_default_branch? + return unless push_to_new_branch? || push_to_existing_branch? + current_language = @project.repository.main_language unless current_language == @project.main_language diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 8490a729e5..1047e32960 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -159,18 +159,28 @@ describe GitPushService, services: true do end describe "Updates main language" do - context "before push" do it { expect(project.main_language).to eq(nil) } end context "after push" do before do - @service = execute_service(project, user, @oldrev, @newrev, @ref) + @service = execute_service(project, user, @oldrev, @newrev, ref) end - it { expect(@service.update_main_language).to eq(true) } - it { expect(project.main_language).to eq("Ruby") } + context "to master" do + let(:ref) { @ref } + + it { expect(@service.update_main_language).to eq(true) } + it { expect(project.main_language).to eq("Ruby") } + end + + context "to other branch" do + let(:ref) { 'refs/heads/feature/branch' } + + it { expect(@service.update_main_language).to eq(nil) } + it { expect(project.main_language).to eq(nil) } + end end end From b248ee93814e8521fa0c73c82ec9ed113698b945 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 5 Apr 2016 13:29:48 +0200 Subject: [PATCH 067/261] Check permissions when importing project members Closes #14899 --- CHANGELOG | 3 ++ .../projects/project_members_controller.rb | 9 +++- .../project_members_controller_spec.rb | 49 +++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 spec/controllers/projects/project_members_controller_spec.rb diff --git a/CHANGELOG b/CHANGELOG index 39239bebcf..362a571bb4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -20,6 +20,9 @@ v 8.7.0 (unreleased) - Fall back to `In-Reply-To` and `References` headers when sub-addressing is not available (David Padilla) - Remove "Congratulations!" tweet button on newly-created project. (Connor Shea) +v 8.6.5 (unreleased) + - Check permissions when user attempts to import members from another project + v 8.6.4 - Don't attempt to fetch any tags from a forked repo (Stan Hu) diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index e7bddc4a6f..cd984f03c6 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -95,8 +95,13 @@ class Projects::ProjectMembersController < Projects::ApplicationController def apply_import giver = Project.find(params[:source_project_id]) - status = @project.team.import(giver, current_user) - notice = status ? "Successfully imported" : "Import failed" + + if current_user.can?(:read_project_member, giver) + status = @project.team.import(giver, current_user) + notice = status ? "Successfully imported" : "Import failed" + else + notice = 'You are not authorized to import members from this project' + end redirect_to(namespace_project_project_members_path(project.namespace, project), notice: notice) diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb new file mode 100644 index 0000000000..6d1df8d9fb --- /dev/null +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -0,0 +1,49 @@ +require('spec_helper') + +describe Projects::ProjectMembersController do + let(:project) { create(:project) } + let(:another_project) { create(:project, :private) } + let(:user) { create(:user) } + let(:member) { create(:user) } + + before do + project.team << [user, :master] + another_project.team << [member, :guest] + sign_in(user) + end + + describe '#apply_import' do + shared_context 'import applied' do + before do + post(:apply_import, namespace_id: project.namespace.to_param, + project_id: project.to_param, + source_project_id: another_project.id) + end + end + + context 'when user can access source project members' do + before { another_project.team << [user, :guest] } + include_context 'import applied' + + it 'imports source project members' do + expect(project.team_members).to include member + expect(response).to set_flash.to 'Successfully imported' + expect(response).to redirect_to( + namespace_project_project_members_path(project.namespace, project) + ) + end + end + + context 'when user is not member of a source project' do + include_context 'import applied' + + it 'does not import team members' do + expect(project.team_members).to_not include member + end + + it 'notifies about invalid permissions' do + expect(response).to set_flash.to /not authorized/ + end + end + end +end From c52b5c92fbd31dc6f76087c43a94243d382d3172 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 5 Apr 2016 13:55:15 +0200 Subject: [PATCH 068/261] Do not leak project exists when importing members When importing members, and user does not have permissions to read members in a source project, do not leak information about source project existence. Notifiy user that project has not been found instead. --- app/controllers/projects/project_members_controller.rb | 8 ++++---- .../projects/project_members_controller_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index cd984f03c6..fd56dfd126 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -94,13 +94,13 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def apply_import - giver = Project.find(params[:source_project_id]) + source_project = Project.find(params[:source_project_id]) - if current_user.can?(:read_project_member, giver) - status = @project.team.import(giver, current_user) + if can?(current_user, :read_project_member, source_project) + status = @project.team.import(source_project, current_user) notice = status ? "Successfully imported" : "Import failed" else - notice = 'You are not authorized to import members from this project' + notice = 'Import failed - source project not found!' end redirect_to(namespace_project_project_members_path(project.namespace, project), diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 6d1df8d9fb..6ff3d4199f 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -41,8 +41,8 @@ describe Projects::ProjectMembersController do expect(project.team_members).to_not include member end - it 'notifies about invalid permissions' do - expect(response).to set_flash.to /not authorized/ + it 'pretends that source projects does not exist' do + expect(response).to set_flash.to /source project not found/ end end end From f22e6515df8c062c3a1c29e21c4d846a9e000c9d Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 5 Apr 2016 13:36:00 +0100 Subject: [PATCH 069/261] Added CHANGELOG for build notifications [ci skip] --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index f0b5e58ded..e1b6b32cff 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -20,6 +20,7 @@ v 8.7.0 (unreleased) - Fall back to `In-Reply-To` and `References` headers when sub-addressing is not available (David Padilla) - Remove "Congratulations!" tweet button on newly-created project. (Connor Shea) - Improved UX of the navigation sidebar + - Build status notifications v 8.6.4 - Don't attempt to fetch any tags from a forked repo (Stan Hu) From bb9c194c23b8b3ffef30c7fdbe244d4fefc93883 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 5 Apr 2016 14:37:06 +0200 Subject: [PATCH 070/261] Respond 404 when unauthorized user imports members --- app/controllers/projects/project_members_controller.rb | 2 +- spec/controllers/projects/project_members_controller_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index fd56dfd126..e457db2f0b 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -100,7 +100,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController status = @project.team.import(source_project, current_user) notice = status ? "Successfully imported" : "Import failed" else - notice = 'Import failed - source project not found!' + return render_404 end redirect_to(namespace_project_project_members_path(project.namespace, project), diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 6ff3d4199f..d47e4ab9a4 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -41,8 +41,8 @@ describe Projects::ProjectMembersController do expect(project.team_members).to_not include member end - it 'pretends that source projects does not exist' do - expect(response).to set_flash.to /source project not found/ + it 'responds with not found' do + expect(response.status).to eq 404 end end end From 7b290e29f0c7484617d4f1c09244db84c00c11b4 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Tue, 5 Apr 2016 17:04:16 +0300 Subject: [PATCH 071/261] Fix LDAP link and codeblock indentation [ci skip] --- doc/administration/auth/ldap.md | 12 ++++++------ doc/integration/ldap.md | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/administration/auth/ldap.md b/doc/administration/auth/ldap.md index 237700bbcd..1009677984 100644 --- a/doc/administration/auth/ldap.md +++ b/doc/administration/auth/ldap.md @@ -261,13 +261,13 @@ tree and traverse it. - Run the following check command to make sure that the LDAP settings are correct and GitLab can see your users: - ```bash - # For Omnibus installations - sudo gitlab-rake gitlab:ldap:check + ```bash + # For Omnibus installations + sudo gitlab-rake gitlab:ldap:check - # For installations from source - sudo -u git -H bundle exec rake gitlab:ldap:check RAILS_ENV=production - ``` + # For installations from source + sudo -u git -H bundle exec rake gitlab:ldap:check RAILS_ENV=production + ``` ### Connection Refused diff --git a/doc/integration/ldap.md b/doc/integration/ldap.md index fb20308c49..30f0c15dac 100644 --- a/doc/integration/ldap.md +++ b/doc/integration/ldap.md @@ -1,3 +1,3 @@ # GitLab LDAP integration -This document was moved under [`administration/auth/ldap`](administration/auth/ldap.md). +This document was moved under [`administration/auth/ldap`](../administration/auth/ldap.md). From 1ba9a91c6d3b98e1825e173fe281ba065d35890c Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Thu, 31 Mar 2016 16:11:49 -0300 Subject: [PATCH 072/261] Fix problem when creating milestones in groups without projects --- .../groups/milestones_controller.rb | 28 +++++++++++++++---- .../groups/milestones_controller_spec.rb | 6 ++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb index b23c3022fb..2c05d9e0fe 100644 --- a/app/controllers/groups/milestones_controller.rb +++ b/app/controllers/groups/milestones_controller.rb @@ -18,14 +18,14 @@ class Groups::MilestonesController < Groups::ApplicationController end def create - project_ids = params[:milestone][:project_ids] + project_ids = params[:milestone][:project_ids].reject(&:blank?) title = milestone_params[:title] - @projects.where(id: project_ids).each do |project| - Milestones::CreateService.new(project, current_user, milestone_params).execute + if project_ids.present? + create_milestones(project_ids, title) + else + render_new_with_error("Select a project(s).") end - - redirect_to milestone_path(title) end def show @@ -41,6 +41,24 @@ class Groups::MilestonesController < Groups::ApplicationController private + def create_milestones(project_ids, title) + begin + @projects.where(id: project_ids).each do |project| + ActiveRecord::Base.transaction { Milestones::CreateService.new(project, current_user, milestone_params).execute } + end + + redirect_to milestone_path(title) + rescue => e + render_new_with_error("Error creating milestones: #{e.message}") + end + end + + def render_new_with_error(error) + @milestone = Milestone.new(milestone_params) + flash[:alert] = error + render :new + end + def authorize_admin_milestones! return render_404 unless can?(current_user, :admin_milestones, group) end diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index eb0c6ac6d8..f258d6fa0c 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -23,5 +23,11 @@ describe Groups::MilestonesController do expect(response).to redirect_to(group_milestone_path(group, title.to_slug.to_s, title: title)) expect(Milestone.where(title: title).count).to eq(2) end + + it "redirects to new when there are no project ids" do + post :create, group_id: group.id, milestone: { title: title, project_ids: [""] } + expect(response).to render_template :new + expect(flash[:alert]).to_not be_nil + end end end From 5d428030451b1fa2bac89f798c40d2f91ac65bac Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Fri, 1 Apr 2016 15:50:17 -0300 Subject: [PATCH 073/261] Improve code --- .../groups/milestones_controller.rb | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb index 2c05d9e0fe..21fc329f23 100644 --- a/app/controllers/groups/milestones_controller.rb +++ b/app/controllers/groups/milestones_controller.rb @@ -21,10 +21,10 @@ class Groups::MilestonesController < Groups::ApplicationController project_ids = params[:milestone][:project_ids].reject(&:blank?) title = milestone_params[:title] - if project_ids.present? - create_milestones(project_ids, title) + if create_milestones(project_ids, title) + redirect_to milestone_path(title) else - render_new_with_error("Select a project(s).") + render_new_with_error(@error) end end @@ -42,14 +42,22 @@ class Groups::MilestonesController < Groups::ApplicationController private def create_milestones(project_ids, title) + unless project_ids.present? + @error = "Please select at least one project." + return false + end + begin - @projects.where(id: project_ids).each do |project| - ActiveRecord::Base.transaction { Milestones::CreateService.new(project, current_user, milestone_params).execute } + ActiveRecord::Base.transaction do + @projects.where(id: project_ids).each do |project| + Milestones::CreateService.new(project, current_user, milestone_params).execute + end end - redirect_to milestone_path(title) + true rescue => e - render_new_with_error("Error creating milestones: #{e.message}") + @error = "Error creating milestone: #{e.message}." + false end end From 32c7e42b612bdda43eeef55d8c8afdc9eeb33785 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Mon, 4 Apr 2016 17:04:35 -0300 Subject: [PATCH 074/261] Improve code --- .../groups/milestones_controller.rb | 31 +++++++++---------- app/views/groups/milestones/new.html.haml | 8 +++++ .../groups/milestones_controller_spec.rb | 2 +- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb index 21fc329f23..fcf19e8066 100644 --- a/app/controllers/groups/milestones_controller.rb +++ b/app/controllers/groups/milestones_controller.rb @@ -24,7 +24,7 @@ class Groups::MilestonesController < Groups::ApplicationController if create_milestones(project_ids, title) redirect_to milestone_path(title) else - render_new_with_error(@error) + render_new_with_error(project_ids.empty?) end end @@ -42,28 +42,25 @@ class Groups::MilestonesController < Groups::ApplicationController private def create_milestones(project_ids, title) - unless project_ids.present? - @error = "Please select at least one project." - return false - end + return false unless project_ids.present? - begin - ActiveRecord::Base.transaction do - @projects.where(id: project_ids).each do |project| - Milestones::CreateService.new(project, current_user, milestone_params).execute - end + ActiveRecord::Base.transaction do + @projects.where(id: project_ids).each do |project| + Milestones::CreateService.new(project, current_user, milestone_params).execute end - - true - rescue => e - @error = "Error creating milestone: #{e.message}." - false end + + true + + rescue => e + + flash.now[:alert] = "An error occurred while creating the milestone: #{e.message}" + false end - def render_new_with_error(error) + def render_new_with_error(empty_project_ids) @milestone = Milestone.new(milestone_params) - flash[:alert] = error + @milestone.errors.add(:project_id, "Please select at least one project.") if empty_project_ids render :new end diff --git a/app/views/groups/milestones/new.html.haml b/app/views/groups/milestones/new.html.haml index a8e1ed77da..4290e0bf72 100644 --- a/app/views/groups/milestones/new.html.haml +++ b/app/views/groups/milestones/new.html.haml @@ -10,6 +10,14 @@ = form_for @milestone, url: group_milestones_path(@group), html: { class: 'form-horizontal milestone-form gfm-form js-quick-submit js-requires-input' } do |f| .row + - if @milestone.errors.any? + #error_explanation + .alert.alert-danger + %ul + - @milestone.errors.full_messages.each do |msg| + %li + = msg + .col-md-6 .form-group = f.label :title, "Title", class: "control-label" diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index f258d6fa0c..9c7b5c74b8 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -27,7 +27,7 @@ describe Groups::MilestonesController do it "redirects to new when there are no project ids" do post :create, group_id: group.id, milestone: { title: title, project_ids: [""] } expect(response).to render_template :new - expect(flash[:alert]).to_not be_nil + expect(assigns(:milestone).errors).to_not be_nil end end end From c8d0f355022956e557c148fd5b5a3176661abba4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 5 Apr 2016 18:50:43 +0200 Subject: [PATCH 075/261] Add 8.5.9 CHANGELOG item MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [ci skip] Signed-off-by: Rémy Coutable --- CHANGELOG | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index e1b6b32cff..071d02f916 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -140,6 +140,9 @@ v 8.6.0 - Trigger a todo for mentions on commits page - Let project owners and admins soft delete issues and merge requests +v 8.5.9 + - Don't attempt to fetch any tags from a forked repo (Stan Hu). + v 8.5.8 - Bump Git version requirement to 2.7.4 From 9f33bf86cf55df8a00357f49bc63a5291b2a8024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 5 Apr 2016 18:56:09 +0200 Subject: [PATCH 076/261] Add 8.4.6 and 8.4.7 CHANGELOG items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [ci skip] Signed-off-by: Rémy Coutable --- CHANGELOG | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 071d02f916..a4bb4589f3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -284,6 +284,12 @@ v 8.5.0 - Show label row when filtering issues or merge requests by label (Nuttanart Pornprasitsakul) - Add Todos +v 8.4.7 + - Don't attempt to fetch any tags from a forked repo (Stan Hu). + +v 8.4.6 + - Bump Git version requirement to 2.7.4 + v 8.4.5 - No CE-specific changes From 5ff9d2a1821ef0413a349388bcdc0fab84a17086 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 28 Mar 2016 15:37:53 -0400 Subject: [PATCH 077/261] Bump rails to 4.2.6 --- Gemfile | 2 +- Gemfile.lock | 72 ++++++++++++++++++------------------- config/environments/test.rb | 1 + 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/Gemfile b/Gemfile index 6327227282..5eaaf0cfb1 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,6 @@ source "https://rubygems.org" -gem 'rails', '4.2.5.2' +gem 'rails', '4.2.6' gem 'rails-deprecated_sanitizer', '~> 1.0.3' # Responders respond_to and respond_with diff --git a/Gemfile.lock b/Gemfile.lock index 229089f431..df18410ece 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,41 +4,41 @@ GEM CFPropertyList (2.3.2) RedCloth (4.2.9) ace-rails-ap (2.0.1) - actionmailer (4.2.5.2) - actionpack (= 4.2.5.2) - actionview (= 4.2.5.2) - activejob (= 4.2.5.2) + actionmailer (4.2.6) + actionpack (= 4.2.6) + actionview (= 4.2.6) + activejob (= 4.2.6) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 1.0, >= 1.0.5) - actionpack (4.2.5.2) - actionview (= 4.2.5.2) - activesupport (= 4.2.5.2) + actionpack (4.2.6) + actionview (= 4.2.6) + activesupport (= 4.2.6) rack (~> 1.6) rack-test (~> 0.6.2) rails-dom-testing (~> 1.0, >= 1.0.5) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (4.2.5.2) - activesupport (= 4.2.5.2) + actionview (4.2.6) + activesupport (= 4.2.6) builder (~> 3.1) erubis (~> 2.7.0) rails-dom-testing (~> 1.0, >= 1.0.5) rails-html-sanitizer (~> 1.0, >= 1.0.2) - activejob (4.2.5.2) - activesupport (= 4.2.5.2) + activejob (4.2.6) + activesupport (= 4.2.6) globalid (>= 0.3.0) - activemodel (4.2.5.2) - activesupport (= 4.2.5.2) + activemodel (4.2.6) + activesupport (= 4.2.6) builder (~> 3.1) - activerecord (4.2.5.2) - activemodel (= 4.2.5.2) - activesupport (= 4.2.5.2) + activerecord (4.2.6) + activemodel (= 4.2.6) + activesupport (= 4.2.6) arel (~> 6.0) activerecord-deprecated_finders (1.0.4) activerecord-session_store (0.1.2) actionpack (>= 4.0.0, < 5) activerecord (>= 4.0.0, < 5) railties (>= 4.0.0, < 5) - activesupport (4.2.5.2) + activesupport (4.2.6) i18n (~> 0.7) json (~> 1.7, >= 1.7.7) minitest (~> 5.1) @@ -464,8 +464,8 @@ GEM nokogiri (>= 1.5.9) macaddr (1.7.1) systemu (~> 2.6.2) - mail (2.6.3) - mime-types (>= 1.16, < 3) + mail (2.6.4) + mime-types (>= 1.16, < 4) mail_room (0.6.1) method_source (0.8.2) mime-types (1.25.1) @@ -595,16 +595,16 @@ GEM rack rack-test (0.6.3) rack (>= 1.0) - rails (4.2.5.2) - actionmailer (= 4.2.5.2) - actionpack (= 4.2.5.2) - actionview (= 4.2.5.2) - activejob (= 4.2.5.2) - activemodel (= 4.2.5.2) - activerecord (= 4.2.5.2) - activesupport (= 4.2.5.2) + rails (4.2.6) + actionmailer (= 4.2.6) + actionpack (= 4.2.6) + actionview (= 4.2.6) + activejob (= 4.2.6) + activemodel (= 4.2.6) + activerecord (= 4.2.6) + activesupport (= 4.2.6) bundler (>= 1.3.0, < 2.0) - railties (= 4.2.5.2) + railties (= 4.2.6) sprockets-rails rails-deprecated_sanitizer (1.0.3) activesupport (>= 4.2.0.alpha) @@ -614,9 +614,9 @@ GEM rails-deprecated_sanitizer (>= 1.0.1) rails-html-sanitizer (1.0.3) loofah (~> 2.0) - railties (4.2.5.2) - actionpack (= 4.2.5.2) - activesupport (= 4.2.5.2) + railties (4.2.6) + actionpack (= 4.2.6) + activesupport (= 4.2.6) rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) rainbow (2.1.0) @@ -788,10 +788,10 @@ GEM spring (>= 0.9.1) sprockets (3.3.5) rack (> 1, < 3) - sprockets-rails (2.3.3) - actionpack (>= 3.0) - activesupport (>= 3.0) - sprockets (>= 2.8, < 4.0) + sprockets-rails (3.0.4) + actionpack (>= 4.0) + activesupport (>= 4.0) + sprockets (>= 3.0.0) state_machines (0.4.0) state_machines-activemodel (0.3.0) activemodel (~> 4.1) @@ -1002,7 +1002,7 @@ DEPENDENCIES rack-attack (~> 4.3.1) rack-cors (~> 0.4.0) rack-oauth2 (~> 1.2.1) - rails (= 4.2.5.2) + rails (= 4.2.6) rails-deprecated_sanitizer (~> 1.0.3) raphael-rails (~> 2.1.2) rblineprof diff --git a/config/environments/test.rb b/config/environments/test.rb index f96ac6f975..a703c0934f 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -8,6 +8,7 @@ Rails.application.configure do config.cache_classes = false # Configure static asset server for tests with Cache-Control for performance + config.assets.digest = false config.serve_static_files = true config.static_cache_control = "public, max-age=3600" From 400a7b995de5c35fa3a2df83e08867c736f72324 Mon Sep 17 00:00:00 2001 From: Sytse Sijbrandij Date: Tue, 5 Apr 2016 13:24:58 -0400 Subject: [PATCH 078/261] Attribution where it is due. --- doc/markdown/markdown.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/markdown/markdown.md b/doc/markdown/markdown.md index e6eb1cf381..75d55b3417 100644 --- a/doc/markdown/markdown.md +++ b/doc/markdown/markdown.md @@ -31,7 +31,7 @@ _GitLab uses the [Redcarpet Ruby library][redcarpet] for Markdown processing._ -For GitLab we developed something we call "GitLab Flavored Markdown" (GFM). It extends the standard Markdown in a few significant ways to add some useful functionality. +For GitLab uses "GitLab Flavored Markdown" (GFM). It extends the standard Markdown in a few significant ways to add some useful functionality. It was inspired by [GitHub Flavored Markdown](https://help.github.com/articles/basic-writing-and-formatting-syntax/). You can use GFM in @@ -47,15 +47,15 @@ You can also use other rich text files in GitLab. You might have to install a de GFM honors the markdown specification in how [paragraphs and line breaks are handled](https://daringfireball.net/projects/markdown/syntax#p). -A paragraph is simply one or more consecutive lines of text, separated by one or more blank lines. +A paragraph is simply one or more consecutive lines of text, separated by one or more blank lines. Line-breaks, or softreturns, are rendered if you end a line with two or more spaces - Roses are red [followed by two or more spaces] + Roses are red [followed by two or more spaces] Violets are blue Sugar is sweet -Roses are red +Roses are red Violets are blue Sugar is sweet @@ -67,7 +67,7 @@ It is not reasonable to italicize just _part_ of a word, especially when you're perform_complicated_task do_this_and_do_that_and_another_thing -perform_complicated_task +perform_complicated_task do_this_and_do_that_and_another_thing ## URL auto-linking @@ -534,7 +534,7 @@ This line is separated from the one above by two newlines, so it will be a *sepa This line is also a separate paragraph, but... This line is only separated by a single newline, so it's a separate line in the *same paragraph*. -This line is also a separate paragraph, and... +This line is also a separate paragraph, and... This line is on its own line, because the previous line ends with two spaces. ``` @@ -546,7 +546,7 @@ This line is separated from the one above by two newlines, so it will be a *sepa This line is also begins a separate paragraph, but... This line is only separated by a single newline, so it's a separate line in the *same paragraph*. -This line is also a separate paragraph, and... +This line is also a separate paragraph, and... This line is on its own line, because the previous line ends with two spaces. From b8d0f174224c883d2b6766ba424a7b5ad7f2431e Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 5 Apr 2016 14:06:56 -0400 Subject: [PATCH 079/261] The Markdown doc is one place where trailing whitespace matters [ci skip] --- doc/markdown/markdown.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/markdown/markdown.md b/doc/markdown/markdown.md index 75d55b3417..4f199b6af6 100644 --- a/doc/markdown/markdown.md +++ b/doc/markdown/markdown.md @@ -31,7 +31,7 @@ _GitLab uses the [Redcarpet Ruby library][redcarpet] for Markdown processing._ -For GitLab uses "GitLab Flavored Markdown" (GFM). It extends the standard Markdown in a few significant ways to add some useful functionality. It was inspired by [GitHub Flavored Markdown](https://help.github.com/articles/basic-writing-and-formatting-syntax/). +GitLab uses "GitLab Flavored Markdown" (GFM). It extends the standard Markdown in a few significant ways to add some useful functionality. It was inspired by [GitHub Flavored Markdown](https://help.github.com/articles/basic-writing-and-formatting-syntax/). You can use GFM in @@ -55,7 +55,7 @@ Line-breaks, or softreturns, are rendered if you end a line with two or more spa Sugar is sweet -Roses are red +Roses are red Violets are blue Sugar is sweet @@ -534,7 +534,7 @@ This line is separated from the one above by two newlines, so it will be a *sepa This line is also a separate paragraph, but... This line is only separated by a single newline, so it's a separate line in the *same paragraph*. -This line is also a separate paragraph, and... +This line is also a separate paragraph, and... This line is on its own line, because the previous line ends with two spaces. ``` @@ -546,7 +546,7 @@ This line is separated from the one above by two newlines, so it will be a *sepa This line is also begins a separate paragraph, but... This line is only separated by a single newline, so it's a separate line in the *same paragraph*. -This line is also a separate paragraph, and... +This line is also a separate paragraph, and... This line is on its own line, because the previous line ends with two spaces. From 934f1e9097485bbaebbe2759e995c77bb4391c5d Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 4 Apr 2016 14:59:54 -0700 Subject: [PATCH 080/261] Fix Error 500 after renaming a project path Closes #14885 --- CHANGELOG | 3 ++- app/controllers/projects_controller.rb | 3 +++ spec/controllers/projects_controller_spec.rb | 22 ++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index e1b6b32cff..7d744a6c49 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -3,11 +3,12 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.7.0 (unreleased) - All images in discussions and wikis now link to their source files !3464 (Connor Shea). - Improved Markdown rendering performance !3389 (Yorick Peterse) - - Don't attempt to look up an avatar in repo if repo directory does not exist (Stan hu) + - Don't attempt to look up an avatar in repo if repo directory does not exist (Stan Hu) - Preserve time notes/comments have been updated at when moving issue - Make HTTP(s) label consistent on clone bar (Stan Hu) - Expose label description in API (Mariusz Jachimowicz) - Allow back dating on issues when created through the API + - Fix Error 500 after renaming a project path (Stan Hu) - Fix avatar stretching by providing a cropping feature - Add endpoints to archive or unarchive a project !3372 - Add links to CI setup documentation from project settings and builds pages diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 8c3a74c823..3cc37e5985 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -40,6 +40,9 @@ class ProjectsController < Projects::ApplicationController def update status = ::Projects::UpdateService.new(@project, current_user, project_params).execute + # Refresh the repo in case anything changed + @repository = project.repository + respond_to do |format| if status flash[:notice] = "Project '#{@project.name}' was successfully updated." diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 1893e946f5..069cd917e5 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -83,6 +83,28 @@ describe ProjectsController do end end + describe "#update" do + render_views + + let(:admin) { create(:admin) } + + it "sets the repository to the right path after a rename" do + new_path = 'renamed_path' + project_params = { path: new_path } + controller.instance_variable_set(:@project, project) + sign_in(admin) + + put :update, + namespace_id: project.namespace.to_param, + id: project.id, + project: project_params + + expect(project.repository.path).to include(new_path) + expect(assigns(:repository).path).to eq(project.repository.path) + expect(response.status).to eq(200) + end + end + describe "#destroy" do let(:admin) { create(:admin) } From 33faf219aaf72bb2c8a6df06fd42adf17bebfd14 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Tue, 5 Apr 2016 11:43:30 -0700 Subject: [PATCH 081/261] Fix data check in update issue response --- app/assets/javascripts/issue.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/issue.js.coffee b/app/assets/javascripts/issue.js.coffee index d663e34871..b30e493592 100644 --- a/app/assets/javascripts/issue.js.coffee +++ b/app/assets/javascripts/issue.js.coffee @@ -49,7 +49,7 @@ class @Issue issueStatus = if isClose then 'close' else 'open' new Flash(issueFailMessage, 'alert') success: (data, textStatus, jqXHR) -> - if data.saved + if 'id' of data $(document).trigger('issuable:change'); if isClose $('a.btn-close').addClass('hidden') From 0ab6b82a234f6dcc208591724036f152eca0e1c4 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Tue, 5 Apr 2016 12:09:19 -0700 Subject: [PATCH 082/261] Update shades of red --- app/assets/stylesheets/framework/variables.scss | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 98fe794d36..dd189a2cb7 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -104,9 +104,9 @@ $orange-light: rgba(252, 109, 38, 0.80); $orange-normal: #e75e40; $orange-dark: #ce5237; -$red-light: #f06559; -$red-normal: #e52c5a; -$red-dark: #d22852; +$red-light: #e52c5a; +$red-normal: #d22852; +$red-dark: darken($red-normal, 5%); $border-white-light: #f1f2f4; $border-white-normal: #d6dae2; @@ -128,9 +128,9 @@ $border-orange-light: #fc6d26; $border-orange-normal: #ce5237; $border-orange-dark: #c14e35; -$border-red-light: #f24f41; -$border-red-normal: #d22852; -$border-red-dark: #ca264f; +$border-red-light: #d22852; +$border-red-normal: #ca264f; +$border-red-dark: darken($border-red-normal, 5%); $help-well-bg: #fafafa; $help-well-border: #e5e5e5; From 5ee6badade3c453c7090e9c1f1f4d636c5bb068e Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 5 Apr 2016 16:33:37 -0300 Subject: [PATCH 083/261] Unblocks user when active_directory is disabled and it can be found --- lib/gitlab/ldap/access.rb | 5 ++++- spec/lib/gitlab/ldap/access_spec.rb | 27 ++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index da4435c730..f2b649e50a 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -33,7 +33,10 @@ module Gitlab def allowed? if ldap_user - return true unless ldap_config.active_directory + unless ldap_config.active_directory + user.activate if user.ldap_blocked? + return true + end # Block user in GitLab if he/she was blocked in AD if Gitlab::LDAP::Person.disabled_via_active_directory?(user.ldap_identity.extern_uid, adapter) diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 32a19bf344..f5b66b8156 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -33,7 +33,7 @@ describe Gitlab::LDAP::Access, lib: true do it { is_expected.to be_falsey } - it 'should block user in GitLab' do + it 'blocks user in GitLab' do access.allowed? expect(user).to be_blocked expect(user).to be_ldap_blocked @@ -78,6 +78,31 @@ describe Gitlab::LDAP::Access, lib: true do end it { is_expected.to be_truthy } + + context 'when user cannot be found' do + before do + allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(nil) + end + + it { is_expected.to be_falsey } + + it 'blocks user in GitLab' do + access.allowed? + expect(user).to be_blocked + expect(user).to be_ldap_blocked + end + end + + context 'when user was previously ldap_blocked' do + before do + user.ldap_block + end + + it 'unblocks the user if it exists' do + access.allowed? + expect(user).not_to be_blocked + end + end end end end From 1eeec7c63ee3a293f134ff16b5f2209e77ab0f6c Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Tue, 5 Apr 2016 13:13:49 -0700 Subject: [PATCH 084/261] Update issue_spec test --- spec/javascripts/issue_spec.js.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/javascripts/issue_spec.js.coffee b/spec/javascripts/issue_spec.js.coffee index 86ba9dd8e9..ea27f36e9b 100644 --- a/spec/javascripts/issue_spec.js.coffee +++ b/spec/javascripts/issue_spec.js.coffee @@ -29,8 +29,8 @@ describe 'reopen/close issue', -> spyOn(jQuery, 'ajax').and.callFake (req) -> expect(req.type).toBe('PUT') expect(req.url).toBe('http://gitlab.com/issues/6/close') - req.success saved: true - + req.success id: 34 + $btnClose = $('a.btn-close') $btnReopen = $('a.btn-reopen') expect($btnReopen).toBeHidden() @@ -94,7 +94,7 @@ describe 'reopen/close issue', -> spyOn(jQuery, 'ajax').and.callFake (req) -> expect(req.type).toBe('PUT') expect(req.url).toBe('http://gitlab.com/issues/6/reopen') - req.success saved: true + req.success id: 34 $btnClose = $('a.btn-close') $btnReopen = $('a.btn-reopen') From ee1de011d1a9e2990fd56a794bbff6b0ec374d00 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 5 Apr 2016 17:03:34 -0400 Subject: [PATCH 085/261] Premailer shouldn't remove script tags from our emails Closes #14943. --- config/initializers/premailer.rb | 3 ++- spec/mailers/shared/notify.rb | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/config/initializers/premailer.rb b/config/initializers/premailer.rb index a44316bc3a..b9176688bc 100644 --- a/config/initializers/premailer.rb +++ b/config/initializers/premailer.rb @@ -3,5 +3,6 @@ Premailer::Rails.config.merge!( generate_text_part: false, preserve_styles: true, remove_comments: true, - remove_ids: true + remove_ids: true, + remove_scripts: false ) diff --git a/spec/mailers/shared/notify.rb b/spec/mailers/shared/notify.rb index 56a6dbf96f..5a85cb501d 100644 --- a/spec/mailers/shared/notify.rb +++ b/spec/mailers/shared/notify.rb @@ -141,10 +141,12 @@ shared_examples 'a new user email' do end shared_examples 'it should have Gmail Actions links' do + it { is_expected.to have_body_text '