From d2a9eefbfe86f1a152673d34f5803107c79c7d51 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 3 Oct 2016 23:18:35 +0800 Subject: [PATCH 01/73] Show commits from source project. Be consistent with: MergeRequest#pipeline Fixes #3596 --- app/views/projects/merge_requests/show/_commits.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/merge_requests/show/_commits.html.haml b/app/views/projects/merge_requests/show/_commits.html.haml index 0b05785430b..61020516bcf 100644 --- a/app/views/projects/merge_requests/show/_commits.html.haml +++ b/app/views/projects/merge_requests/show/_commits.html.haml @@ -3,4 +3,4 @@ Most recent commits displayed first %ol#commits-list.list-unstyled - = render "projects/commits/commits", project: @merge_request.project + = render "projects/commits/commits", project: @merge_request.source_project From f11ebb9556612f663264868a24ee8b774babda32 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 5 Oct 2016 18:08:16 +0800 Subject: [PATCH 02/73] Add a view test for showing source commits --- .../merge_requests/_commits.html.haml_spec.rb | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 spec/views/projects/merge_requests/_commits.html.haml_spec.rb diff --git a/spec/views/projects/merge_requests/_commits.html.haml_spec.rb b/spec/views/projects/merge_requests/_commits.html.haml_spec.rb new file mode 100644 index 00000000000..6f70b3daf8e --- /dev/null +++ b/spec/views/projects/merge_requests/_commits.html.haml_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe 'projects/merge_requests/show/_commits.html.haml' do + include Devise::Test::ControllerHelpers + + let(:user) { create(:user) } + let(:target_project) { create(:project) } + + let(:source_project) do + create(:project, forked_from_project: target_project) + end + + let(:merge_request) do + create(:merge_request, :simple, + source_project: source_project, + target_project: target_project, + author: user) + end + + before do + controller.prepend_view_path('app/views/projects') + + assign(:merge_request, merge_request) + assign(:commits, merge_request.commits) + end + + it 'shows commits from source project' do + render + + commit = source_project.commit(merge_request.source_branch) + href = namespace_project_commit_path( + source_project.namespace, + source_project, + commit) + + expect(rendered).to have_link(Commit.truncate_sha(commit.sha), href: href) + end +end From b5707a80ac1caa3aad809c0c9a0ad4fa91c9a834 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 5 Oct 2016 21:54:55 +0800 Subject: [PATCH 03/73] Add CHANGELOG entry [ci skip] --- CHANGELOG | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 58f1e4a59c2..5bb9ff1ec0e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -19,6 +19,7 @@ v 8.13.0 (unreleased) - Add word-wrap to issue title on issue and milestone boards (ClemMakesApps) - Fix robots.txt disallowing access to groups starting with "s" (Matt Harrison) - Close open merge request without source project (Katarzyna Kobierska Ula Budziszewska) + - Fix showing commits from source project for merge request !6658 - Add configurable email subject suffix (Fu Xu) - Use a ConnectionPool for Rails.cache on Sidekiq servers - Replace `alias_method_chain` with `Module#prepend` @@ -42,7 +43,7 @@ v 8.13.0 (unreleased) - Notify the Merger about merge after successful build (Dimitris Karakasilis) - Fix broken repository 500 errors in project list - Close todos when accepting merge requests via the API !6486 (tonygambone) - - Changed Slack service user referencing from full name to username (Sebastian Poxhofer) + - Changed Slack service user referencing from full name to username (Sebastian Poxhofer) v 8.12.4 (unreleased) From a04e9f9b61d93ca872fe108d83f519ba6151631e Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 11 Oct 2016 03:32:52 +0000 Subject: [PATCH 04/73] Remove bad stuffs from auto-merging --- CHANGELOG | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index eb9905a6f52..da4e66762e4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -83,8 +83,6 @@ v 8.13.0 (unreleased) - Fix Pipeline list commit column width should be adjusted - Close todos when accepting merge requests via the API !6486 (tonygambone) - Changed Slack service user referencing from full name to username (Sebastian Poxhofer) - -v 8.12.4 (unreleased) - Retouch environments list and deployments list - Add Container Registry on/off status to Admin Area !6638 (the-undefined) - Grouped pipeline dropdown is a scrollable container From 5f0b7fe429d75de2dbcfef142d2389bf99d199ec Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Mon, 17 Oct 2016 11:44:25 +0200 Subject: [PATCH 05/73] Stop injecting field errors where they won't be used. --- app/assets/javascripts/gl_field_errors.js.es6 | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/gl_field_errors.js.es6 b/app/assets/javascripts/gl_field_errors.js.es6 index 8657e7b4abf..8e8f9f29ab3 100644 --- a/app/assets/javascripts/gl_field_errors.js.es6 +++ b/app/assets/javascripts/gl_field_errors.js.es6 @@ -137,8 +137,11 @@ } initValidators () { - // select all non-hidden inputs in form - this.state.inputs = this.form.find(':input:not([type=hidden])').toArray() + // register selectors here as needed + const validateSelectors = [':text', ':password', '[type=email]'] + .map((selector) => `input${selector}`).join(','); + + this.state.inputs = this.form.find(validateSelectors).toArray() .filter((input) => !input.classList.contains(customValidationFlag)) .map((input) => new GlFieldError({ input, formErrors: this })); From 06564f9e049417087fa53cf8ec15c22ec65724d5 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Thu, 20 Oct 2016 12:47:32 +0200 Subject: [PATCH 06/73] Update gl_field_error tests for better input filtering. --- spec/javascripts/gl_field_errors_spec.js.es6 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/javascripts/gl_field_errors_spec.js.es6 b/spec/javascripts/gl_field_errors_spec.js.es6 index 36feb2b2aa5..da9259edd78 100644 --- a/spec/javascripts/gl_field_errors_spec.js.es6 +++ b/spec/javascripts/gl_field_errors_spec.js.es6 @@ -11,12 +11,12 @@ this.fieldErrors = new global.GlFieldErrors($form); }); - it('should properly initialize the form', function() { + it('should select the correct input elements', function() { expect(this.$form).toBeDefined(); expect(this.$form.length).toBe(1); expect(this.fieldErrors).toBeDefined(); const inputs = this.fieldErrors.state.inputs; - expect(inputs.length).toBe(5); + expect(inputs.length).toBe(4); }); it('should ignore elements with custom error handling', function() { From af669cbea386d900ec86510f7d43de6ca4c1771e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 17 Oct 2016 18:52:14 +0200 Subject: [PATCH 07/73] Change the approach to check if patches apply cleanly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .gitlab-ci.yml | 2 +- lib/gitlab/ee_compat_check.rb | 260 ++++++++++++++++++++++++++++ lib/tasks/ce_to_ee_merge_check.rake | 4 - lib/tasks/ee_compat_check.rake | 4 + lib/tasks/gitlab/dev.rake | 107 ++---------- 5 files changed, 276 insertions(+), 101 deletions(-) create mode 100644 lib/gitlab/ee_compat_check.rb delete mode 100644 lib/tasks/ce_to_ee_merge_check.rake create mode 100644 lib/tasks/ee_compat_check.rake diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 76117a48730..9c4b4acbaf5 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -210,7 +210,7 @@ rake brakeman: *exec rake flay: *exec license_finder: *exec rake downtime_check: *exec -rake ce_to_ee_merge_check: +rake ee_compat_check: <<: *exec only: - branches diff --git a/lib/gitlab/ee_compat_check.rb b/lib/gitlab/ee_compat_check.rb new file mode 100644 index 00000000000..5e8a0c14eba --- /dev/null +++ b/lib/gitlab/ee_compat_check.rb @@ -0,0 +1,260 @@ +module Gitlab + # Checks if a set of migrations requires downtime or not. + class EeCompatCheck + EE_REPO = 'https://gitlab.com/gitlab-org/gitlab-ee.git'.freeze + + attr_reader :ce_branch, :check_dir, :ce_repo + + def initialize(branch:, check_dir:, ce_repo: nil) + @ce_branch = branch + @check_dir = check_dir + @ce_repo = ce_repo || 'https://gitlab.com/gitlab-org/gitlab-ce.git' + end + + def check + ensure_ee_repo + delete_patches + + generate_patch(ce_branch, ce_patch_full_path) + + Dir.chdir(check_dir) do + step("In the #{check_dir} directory") + + step("Pulling latest master", %w[git pull --ff-only origin master]) + + status = catch(:halt_check) do + ce_branch_compat_check! + + delete_ee_branch_locally + + ee_branch_presence_check! + + ee_branch_compat_check! + end + + delete_ee_branch_locally + delete_patches + + if status.nil? + true + else + false + end + end + end + + private + + def ensure_ee_repo + if Dir.exist?(check_dir) + step("#{check_dir} already exists") + else + cmd = %W[git clone --branch master --single-branch --depth 1 #{EE_REPO} #{check_dir}] + step("Cloning #{EE_REPO} into #{check_dir}", cmd) + end + end + + def ce_branch_compat_check! + cmd = %W[git apply --check #{ce_patch_full_path}] + status = step("Checking if #{ce_patch_name} applies cleanly to EE/master", cmd) + + if status.zero? + puts ce_applies_cleanly_msg(ce_branch) + throw(:halt_check) + end + end + + def ee_branch_presence_check! + status = step("Fetching origin/#{ee_branch}", %W[git fetch origin #{ee_branch}]) + + unless status.zero? + puts + puts ce_branch_doesnt_apply_cleanly_and_no_ee_branch_msg + + throw(:halt_check, :ko) + end + end + + def ee_branch_compat_check! + step("Checking out origin/#{ee_branch}", %W[git checkout -b #{ee_branch} FETCH_HEAD]) + + generate_patch(ee_branch, ee_patch_full_path) + cmd = %W[git apply --check #{ee_patch_full_path}] + status = step("Checking if #{ee_patch_name} applies cleanly to EE/master", cmd) + + unless status.zero? + puts + puts ee_branch_doesnt_apply_cleanly_msg + + throw(:halt_check, :ko) + end + + puts + puts ee_applies_cleanly_msg + end + + def generate_patch(branch, filepath) + FileUtils.rm(filepath, force: true) + + depth = 0 + loop do + depth += 10 + step("Fetching origin/master", %W[git fetch origin master --depth=#{depth}]) + status = step("Finding merge base with master", %W[git merge-base FETCH_HEAD #{branch}]) + + break if status.zero? || depth > 500 + end + + raise "#{branch} is too far behind master, please rebase it!" if depth > 500 + + step("Generating the patch against master") + output, status = Gitlab::Popen.popen(%w[git format-patch FETCH_HEAD --stdout]) + throw(:halt_check, :ko) unless status.zero? + + File.open(filepath, 'w+') { |f| f.write(output) } + throw(:halt_check, :ko) unless File.exist?(filepath) + end + + def delete_ee_branch_locally + step("Checking out origin/master", %w[git checkout master]) + step("Deleting the local #{ee_branch} branch", %W[git branch -D #{ee_branch}]) + end + + def delete_patches + step("Deleting #{ce_patch_full_path}") + FileUtils.rm(ce_patch_full_path, force: true) + + step("Deleting #{ee_patch_full_path}") + FileUtils.rm(ee_patch_full_path, force: true) + end + + def ce_patch_name + @ce_patch_name ||= "#{ce_branch}.patch" + end + + def ce_patch_full_path + @ce_patch_full_path ||= File.expand_path(ce_patch_name, check_dir) + end + + def ee_branch + @ee_branch ||= "#{ce_branch}-ee" + end + + def ee_patch_name + @ee_patch_name ||= "#{ee_branch}.patch" + end + + def ee_patch_full_path + @ee_patch_full_path ||= File.expand_path(ee_patch_name, check_dir) + end + + def step(desc, cmd = nil) + puts "\n=> #{desc}\n" + + if cmd + puts "\n$ #{cmd.join(' ')}" + command(cmd) + end + end + + def command(cmd) + output, status = Gitlab::Popen.popen(cmd) + puts output + + status + end + + def ce_applies_cleanly_msg(ce_branch) + <<-MSG.strip_heredoc + ================================================================= + 🎉 Congratulations!! 🎉 + + The #{ce_branch} branch applies cleanly to EE/master! + + Much ❤️!! + =================================================================\n + MSG + end + + def ce_branch_doesnt_apply_cleanly_and_no_ee_branch_msg + <<-MSG.strip_heredoc + ================================================================= + 💥 Oh no! 💥 + + The #{ce_branch} branch does not apply cleanly to the current + EE/master, and no #{ee_branch} branch was found in the EE repository. + + Please create a #{ee_branch} branch that includes changes from + #{ce_branch} but also specific changes than can be applied cleanly + to EE/master. + + There are different ways to create such branch: + + 1. Create a new branch based on the CE branch and rebase it on top of EE/master + + # In the EE repo + $ git fetch #{ce_repo} #{ce_branch} + $ git checkout -b #{ee_branch} FETCH_HEAD + + # You can squash the #{ce_branch} commits into a single "Port of #{ce_branch} to EE" commit + # before rebasing to limit the conflicts-resolving steps during the rebase + $ git fetch origin + $ git rebase origin/master + + At this point you will likely have conflicts. + Solve them, and continue/finish the rebase. + + You can squash the #{ce_branch} commits into a single "Port of #{ce_branch} to EE". + + 2. Create a new branch from master and cherry-pick your CE commits + + # In the EE repo + $ git fetch origin + $ git checkout -b #{ee_branch} FETCH_HEAD + $ git fetch #{ce_repo} #{ce_branch} + $ git cherry-pick SHA # Repeat for all the commits you want to pick + + You can squash the #{ce_branch} commits into a single "Port of #{ce_branch} to EE" commit. + + Don't forget to push your branch to #{EE_REPO}: + + # In the EE repo + $ git push origin #{ee_branch} + + You can then retry this failed build, and hopefully it should pass. + + Stay 💪 ! + =================================================================\n + MSG + end + + def ee_branch_doesnt_apply_cleanly_msg + <<-MSG.strip_heredoc + ================================================================= + 💥 Oh no! 💥 + + The #{ce_branch} does not apply cleanly to the current + EE/master, and even though a #{ee_branch} branch exists in the EE + repository, it does not apply cleanly either to EE/master! + + Please update the #{ee_branch}, push it again to #{EE_REPO}, and + retry this build. + + Stay 💪 ! + =================================================================\n + MSG + end + + def ee_applies_cleanly_msg + <<-MSG.strip_heredoc + ================================================================= + 🎉 Congratulations!! 🎉 + + The #{ee_branch} branch applies cleanly to EE/master! + + Much ❤️!! + =================================================================\n + MSG + end + end +end diff --git a/lib/tasks/ce_to_ee_merge_check.rake b/lib/tasks/ce_to_ee_merge_check.rake deleted file mode 100644 index 424e7883060..00000000000 --- a/lib/tasks/ce_to_ee_merge_check.rake +++ /dev/null @@ -1,4 +0,0 @@ -desc 'Checks if the branch would apply cleanly to EE' -task ce_to_ee_merge_check: :environment do - Rake::Task['gitlab:dev:ce_to_ee_merge_check'].invoke -end diff --git a/lib/tasks/ee_compat_check.rake b/lib/tasks/ee_compat_check.rake new file mode 100644 index 00000000000..f494fa5c5c2 --- /dev/null +++ b/lib/tasks/ee_compat_check.rake @@ -0,0 +1,4 @@ +desc 'Checks if the branch would apply cleanly to EE' +task ee_compat_check: :environment do + Rake::Task['gitlab:dev:ee_compat_check'].invoke +end diff --git a/lib/tasks/gitlab/dev.rake b/lib/tasks/gitlab/dev.rake index 47bdb2d32d2..5ee99dfc810 100644 --- a/lib/tasks/gitlab/dev.rake +++ b/lib/tasks/gitlab/dev.rake @@ -1,106 +1,21 @@ namespace :gitlab do namespace :dev do desc 'Checks if the branch would apply cleanly to EE' - task ce_to_ee_merge_check: :environment do + task ee_compat_check: :environment do return if defined?(Gitlab::License) return unless ENV['CI'] - ce_repo = ENV['CI_BUILD_REPO'] - ce_branch = ENV['CI_BUILD_REF_NAME'] + success = + Gitlab::EeCompatCheck.new( + branch: ENV['CI_BUILD_REF_NAME'], + check_dir: File.expand_path('ee-compat-check', __dir__), + ce_repo: ENV['CI_BUILD_REPO'] + ).check - ee_repo = 'https://gitlab.com/gitlab-org/gitlab-ee.git' - ee_branch = "#{ce_branch}-ee" - ee_dir = 'gitlab-ee-merge-check' - - puts "\n=> Cloning #{ee_repo} into #{ee_dir}\n" - `git clone #{ee_repo} #{ee_dir} --depth 1` - Dir.chdir(ee_dir) do - puts "\n => Fetching #{ce_repo}/#{ce_branch}\n" - `git fetch #{ce_repo} #{ce_branch} --depth 1` - - # Try to merge the current tested branch to EE/master... - puts "\n => Merging #{ce_repo}/#{ce_branch} into #{ee_repo}/master\n" - `git merge FETCH_HEAD` - - exit 0 if $?.success? - - # Check if the -ee branch exists... - puts "\n => Check if #{ee_repo}/#{ee_branch} exists\n" - `git rev-parse --verify #{ee_branch}` - - # The -ee doesn't exist - unless $?.success? - puts - puts <<-MSG.strip_heredoc - ================================================================= - The #{ce_branch} branch cannot be merged without conflicts to the - current EE/master, and no #{ee_branch} branch was detected in - the EE repository. - - Please create a #{ee_branch} branch that includes changes from - #{ce_branch} but also specific changes than can be applied cleanly - to EE/master. - - You can create this branch as follows: - - 1. In the EE repo: - $ git fetch origin - $ git fetch #{ce_repo} #{ce_branch} - $ git checkout -b #{ee_branch} FETCH_HEAD - $ git rebase origin/master - 2. At this point you will likely have conflicts, solve them, and - continue/finish the rebase. Note: You can squash the CE commits - before rebasing. - 3. You can squash all the original #{ce_branch} commits into a - single "Port of #{ce_branch} to EE". - 4. Push your branch to #{ee_repo}: - $ git push origin #{ee_branch} - =================================================================\n - MSG - - exit 1 - end - - # Try to merge the -ee branch to EE/master... - puts "\n => Merging #{ee_repo}/#{ee_branch} into #{ee_repo}/master\n" - `git merge #{ee_branch} master` - - # The -ee cannot be merged cleanly to EE/master... - unless $?.success? - puts - puts <<-MSG.strip_heredoc - ================================================================= - The #{ce_branch} branch cannot be merged without conflicts to - EE/master, and even though the #{ee_branch} branch exists in the EE - repository, it cannot be merged without conflicts to EE/master. - - Please update the #{ee_branch}, push it again to #{ee_repo}, and - retry this job. - =================================================================\n - MSG - - exit 2 - end - - puts "\n => Merging #{ce_repo}/#{ce_branch} into #{ee_repo}/master\n" - `git merge FETCH_HEAD` - exit 0 if $?.success? - - # The -ee can be merged cleanly to EE/master, but still - # cannot be merged cleanly to EE/master... - puts - puts <<-MSG.strip_heredoc - ================================================================= - The #{ce_branch} branch cannot be merged without conflicts to EE, and - even though the #{ee_branch} branch exists in the EE repository and - applies cleanly to EE/master, it doesn't prevent conflicts when - merging #{ce_branch} into EE. - - We may be in a complex situation here. - =================================================================\n - MSG - - exit 3 + if success + exit 0 + else + exit 1 end end end From 68af55c3f93be79841957ff12b736e104fdc40de Mon Sep 17 00:00:00 2001 From: Daniel Klischies Date: Fri, 21 Oct 2016 18:38:11 +0000 Subject: [PATCH 08/73] Add a note regarding syncing the git submodule conf to CI doc. --- doc/user/project/new_ci_build_permissions_model.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/user/project/new_ci_build_permissions_model.md b/doc/user/project/new_ci_build_permissions_model.md index 8827b501901..608475116a1 100644 --- a/doc/user/project/new_ci_build_permissions_model.md +++ b/doc/user/project/new_ci_build_permissions_model.md @@ -254,6 +254,12 @@ test: This will make GitLab CI initialize (fetch) and update (checkout) all your submodules recursively. +If git does not use the newly added relative URLs but still uses your old URLs, +you might need to add `git submodule sync --recursive` to your `.gitlab-ci.yml`, +prior to running `git submodule update --init --recursive`. This transfers the +changes from your `.gitmodules` file into the `.git` folder, which is kept by +runners between runs. + In case your environment or your Docker image doesn't have Git installed, you have to either ask your Administrator or install the missing dependency yourself: From fadaba000a2faba191177793ff6aba5a0ecdbe24 Mon Sep 17 00:00:00 2001 From: DJ Mountney Date: Fri, 21 Oct 2016 11:31:15 -0700 Subject: [PATCH 09/73] Add an example of how to run the backups when using docker to the docs --- doc/raketasks/backup_restore.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index 26baffdf792..fc0cd1b8af2 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -30,6 +30,10 @@ Use this if you've installed GitLab from source: ``` sudo -u git -H bundle exec rake gitlab:backup:create RAILS_ENV=production ``` +If you are running GitLab within a Docker container, you can run the backup from the host: +``` +docker -t exec gitlab-rake gitlab:backup:create +``` You can specify that portions of the application data be skipped using the environment variable `SKIP`. You can skip: From 3b7ca69e530d684faf89a8dc1ea48c26eed5ccb7 Mon Sep 17 00:00:00 2001 From: Bernardo Anderson Date: Sat, 22 Oct 2016 14:20:13 -0500 Subject: [PATCH 10/73] Fix sign in page Forgot your password link overlap --- CHANGELOG.md | 1 + app/assets/stylesheets/pages/login.scss | 6 ++++++ app/views/devise/sessions/_new_base.html.haml | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c39ddca7cf..d4a975e6c3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix: Backup restore doesn't clear cache - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method + - Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens ## 8.13.0 (2016-10-22) diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss index bdb13bee178..51048649383 100644 --- a/app/assets/stylesheets/pages/login.scss +++ b/app/assets/stylesheets/pages/login.scss @@ -255,6 +255,12 @@ .new_user { position: relative; padding-bottom: 35px; + @media (min-width: $screen-sm-min) and (max-width: $screen-sm-max) { + .forgot-password { + float: none !important; + margin-top: 5px; + } + } } .move-submit-down { diff --git a/app/views/devise/sessions/_new_base.html.haml b/app/views/devise/sessions/_new_base.html.haml index 525e7d99d71..5fd896f6835 100644 --- a/app/views/devise/sessions/_new_base.html.haml +++ b/app/views/devise/sessions/_new_base.html.haml @@ -12,5 +12,5 @@ %label{for: "user_remember_me"} = f.check_box :remember_me %span Remember me - .pull-right + .pull-right.forgot-password = link_to "Forgot your password?", new_password_path(resource_name) From 2eb452402f6d9ac7dc6a5b083dab824fa9767c4d Mon Sep 17 00:00:00 2001 From: Bernardo Anderson Date: Sat, 22 Oct 2016 14:38:58 -0500 Subject: [PATCH 11/73] Add empty line before media query --- app/assets/stylesheets/pages/login.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss index 51048649383..02c4bbf3710 100644 --- a/app/assets/stylesheets/pages/login.scss +++ b/app/assets/stylesheets/pages/login.scss @@ -255,6 +255,7 @@ .new_user { position: relative; padding-bottom: 35px; + @media (min-width: $screen-sm-min) and (max-width: $screen-sm-max) { .forgot-password { float: none !important; From 204da00ea0dc52c6746b160fd698e07f55a7fc00 Mon Sep 17 00:00:00 2001 From: Jared Ready Date: Thu, 29 Sep 2016 20:18:46 -0500 Subject: [PATCH 12/73] Move spec/mailers/shared/notify.rb to spec/support --- spec/mailers/emails/builds_spec.rb | 1 - spec/mailers/emails/merge_requests_spec.rb | 1 - spec/mailers/emails/profile_spec.rb | 1 - spec/mailers/notify_spec.rb | 1 - .../shared/notify.rb => support/notify_shared_examples.rb} | 0 5 files changed, 4 deletions(-) rename spec/{mailers/shared/notify.rb => support/notify_shared_examples.rb} (100%) diff --git a/spec/mailers/emails/builds_spec.rb b/spec/mailers/emails/builds_spec.rb index 0df89938e97..d968096783c 100644 --- a/spec/mailers/emails/builds_spec.rb +++ b/spec/mailers/emails/builds_spec.rb @@ -1,6 +1,5 @@ require 'spec_helper' require 'email_spec' -require 'mailers/shared/notify' describe Notify do include EmailSpec::Matchers diff --git a/spec/mailers/emails/merge_requests_spec.rb b/spec/mailers/emails/merge_requests_spec.rb index 4d3811af254..e22858d1d8f 100644 --- a/spec/mailers/emails/merge_requests_spec.rb +++ b/spec/mailers/emails/merge_requests_spec.rb @@ -1,6 +1,5 @@ require 'spec_helper' require 'email_spec' -require 'mailers/shared/notify' describe Notify, "merge request notifications" do include EmailSpec::Matchers diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb index 781472d0c00..14bc062ef12 100644 --- a/spec/mailers/emails/profile_spec.rb +++ b/spec/mailers/emails/profile_spec.rb @@ -1,6 +1,5 @@ require 'spec_helper' require 'email_spec' -require 'mailers/shared/notify' describe Notify do include EmailSpec::Matchers diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index c8207e58e90..f5f3f58613d 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -1,6 +1,5 @@ require 'spec_helper' require 'email_spec' -require 'mailers/shared/notify' describe Notify do include EmailSpec::Helpers diff --git a/spec/mailers/shared/notify.rb b/spec/support/notify_shared_examples.rb similarity index 100% rename from spec/mailers/shared/notify.rb rename to spec/support/notify_shared_examples.rb From b1ce2eb1e5f6a4a5b413381489fbb7e63ff3e1e5 Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Mon, 23 May 2016 14:19:27 +0500 Subject: [PATCH 13/73] refactor(email): use setter method instead AR callbacks --- CHANGELOG.md | 1 + app/models/email.rb | 6 ++---- spec/models/email_spec.rb | 5 +++++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b25431278bd..6b8ecb68c8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method - Fix documents and comments on Build API `scope` + - Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov) ## 8.13.1 (unreleased) - Fix error in generating labels diff --git a/app/models/email.rb b/app/models/email.rb index 32a412ab878..826d4f16edb 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -7,10 +7,8 @@ class Email < ActiveRecord::Base validates :email, presence: true, uniqueness: true, email: true validate :unique_email, if: ->(email) { email.email_changed? } - before_validation :cleanup_email - - def cleanup_email - self.email = self.email.downcase.strip + def email=(value) + write_attribute(:email, value.downcase.strip) end def unique_email diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index d9df9e0f907..fe4de1b2afb 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -6,4 +6,9 @@ describe Email, models: true do subject { build(:email) } end end + + it 'normalize email value' do + expect(described_class.new(email: ' inFO@exAMPLe.com ').email) + .to eq 'info@example.com' + end end From 4ea1973f5971f9c72230779d28f228b5da1148af Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 23 Oct 2016 10:31:18 -0700 Subject: [PATCH 14/73] Expire and build repository cache after project import After a project import, there's a chance that the UI checks the branch count before the project has been imported. This change causes more of the keys to be flushed after an import and forces a rebuild of the repository cache. Closes #13518 --- CHANGELOG.md | 1 + app/models/repository.rb | 18 ++++++++++++++---- spec/models/repository_spec.rb | 21 +++++---------------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b25431278bd..9999864662a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.13.1 (unreleased) - Fix error in generating labels + - Expire and build repository cache after project import ## 8.13.0 (2016-10-22) diff --git a/app/models/repository.rb b/app/models/repository.rb index 1b7f20a2134..c0460e3952f 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -419,6 +419,17 @@ class Repository @exists = nil end + # expire cache that doesn't depend on repository data (when expiring) + def expire_content_cache + expire_tags_cache + expire_tag_count_cache + expire_branches_cache + expire_branch_count_cache + expire_root_ref_cache + expire_emptiness_caches + expire_exists_cache + end + # Runs code after a repository has been created. def after_create expire_exists_cache @@ -473,14 +484,13 @@ class Repository end def before_import - expire_emptiness_caches - expire_exists_cache + expire_content_cache end # Runs code after a repository has been forked/imported. def after_import - expire_emptiness_caches - expire_exists_cache + expire_content_cache + build_cache end # Runs code after a new commit has been pushed. diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index f977cf73673..187a1bf2d79 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1146,28 +1146,17 @@ describe Repository, models: true do 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) + it 'flushes the repository caches' do + expect(repository).to receive(:expire_content_cache) repository.before_import end end describe '#after_import' do - it 'flushes the emptiness cachess' do - expect(repository).to receive(:expire_emptiness_caches) - - repository.after_import - end - - it 'flushes the exists cache' do - expect(repository).to receive(:expire_exists_cache) + it 'flushes and builds the cache' do + expect(repository).to receive(:expire_content_cache) + expect(repository).to receive(:build_cache) repository.after_import end From 8a3710f452dc2192c936b1b418d2e1a90f5ae65f Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 23 Oct 2016 10:45:08 -0700 Subject: [PATCH 15/73] Remove duplicate code in repository cache clearing --- app/models/repository.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index c0460e3952f..4ae9c20726f 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -445,14 +445,7 @@ class Repository expire_cache if exists? - # expire cache that don't depend on repository data (when expiring) - expire_tags_cache - expire_tag_count_cache - expire_branches_cache - expire_branch_count_cache - expire_root_ref_cache - expire_emptiness_caches - expire_exists_cache + expire_content_cache repository_event(:remove_repository) end From 59ed1d3cbbf6c89fde202889af798fc189035313 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 23 Oct 2016 21:17:23 -0700 Subject: [PATCH 16/73] Fix reply-by-email not working due to queue name mismatch mail_room was configured to deliver mail to the `incoming_email` queue while `EmailReceiveWorker` was reading the `email_receiver` queue. Adds a migration that repeats the work of a previous migration to ensure all mails that wound up in the old queue get processed. Closes #23689 --- CHANGELOG.md | 1 + config/mail_room.yml | 2 +- ...317_migrate_mailroom_queue_from_default.rb | 63 +++++++++++++++++++ db/schema.rb | 2 +- 4 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20161024042317_migrate_mailroom_queue_from_default.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b25431278bd..99f03536f01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.13.1 (unreleased) - Fix error in generating labels + - Fix reply-by-email not working due to queue name mismatch ## 8.13.0 (2016-10-22) diff --git a/config/mail_room.yml b/config/mail_room.yml index c639f8260aa..68697bd1dc4 100644 --- a/config/mail_room.yml +++ b/config/mail_room.yml @@ -25,7 +25,7 @@ :delivery_options: :redis_url: <%= config[:redis_url].to_json %> :namespace: <%= Gitlab::Redis::SIDEKIQ_NAMESPACE %> - :queue: incoming_email + :queue: email_receiver :worker: EmailReceiverWorker :arbitration_method: redis diff --git a/db/migrate/20161024042317_migrate_mailroom_queue_from_default.rb b/db/migrate/20161024042317_migrate_mailroom_queue_from_default.rb new file mode 100644 index 00000000000..06d07bdb835 --- /dev/null +++ b/db/migrate/20161024042317_migrate_mailroom_queue_from_default.rb @@ -0,0 +1,63 @@ +require 'json' + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class MigrateMailroomQueueFromDefault < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = true + + DOWNTIME_REASON = <<-EOF + Moving Sidekiq jobs from queues requires Sidekiq to be stopped. Not stopping + Sidekiq will result in the loss of jobs that are scheduled after this + migration completes. + EOF + + disable_ddl_transaction! + + # Jobs for which the queue names have been changed (e.g. multiple workers + # using the same non-default queue). + # + # The keys are the old queue names, the values the jobs to move and their new + # queue names. + RENAMED_QUEUES = { + incoming_email: { + 'EmailReceiverWorker' => :email_receiver + } + } + + def up + Sidekiq.redis do |redis| + RENAMED_QUEUES.each do |queue, jobs| + migrate_from_queue(redis, queue, jobs) + end + end + end + + def down + Sidekiq.redis do |redis| + RENAMED_QUEUES.each do |dest_queue, jobs| + jobs.each do |worker, from_queue| + migrate_from_queue(redis, from_queue, worker => dest_queue) + end + end + end + end + + def migrate_from_queue(redis, queue, job_mapping) + while job = redis.lpop("queue:#{queue}") + payload = JSON.load(job) + new_queue = job_mapping[payload['class']] + + # If we have no target queue to migrate to we're probably dealing with + # some ancient job for which the worker no longer exists. In that case + # there's no sane option we can take, other than just dropping the job. + next unless new_queue + + payload['queue'] = new_queue + + redis.lpush("queue:#{new_queue}", JSON.dump(payload)) + end + end +end diff --git a/db/schema.rb b/db/schema.rb index f5c01511195..02282b0f666 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161019213545) do +ActiveRecord::Schema.define(version: 20161024042317) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" From f79f3a1dd621362b0894eff0a54f220f8415a2e0 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 6 Sep 2016 10:35:34 +0530 Subject: [PATCH 17/73] Fix branch protection API. 1. Previously, we were not removing existing access levels before creating new ones. This is not a problem for EE, but _is_ for CE, since we restrict the number of access levels in CE to 1. 2. The correct approach is: CE -> delete all access levels before updating a protected branch EE -> delete developer access levels if "developers_can_{merge,push}" is switched off 3. The dispatch is performed by checking if a "length: 1" validation is present on the access levels or not. 4. Another source of problems was that we didn't put multiple queries in a transaction. If the `destroy_all` passes, but the `update` fails, we should have a rollback. 5. Modifying the API to provide users direct access to CRUD access levels will make things a lot simpler. 6. Create `create/update` services separately for this API, which perform the necessary data translation, before calling the regular `create/update` services. The translation code was getting too large for the API endpoint itself, so this move makes sense. --- .../concerns/protected_branch_access.rb | 5 + .../protected_branches/api_create_service.rb | 30 +++ .../protected_branches/api_update_service.rb | 79 +++++++ lib/api/branches.rb | 47 ++-- spec/requests/api/branches_spec.rb | 218 +++++++++++------- 5 files changed, 267 insertions(+), 112 deletions(-) create mode 100644 app/services/protected_branches/api_create_service.rb create mode 100644 app/services/protected_branches/api_update_service.rb diff --git a/app/models/concerns/protected_branch_access.rb b/app/models/concerns/protected_branch_access.rb index 5a7b36070e7..dd6b34d934b 100644 --- a/app/models/concerns/protected_branch_access.rb +++ b/app/models/concerns/protected_branch_access.rb @@ -1,6 +1,11 @@ module ProtectedBranchAccess extend ActiveSupport::Concern + included do + scope :master, -> () { where(access_level: Gitlab::Access::MASTER) } + scope :developer, -> () { where(access_level: Gitlab::Access::DEVELOPER) } + end + def humanize self.class.human_access_levels[self.access_level] end diff --git a/app/services/protected_branches/api_create_service.rb b/app/services/protected_branches/api_create_service.rb new file mode 100644 index 00000000000..a28056035b8 --- /dev/null +++ b/app/services/protected_branches/api_create_service.rb @@ -0,0 +1,30 @@ +# The protected branches API still uses the `developers_can_push` and `developers_can_merge` +# flags for backward compatibility, and so performs translation between that format and the +# internal data model (separate access levels). The translation code is non-trivial, and so +# lives in this service. +module ProtectedBranches + class ApiCreateService < BaseService + def initialize(project, user, params, developers_can_push:, developers_can_merge:) + super(project, user, params) + @developers_can_merge = developers_can_merge + @developers_can_push = developers_can_push + end + + def execute + if @developers_can_push + @params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) + else + @params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) + end + + if @developers_can_merge + @params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) + else + @params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) + end + + service = ProtectedBranches::CreateService.new(@project, @current_user, @params) + service.execute + end + end +end diff --git a/app/services/protected_branches/api_update_service.rb b/app/services/protected_branches/api_update_service.rb new file mode 100644 index 00000000000..41d3d413caa --- /dev/null +++ b/app/services/protected_branches/api_update_service.rb @@ -0,0 +1,79 @@ +# The protected branches API still uses the `developers_can_push` and `developers_can_merge` +# flags for backward compatibility, and so performs translation between that format and the +# internal data model (separate access levels). The translation code is non-trivial, and so +# lives in this service. +module ProtectedBranches + class ApiUpdateService < BaseService + def initialize(project, user, params, developers_can_push:, developers_can_merge:) + super(project, user, params) + @developers_can_merge = developers_can_merge + @developers_can_push = developers_can_push + end + + def execute(protected_branch) + @protected_branch = protected_branch + + protected_branch.transaction do + # If a protected branch can have only a single access level (CE), delete it to + # make room for the new access level. If a protected branch can have more than + # one access level (EE), only remove the relevant access levels. If we don't do this, + # we'll have a failed validation. + if protected_branch_restricted_to_single_access_level? + delete_redundant_ce_access_levels + else + delete_redundant_ee_access_levels + end + + if @developers_can_push + params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) + elsif @developers_can_push == false + params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) + end + + if @developers_can_merge + params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) + elsif @developers_can_merge == false + params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) + end + + service = ProtectedBranches::UpdateService.new(@project, @current_user, @params) + service.execute(protected_branch) + end + end + + private + + def delete_redundant_ce_access_levels + if @developers_can_merge || @developers_can_merge == false + @protected_branch.merge_access_levels.destroy_all + end + + if @developers_can_push || @developers_can_push == false + @protected_branch.push_access_levels.destroy_all + end + end + + def delete_redundant_ee_access_levels + if @developers_can_merge + @protected_branch.merge_access_levels.developer.destroy_all + elsif @developers_can_merge == false + @protected_branch.merge_access_levels.developer.destroy_all + @protected_branch.merge_access_levels.master.destroy_all + end + + if @developers_can_push + @protected_branch.push_access_levels.developer.destroy_all + elsif @developers_can_push == false + @protected_branch.push_access_levels.developer.destroy_all + @protected_branch.push_access_levels.master.destroy_all + end + end + + def protected_branch_restricted_to_single_access_level? + length_validator = ProtectedBranch.validators_on(:push_access_levels).find do |validator| + validator.is_a? ActiveModel::Validations::LengthValidator + end + length_validator.options[:is] == 1 if length_validator + end + end +end diff --git a/lib/api/branches.rb b/lib/api/branches.rb index b615703df93..6941cc4aca6 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -57,40 +57,25 @@ module API developers_can_merge = to_boolean(params[:developers_can_merge]) developers_can_push = to_boolean(params[:developers_can_push]) - protected_branch_params = { - name: @branch.name + params = { + name: @branch.name, } - # If `developers_can_merge` is switched off, _all_ `DEVELOPER` - # merge_access_levels need to be deleted. - if developers_can_merge == false - protected_branch.merge_access_levels.where(access_level: Gitlab::Access::DEVELOPER).destroy_all - end + service_args = [user_project, current_user, params, + developers_can_push: developers_can_push, + developers_can_merge: developers_can_merge] - # If `developers_can_push` is switched off, _all_ `DEVELOPER` - # push_access_levels need to be deleted. - if developers_can_push == false - protected_branch.push_access_levels.where(access_level: Gitlab::Access::DEVELOPER).destroy_all - end + protected_branch = if protected_branch + ProtectedBranches::ApiUpdateService.new(*service_args).execute(protected_branch) + else + ProtectedBranches::ApiCreateService.new(*service_args).execute + end - protected_branch_params.merge!( - merge_access_levels_attributes: [{ - access_level: developers_can_merge ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - }], - push_access_levels_attributes: [{ - access_level: developers_can_push ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - }] - ) - - if protected_branch - service = ProtectedBranches::UpdateService.new(user_project, current_user, protected_branch_params) - service.execute(protected_branch) + if protected_branch.valid? + present @branch, with: Entities::RepoBranch, project: user_project else - service = ProtectedBranches::CreateService.new(user_project, current_user, protected_branch_params) - service.execute + render_api_error!(protected_branch.errors.full_messages, 422) end - - present @branch, with: Entities::RepoBranch, project: user_project end # Unprotect a single branch @@ -123,7 +108,7 @@ module API post ":id/repository/branches" do authorize_push_project result = CreateBranchService.new(user_project, current_user). - execute(params[:branch_name], params[:ref]) + execute(params[:branch_name], params[:ref]) if result[:status] == :success present result[:branch], @@ -142,10 +127,10 @@ module API # Example Request: # DELETE /projects/:id/repository/branches/:branch delete ":id/repository/branches/:branch", - requirements: { branch: /.+/ } do + requirements: { branch: /.+/ } do authorize_push_project result = DeleteBranchService.new(user_project, current_user). - execute(params[:branch]) + execute(params[:branch]) if result[:status] == :success { diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 3fd989dd7a6..905f762d578 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -48,92 +48,154 @@ describe API::API, api: true do end describe 'PUT /projects/:id/repository/branches/:branch/protect' do - it 'protects a single branch' do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) - - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['commit']['id']).to eq(branch_sha) - expect(json_response['protected']).to eq(true) - expect(json_response['developers_can_push']).to eq(false) - expect(json_response['developers_can_merge']).to eq(false) - end - - it 'protects a single branch and developers can push' do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), - developers_can_push: true - - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['commit']['id']).to eq(branch_sha) - expect(json_response['protected']).to eq(true) - expect(json_response['developers_can_push']).to eq(true) - expect(json_response['developers_can_merge']).to eq(false) - end - - it 'protects a single branch and developers can merge' do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), - developers_can_merge: true - - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['commit']['id']).to eq(branch_sha) - expect(json_response['protected']).to eq(true) - expect(json_response['developers_can_push']).to eq(false) - expect(json_response['developers_can_merge']).to eq(true) - end - - it 'protects a single branch and developers can push and merge' do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), - developers_can_push: true, developers_can_merge: true - - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['commit']['id']).to eq(branch_sha) - expect(json_response['protected']).to eq(true) - expect(json_response['developers_can_push']).to eq(true) - expect(json_response['developers_can_merge']).to eq(true) - end - - it 'protects a single branch and developers cannot push and merge' do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), - developers_can_push: 'tru', developers_can_merge: 'tr' - - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['commit']['id']).to eq(branch_sha) - expect(json_response['protected']).to eq(true) - expect(json_response['developers_can_push']).to eq(false) - expect(json_response['developers_can_merge']).to eq(false) - end - - context 'on a protected branch' do - let(:protected_branch) { 'foo' } - - before do - project.repository.add_branch(user, protected_branch, 'master') - create(:protected_branch, :developers_can_push, :developers_can_merge, project: project, name: protected_branch) - end - - it 'updates that a developer can push' do - put api("/projects/#{project.id}/repository/branches/#{protected_branch}/protect", user), - developers_can_push: false, developers_can_merge: false + context "when a protected branch doesn't already exist" do + it 'protects a single branch' do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) expect(response).to have_http_status(200) - expect(json_response['name']).to eq(protected_branch) + expect(json_response['name']).to eq(branch_name) + expect(json_response['commit']['id']).to eq(branch_sha) expect(json_response['protected']).to eq(true) expect(json_response['developers_can_push']).to eq(false) expect(json_response['developers_can_merge']).to eq(false) end - it 'does not update that a developer can push' do - put api("/projects/#{project.id}/repository/branches/#{protected_branch}/protect", user), - developers_can_push: 'foobar', developers_can_merge: 'foo' + it 'protects a single branch and developers can push' do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), + developers_can_push: true expect(response).to have_http_status(200) - expect(json_response['name']).to eq(protected_branch) + expect(json_response['name']).to eq(branch_name) + expect(json_response['commit']['id']).to eq(branch_sha) expect(json_response['protected']).to eq(true) expect(json_response['developers_can_push']).to eq(true) + expect(json_response['developers_can_merge']).to eq(false) + end + + it 'protects a single branch and developers can merge' do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), + developers_can_merge: true + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(branch_name) + expect(json_response['commit']['id']).to eq(branch_sha) + expect(json_response['protected']).to eq(true) + expect(json_response['developers_can_push']).to eq(false) + expect(json_response['developers_can_merge']).to eq(true) + end + + it 'protects a single branch and developers can push and merge' do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), + developers_can_push: true, developers_can_merge: true + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(branch_name) + expect(json_response['commit']['id']).to eq(branch_sha) + expect(json_response['protected']).to eq(true) + expect(json_response['developers_can_push']).to eq(true) + expect(json_response['developers_can_merge']).to eq(true) + end + + it 'protects a single branch and developers cannot push and merge' do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), + developers_can_push: 'tru', developers_can_merge: 'tr' + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(branch_name) + expect(json_response['commit']['id']).to eq(branch_sha) + expect(json_response['protected']).to eq(true) + expect(json_response['developers_can_push']).to eq(false) + expect(json_response['developers_can_merge']).to eq(false) + end + end + + context 'for an existing protected branch' do + before do + project.repository.add_branch(user, protected_branch.name, 'master') + end + + context "when developers can push and merge" do + let(:protected_branch) { create(:protected_branch, :developers_can_push, :developers_can_merge, project: project, name: 'protected_branch') } + + it 'updates that a developer cannot push or merge' do + put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), + developers_can_push: false, developers_can_merge: false + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(protected_branch.name) + expect(json_response['protected']).to eq(true) + expect(json_response['developers_can_push']).to eq(false) + expect(json_response['developers_can_merge']).to eq(false) + end + + it "doesn't result in 0 access levels when 'developers_can_push' is switched off" do + put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), + developers_can_push: false + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(protected_branch.name) + expect(protected_branch.reload.push_access_levels.first).to be_present + expect(protected_branch.reload.push_access_levels.first.access_level).to eq(Gitlab::Access::MASTER) + end + + it "doesn't result in 0 access levels when 'developers_can_merge' is switched off" do + put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), + developers_can_merge: false + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(protected_branch.name) + expect(protected_branch.reload.merge_access_levels.first).to be_present + expect(protected_branch.reload.merge_access_levels.first.access_level).to eq(Gitlab::Access::MASTER) + end + end + + context "when developers cannot push or merge" do + let(:protected_branch) { create(:protected_branch, project: project, name: 'protected_branch') } + + it 'updates that a developer can push and merge' do + put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), + developers_can_push: true, developers_can_merge: true + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(protected_branch.name) + expect(json_response['protected']).to eq(true) + expect(json_response['developers_can_push']).to eq(true) + expect(json_response['developers_can_merge']).to eq(true) + end + end + end + + context "multiple API calls" do + it "returns success when `protect` is called twice" do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(branch_name) + expect(json_response['protected']).to eq(true) + expect(json_response['developers_can_push']).to eq(false) + expect(json_response['developers_can_merge']).to eq(false) + end + + it "returns success when `protect` is called twice with `developers_can_push` turned on" do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), developers_can_push: true + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), developers_can_push: true + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(branch_name) + expect(json_response['protected']).to eq(true) + expect(json_response['developers_can_push']).to eq(true) + expect(json_response['developers_can_merge']).to eq(false) + end + + it "returns success when `protect` is called twice with `developers_can_merge` turned on" do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), developers_can_merge: true + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), developers_can_merge: true + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(branch_name) + expect(json_response['protected']).to eq(true) + expect(json_response['developers_can_push']).to eq(false) expect(json_response['developers_can_merge']).to eq(true) end end @@ -147,12 +209,6 @@ describe API::API, api: true do put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user2) expect(response).to have_http_status(403) end - - it "returns success when protect branch again" do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) - expect(response).to have_http_status(200) - end end describe "PUT /projects/:id/repository/branches/:branch/unprotect" do From b116df7e9146baf70f04eb6ec1d19091278c14d5 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 6 Sep 2016 11:19:36 +0530 Subject: [PATCH 18/73] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b25431278bd..2538585da7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -387,6 +387,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix inconsistent checkbox alignment (ClemMakesApps) - Use the default branch for displaying the project icon instead of master !5792 (Hannes Rosenögger) - Adds response mime type to transaction metric action when it's not HTML + - Fix branch protection API !6215 - Fix hover leading space bug in pipeline graph !5980 - Avoid conflict with admin labels when importing GitHub labels - User can edit closed MR with deleted fork (Katarzyna Kobierska Ula Budziszewska) !5496 From cef10ef7d7a20a78d377f711867e361bb51fbaf2 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 14 Sep 2016 08:04:29 +0530 Subject: [PATCH 19/73] Implement review comments from @dbalexandre. 1. Don't have any EE-only code in CE. Ok to have CE-only code in EE. 2. Use `case` instead of `if/elsif` --- .../protected_branches/api_update_service.rb | 45 ++++--------------- 1 file changed, 8 insertions(+), 37 deletions(-) diff --git a/app/services/protected_branches/api_update_service.rb b/app/services/protected_branches/api_update_service.rb index 41d3d413caa..1c27d32aad8 100644 --- a/app/services/protected_branches/api_update_service.rb +++ b/app/services/protected_branches/api_update_service.rb @@ -14,25 +14,19 @@ module ProtectedBranches @protected_branch = protected_branch protected_branch.transaction do - # If a protected branch can have only a single access level (CE), delete it to - # make room for the new access level. If a protected branch can have more than - # one access level (EE), only remove the relevant access levels. If we don't do this, - # we'll have a failed validation. - if protected_branch_restricted_to_single_access_level? - delete_redundant_ce_access_levels - else - delete_redundant_ee_access_levels - end + delete_redundant_access_levels - if @developers_can_push + case @developers_can_push + when true params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) - elsif @developers_can_push == false + when false params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) end - if @developers_can_merge + case @developers_can_merge + when true params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) - elsif @developers_can_merge == false + when false params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) end @@ -43,7 +37,7 @@ module ProtectedBranches private - def delete_redundant_ce_access_levels + def delete_redundant_access_levels if @developers_can_merge || @developers_can_merge == false @protected_branch.merge_access_levels.destroy_all end @@ -52,28 +46,5 @@ module ProtectedBranches @protected_branch.push_access_levels.destroy_all end end - - def delete_redundant_ee_access_levels - if @developers_can_merge - @protected_branch.merge_access_levels.developer.destroy_all - elsif @developers_can_merge == false - @protected_branch.merge_access_levels.developer.destroy_all - @protected_branch.merge_access_levels.master.destroy_all - end - - if @developers_can_push - @protected_branch.push_access_levels.developer.destroy_all - elsif @developers_can_push == false - @protected_branch.push_access_levels.developer.destroy_all - @protected_branch.push_access_levels.master.destroy_all - end - end - - def protected_branch_restricted_to_single_access_level? - length_validator = ProtectedBranch.validators_on(:push_access_levels).find do |validator| - validator.is_a? ActiveModel::Validations::LengthValidator - end - length_validator.options[:is] == 1 if length_validator - end end end From b803bc7bb8ad481790d01848902e80602e77da67 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Thu, 22 Sep 2016 20:38:05 +0530 Subject: [PATCH 20/73] Implement review comments from @DouweM. --- .../concerns/protected_branch_access.rb | 4 +-- .../protected_branches/api_create_service.rb | 25 +++++++++++-------- lib/api/branches.rb | 4 +-- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/app/models/concerns/protected_branch_access.rb b/app/models/concerns/protected_branch_access.rb index dd6b34d934b..7fd0905ee81 100644 --- a/app/models/concerns/protected_branch_access.rb +++ b/app/models/concerns/protected_branch_access.rb @@ -2,8 +2,8 @@ module ProtectedBranchAccess extend ActiveSupport::Concern included do - scope :master, -> () { where(access_level: Gitlab::Access::MASTER) } - scope :developer, -> () { where(access_level: Gitlab::Access::DEVELOPER) } + scope :master, -> { where(access_level: Gitlab::Access::MASTER) } + scope :developer, -> { where(access_level: Gitlab::Access::DEVELOPER) } end def humanize diff --git a/app/services/protected_branches/api_create_service.rb b/app/services/protected_branches/api_create_service.rb index a28056035b8..cbb99ede9f3 100644 --- a/app/services/protected_branches/api_create_service.rb +++ b/app/services/protected_branches/api_create_service.rb @@ -11,17 +11,22 @@ module ProtectedBranches end def execute - if @developers_can_push - @params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) - else - @params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) - end + push_access_level = + if @developers_can_push + Gitlab::Access::DEVELOPER + else + Gitlab::Access::MASTER + end - if @developers_can_merge - @params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) - else - @params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) - end + merge_access_level = + if @developers_can_merge + Gitlab::Access::DEVELOPER + else + Gitlab::Access::MASTER + end + + @params.merge!(push_access_levels_attributes: [{ access_level: push_access_level }], + merge_access_levels_attributes: [{ access_level: merge_access_level }]) service = ProtectedBranches::CreateService.new(@project, @current_user, @params) service.execute diff --git a/lib/api/branches.rb b/lib/api/branches.rb index 6941cc4aca6..7feb47784b7 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -57,11 +57,11 @@ module API developers_can_merge = to_boolean(params[:developers_can_merge]) developers_can_push = to_boolean(params[:developers_can_push]) - params = { + protected_branch_params = { name: @branch.name, } - service_args = [user_project, current_user, params, + service_args = [user_project, current_user, protected_branch_params, developers_can_push: developers_can_push, developers_can_merge: developers_can_merge] From 1051087ac4efc3dbf45bd075e36af647d2b66d62 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Fri, 30 Sep 2016 13:14:46 +0530 Subject: [PATCH 21/73] Implement second round of review comments from @DouweM. - Pass `developers_and_merge` and `developers_can_push` in `params` instead of using keyword arguments. - Refactor a slightly complex boolean check to a simple `nil?` check. --- app/services/protected_branches/api_create_service.rb | 6 +++--- app/services/protected_branches/api_update_service.rb | 10 +++++----- lib/api/branches.rb | 9 +++------ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/app/services/protected_branches/api_create_service.rb b/app/services/protected_branches/api_create_service.rb index cbb99ede9f3..d714a8aaf01 100644 --- a/app/services/protected_branches/api_create_service.rb +++ b/app/services/protected_branches/api_create_service.rb @@ -4,10 +4,10 @@ # lives in this service. module ProtectedBranches class ApiCreateService < BaseService - def initialize(project, user, params, developers_can_push:, developers_can_merge:) + def initialize(project, user, params) + @developers_can_merge = params.delete(:developers_can_merge) + @developers_can_push = params.delete(:developers_can_push) super(project, user, params) - @developers_can_merge = developers_can_merge - @developers_can_push = developers_can_push end def execute diff --git a/app/services/protected_branches/api_update_service.rb b/app/services/protected_branches/api_update_service.rb index 1c27d32aad8..c28bffee2f4 100644 --- a/app/services/protected_branches/api_update_service.rb +++ b/app/services/protected_branches/api_update_service.rb @@ -4,10 +4,10 @@ # lives in this service. module ProtectedBranches class ApiUpdateService < BaseService - def initialize(project, user, params, developers_can_push:, developers_can_merge:) + def initialize(project, user, params) + @developers_can_merge = params.delete(:developers_can_merge) + @developers_can_push = params.delete(:developers_can_push) super(project, user, params) - @developers_can_merge = developers_can_merge - @developers_can_push = developers_can_push end def execute(protected_branch) @@ -38,11 +38,11 @@ module ProtectedBranches private def delete_redundant_access_levels - if @developers_can_merge || @developers_can_merge == false + unless @developers_can_merge.nil? @protected_branch.merge_access_levels.destroy_all end - if @developers_can_push || @developers_can_push == false + unless @developers_can_push.nil? @protected_branch.push_access_levels.destroy_all end end diff --git a/lib/api/branches.rb b/lib/api/branches.rb index 7feb47784b7..6d827448994 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -54,16 +54,13 @@ module API not_found!('Branch') unless @branch protected_branch = user_project.protected_branches.find_by(name: @branch.name) - developers_can_merge = to_boolean(params[:developers_can_merge]) - developers_can_push = to_boolean(params[:developers_can_push]) - protected_branch_params = { name: @branch.name, + developers_can_push: to_boolean(params[:developers_can_push]), + developers_can_merge: to_boolean(params[:developers_can_merge]) } - service_args = [user_project, current_user, protected_branch_params, - developers_can_push: developers_can_push, - developers_can_merge: developers_can_merge] + service_args = [user_project, current_user, protected_branch_params] protected_branch = if protected_branch ProtectedBranches::ApiUpdateService.new(*service_args).execute(protected_branch) From db0182e261936c3e800b546d307a3d3834ff9927 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Mon, 24 Oct 2016 11:32:09 +0530 Subject: [PATCH 22/73] Implement third round of review comments from @DouweM. Extract/mutate `params` in the `execute` method of the API services, rather than in `initialize`. --- app/services/protected_branches/api_create_service.rb | 10 ++-------- app/services/protected_branches/api_update_service.rb | 9 +++------ 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/app/services/protected_branches/api_create_service.rb b/app/services/protected_branches/api_create_service.rb index d714a8aaf01..f2040dfa03a 100644 --- a/app/services/protected_branches/api_create_service.rb +++ b/app/services/protected_branches/api_create_service.rb @@ -4,22 +4,16 @@ # lives in this service. module ProtectedBranches class ApiCreateService < BaseService - def initialize(project, user, params) - @developers_can_merge = params.delete(:developers_can_merge) - @developers_can_push = params.delete(:developers_can_push) - super(project, user, params) - end - def execute push_access_level = - if @developers_can_push + if params.delete(:developers_can_push) Gitlab::Access::DEVELOPER else Gitlab::Access::MASTER end merge_access_level = - if @developers_can_merge + if params.delete(:developers_can_merge) Gitlab::Access::DEVELOPER else Gitlab::Access::MASTER diff --git a/app/services/protected_branches/api_update_service.rb b/app/services/protected_branches/api_update_service.rb index c28bffee2f4..050cb3b738b 100644 --- a/app/services/protected_branches/api_update_service.rb +++ b/app/services/protected_branches/api_update_service.rb @@ -4,13 +4,10 @@ # lives in this service. module ProtectedBranches class ApiUpdateService < BaseService - def initialize(project, user, params) - @developers_can_merge = params.delete(:developers_can_merge) - @developers_can_push = params.delete(:developers_can_push) - super(project, user, params) - end - def execute(protected_branch) + @developers_can_push = params.delete(:developers_can_push) + @developers_can_merge = params.delete(:developers_can_merge) + @protected_branch = protected_branch protected_branch.transaction do From cc6491ef6405de855b071e128631df6fdb47db8e Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 20 Oct 2016 12:43:08 +0100 Subject: [PATCH 23/73] Use root_url for issue boards user link Closes #23556 --- CHANGELOG.md | 2 +- app/views/projects/boards/components/_card.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f47b27d5aa..fb4dbe1062b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,6 @@ Please view this file on the master branch, on stable branches it's out of date. - Add hover to trash icon in notes !7008 (blackst0ne) - Simpler arguments passed to named_route on toggle_award_url helper method - Fix: Backup restore doesn't clear cache - - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method - Fix documents and comments on Build API `scope` @@ -44,6 +43,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Clarify documentation for Runners API (Gennady Trafimenkov) - The instrumentation for Banzai::Renderer has been restored - Change user & group landing page routing from /u/:username to /:username + - Fixed issue boards user link when in subdirectory - Added documentation for .gitattributes files - Move Pipeline Metrics to separate worker - AbstractReferenceFilter caches project_refs on RequestStore when active diff --git a/app/views/projects/boards/components/_card.html.haml b/app/views/projects/boards/components/_card.html.haml index d8f16022407..c6d718a1cd1 100644 --- a/app/views/projects/boards/components/_card.html.haml +++ b/app/views/projects/boards/components/_card.html.haml @@ -26,7 +26,7 @@ ":title" => "label.description", data: { container: 'body' } } {{ label.title }} - %a.has-tooltip{ ":href" => "'/' + issue.assignee.username", + %a.has-tooltip{ ":href" => "'#{root_path}' + issue.assignee.username", ":title" => "'Assigned to ' + issue.assignee.name", "v-if" => "issue.assignee", data: { container: 'body' } } From 9dbd5b3cfad10b214ae5ef27c39246bbb74a5077 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 24 Oct 2016 10:19:36 +0300 Subject: [PATCH 24/73] Revert "Change "Group#web_url" to return "/groups/twitter" rather than "/twitter"." This reverts commit c81ff152e08d58c13efbd50c40dd2e083ac65083. --- app/models/group.rb | 2 +- config/routes/group.rb | 39 ++++++++++++++++++--------------------- spec/models/group_spec.rb | 6 ------ 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index 6865e610718..00a595d2705 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -68,7 +68,7 @@ class Group < Namespace end def web_url - Gitlab::Routing.url_helpers.group_canonical_url(self) + Gitlab::Routing.url_helpers.group_url(self) end def human_name diff --git a/config/routes/group.rb b/config/routes/group.rb index 826048ba196..4838c9d91c6 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -12,26 +12,23 @@ constraints(GroupUrlConstrainer.new) do end end -scope constraints: { id: /[a-zA-Z.0-9_\-]+(? 'groups#show', as: :group_canonical end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 47f89f744cb..ac862055ebc 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -265,10 +265,4 @@ describe Group, models: true do members end - - describe '#web_url' do - it 'returns the canonical URL' do - expect(group.web_url).to include("groups/#{group.name}") - end - end end From 036fac06d18e82f0d0696bd1b350548bb47125e8 Mon Sep 17 00:00:00 2001 From: Linus G Thiel Date: Wed, 5 Oct 2016 21:58:34 +0200 Subject: [PATCH 25/73] Gracefully handle adding of no users to projects and groups - Disable {project, group} members submit button if no users If no users are selected, the submit button should be disabled. - Alert user when no users were added to {project, group}. When no users were selected for adding, an alert message is flashed that no users were added. - Also, this commit adds a feedback when users were actually added to a project, in symmetry with how group members are handled. Closes #22967, #23270. --- CHANGELOG.md | 1 + app/assets/javascripts/application.js | 5 +- .../groups/group_members_controller.rb | 4 ++ .../projects/project_members_controller.rb | 6 +- .../groups/group_members/index.html.haml | 3 + .../projects/project_members/index.html.haml | 3 + .../groups/group_members_controller_spec.rb | 54 ++++++++++++++++++ .../project_members_controller_spec.rb | 57 +++++++++++++++++++ 8 files changed, 130 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 909af5fc053..3fa0e67037e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Update Gitlab Shell to fix some problems with moving projects between storages - Cache rendered markdown in the database, rather than Redis - Avoid database queries on Banzai::ReferenceParser::BaseParser for nodes without references + - Better handle when no users were selected for adding to group or project. (Linus Thiel) - Simplify Mentionable concern instance methods - API: Ability to retrieve version information (Robert Schilling) - Fix permission for setting an issue's due date diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 8a61669822c..17cbfd0e66f 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -83,14 +83,15 @@ }; // Disable button if text field is empty - window.disableButtonIfEmptyField = function(field_selector, button_selector) { + window.disableButtonIfEmptyField = function(field_selector, button_selector, event_name) { + event_name = event_name || 'input'; var closest_submit, field; field = $(field_selector); closest_submit = field.closest('form').find(button_selector); if (rstrip(field.val()) === "") { closest_submit.disable(); } - return field.on('input', function() { + return field.on(event_name, function() { if (rstrip($(this).val()) === "") { return closest_submit.disable(); } else { diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 18cd800c619..3a373e4a946 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -21,6 +21,10 @@ class Groups::GroupMembersController < Groups::ApplicationController end def create + if params[:user_ids].empty? + return redirect_to group_group_members_path(@group), alert: 'No users specified.' + end + @group.add_users( params[:user_ids].split(','), params[:access_level], diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 2a07d154853..2bac48c5490 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -25,6 +25,10 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def create + if params[:user_ids].empty? + return redirect_to namespace_project_project_members_path(@project.namespace, @project), alert: 'No users specified.' + end + @project.team.add_users( params[:user_ids].split(','), params[:access_level], @@ -32,7 +36,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController current_user: current_user ) - redirect_to namespace_project_project_members_path(@project.namespace, @project) + redirect_to namespace_project_project_members_path(@project.namespace, @project), notice: 'Users were successfully added.' end def update diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml index ebf9aca7700..b295ad0e182 100644 --- a/app/views/groups/group_members/index.html.haml +++ b/app/views/groups/group_members/index.html.haml @@ -29,3 +29,6 @@ %ul.content-list = render partial: 'shared/members/member', collection: @members, as: :member = paginate @members, theme: 'gitlab' + +:javascript + window.disableButtonIfEmptyField('#user_ids', 'input[name=commit]', 'change'); diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml index bdeb704b6da..3904647b608 100644 --- a/app/views/projects/project_members/index.html.haml +++ b/app/views/projects/project_members/index.html.haml @@ -26,3 +26,6 @@ = render 'team', members: @project_members = paginate @project_members, theme: "gitlab" + +:javascript + window.disableButtonIfEmptyField('#user_ids', 'input[name=commit]', 'change'); diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index ad15b3f8f40..82eebe6f2d4 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -13,6 +13,60 @@ describe Groups::GroupMembersController do end end + describe '#create' do + let(:group) { create(:group, :public) } + + context 'when users are added' do + let(:user) { create(:user) } + let(:group_user) { create(:user) } + let(:member) do + group.add_developer(group_user) + group.members.find_by(user_id: group_user) + end + + context 'when user does not have enough rights' do + before do + group.members.delete(member) + group.add_developer(user) + sign_in(user) + end + + it 'returns 403' do + post :create, group_id: group, + user_ids: member + + expect(response).to have_http_status(403) + expect(group.users).not_to include group_user + end + end + + context 'when user has enough rights' do + before do + group.add_owner(user) + sign_in(user) + end + + it 'adds user to members' do + post :create, group_id: group, + user_ids: member + + expect(response).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(group_group_members_path(group)) + expect(group.users).to include group_user + end + + it 'adds no user to members' do + post :create, group_id: group, + user_ids: '' + + expect(response).to set_flash.to 'No users specified.' + expect(response).to redirect_to(group_group_members_path(group)) + expect(group.users).not_to include group_user + end + end + end + end + describe 'DELETE destroy' do let(:member) { create(:group_member, :developer, group: group) } diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 5e487241d07..44d907eafd7 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -13,6 +13,63 @@ describe Projects::ProjectMembersController do end end + describe '#create' do + let(:project) { create(:project, :public) } + + context 'when users are added' do + let(:user) { create(:user) } + let(:team_user) { create(:user) } + let(:member) do + project.team << [team_user, :developer] + project.members.find_by(user_id: team_user.id) + end + + context 'when user does not have enough rights' do + before do + project.members.delete(member) + project.team << [user, :developer] + sign_in(user) + end + + it 'returns 404' do + post :create, namespace_id: project.namespace, + project_id: project, + user_ids: member + + expect(response).to have_http_status(404) + expect(project.users).not_to include team_user + end + end + + context 'when user has enough rights' do + before do + project.team << [user, :master] + sign_in(user) + end + + it 'adds user to members' do + post :create, namespace_id: project.namespace, + project_id: project, + user_ids: member + + expect(response).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project)) + expect(project.users).to include team_user + end + + it 'adds no user to members' do + post :create, namespace_id: project.namespace, + project_id: project, + user_ids: '' + + expect(response).to set_flash.to 'No users specified.' + expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project)) + expect(project.users).not_to include team_user + end + end + end + end + describe 'DELETE destroy' do let(:member) { create(:project_member, :developer, project: project) } From c82278898d4e7932da1e0071d4dcfa13f65967f0 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 19 Oct 2016 17:27:28 +0300 Subject: [PATCH 26/73] Refactor groups/projects members controller Signed-off-by: Dmitriy Zaporozhets --- .../groups/group_members_controller.rb | 2 +- .../projects/project_members_controller.rb | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 3a373e4a946..5a6e26ab8cc 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -21,7 +21,7 @@ class Groups::GroupMembersController < Groups::ApplicationController end def create - if params[:user_ids].empty? + if params[:user_ids].blank? return redirect_to group_group_members_path(@group), alert: 'No users specified.' end diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 2bac48c5490..ec8512bbaba 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -25,16 +25,18 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def create - if params[:user_ids].empty? - return redirect_to namespace_project_project_members_path(@project.namespace, @project), alert: 'No users specified.' + if params[:user_ids].blank? && params[:group_ids].blank? + return redirect_to namespace_project_project_members_path(@project.namespace, @project), alert: 'No users or groups specified.' end - @project.team.add_users( - params[:user_ids].split(','), - params[:access_level], - expires_at: params[:expires_at], - current_user: current_user - ) + if params[:user_ids].present? + @project.team.add_users( + params[:user_ids].split(','), + params[:access_level], + expires_at: params[:expires_at], + current_user: current_user + ) + end redirect_to namespace_project_project_members_path(@project.namespace, @project), notice: 'Users were successfully added.' end From 1eba14ae2ecd829642e88610b0b2964e5d04158e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 19 Oct 2016 17:50:41 +0300 Subject: [PATCH 27/73] Refactor create member tests from group_members_controller_spec Signed-off-by: Dmitriy Zaporozhets --- .../groups/group_members_controller_spec.rb | 81 ++++++++----------- 1 file changed, 35 insertions(+), 46 deletions(-) diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 82eebe6f2d4..c7db84dd5f9 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -13,56 +13,45 @@ describe Groups::GroupMembersController do end end - describe '#create' do - let(:group) { create(:group, :public) } + describe 'POST create' do + let(:group_user) { create(:user) } - context 'when users are added' do - let(:user) { create(:user) } - let(:group_user) { create(:user) } - let(:member) do - group.add_developer(group_user) - group.members.find_by(user_id: group_user) + before { sign_in(user) } + + context 'when user does not have enough rights' do + before { group.add_developer(user) } + + it 'returns 403' do + post :create, group_id: group, + user_ids: group_user.id, + access_level: Gitlab::Access::GUEST + + expect(response).to have_http_status(403) + expect(group.users).not_to include group_user + end + end + + context 'when user has enough rights' do + before { group.add_owner(user) } + + it 'adds user to members' do + post :create, group_id: group, + user_ids: group_user.id, + access_level: Gitlab::Access::GUEST + + expect(response).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(group_group_members_path(group)) + expect(group.users).to include group_user end - context 'when user does not have enough rights' do - before do - group.members.delete(member) - group.add_developer(user) - sign_in(user) - end + it 'adds no user to members' do + post :create, group_id: group, + user_ids: '', + access_level: Gitlab::Access::GUEST - it 'returns 403' do - post :create, group_id: group, - user_ids: member - - expect(response).to have_http_status(403) - expect(group.users).not_to include group_user - end - end - - context 'when user has enough rights' do - before do - group.add_owner(user) - sign_in(user) - end - - it 'adds user to members' do - post :create, group_id: group, - user_ids: member - - expect(response).to set_flash.to 'Users were successfully added.' - expect(response).to redirect_to(group_group_members_path(group)) - expect(group.users).to include group_user - end - - it 'adds no user to members' do - post :create, group_id: group, - user_ids: '' - - expect(response).to set_flash.to 'No users specified.' - expect(response).to redirect_to(group_group_members_path(group)) - expect(group.users).not_to include group_user - end + expect(response).to set_flash.to 'No users specified.' + expect(response).to redirect_to(group_group_members_path(group)) + expect(group.users).not_to include group_user end end end From 52707acfa73e15896c4794d55e430eb14a532f66 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 20 Oct 2016 10:43:05 +0300 Subject: [PATCH 28/73] Move changelog item to 8.14 Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fa0e67037e..444320bb35c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix error in generating labels - Fix reply-by-email not working due to queue name mismatch - Expire and build repository cache after project import + - Simpler arguments passed to named_route on toggle_award_url helper method + - Better handle when no users were selected for adding to group or project. (Linus Thiel) ## 8.13.0 (2016-10-22) - Removes extra line for empty issue description. (!7045) @@ -69,7 +71,6 @@ Please view this file on the master branch, on stable branches it's out of date. - Update Gitlab Shell to fix some problems with moving projects between storages - Cache rendered markdown in the database, rather than Redis - Avoid database queries on Banzai::ReferenceParser::BaseParser for nodes without references - - Better handle when no users were selected for adding to group or project. (Linus Thiel) - Simplify Mentionable concern instance methods - API: Ability to retrieve version information (Robert Schilling) - Fix permission for setting an issue's due date From 3608f9284e676437e306b4f08a157f997471f4d3 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 20 Oct 2016 10:59:36 +0300 Subject: [PATCH 29/73] Improve create project member test at project_members_controller_spec Signed-off-by: Dmitriy Zaporozhets --- .../project_members_controller_spec.rb | 41 ++++++++----------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 44d907eafd7..b4f066d8600 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -13,58 +13,49 @@ describe Projects::ProjectMembersController do end end - describe '#create' do - let(:project) { create(:project, :public) } - + describe 'POST create' do context 'when users are added' do - let(:user) { create(:user) } - let(:team_user) { create(:user) } - let(:member) do - project.team << [team_user, :developer] - project.members.find_by(user_id: team_user.id) - end + let(:project_user) { create(:user) } + + before { sign_in(user) } context 'when user does not have enough rights' do - before do - project.members.delete(member) - project.team << [user, :developer] - sign_in(user) - end + before { project.team << [user, :developer] } it 'returns 404' do post :create, namespace_id: project.namespace, project_id: project, - user_ids: member + user_ids: project_user.id, + access_level: Gitlab::Access::GUEST expect(response).to have_http_status(404) - expect(project.users).not_to include team_user + expect(project.users).not_to include project_user end end context 'when user has enough rights' do - before do - project.team << [user, :master] - sign_in(user) - end + before { project.team << [user, :master] } it 'adds user to members' do post :create, namespace_id: project.namespace, project_id: project, - user_ids: member + user_ids: project_user.id, + access_level: Gitlab::Access::GUEST expect(response).to set_flash.to 'Users were successfully added.' expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project)) - expect(project.users).to include team_user + expect(project.users).to include project_user end it 'adds no user to members' do post :create, namespace_id: project.namespace, project_id: project, - user_ids: '' + user_ids: '', + access_level: Gitlab::Access::GUEST - expect(response).to set_flash.to 'No users specified.' + expect(response).to set_flash.to 'No users or groups specified.' expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project)) - expect(project.users).not_to include team_user + expect(project.users).not_to include project_user end end end From bedbb7b744504a0008e0bc33b21f229a84d616d6 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 20 Oct 2016 11:07:06 +0300 Subject: [PATCH 30/73] Refactor js that disable form submit if no members selected Signed-off-by: Dmitriy Zaporozhets --- app/assets/javascripts/members.js.es6 | 1 + app/views/groups/group_members/index.html.haml | 3 --- app/views/projects/project_members/index.html.haml | 3 --- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/app/assets/javascripts/members.js.es6 b/app/assets/javascripts/members.js.es6 index a0cd20f21e8..2bdd0f7a637 100644 --- a/app/assets/javascripts/members.js.es6 +++ b/app/assets/javascripts/members.js.es6 @@ -10,6 +10,7 @@ $('.project_member, .group_member').off('ajax:success').on('ajax:success', this.removeRow); $('.js-member-update-control').off('change').on('change', this.formSubmit); $('.js-edit-member-form').off('ajax:success').on('ajax:success', this.formSuccess); + disableButtonIfEmptyField('#user_ids', 'input[name=commit]', 'change'); } removeRow(e) { diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml index b295ad0e182..ebf9aca7700 100644 --- a/app/views/groups/group_members/index.html.haml +++ b/app/views/groups/group_members/index.html.haml @@ -29,6 +29,3 @@ %ul.content-list = render partial: 'shared/members/member', collection: @members, as: :member = paginate @members, theme: 'gitlab' - -:javascript - window.disableButtonIfEmptyField('#user_ids', 'input[name=commit]', 'change'); diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml index 3904647b608..bdeb704b6da 100644 --- a/app/views/projects/project_members/index.html.haml +++ b/app/views/projects/project_members/index.html.haml @@ -26,6 +26,3 @@ = render 'team', members: @project_members = paginate @project_members, theme: "gitlab" - -:javascript - window.disableButtonIfEmptyField('#user_ids', 'input[name=commit]', 'change'); From 80ea344e427379b5e614f14c90af5369ae1c4a16 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 20 Oct 2016 11:48:38 +0300 Subject: [PATCH 31/73] Trigger change even in select2 test helper to produce production-like behaviour Signed-off-by: Dmitriy Zaporozhets --- spec/support/select2_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/support/select2_helper.rb b/spec/support/select2_helper.rb index 35cc51725c6..d30cc8ff9f2 100644 --- a/spec/support/select2_helper.rb +++ b/spec/support/select2_helper.rb @@ -17,9 +17,9 @@ module Select2Helper selector = options.fetch(:from) if options[:multiple] - execute_script("$('#{selector}').select2('val', ['#{value}'], true);") + execute_script("$('#{selector}').select2('val', ['#{value}']).trigger('change');") else - execute_script("$('#{selector}').select2('val', '#{value}', true);") + execute_script("$('#{selector}').select2('val', '#{value}').trigger('change');") end end end From f5659ac4d1ad26d4d0ba5ea10adb92511cc911f2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 20 Oct 2016 14:58:26 +0300 Subject: [PATCH 32/73] Add parentheses around return redirect_to method Signed-off-by: Dmitriy Zaporozhets --- app/controllers/groups/group_members_controller.rb | 2 +- app/controllers/projects/project_members_controller.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 5a6e26ab8cc..940a3ad20ba 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -22,7 +22,7 @@ class Groups::GroupMembersController < Groups::ApplicationController def create if params[:user_ids].blank? - return redirect_to group_group_members_path(@group), alert: 'No users specified.' + return redirect_to(group_group_members_path(@group), alert: 'No users specified.') end @group.add_users( diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index ec8512bbaba..073d4338338 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -26,7 +26,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController def create if params[:user_ids].blank? && params[:group_ids].blank? - return redirect_to namespace_project_project_members_path(@project.namespace, @project), alert: 'No users or groups specified.' + return redirect_to(namespace_project_project_members_path(@project.namespace, @project), alert: 'No users or groups specified.') end if params[:user_ids].present? From 7ded7c17d4da00e60d2855560a5c53b6dbaf98c0 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 20 Oct 2016 15:27:02 +0300 Subject: [PATCH 33/73] Update project member controller to match recent master logic Signed-off-by: Dmitriy Zaporozhets --- .../projects/project_members_controller.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 073d4338338..d08f490de18 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -25,18 +25,16 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def create - if params[:user_ids].blank? && params[:group_ids].blank? + if params[:user_ids].blank? return redirect_to(namespace_project_project_members_path(@project.namespace, @project), alert: 'No users or groups specified.') end - if params[:user_ids].present? - @project.team.add_users( - params[:user_ids].split(','), - params[:access_level], - expires_at: params[:expires_at], - current_user: current_user - ) - end + @project.team.add_users( + params[:user_ids].split(','), + params[:access_level], + expires_at: params[:expires_at], + current_user: current_user + ) redirect_to namespace_project_project_members_path(@project.namespace, @project), notice: 'Users were successfully added.' end From 672704f6389104b3d93c00a116dd4dc8ece19136 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 24 Oct 2016 13:51:17 +0300 Subject: [PATCH 34/73] Add relative url support to routing contrainers Signed-off-by: Dmitriy Zaporozhets --- lib/constraints/namespace_url_constrainer.rb | 13 ++++++++++++- .../constraints/namespace_url_constrainer_spec.rb | 10 ++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/constraints/namespace_url_constrainer.rb b/lib/constraints/namespace_url_constrainer.rb index 23920193743..91b70143f11 100644 --- a/lib/constraints/namespace_url_constrainer.rb +++ b/lib/constraints/namespace_url_constrainer.rb @@ -1,6 +1,9 @@ class NamespaceUrlConstrainer def matches?(request) - id = request.path.sub(/\A\/+/, '').split('/').first.sub(/.atom\z/, '') + id = request.path + id = id.sub(/\A#{relative_url_root}/, '') if relative_url_root + id = id.sub(/\A\/+/, '').split('/').first + id = id.sub(/.atom\z/, '') if id if id =~ Gitlab::Regex.namespace_regex find_resource(id) @@ -10,4 +13,12 @@ class NamespaceUrlConstrainer def find_resource(id) Namespace.find_by_path(id) end + + private + + def relative_url_root + if defined?(Gitlab::Application.config.relative_url_root) + Gitlab::Application.config.relative_url_root + end + end end diff --git a/spec/lib/constraints/namespace_url_constrainer_spec.rb b/spec/lib/constraints/namespace_url_constrainer_spec.rb index a5feaacb8ee..7814711fe27 100644 --- a/spec/lib/constraints/namespace_url_constrainer_spec.rb +++ b/spec/lib/constraints/namespace_url_constrainer_spec.rb @@ -17,6 +17,16 @@ describe NamespaceUrlConstrainer, lib: true do it { expect(subject.matches?(request '/g/gitlab')).to be_falsey } it { expect(subject.matches?(request '/.gitlab')).to be_falsey } end + + context 'relative url' do + before do + allow(Gitlab::Application.config).to receive(:relative_url_root) { '/gitlab' } + end + + it { expect(subject.matches?(request '/gitlab/gitlab')).to be_truthy } + it { expect(subject.matches?(request '/gitlab/gitlab-ce')).to be_falsey } + it { expect(subject.matches?(request '/gitlab/')).to be_falsey } + end end def request(path) From ccd81a872cbb28fe1dc3f8722fa2a8b85fd16ee8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 24 Oct 2016 13:53:15 +0300 Subject: [PATCH 35/73] Add changelog item for groups 404 on relative url Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 909af5fc053..af88bdc8988 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix error in generating labels - Fix reply-by-email not working due to queue name mismatch - Expire and build repository cache after project import + - Fix 404 for group pages when GitLab setup uses relative url ## 8.13.0 (2016-10-22) - Removes extra line for empty issue description. (!7045) From 8f527cbf06d31c084bdf71c7f882d73ff6591e29 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Mon, 24 Oct 2016 13:06:17 +0200 Subject: [PATCH 36/73] Grapify builds API --- lib/api/builds.rb | 162 ++++++++++++++++++++++------------------------ 1 file changed, 79 insertions(+), 83 deletions(-) diff --git a/lib/api/builds.rb b/lib/api/builds.rb index 7b00c5037f1..67adca6605f 100644 --- a/lib/api/builds.rb +++ b/lib/api/builds.rb @@ -3,15 +3,32 @@ module API class Builds < Grape::API before { authenticate! } + params do + requires :id, type: String, desc: 'The ID of a project' + end resource :projects do - # Get a project builds - # - # Parameters: - # id (required) - The ID of a project - # scope (optional) - The scope of builds to show (one or array of: created, pending, running, failed, success, canceled, skipped; - # if none provided showing all builds) - # Example Request: - # GET /projects/:id/builds + helpers do + params :optional_scope do + optional :scope, types: [String, Array[String]], desc: 'The scope of builds to show', + values: ['pending', 'running', 'failed', 'success', 'canceled'], + coerce_with: ->(scope) { + if scope.is_a?(String) + [scope] + elsif scope.is_a?(Hashie::Mash) + scope.values + else + ['unknown'] + end + } + end + end + + desc 'Get a project builds' do + success Entities::Build + end + params do + use :optional_scope + end get ':id/builds' do builds = user_project.builds.order('id DESC') builds = filter_builds(builds, params[:scope]) @@ -20,15 +37,13 @@ module API user_can_download_artifacts: can?(current_user, :read_build, user_project) end - # Get builds for a specific commit of a project - # - # Parameters: - # id (required) - The ID of a project - # sha (required) - The SHA id of a commit - # scope (optional) - The scope of builds to show (one or array of: created, pending, running, failed, success, canceled, skipped; - # if none provided showing all builds) - # Example Request: - # GET /projects/:id/repository/commits/:sha/builds + desc 'Get builds for a specific commit of a project' do + success Entities::Build + end + params do + requires :sha, type: String, desc: 'The SHA id of a commit' + use :optional_scope + end get ':id/repository/commits/:sha/builds' do authorize_read_builds! @@ -42,13 +57,12 @@ module API user_can_download_artifacts: can?(current_user, :read_build, user_project) end - # Get a specific build of a project - # - # Parameters: - # id (required) - The ID of a project - # build_id (required) - The ID of a build - # Example Request: - # GET /projects/:id/builds/:build_id + desc 'Get a specific build of a project' do + success Entities::Build + end + params do + requires :build_id, type: Integer, desc: 'The ID of a build' + end get ':id/builds/:build_id' do authorize_read_builds! @@ -58,13 +72,12 @@ module API user_can_download_artifacts: can?(current_user, :read_build, user_project) end - # Download the artifacts file from build - # - # Parameters: - # id (required) - The ID of a build - # token (required) - The build authorization token - # Example Request: - # GET /projects/:id/builds/:build_id/artifacts + desc 'Download the artifacts file from build' do + detail 'This feature was introduced in GitLab 8.5' + end + params do + requires :build_id, type: Integer, desc: 'The ID of a build' + end get ':id/builds/:build_id/artifacts' do authorize_read_builds! @@ -73,14 +86,13 @@ module API present_artifacts!(build.artifacts_file) end - # Download the artifacts file from ref_name and job - # - # Parameters: - # id (required) - The ID of a project - # ref_name (required) - The ref from repository - # job (required) - The name for the build - # Example Request: - # GET /projects/:id/builds/artifacts/:ref_name/download?job=name + desc 'Download the artifacts file from build' do + detail 'This feature was introduced in GitLab 8.10' + end + params do + requires :ref_name, type: String, desc: 'The ref from repository' + requires :job, type: String, desc: 'The name for the build' + end get ':id/builds/artifacts/:ref_name/download', requirements: { ref_name: /.+/ } do authorize_read_builds! @@ -91,17 +103,13 @@ module API present_artifacts!(latest_build.artifacts_file) end - # Get a trace of a specific build of a project - # - # Parameters: - # id (required) - The ID of a project - # build_id (required) - The ID of a build - # Example Request: - # GET /projects/:id/build/:build_id/trace - # # TODO: We should use `present_file!` and leave this implementation for backward compatibility (when build trace # is saved in the DB instead of file). But before that, we need to consider how to replace the value of # `runners_token` with some mask (like `xxxxxx`) when sending trace file directly by workhorse. + desc 'Get a trace of a specific build of a project' + params do + requires :build_id, type: Integer, desc: 'The ID of a build' + end get ':id/builds/:build_id/trace' do authorize_read_builds! @@ -115,13 +123,12 @@ module API body trace end - # Cancel a specific build of a project - # - # parameters: - # id (required) - the id of a project - # build_id (required) - the id of a build - # example request: - # post /projects/:id/build/:build_id/cancel + desc 'Cancel a specific build of a project' do + success Entities::Build + end + params do + requires :build_id, type: Integer, desc: 'The ID of a build' + end post ':id/builds/:build_id/cancel' do authorize_update_builds! @@ -133,13 +140,12 @@ module API user_can_download_artifacts: can?(current_user, :read_build, user_project) end - # Retry a specific build of a project - # - # parameters: - # id (required) - the id of a project - # build_id (required) - the id of a build - # example request: - # post /projects/:id/build/:build_id/retry + desc 'Retry a specific build of a project' do + success Entities::Build + end + params do + requires :build_id, type: Integer, desc: 'The ID of a build' + end post ':id/builds/:build_id/retry' do authorize_update_builds! @@ -152,13 +158,12 @@ module API user_can_download_artifacts: can?(current_user, :read_build, user_project) end - # Erase build (remove artifacts and build trace) - # - # Parameters: - # id (required) - the id of a project - # build_id (required) - the id of a build - # example Request: - # post /projects/:id/build/:build_id/erase + desc 'Erase build (remove artifacts and build trace)' do + success Entities::Build + end + params do + requires :build_id, type: Integer, desc: 'The ID of a build' + end post ':id/builds/:build_id/erase' do authorize_update_builds! @@ -170,13 +175,12 @@ module API user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project) end - # Keep the artifacts to prevent them from being deleted - # - # Parameters: - # id (required) - the id of a project - # build_id (required) - The ID of a build - # Example Request: - # POST /projects/:id/builds/:build_id/artifacts/keep + desc 'Keep the artifacts to prevent them from being deleted' do + success Entities::Build + end + params do + requires :build_id, type: Integer, desc: 'The ID of a build' + end post ':id/builds/:build_id/artifacts/keep' do authorize_update_builds! @@ -235,14 +239,6 @@ module API return builds if scope.nil? || scope.empty? available_statuses = ::CommitStatus::AVAILABLE_STATUSES - scope = - if scope.is_a?(String) - [scope] - elsif scope.is_a?(Hashie::Mash) - scope.values - else - ['unknown'] - end unknown = scope - available_statuses render_api_error!('Scope contains invalid value(s)', 400) unless unknown.empty? From ea6c5c1c0ee8f09fff4dbf02d7bfb65107020996 Mon Sep 17 00:00:00 2001 From: barthc Date: Tue, 27 Sep 2016 15:24:50 +0100 Subject: [PATCH 37/73] Fix authored vote from notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG.md | 1 + app/services/notes/create_service.rb | 6 ++++-- spec/requests/api/notes_spec.rb | 15 +++++++++++++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 909af5fc053..b5525c32465 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.14.0 (2016-11-22) - Adds user project membership expired event to clarify why user was removed (Callum Dryden) - Trim leading and trailing whitespace on project_path (Linus Thiel) + - Prevent award emoji via notes for issues/MRs authored by user (barthc) - Fix HipChat notifications rendering (airatshigapov, eisnerd) - Add hover to trash icon in notes !7008 (blackst0ne) - Simpler arguments passed to named_route on toggle_award_url helper method diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index a36008c3ef5..723cc0e6834 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -7,8 +7,10 @@ module Notes if note.award_emoji? noteable = note.noteable - todo_service.new_award_emoji(noteable, current_user) - return noteable.create_award_emoji(note.award_emoji_name, current_user) + if noteable.user_can_award?(current_user, note.award_emoji_name) + todo_service.new_award_emoji(noteable, current_user) + return noteable.create_award_emoji(note.award_emoji_name, current_user) + end end # We execute commands (extracted from `params[:note]`) on the noteable diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index d58bedc3bf7..0124b7271b3 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -221,12 +221,23 @@ describe API::API, api: true do end end - context 'when the user is posting an award emoji' do + context 'when the user is posting an award emoji on an issue created by someone else' do + let(:issue2) { create(:issue, project: project) } + it 'returns an award emoji' do + post api("/projects/#{project.id}/issues/#{issue2.id}/notes", user), body: ':+1:' + + expect(response).to have_http_status(201) + expect(json_response['awardable_id']).to eq issue2.id + end + end + + context 'when the user is posting an award emoji on his/her own issue' do + it 'creates a new issue note' do post api("/projects/#{project.id}/issues/#{issue.id}/notes", user), body: ':+1:' expect(response).to have_http_status(201) - expect(json_response['awardable_id']).to eq issue.id + expect(json_response['body']).to eq(':+1:') end end end From 4a0e8f59e25a0b33e8e8cf33678688df912da8eb Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 20 Oct 2016 14:54:55 +0000 Subject: [PATCH 38/73] Merge branch 'security-fix-leaking-namespace-name' into 'security' Check that user has access to a given namespace to prevent leaking namespace names. See merge request !2009 --- app/controllers/import/gitlab_projects_controller.rb | 4 ++-- app/views/import/gitlab_projects/new.html.haml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/import/gitlab_projects_controller.rb b/app/controllers/import/gitlab_projects_controller.rb index 3ec173abcdb..36d246d185b 100644 --- a/app/controllers/import/gitlab_projects_controller.rb +++ b/app/controllers/import/gitlab_projects_controller.rb @@ -2,8 +2,8 @@ class Import::GitlabProjectsController < Import::BaseController before_action :verify_gitlab_project_import_enabled def new - @namespace_id = project_params[:namespace_id] - @namespace_name = Namespace.find(project_params[:namespace_id]).name + @namespace = Namespace.find(project_params[:namespace_id]) + return render_404 unless current_user.can?(:create_projects, @namespace) @path = project_params[:path] end diff --git a/app/views/import/gitlab_projects/new.html.haml b/app/views/import/gitlab_projects/new.html.haml index 44e2653ca4a..767dffb5589 100644 --- a/app/views/import/gitlab_projects/new.html.haml +++ b/app/views/import/gitlab_projects/new.html.haml @@ -9,12 +9,12 @@ %p Project will be imported as %strong - #{@namespace_name}/#{@path} + #{@namespace.name}/#{@path} %p To move or copy an entire GitLab project from another GitLab installation to this one, navigate to the original project's settings page, generate an export file, and upload it here. .form-group - = hidden_field_tag :namespace_id, @namespace_id + = hidden_field_tag :namespace_id, @namespace.id = hidden_field_tag :path, @path = label_tag :file, class: 'control-label' do %span GitLab project export From 4eb9056890d7789abc2f30be30fe3536336bdf42 Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Mon, 24 Oct 2016 14:59:20 +0100 Subject: [PATCH 39/73] fixes build with cache:clear issue --- spec/tasks/gitlab/backup_rake_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index 548e7780c36..73bc8326f02 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -9,6 +9,7 @@ describe 'gitlab:app namespace rake task' do Rake.application.rake_require 'tasks/gitlab/backup' Rake.application.rake_require 'tasks/gitlab/shell' Rake.application.rake_require 'tasks/gitlab/db' + Rake.application.rake_require 'tasks/cache' # empty task as env is already loaded Rake::Task.define_task :environment From f73d83db76ae8386ad4db604efb40156593b7a7a Mon Sep 17 00:00:00 2001 From: Luis HGO Date: Mon, 20 Jun 2016 22:37:40 -0300 Subject: [PATCH 40/73] Added path parameter to Commits API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG.md | 1 + lib/api/commits.rb | 2 ++ spec/requests/api/commits_spec.rb | 11 +++++++++++ 3 files changed, 14 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53406425f3d..ae578852fde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Adds user project membership expired event to clarify why user was removed (Callum Dryden) - Trim leading and trailing whitespace on project_path (Linus Thiel) - Prevent award emoji via notes for issues/MRs authored by user (barthc) + - Adds an optional path parameter to the Commits API to filter commits by path (Luis HGO) - Fix HipChat notifications rendering (airatshigapov, eisnerd) - Add hover to trash icon in notes !7008 (blackst0ne) - Simpler arguments passed to named_route on toggle_award_url helper method diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 617a240318a..2f2cf769481 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -19,6 +19,7 @@ module API optional :until, type: String, desc: 'Only commits before or in this date will be returned' optional :page, type: Integer, default: 0, desc: 'The page for pagination' optional :per_page, type: Integer, default: 20, desc: 'The number of results per page' + optional :path, type: String, desc: 'The file path' end get ":id/repository/commits" do # TODO remove the next line for 9.0, use DateTime type in the params block @@ -28,6 +29,7 @@ module API offset = params[:page] * params[:per_page] commits = user_project.repository.commits(ref, + path: params[:path], limit: params[:per_page], offset: offset, after: params[:since], diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 66fa0c0c01f..a6e8550fac3 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -72,6 +72,17 @@ describe API::API, api: true do expect(json_response['message']).to include "\"since\" must be a timestamp in ISO 8601 format" end end + + context "path optional parameter" do + it "returns project commits matching provided path parameter" do + path = 'files/ruby/popen.rb' + + get api("/projects/#{project.id}/repository/commits?path=#{path}", user) + + expect(json_response.size).to eq(3) + expect(json_response.first["id"]).to eq("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") + end + end end describe "Create a commit with multiple files and actions" do From 8b7634c2b665f6c3dc8b0f82b870fd2032f7d247 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Mon, 24 Oct 2016 16:45:00 +0200 Subject: [PATCH 41/73] Fix old monitoring links to point to the new location --- doc/development/performance.md | 2 +- doc/monitoring/performance/gitlab_configuration.md | 2 +- doc/monitoring/performance/influxdb_configuration.md | 2 +- doc/monitoring/performance/influxdb_schema.md | 2 +- doc/monitoring/performance/introduction.md | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/development/performance.md b/doc/development/performance.md index c4a964d1da3..8337c2d9cb3 100644 --- a/doc/development/performance.md +++ b/doc/development/performance.md @@ -37,7 +37,7 @@ graphs/dashboards. GitLab provides built-in tools to aid the process of improving performance: * [Sherlock](profiling.md#sherlock) -* [GitLab Performance Monitoring](../monitoring/performance/monitoring.md) +* [GitLab Performance Monitoring](../administration/monitoring/performance/monitoring.md) * [Request Profiling](../administration/monitoring/performance/request_profiling.md) GitLab employees can use GitLab.com's performance monitoring systems located at diff --git a/doc/monitoring/performance/gitlab_configuration.md b/doc/monitoring/performance/gitlab_configuration.md index a669bb28904..19d46135930 100644 --- a/doc/monitoring/performance/gitlab_configuration.md +++ b/doc/monitoring/performance/gitlab_configuration.md @@ -1 +1 @@ -This document was moved to [administration/monitoring/performance/gitlab_configuration](../administration/monitoring/performance/gitlab_configuration.md). +This document was moved to [administration/monitoring/performance/gitlab_configuration](../../administration/monitoring/performance/gitlab_configuration.md). diff --git a/doc/monitoring/performance/influxdb_configuration.md b/doc/monitoring/performance/influxdb_configuration.md index 02647de1eb0..15fd275e916 100644 --- a/doc/monitoring/performance/influxdb_configuration.md +++ b/doc/monitoring/performance/influxdb_configuration.md @@ -1 +1 @@ -This document was moved to [administration/monitoring/performance/influxdb_configuration](../administration/monitoring/performance/influxdb_configuration.md). +This document was moved to [administration/monitoring/performance/influxdb_configuration](../../administration/monitoring/performance/influxdb_configuration.md). diff --git a/doc/monitoring/performance/influxdb_schema.md b/doc/monitoring/performance/influxdb_schema.md index a989e323e04..e53f9701dc3 100644 --- a/doc/monitoring/performance/influxdb_schema.md +++ b/doc/monitoring/performance/influxdb_schema.md @@ -1 +1 @@ -This document was moved to [administration/monitoring/performance/influxdb_schema](../administration/monitoring/performance/influxdb_schema.md). +This document was moved to [administration/monitoring/performance/influxdb_schema](../../administration/monitoring/performance/influxdb_schema.md). diff --git a/doc/monitoring/performance/introduction.md b/doc/monitoring/performance/introduction.md index ab3f3ac1664..ae88baa0c14 100644 --- a/doc/monitoring/performance/introduction.md +++ b/doc/monitoring/performance/introduction.md @@ -1 +1 @@ -This document was moved to [administration/monitoring/performance/introduction](../administration/monitoring/performance/introduction.md). +This document was moved to [administration/monitoring/performance/introduction](../../administration/monitoring/performance/introduction.md). From f3cf51523067149ac39e61ebdb4fda372122d65f Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Fri, 21 Oct 2016 14:12:00 +0200 Subject: [PATCH 42/73] Fix typo in project settings that prevents users from enabling container registry. Fixes #23575. --- app/views/projects/edit.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 55b6580e640..30473d14b9b 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -101,7 +101,7 @@ Git Large File Storage = link_to icon('question-circle'), help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs') - - if Gitlab.config.lfs.enabled && current_user.admin? + - if Gitlab.config.registry.enabled .form-group .checkbox = f.label :container_registry_enabled do From 21666dbe119df7f2d86f0ac13c4addc814af4485 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Mon, 24 Oct 2016 14:05:33 +0200 Subject: [PATCH 43/73] Grapify the labels API --- lib/api/labels.rb | 91 +++++++++++++------------------- spec/requests/api/labels_spec.rb | 6 +-- 2 files changed, 41 insertions(+), 56 deletions(-) diff --git a/lib/api/labels.rb b/lib/api/labels.rb index 642e6345b9e..326e1e7ae00 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -3,37 +3,32 @@ module API class Labels < Grape::API before { authenticate! } + params do + requires :id, type: String, desc: 'The ID of a project' + end resource :projects do - # Get all labels of the project - # - # Parameters: - # id (required) - The ID of a project - # Example Request: - # GET /projects/:id/labels + desc 'Get all labels of the project' do + success Entities::Label + end get ':id/labels' do present available_labels, with: Entities::Label, current_user: current_user end - # Creates a new label - # - # Parameters: - # id (required) - The ID of a project - # name (required) - The name of the label to be created - # color (required) - Color of the label given in 6-digit hex - # notation with leading '#' sign (e.g. #FFAABB) - # description (optional) - The description of label to be created - # Example Request: - # POST /projects/:id/labels + desc 'Create a new label' do + success Entities::Label + end + params do + requires :name, type: String, desc: 'The name of the label to be created' + requires :color, type: String, desc: "The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)" + optional :description, type: String, desc: 'The description of label to be created' + end post ':id/labels' do authorize! :admin_label, user_project - required_attributes! [:name, :color] - - attrs = attributes_for_keys [:name, :color, :description] - label = user_project.find_label(attrs[:name]) + label = user_project.find_label(params[:name]) conflict!('Label already exists') if label - label = user_project.labels.create(attrs) + label = user_project.labels.create(declared(params, include_parent_namespaces: false).to_h) if label.valid? present label, with: Entities::Label, current_user: current_user @@ -42,54 +37,44 @@ module API end end - # Deletes an existing label - # - # Parameters: - # id (required) - The ID of a project - # name (required) - The name of the label to be deleted - # - # Example Request: - # DELETE /projects/:id/labels + desc 'Delete an existing label' do + success Entities::Label + end + params do + requires :name, type: String, desc: 'The name of the label to be deleted' + end delete ':id/labels' do authorize! :admin_label, user_project - required_attributes! [:name] label = user_project.find_label(params[:name]) not_found!('Label') unless label - label.destroy + present label.destroy, with: Entities::Label, current_user: current_user end - # Updates an existing label. At least one optional parameter is required. - # - # Parameters: - # id (required) - The ID of a project - # name (required) - The name of the label to be deleted - # new_name (optional) - The new name of the label - # color (optional) - Color of the label given in 6-digit hex - # notation with leading '#' sign (e.g. #FFAABB) - # description (optional) - The description of label to be created - # Example Request: - # PUT /projects/:id/labels + desc 'Update an existing label. At least one optional parameter is required.' do + success Entities::Label + end + params do + requires :name, type: String, desc: 'The name of the label to be updated' + optional :new_name, type: String, desc: 'The new name of the label' + optional :color, type: String, desc: "The new color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)" + optional :description, type: String, desc: 'The new description of label' + at_least_one_of :new_name, :color, :description + end put ':id/labels' do authorize! :admin_label, user_project - required_attributes! [:name] label = user_project.find_label(params[:name]) not_found!('Label not found') unless label - attrs = attributes_for_keys [:new_name, :color, :description] - - if attrs.empty? - render_api_error!('Required parameters "new_name" or "color" ' \ - 'missing', - 400) - end - + update_params = declared(params, + include_parent_namespaces: false, + include_missing: false).to_h # Rename new name to the actual label attribute name - attrs[:name] = attrs.delete(:new_name) if attrs.key?(:new_name) + update_params['name'] = update_params.delete('new_name') if update_params.key?('new_name') - if label.update(attrs) + if label.update(update_params) present label, with: Entities::Label, current_user: current_user else render_validation_error!(label) diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 867bc615b97..46641fcd846 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -159,14 +159,14 @@ describe API::API, api: true do it 'returns 400 if no label name given' do put api("/projects/#{project.id}/labels", user), new_name: 'label2' expect(response).to have_http_status(400) - expect(json_response['message']).to eq('400 (Bad request) "name" not given') + expect(json_response['error']).to eq('name is missing') end it 'returns 400 if no new parameters given' do put api("/projects/#{project.id}/labels", user), name: 'label1' expect(response).to have_http_status(400) - expect(json_response['message']).to eq('Required parameters '\ - '"new_name" or "color" missing') + expect(json_response['error']).to eq('new_name, color, description are missing, '\ + 'at least one parameter must be provided') end it 'returns 400 for invalid name' do From ef56ceb40be13500af2cf727735ab24b87a753d6 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Fri, 21 Oct 2016 15:18:41 -0500 Subject: [PATCH 44/73] Change overflow scroll to auto --- app/assets/stylesheets/framework/dropdowns.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index a839371a6f2..224bc58f7a7 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -402,7 +402,7 @@ .dropdown-content { max-height: 215px; - overflow-y: scroll; + overflow-y: auto; } .dropdown-footer { From 413c012db83695753a5dbf7bb8541fb3a7248aa9 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Sat, 22 Oct 2016 20:50:24 +0200 Subject: [PATCH 45/73] Only show register tab if signup enabled. --- CHANGELOG.md | 1 + app/views/devise/shared/_tabs_normal.html.haml | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae578852fde..281ea2e3799 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix 404 for group pages when GitLab setup uses relative url - Simpler arguments passed to named_route on toggle_award_url helper method - Better handle when no users were selected for adding to group or project. (Linus Thiel) + - Only show register tab if signup enabled. ## 8.13.0 (2016-10-22) - Removes extra line for empty issue description. (!7045) diff --git a/app/views/devise/shared/_tabs_normal.html.haml b/app/views/devise/shared/_tabs_normal.html.haml index 79b1d447a92..05246303fb6 100644 --- a/app/views/devise/shared/_tabs_normal.html.haml +++ b/app/views/devise/shared/_tabs_normal.html.haml @@ -1,5 +1,6 @@ %ul.nav-links.new-session-tabs.nav-tabs{ role: 'tablist'} %li.active{ role: 'presentation' } %a{ href: '#login-pane', data: { toggle: 'tab' }, role: 'tab'} Sign in - %li{ role: 'presentation'} - %a{ href: '#register-pane', data: { toggle: 'tab' }, role: 'tab'} Register + - if signin_enabled? && signup_enabled? + %li{ role: 'presentation'} + %a{ href: '#register-pane', data: { toggle: 'tab' }, role: 'tab'} Register From 5bc42660ef7f85ff7d7c56f93b5ec4cfaca0aff9 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Mon, 24 Oct 2016 13:26:48 +0200 Subject: [PATCH 46/73] Test login tab/pane rendering in varying configurations. --- spec/features/login_spec.rb | 63 +++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb index 996f39ea06d..e91667f096d 100644 --- a/spec/features/login_spec.rb +++ b/spec/features/login_spec.rb @@ -215,4 +215,67 @@ feature 'Login', feature: true do end end end + + describe 'UI tabs and panes' do + context 'when no defaults are changed' do + it 'should correctly render tabs and panes' do + ensure_tab_pane_correctness + end + end + + context 'when signup is disabled' do + before do + stub_application_setting(signup_enabled: false) + end + it 'should correctly render tabs and panes' do + ensure_tab_pane_correctness + end + end + + context 'when ldap is enabled' do + before do + visit new_user_session_path + allow(page).to receive(:form_based_providers).and_return([:ldapmain]) + allow(page).to receive(:ldap_enabled).and_return(true) + end + + it 'should correctly render tabs and panes' do + ensure_tab_pane_correctness(false) + end + end + + context 'when crowd is enabled' do + before do + visit new_user_session_path + allow(page).to receive(:form_based_providers).and_return([:crowd]) + allow(page).to receive(:crowd_enabled?).and_return(true) + end + + it 'tabs and panes should be configured correctly' do + ensure_tab_pane_correctness(false) + end + end + + def ensure_tab_pane_correctness(visit_path=true) + if visit_path + visit new_user_session_path + end + ensure_tab_pane_counts + ensure_one_active_tab + ensure_one_active_pane + end + + def ensure_tab_pane_counts + tabs_count = page.all('[role="tab"]').size + expect(page).to have_selector('[role="tabpanel"]', count: tabs_count) + end + + def ensure_one_active_tab + expect(page).to have_selector('.nav-tabs > li.active', count: 1) + end + + def ensure_one_active_pane + expect(page).to have_selector('.tab-pane.active', count: 1) + end + end end From ffb1482193e4c60e7c5915e28917b0e58963b017 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Mon, 24 Oct 2016 17:53:32 +0200 Subject: [PATCH 47/73] Use proper tense and spacing in login_specs. --- spec/features/login_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb index e91667f096d..a3e1ca923ef 100644 --- a/spec/features/login_spec.rb +++ b/spec/features/login_spec.rb @@ -218,7 +218,7 @@ feature 'Login', feature: true do describe 'UI tabs and panes' do context 'when no defaults are changed' do - it 'should correctly render tabs and panes' do + it 'correctly renders tabs and panes' do ensure_tab_pane_correctness end end @@ -227,7 +227,8 @@ feature 'Login', feature: true do before do stub_application_setting(signup_enabled: false) end - it 'should correctly render tabs and panes' do + + it 'correctly renders tabs and panes' do ensure_tab_pane_correctness end end @@ -239,7 +240,7 @@ feature 'Login', feature: true do allow(page).to receive(:ldap_enabled).and_return(true) end - it 'should correctly render tabs and panes' do + it 'correctly renders tabs and panes' do ensure_tab_pane_correctness(false) end end @@ -251,7 +252,7 @@ feature 'Login', feature: true do allow(page).to receive(:crowd_enabled?).and_return(true) end - it 'tabs and panes should be configured correctly' do + it 'correctly renders tabs and panes' do ensure_tab_pane_correctness(false) end end @@ -260,6 +261,7 @@ feature 'Login', feature: true do if visit_path visit new_user_session_path end + ensure_tab_pane_counts ensure_one_active_tab ensure_one_active_pane From 78de8816f587bd90725e7b46fcbd3860d3eb2889 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 25 Oct 2016 00:08:30 +0800 Subject: [PATCH 48/73] Also keep commits from source_project around, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6658#note_17190236 --- app/models/merge_request_diff.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index b8a10b7968e..dd65a9a8b86 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -299,8 +299,10 @@ class MergeRequestDiff < ActiveRecord::Base end def keep_around_commits - repository.keep_around(start_commit_sha) - repository.keep_around(head_commit_sha) - repository.keep_around(base_commit_sha) + [repository, merge_request.source_project.repository].each do |repo| + repo.keep_around(start_commit_sha) + repo.keep_around(head_commit_sha) + repo.keep_around(base_commit_sha) + end end end From b4a9247f7e59b454385890c2db46c86f076499ce Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Sat, 22 Oct 2016 23:14:52 +0100 Subject: [PATCH 49/73] Replaced deprecated use of document.body.scrollTop --- app/assets/javascripts/build.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/build.js b/app/assets/javascripts/build.js index 97462a5959c..f4c387a1a05 100644 --- a/app/assets/javascripts/build.js +++ b/app/assets/javascripts/build.js @@ -148,7 +148,7 @@ }; Build.prototype.translateSidebar = function(e) { - var newPosition = this.sidebarTranslationLimits.max - document.body.scrollTop; + var newPosition = this.sidebarTranslationLimits.max - (document.body.scrollTop || document.documentElement.scrollTop); if (newPosition < this.sidebarTranslationLimits.min) newPosition = this.sidebarTranslationLimits.min; this.$sidebar.css({ top: newPosition From ed545d9f754374c7a5fe98a81544f0d1cdc88cf0 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 25 Oct 2016 01:56:39 +0800 Subject: [PATCH 50/73] Make sure merge request was created before deleting source --- spec/models/merge_request_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 6e5137602aa..6f66a0ebe75 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1286,7 +1286,8 @@ describe MergeRequest, models: true do let(:project) { create(:project) } let(:user) { create(:user) } let(:fork_project) { create(:project, forked_from_project: project, namespace: user.namespace) } - let(:merge_request) do + + let!(:merge_request) do create(:closed_merge_request, source_project: fork_project, target_project: project) From 4a880b292c13231f20c167ffe733bce7391b0c23 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 24 Oct 2016 12:57:40 -0500 Subject: [PATCH 51/73] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f02cc1f102..0af4d0eefa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Trim leading and trailing whitespace on project_path (Linus Thiel) - Prevent award emoji via notes for issues/MRs authored by user (barthc) - Adds an optional path parameter to the Commits API to filter commits by path (Luis HGO) + - Fix extra space on Build sidebar on Firefox !7060 - Fix HipChat notifications rendering (airatshigapov, eisnerd) - Add hover to trash icon in notes !7008 (blackst0ne) - Simpler arguments passed to named_route on toggle_award_url helper method From 3ea3a6a152c88325ca2fd1161a27b4a9e2d5ce51 Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Fri, 14 Oct 2016 18:05:48 +0100 Subject: [PATCH 52/73] invoked the pipelines class when builds are dynamically loaded and dispatched for commit builds page --- CHANGELOG.md | 1 + app/assets/javascripts/dispatcher.js.es6 | 3 +++ app/assets/javascripts/merge_request_tabs.js | 1 + 3 files changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f02cc1f102..51b0adcde1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.13.1 (unreleased) - Fix error in generating labels - Fix reply-by-email not working due to queue name mismatch + - Fixed hidden pipeline graph on commit and MR page !6895 - Expire and build repository cache after project import - Fix 404 for group pages when GitLab setup uses relative url - Simpler arguments passed to named_route on toggle_award_url helper method diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index afc0d6f8c62..a1fe57562fa 100644 --- a/app/assets/javascripts/dispatcher.js.es6 +++ b/app/assets/javascripts/dispatcher.js.es6 @@ -117,6 +117,9 @@ new ZenMode(); shortcut_handler = new ShortcutsNavigation(); break; + case 'projects:commit:builds': + new gl.Pipelines(); + break; case 'projects:commits:show': case 'projects:activity': shortcut_handler = new ShortcutsNavigation(); diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 9f28738e06b..3dde979185b 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -282,6 +282,7 @@ document.querySelector("div#builds").innerHTML = data.html; gl.utils.localTimeAgo($('.js-timeago', 'div#builds')); _this.buildsLoaded = true; + if (!this.pipelines) this.pipelines = new gl.Pipelines(); return _this.scrollToElement("#builds"); }; })(this) From c7af4cf83027863a2d109ba82135e56092c71a09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 24 Oct 2016 20:29:00 +0200 Subject: [PATCH 53/73] Don't print out implementation detail step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/gitlab/ee_compat_check.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/ee_compat_check.rb b/lib/gitlab/ee_compat_check.rb index 5e8a0c14eba..6f8d2ca3220 100644 --- a/lib/gitlab/ee_compat_check.rb +++ b/lib/gitlab/ee_compat_check.rb @@ -116,7 +116,7 @@ module Gitlab end def delete_ee_branch_locally - step("Checking out origin/master", %w[git checkout master]) + command(%w[git checkout master]) step("Deleting the local #{ee_branch} branch", %W[git branch -D #{ee_branch}]) end From 402dcab4f1a9e2ea7310e9d6137abe1299f48bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 24 Oct 2016 20:47:50 +0200 Subject: [PATCH 54/73] Use File.write instead of File.open + File#write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/gitlab/ee_compat_check.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/ee_compat_check.rb b/lib/gitlab/ee_compat_check.rb index 6f8d2ca3220..fd9fd56510f 100644 --- a/lib/gitlab/ee_compat_check.rb +++ b/lib/gitlab/ee_compat_check.rb @@ -111,7 +111,7 @@ module Gitlab output, status = Gitlab::Popen.popen(%w[git format-patch FETCH_HEAD --stdout]) throw(:halt_check, :ko) unless status.zero? - File.open(filepath, 'w+') { |f| f.write(output) } + File.write(filepath, output) throw(:halt_check, :ko) unless File.exist?(filepath) end From d38d4a9e8e4b8aff1419f146c40bf7f4af8669ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 24 Oct 2016 21:19:46 +0200 Subject: [PATCH 55/73] Disable Rails/Output cop since it makes no sense here MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/gitlab/ee_compat_check.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/gitlab/ee_compat_check.rb b/lib/gitlab/ee_compat_check.rb index fd9fd56510f..b1a6d5fe0f6 100644 --- a/lib/gitlab/ee_compat_check.rb +++ b/lib/gitlab/ee_compat_check.rb @@ -1,3 +1,4 @@ +# rubocop: disable Rails/Output module Gitlab # Checks if a set of migrations requires downtime or not. class EeCompatCheck From 5480114477fcb0f6a7dcc49eb75582652cb007e5 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Mon, 24 Oct 2016 14:23:40 -0500 Subject: [PATCH 56/73] Enable trailingWhitespace in scss-lint --- .scss-lint.yml | 2 +- .../stylesheets/framework/animations.scss | 4 ++-- app/assets/stylesheets/framework/logo.scss | 2 +- .../stylesheets/pages/cycle_analytics.scss | 18 +++++++++--------- app/assets/stylesheets/pages/diff.scss | 4 ++-- app/assets/stylesheets/pages/environments.scss | 8 ++++---- app/assets/stylesheets/pages/events.scss | 2 +- app/assets/stylesheets/pages/login.scss | 2 +- .../stylesheets/pages/merge_conflicts.scss | 2 +- app/assets/stylesheets/pages/notes.scss | 2 +- app/assets/stylesheets/pages/status.scss | 2 +- app/assets/stylesheets/pages/tree.scss | 4 ++-- 12 files changed, 26 insertions(+), 26 deletions(-) diff --git a/.scss-lint.yml b/.scss-lint.yml index 5093702519b..89f9e74f03d 100644 --- a/.scss-lint.yml +++ b/.scss-lint.yml @@ -223,7 +223,7 @@ linters: # Reports lines containing trailing whitespace. TrailingWhitespace: - enabled: false + enabled: true # Don't write trailing zeros for numeric values with a decimal point. TrailingZero: diff --git a/app/assets/stylesheets/framework/animations.scss b/app/assets/stylesheets/framework/animations.scss index 1e9a45c19b8..98d3889cd44 100644 --- a/app/assets/stylesheets/framework/animations.scss +++ b/app/assets/stylesheets/framework/animations.scss @@ -1,10 +1,10 @@ // This file is based off animate.css 3.5.1, available here: // https://github.com/daneden/animate.css/blob/3.5.1/animate.css -// +// // animate.css - http://daneden.me/animate // Version - 3.5.1 // Licensed under the MIT license - http://opensource.org/licenses/MIT -// +// // Copyright (c) 2016 Daniel Eden .animated { diff --git a/app/assets/stylesheets/framework/logo.scss b/app/assets/stylesheets/framework/logo.scss index a90e45bb5f4..429cfbe7235 100644 --- a/app/assets/stylesheets/framework/logo.scss +++ b/app/assets/stylesheets/framework/logo.scss @@ -61,7 +61,7 @@ 10%, 80% { fill: $tanuki-red; } - + 20%, 90% { fill: lighten($tanuki-red, 25%); } diff --git a/app/assets/stylesheets/pages/cycle_analytics.scss b/app/assets/stylesheets/pages/cycle_analytics.scss index d732008de3d..572e1e7d558 100644 --- a/app/assets/stylesheets/pages/cycle_analytics.scss +++ b/app/assets/stylesheets/pages/cycle_analytics.scss @@ -9,15 +9,15 @@ padding: 24px 0; border-bottom: none; position: relative; - + @media (max-width: $screen-sm-min) { padding: 6px 0 24px; - } + } } .column { text-align: center; - + @media (max-width: $screen-sm-min) { padding: 15px 0; } @@ -36,7 +36,7 @@ &:last-child { text-align: right; - + @media (max-width: $screen-sm-min) { text-align: center; } @@ -51,7 +51,7 @@ .bordered-box { border: 1px solid $border-color; border-radius: $border-radius-default; - + } .content-list { @@ -73,10 +73,10 @@ font-weight: 600; color: $gl-title-color; } - + &.text { color: $layout-link-gray; - + &.value-col { color: $gl-title-color; } @@ -108,13 +108,13 @@ .svg-container { text-align: center; - + svg { width: 136px; height: 136px; } } - + .inner-content { @media (max-width: $screen-sm-min) { padding: 0 28px; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index fe6421f8b3f..f8e3ca29a2b 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -222,12 +222,12 @@ top: 13px; right: 7px; } - + .frame { top: 0; right: 0; position: absolute; - + &.deleted { margin: 0; display: block; diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index 12ee0a5dc3d..fc49ff780fc 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -37,10 +37,10 @@ .branch-name { color: $gl-dark-link-color; } - + .stop-env-link { color: $table-text-gray; - + .stop-env-icon { font-size: 14px; } @@ -48,11 +48,11 @@ .deployment { .build-column { - + .build-link { color: $gl-dark-link-color; } - + .avatar { float: none; } diff --git a/app/assets/stylesheets/pages/events.scss b/app/assets/stylesheets/pages/events.scss index 5d9a76dac05..3004959ff7b 100644 --- a/app/assets/stylesheets/pages/events.scss +++ b/app/assets/stylesheets/pages/events.scss @@ -142,7 +142,7 @@ .event-last-push { overflow: auto; width: 100%; - + .event-last-push-text { @include str-truncated(100%); padding: 4px 0; diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss index 8dac6ab999e..2be9453aaee 100644 --- a/app/assets/stylesheets/pages/login.scss +++ b/app/assets/stylesheets/pages/login.scss @@ -286,7 +286,7 @@ .new_user { position: relative; padding-bottom: 35px; - + @media (min-width: $screen-sm-min) and (max-width: $screen-sm-max) { .forgot-password { float: none !important; diff --git a/app/assets/stylesheets/pages/merge_conflicts.scss b/app/assets/stylesheets/pages/merge_conflicts.scss index eed2b0ab7cc..2e917361b25 100644 --- a/app/assets/stylesheets/pages/merge_conflicts.scss +++ b/app/assets/stylesheets/pages/merge_conflicts.scss @@ -254,7 +254,7 @@ $colors: ( border-top: solid 2px $border-green-extra-light; } } - + .editor { pre { height: 350px; diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index fffcdc812a7..faa0fc82ca8 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -458,7 +458,7 @@ ul.notes { .discussion-next-btn { svg { margin: 0; - + path { fill: $gray-darkest; } diff --git a/app/assets/stylesheets/pages/status.scss b/app/assets/stylesheets/pages/status.scss index f1d53c7b8bc..01426e28e92 100644 --- a/app/assets/stylesheets/pages/status.scss +++ b/app/assets/stylesheets/pages/status.scss @@ -74,7 +74,7 @@ .ci-status-icon-success_with_warning { color: $gl-warning; } - + .ci-status-icon-running { color: $blue-normal; } diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index 6ea7a2b5498..84dcd6835d5 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -29,11 +29,11 @@ .last-commit { @include str-truncated(506px); - + @media (min-width: $screen-sm-max) and (max-width: $screen-md-max) { @include str-truncated(450px); } - + } .commit-history-link-spacer { From 2004b30ed60b2de37edcc0e174b0cfdc20e33db4 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Mon, 24 Oct 2016 14:29:44 -0500 Subject: [PATCH 57/73] Enable SpaceAroundOperator in scss-lint --- .scss-lint.yml | 2 +- app/assets/stylesheets/framework/modal.scss | 2 +- app/assets/stylesheets/pages/note_form.scss | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.scss-lint.yml b/.scss-lint.yml index 5093702519b..9eb6fdd767f 100644 --- a/.scss-lint.yml +++ b/.scss-lint.yml @@ -201,7 +201,7 @@ linters: # Operators should be formatted with a single space on both sides of an # infix operator. SpaceAroundOperator: - enabled: false + enabled: true # Opening braces should be preceded by a single space. SpaceBeforeBrace: diff --git a/app/assets/stylesheets/framework/modal.scss b/app/assets/stylesheets/framework/modal.scss index 8374f30d0b2..8cd49280e1c 100644 --- a/app/assets/stylesheets/framework/modal.scss +++ b/app/assets/stylesheets/framework/modal.scss @@ -3,7 +3,7 @@ padding: 15px; .form-actions { - margin: -$gl-padding+1; + margin: -$gl-padding + 1; margin-top: 15px; } diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 17f28959414..25c1bbdc1c9 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -8,7 +8,7 @@ .diff-file .diff-content { tr.line_holder:hover > td .line_note_link { opacity: 1.0; - filter: alpha(opacity=100); + filter: alpha(opacity = 100); } } From f035cd486a5d929881fc7b36c9bd716d6b5c0ae4 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Mon, 24 Oct 2016 14:49:06 -0500 Subject: [PATCH 58/73] Enable SpaceAfterVariableColon in scss-lint --- .scss-lint.yml | 2 +- .../framework/tw_bootstrap_variables.scss | 122 +++++++++--------- .../stylesheets/framework/variables.scss | 44 +++---- app/assets/stylesheets/mailers/devise.scss | 10 +- 4 files changed, 89 insertions(+), 89 deletions(-) diff --git a/.scss-lint.yml b/.scss-lint.yml index 5093702519b..1958803cfde 100644 --- a/.scss-lint.yml +++ b/.scss-lint.yml @@ -191,7 +191,7 @@ linters: # Variables should be formatted with a single space separating the colon # from the variable's value. SpaceAfterVariableColon: - enabled: false + enabled: true # Variables should be formatted with no space between the name and the # colon. diff --git a/app/assets/stylesheets/framework/tw_bootstrap_variables.scss b/app/assets/stylesheets/framework/tw_bootstrap_variables.scss index 915aa631ef8..44fe37d3a4a 100644 --- a/app/assets/stylesheets/framework/tw_bootstrap_variables.scss +++ b/app/assets/stylesheets/framework/tw_bootstrap_variables.scss @@ -16,21 +16,21 @@ // $gray-light: lighten($gray-base, 46.7%) // #777 // $gray-lighter: lighten($gray-base, 93.5%) // #eee -$brand-primary: $gl-primary; -$brand-success: $gl-success; -$brand-info: $gl-info; -$brand-warning: $gl-warning; -$brand-danger: $gl-danger; +$brand-primary: $gl-primary; +$brand-success: $gl-success; +$brand-info: $gl-info; +$brand-warning: $gl-warning; +$brand-danger: $gl-danger; -$border-radius-base: 3px !default; -$border-radius-large: 3px !default; -$border-radius-small: 3px !default; +$border-radius-base: 3px !default; +$border-radius-large: 3px !default; +$border-radius-small: 3px !default; //== Scaffolding // -$text-color: $gl-text-color; -$link-color: $gl-link-color; +$text-color: $gl-text-color; +$link-color: $gl-link-color; //== Typography @@ -38,112 +38,112 @@ $link-color: $gl-link-color; //## Font, line-height, and color for body text, headings, and more. $font-family-sans-serif: $regular_font; -$font-family-monospace: $monospace_font; -$font-size-base: $gl-font-size; +$font-family-monospace: $monospace_font; +$font-size-base: $gl-font-size; //== Components // //## Define common padding and border radius sizes and more. Values based on 14px text and 1.428 line-height (~20px to start). -$padding-base-vertical: $gl-vert-padding; -$padding-base-horizontal: $gl-padding; -$component-active-color: #fff; -$component-active-bg: $brand-info; +$padding-base-vertical: $gl-vert-padding; +$padding-base-horizontal: $gl-padding; +$component-active-color: #fff; +$component-active-bg: $brand-info; //== Forms // //## -$input-color: $text-color; -$input-border: $border-color; -$input-border-focus: $focus-border-color; -$legend-color: $text-color; +$input-color: $text-color; +$input-border: $border-color; +$input-border-focus: $focus-border-color; +$legend-color: $text-color; //== Pagination // //## -$pagination-color: $gl-gray; -$pagination-bg: #fff; -$pagination-border: $border-color; +$pagination-color: $gl-gray; +$pagination-bg: #fff; +$pagination-border: $border-color; -$pagination-hover-color: $gl-gray; -$pagination-hover-bg: $row-hover; -$pagination-hover-border: $border-color; +$pagination-hover-color: $gl-gray; +$pagination-hover-bg: $row-hover; +$pagination-hover-border: $border-color; -$pagination-active-color: $blue-dark; -$pagination-active-bg: #fff; -$pagination-active-border: $border-color; +$pagination-active-color: $blue-dark; +$pagination-active-bg: #fff; +$pagination-active-border: $border-color; -$pagination-disabled-color: #cdcdcd; -$pagination-disabled-bg: $background-color; -$pagination-disabled-border: $border-color; +$pagination-disabled-color: #cdcdcd; +$pagination-disabled-bg: $background-color; +$pagination-disabled-border: $border-color; //== Form states and alerts // //## Define colors for form feedback states and, by default, alerts. -$state-success-text: #fff; -$state-success-bg: $brand-success; -$state-success-border: $brand-success; +$state-success-text: #fff; +$state-success-bg: $brand-success; +$state-success-border: $brand-success; -$state-info-text: #fff; -$state-info-bg: $brand-info; -$state-info-border: $brand-info; +$state-info-text: #fff; +$state-info-bg: $brand-info; +$state-info-border: $brand-info; -$state-warning-text: #fff; -$state-warning-bg: $brand-warning; -$state-warning-border: $brand-warning; +$state-warning-text: #fff; +$state-warning-bg: $brand-warning; +$state-warning-border: $brand-warning; -$state-danger-text: #fff; -$state-danger-bg: $brand-danger; -$state-danger-border: $brand-danger; +$state-danger-text: #fff; +$state-danger-bg: $brand-danger; +$state-danger-border: $brand-danger; //== Alerts // //## Define alert colors, border radius, and padding. -$alert-border-radius: 0; +$alert-border-radius: 0; //== Panels // //## -$panel-border-radius: 2px; -$panel-default-text: $text-color; -$panel-default-border: $border-color; +$panel-border-radius: 2px; +$panel-default-text: $text-color; +$panel-default-border: $border-color; $panel-default-heading-bg: $background-color; -$panel-footer-bg: $background-color; -$panel-inner-border: $border-color; +$panel-footer-bg: $background-color; +$panel-inner-border: $border-color; //== Wells // //## -$well-bg: $gray-light; -$well-border: #eee; +$well-bg: $gray-light; +$well-border: #eee; //== Code // //## -$code-color: #c7254e; -$code-bg: #f9f2f4; +$code-color: #c7254e; +$code-bg: #f9f2f4; -$kbd-color: #fff; -$kbd-bg: #333; +$kbd-color: #fff; +$kbd-bg: #333; //== Buttons // //## -$btn-default-color: $gl-text-color; -$btn-default-bg: #fff; -$btn-default-border: #e7e9ed; +$btn-default-color: $gl-text-color; +$btn-default-bg: #fff; +$btn-default-border: #e7e9ed; //== Nav // @@ -153,8 +153,8 @@ $nav-link-padding: 13px $gl-padding; //== Code // //## -$pre-bg: $background-color !default; -$pre-color: $gl-gray !default; +$pre-bg: $background-color !default; +$pre-color: $gl-gray !default; $pre-border-color: $border-color; $table-bg-accent: $background-color; diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index eafe84570a8..b271f8cf332 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -84,39 +84,39 @@ $warning-message-border: #f0e2bb; /* * UI elements */ -$border-color: #e5e5e5; -$focus-border-color: #3aabf0; -$table-border-color: #f0f0f0; -$background-color: $gray-light; +$border-color: #e5e5e5; +$focus-border-color: #3aabf0; +$table-border-color: #f0f0f0; +$background-color: $gray-light; $dark-background-color: #f5f5f5; -$table-text-gray: #8f8f8f; +$table-text-gray: #8f8f8f; /* * Text */ -$gl-font-size: 15px; -$gl-title-color: #333; -$gl-text-color: #5c5c5c; -$gl-text-color-light: #8c8c8c; -$gl-text-green: #4a2; -$gl-text-red: #d12f19; -$gl-text-orange: #d90; -$gl-link-color: #3084bb; -$gl-dark-link-color: #333; +$gl-font-size: 15px; +$gl-title-color: #333; +$gl-text-color: #5c5c5c; +$gl-text-color-light: #8c8c8c; +$gl-text-green: #4a2; +$gl-text-red: #d12f19; +$gl-text-orange: #d90; +$gl-link-color: #3084bb; +$gl-dark-link-color: #333; $gl-placeholder-color: #8f8f8f; -$gl-icon-color: $gl-placeholder-color; -$gl-grayish-blue: #7f8fa4; -$gl-gray: $gl-text-color; -$gl-gray-dark: #313236; -$gl-gray-light: $gl-placeholder-color; -$gl-header-color: #4c4e54; +$gl-icon-color: $gl-placeholder-color; +$gl-grayish-blue: #7f8fa4; +$gl-gray: $gl-text-color; +$gl-gray-dark: #313236; +$gl-gray-light: $gl-placeholder-color; +$gl-header-color: #4c4e54; /* * Lists */ -$list-font-size: $gl-font-size; +$list-font-size: $gl-font-size; $list-title-color: $gl-title-color; -$list-text-color: $gl-text-color; +$list-text-color: $gl-text-color; $list-text-height: 42px; /* diff --git a/app/assets/stylesheets/mailers/devise.scss b/app/assets/stylesheets/mailers/devise.scss index 9495c5b3f37..b2bce482fde 100644 --- a/app/assets/stylesheets/mailers/devise.scss +++ b/app/assets/stylesheets/mailers/devise.scss @@ -5,13 +5,13 @@ // Styles defined here are embedded directly into the resulting email HTML via // the `premailer` gem. -$body-background-color: #363636; +$body-background-color: #363636; $message-background-color: #fafafa; -$header-color: #6b4fbb; -$body-color: #444; -$cta-color: #e14329; -$footer-link-color: #7e7e7e; +$header-color: #6b4fbb; +$body-color: #444; +$cta-color: #e14329; +$footer-link-color: #7e7e7e; $font-family: Helvetica, Arial, sans-serif; From a31a719a4fa29373fd39eab7b84ea4955121e9fd Mon Sep 17 00:00:00 2001 From: winniehell Date: Wed, 31 Aug 2016 21:25:34 +0200 Subject: [PATCH 59/73] Add failing test for #21420 --- .../filter/relative_link_filter_spec.rb | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb index 6b58f3e43ee..2bfa51deb20 100644 --- a/spec/lib/banzai/filter/relative_link_filter_spec.rb +++ b/spec/lib/banzai/filter/relative_link_filter_spec.rb @@ -50,14 +50,6 @@ describe Banzai::Filter::RelativeLinkFilter, lib: true do end end - shared_examples :relative_to_requested do - it 'rebuilds URL relative to the requested path' do - doc = filter(link('users.md')) - expect(doc.at_css('a')['href']). - to eq "/#{project_path}/blob/#{ref}/doc/api/users.md" - end - end - context 'with a project_wiki' do let(:project_wiki) { double('ProjectWiki') } include_examples :preserve_unchanged @@ -188,12 +180,38 @@ describe Banzai::Filter::RelativeLinkFilter, lib: true do context 'when requested path is a file in the repo' do let(:requested_path) { 'doc/api/README.md' } - include_examples :relative_to_requested + it 'rebuilds URL relative to the containing directory' do + doc = filter(link('users.md')) + expect(doc.at_css('a')['href']).to eq "/#{project_path}/blob/#{Addressable::URI.escape(ref)}/doc/api/users.md" + end end context 'when requested path is a directory in the repo' do - let(:requested_path) { 'doc/api' } - include_examples :relative_to_requested + let(:requested_path) { 'doc/api/' } + it 'rebuilds URL relative to the directory' do + doc = filter(link('users.md')) + expect(doc.at_css('a')['href']).to eq "/#{project_path}/blob/#{Addressable::URI.escape(ref)}/doc/api/users.md" + end + end + + context 'when ref name contains percent sign' do + let(:ref) { '100%branch' } + let(:commit) { project.commit('1b12f15a11fc6e62177bef08f47bc7b5ce50b141') } + let(:requested_path) { 'foo/bar/' } + it 'correctly escapes the ref' do + doc = filter(link('.gitkeep')) + expect(doc.at_css('a')['href']).to eq "/#{project_path}/blob/#{Addressable::URI.escape(ref)}/foo/bar/.gitkeep" + end + end + + context 'when requested path is a directory with space in the repo' do + let(:ref) { 'master' } + let(:commit) { project.commit('38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e') } + let(:requested_path) { 'with space/' } + it 'does not escape the space twice' do + doc = filter(link('README.md')) + expect(doc.at_css('a')['href']).to eq "/#{project_path}/blob/#{Addressable::URI.escape(ref)}/with%20space/README.md" + end end end From ac2eb1cd012914b730911422e31410a78fe242b3 Mon Sep 17 00:00:00 2001 From: winniehell Date: Fri, 26 Aug 2016 10:01:32 +0200 Subject: [PATCH 60/73] Escape ref and path for relative links (!6050) --- CHANGELOG.md | 1 + lib/banzai/filter/relative_link_filter.rb | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86878e5af6c..8522f2adaf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix extra space on Build sidebar on Firefox !7060 - Fix HipChat notifications rendering (airatshigapov, eisnerd) - Add hover to trash icon in notes !7008 (blackst0ne) + - Escape ref and path for relative links !6050 (winniehell) - Simpler arguments passed to named_route on toggle_award_url helper method - Fix: Backup restore doesn't clear cache - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method diff --git a/lib/banzai/filter/relative_link_filter.rb b/lib/banzai/filter/relative_link_filter.rb index 4fa8d05481f..f09d78be0ce 100644 --- a/lib/banzai/filter/relative_link_filter.rb +++ b/lib/banzai/filter/relative_link_filter.rb @@ -52,8 +52,8 @@ module Banzai relative_url_root, context[:project].path_with_namespace, uri_type(file_path), - ref, - file_path + Addressable::URI.escape(ref), + Addressable::URI.escape(file_path) ].compact.join('/').squeeze('/').chomp('/') uri From 791c9d0dd3464eb471262b73a62632ed39d47eb2 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Mon, 24 Oct 2016 15:22:06 -0500 Subject: [PATCH 61/73] Enable SingleLinePerSelector in scss-lint --- .scss-lint.yml | 2 +- .../stylesheets/framework/animations.scss | 3 +- app/assets/stylesheets/framework/blocks.scss | 3 +- app/assets/stylesheets/framework/buttons.scss | 3 +- app/assets/stylesheets/framework/common.scss | 6 ++-- .../stylesheets/framework/dropdowns.scss | 6 ++-- app/assets/stylesheets/framework/flash.scss | 6 ++-- .../stylesheets/framework/gitlab-theme.scss | 4 ++- app/assets/stylesheets/framework/header.scss | 13 ++++++--- app/assets/stylesheets/framework/lists.scss | 6 ++-- app/assets/stylesheets/framework/mobile.scss | 12 +++++--- app/assets/stylesheets/framework/nav.scss | 22 +++++++++++---- app/assets/stylesheets/framework/selects.scss | 6 ++-- app/assets/stylesheets/framework/tables.scss | 3 +- .../stylesheets/framework/tw_bootstrap.scss | 3 +- .../stylesheets/framework/typography.scss | 28 +++++++++++++++---- app/assets/stylesheets/highlight/dark.scss | 19 +++++++++---- app/assets/stylesheets/highlight/monokai.scss | 19 +++++++++---- .../stylesheets/highlight/solarized_dark.scss | 19 +++++++++---- .../highlight/solarized_light.scss | 19 +++++++++---- app/assets/stylesheets/highlight/white.scss | 13 ++++++--- app/assets/stylesheets/pages/admin.scss | 3 +- app/assets/stylesheets/pages/ci_projects.scss | 3 +- app/assets/stylesheets/pages/commit.scss | 6 ++-- app/assets/stylesheets/pages/commits.scss | 6 ++-- .../stylesheets/pages/confirmation.scss | 10 +++++-- app/assets/stylesheets/pages/detail_page.scss | 3 +- app/assets/stylesheets/pages/diff.scss | 9 ++++-- app/assets/stylesheets/pages/editor.scss | 4 ++- app/assets/stylesheets/pages/errors.scss | 4 ++- app/assets/stylesheets/pages/issues.scss | 3 +- app/assets/stylesheets/pages/login.scss | 9 ++++-- .../stylesheets/pages/merge_conflicts.scss | 3 +- app/assets/stylesheets/pages/milestone.scss | 3 +- app/assets/stylesheets/pages/note_form.scss | 3 +- app/assets/stylesheets/pages/notes.scss | 3 +- app/assets/stylesheets/pages/pipelines.scss | 12 +++++--- app/assets/stylesheets/pages/profile.scss | 3 +- app/assets/stylesheets/pages/projects.scss | 12 +++++--- app/assets/stylesheets/pages/search.scss | 6 ++-- app/assets/stylesheets/pages/tree.scss | 6 ++-- app/assets/stylesheets/print.scss | 25 ++++++++++++++--- 42 files changed, 247 insertions(+), 104 deletions(-) diff --git a/.scss-lint.yml b/.scss-lint.yml index 5093702519b..f8fc1c077b8 100644 --- a/.scss-lint.yml +++ b/.scss-lint.yml @@ -172,7 +172,7 @@ linters: # Split selectors onto separate lines after each comma, and have each # individual selector occupy a single line. SingleLinePerSelector: - enabled: false + enabled: true # Commas in lists should be followed by a space. SpaceAfterComma: diff --git a/app/assets/stylesheets/framework/animations.scss b/app/assets/stylesheets/framework/animations.scss index 1e9a45c19b8..0224cc2df21 100644 --- a/app/assets/stylesheets/framework/animations.scss +++ b/app/assets/stylesheets/framework/animations.scss @@ -37,7 +37,8 @@ } @include keyframes(pulse) { - from, to { + from, + to { @include webkit-prefix(transform, scale3d(1, 1, 1)); } diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss index df2e2ea8d2c..7e168092522 100644 --- a/app/assets/stylesheets/framework/blocks.scss +++ b/app/assets/stylesheets/framework/blocks.scss @@ -128,7 +128,8 @@ position: relative; .avatar-holder { - .avatar, .identicon { + .avatar, + .identicon { margin: 0 auto; float: none; } diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index e6656c2d69a..c0e9c8bf829 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -213,7 +213,8 @@ top: 2px; } - svg, .fa { + svg, + .fa { &:not(:last-child) { margin-right: 3px; } diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 800e2dba018..ad5ac589d0f 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -143,7 +143,8 @@ li.note { } } -.wiki_content code, .readme code { +.wiki_content code, +.readme code { background-color: inherit; } @@ -350,7 +351,8 @@ table { margin-right: 10px; } -.alert, .progress { +.alert, +.progress { margin-bottom: $gl-padding; } diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index a839371a6f2..a2d0b1353da 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -275,7 +275,8 @@ a { padding-left: 25px; - &.is-indeterminate, &.is-active { + &.is-indeterminate, + &.is-active { &::before { position: absolute; left: 5px; @@ -373,7 +374,8 @@ } } -.dropdown-input-field, .default-dropdown-input { +.dropdown-input-field, +.default-dropdown-input { width: 100%; min-height: 30px; padding: 0 7px; diff --git a/app/assets/stylesheets/framework/flash.scss b/app/assets/stylesheets/framework/flash.scss index a55dcf4a699..a9006de6d3e 100644 --- a/app/assets/stylesheets/framework/flash.scss +++ b/app/assets/stylesheets/framework/flash.scss @@ -18,7 +18,8 @@ margin: 0; } - .flash-notice, .flash-alert { + .flash-notice, + .flash-alert { border-radius: $border-radius-default; .container-fluid, @@ -30,7 +31,8 @@ &.flash-container-page { margin-bottom: 0; - .flash-notice, .flash-alert { + .flash-notice, + .flash-alert { border-radius: 0; } } diff --git a/app/assets/stylesheets/framework/gitlab-theme.scss b/app/assets/stylesheets/framework/gitlab-theme.scss index fe834f4e2f6..3f877d86a26 100644 --- a/app/assets/stylesheets/framework/gitlab-theme.scss +++ b/app/assets/stylesheets/framework/gitlab-theme.scss @@ -25,7 +25,9 @@ a { color: $color-light; - &:hover, &:focus, &:active { + &:hover, + &:focus, + &:active { background: $color-dark; } diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index 3a4fdd0da22..142076f65b2 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -15,7 +15,8 @@ header { margin: 8px 0; text-align: center; - .tanuki-logo, img { + .tanuki-logo, + img { height: 36px; } } @@ -54,7 +55,9 @@ header { line-height: 28px; text-align: center; - &:hover, &:focus, &:active { + &:hover, + &:focus, + &:active { background-color: $background-color; } @@ -125,7 +128,8 @@ header { left: -50%; } - svg, img { + svg, + img { height: 36px; } @@ -222,7 +226,8 @@ header { margin: 0; float: none !important; - .visible-xs, .visable-sm { + .visible-xs, + .visable-sm { display: table-cell !important; } } diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss index 4b2627c1b87..48e34a0066e 100644 --- a/app/assets/stylesheets/framework/lists.scss +++ b/app/assets/stylesheets/framework/lists.scss @@ -76,14 +76,16 @@ /** light list with border-bottom between li **/ -ul.bordered-list, ul.unstyled-list { +ul.bordered-list, +ul.unstyled-list { @include basic-list; &.top-list { li:first-child { padding-top: 0; - h4, h5 { + h4, + h5 { margin-top: 0; } } diff --git a/app/assets/stylesheets/framework/mobile.scss b/app/assets/stylesheets/framework/mobile.scss index 9fe390eb09d..c1ed43bc20f 100644 --- a/app/assets/stylesheets/framework/mobile.scss +++ b/app/assets/stylesheets/framework/mobile.scss @@ -79,7 +79,8 @@ padding-left: 15px !important; } - .nav-links, .nav-links { + .nav-links, + .nav-links { li a { font-size: 14px; padding: 19px 10px; @@ -99,18 +100,21 @@ @media (max-width: $screen-sm-max) { .issues-filters { - .milestone-filter, .labels-filter { + .milestone-filter, + .labels-filter { display: none; } } .page-title { - .note-created-ago, .new-issue-link { + .note-created-ago, + .new-issue-link { display: none; } } - .issue_edited_ago, .note_edited_ago { + .issue_edited_ago, + .note_edited_ago { display: none; } diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index 899db045b74..fcaf5e18633 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -54,7 +54,9 @@ color: #959494; border-bottom: 2px solid transparent; - &:hover, &:active, &:focus { + &:hover, + &:active, + &:focus { text-decoration: none; outline: none; } @@ -211,7 +213,11 @@ padding-bottom: 0; width: 100%; - .btn, form, .dropdown, .dropdown-menu-toggle, .form-control { + .btn, + form, + .dropdown, + .dropdown-menu-toggle, + .form-control { margin: 0 0 10px; display: block; width: 100%; @@ -245,7 +251,8 @@ } &.adjust { - .nav-text, .nav-controls { + .nav-text, + .nav-controls { width: auto; } } @@ -309,13 +316,15 @@ padding-top: 10px; } - a, i { + a, + i { color: $layout-link-gray; } &.active { - a, i { + a, + i { color: $black; } @@ -328,7 +337,8 @@ } &:hover { - a, i { + a, + i { color: $black; } } diff --git a/app/assets/stylesheets/framework/selects.scss b/app/assets/stylesheets/framework/selects.scss index e0708c65695..ecdf0be1a05 100644 --- a/app/assets/stylesheets/framework/selects.scss +++ b/app/assets/stylesheets/framework/selects.scss @@ -3,7 +3,8 @@ width: 100% !important; } -.select2-container, .select2-container.select2-drop-above { +.select2-container, +.select2-container.select2-drop-above { .select2-choice { background: #fff; border-color: $input-border; @@ -71,7 +72,8 @@ } .select2-container-active { - .select2-choice, .select2-choices { + .select2-choice, + .select2-choices { box-shadow: none; } } diff --git a/app/assets/stylesheets/framework/tables.scss b/app/assets/stylesheets/framework/tables.scss index b42075c98d0..9a90d3794fd 100644 --- a/app/assets/stylesheets/framework/tables.scss +++ b/app/assets/stylesheets/framework/tables.scss @@ -23,7 +23,8 @@ table { } tr { - td, th { + td, + th { padding: 10px $gl-padding; line-height: 20px; vertical-align: middle; diff --git a/app/assets/stylesheets/framework/tw_bootstrap.scss b/app/assets/stylesheets/framework/tw_bootstrap.scss index f4106641269..59f4594bb83 100644 --- a/app/assets/stylesheets/framework/tw_bootstrap.scss +++ b/app/assets/stylesheets/framework/tw_bootstrap.scss @@ -126,7 +126,8 @@ box-shadow: none; .panel-body { - form, pre { + form, + pre { margin: 0; } diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 55de9053be5..266a8024809 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -131,12 +131,14 @@ font-weight: inherit; } - ul, ol { + ul, + ol { padding: 0; margin: 3px 0 3px 28px !important; } - ul:dir(rtl), ol:dir(rtl) { + ul:dir(rtl), + ol:dir(rtl) { margin: 3px 28px 3px 0 !important; } @@ -144,7 +146,8 @@ line-height: 1.6em; } - a[href*="/uploads/"], a[href*="storage.googleapis.com/google-code-attachments/"] { + a[href*="/uploads/"], + a[href*="storage.googleapis.com/google-code-attachments/"] { &:before { margin-right: 4px; @@ -167,7 +170,12 @@ } /* Link to current header. */ - h1, h2, h3, h4, h5, h6 { + h1, + h2, + h3, + h4, + h5, + h6 { position: relative; a.anchor { @@ -215,7 +223,12 @@ body { margin: 12px 7px; } -h1, h2, h3, h4, h5, h6 { +h1, +h2, +h3, +h4, +h5, +h6 { color: $gl-title-color; font-weight: 600; } @@ -273,7 +286,10 @@ a > code { text-decoration: line-through; } -h1, h2, h3, h4 { +h1, +h2, +h3, +h4 { small { color: $gl-gray; } diff --git a/app/assets/stylesheets/highlight/dark.scss b/app/assets/stylesheets/highlight/dark.scss index a3acee299e3..d22d9b01495 100644 --- a/app/assets/stylesheets/highlight/dark.scss +++ b/app/assets/stylesheets/highlight/dark.scss @@ -1,20 +1,25 @@ /* https://github.com/MozMorris/tomorrow-pygments */ .code.dark { // Line numbers - .line-numbers, .diff-line-num { + .line-numbers, + .diff-line-num { background-color: #1d1f21; } - .diff-line-num, .diff-line-num a { + .diff-line-num, + .diff-line-num a { color: rgba(255, 255, 255, 0.3); } // Code itself - pre.code, .diff-line-num { + pre.code, + .diff-line-num { border-color: #666; } - &, pre.code, .line_holder .line_content { + &, + pre.code, + .line_holder .line_content { background-color: #1d1f21; color: #c5c8c6; } @@ -31,11 +36,13 @@ border-color: darken(#557, 15%); } - .diff-line-num.new, .line_content.new { + .diff-line-num.new, + .line_content.new { @include diff_background(rgba(51, 255, 51, 0.1), rgba(51, 255, 51, 0.2), #808080); } - .diff-line-num.old, .line_content.old { + .diff-line-num.old, + .line_content.old { @include diff_background(rgba(255, 51, 51, 0.2), rgba(255, 51, 51, 0.25), #808080); } diff --git a/app/assets/stylesheets/highlight/monokai.scss b/app/assets/stylesheets/highlight/monokai.scss index e9228c94db9..db8da8aab10 100644 --- a/app/assets/stylesheets/highlight/monokai.scss +++ b/app/assets/stylesheets/highlight/monokai.scss @@ -1,20 +1,25 @@ /* https://github.com/richleland/pygments-css/blob/master/monokai.css */ .code.monokai { // Line numbers - .line-numbers, .diff-line-num { + .line-numbers, + .diff-line-num { background-color: #272822; } - .diff-line-num, .diff-line-num a { + .diff-line-num, + .diff-line-num a { color: rgba(255, 255, 255, 0.3); } // Code itself - pre.code, .diff-line-num { + pre.code, + .diff-line-num { border-color: #555; } - &, pre.code, .line_holder .line_content { + &, + pre.code, + .line_holder .line_content { background-color: #272822; color: #f8f8f2; } @@ -31,11 +36,13 @@ border-color: darken(#49483e, 15%); } - .diff-line-num.new, .line_content.new { + .diff-line-num.new, + .line_content.new { @include diff_background(rgba(166, 226, 46, 0.1), rgba(166, 226, 46, 0.15), #808080); } - .diff-line-num.old, .line_content.old { + .diff-line-num.old, + .line_content.old { @include diff_background(rgba(254, 147, 140, 0.15), rgba(254, 147, 140, 0.2), #808080); } diff --git a/app/assets/stylesheets/highlight/solarized_dark.scss b/app/assets/stylesheets/highlight/solarized_dark.scss index c3c7773b9e2..a87333146de 100644 --- a/app/assets/stylesheets/highlight/solarized_dark.scss +++ b/app/assets/stylesheets/highlight/solarized_dark.scss @@ -1,20 +1,25 @@ /* https://gist.github.com/qguv/7936275 */ .code.solarized-dark { // Line numbers - .line-numbers, .diff-line-num { + .line-numbers, + .diff-line-num { background-color: #002b36; } - .diff-line-num, .diff-line-num a { + .diff-line-num, + .diff-line-num a { color: rgba(255, 255, 255, 0.3); } // Code itself - pre.code, .diff-line-num { + pre.code, + .diff-line-num { border-color: #113b46; } - &, pre.code, .line_holder .line_content { + &, + pre.code, + .line_holder .line_content { background-color: #002b36; color: #93a1a1; } @@ -31,11 +36,13 @@ border-color: darken(#174652, 15%); } - .diff-line-num.new, .line_content.new { + .diff-line-num.new, + .line_content.new { @include diff_background(rgba(133, 153, 0, 0.15), rgba(133, 153, 0, 0.25), #113b46); } - .diff-line-num.old, .line_content.old { + .diff-line-num.old, + .line_content.old { @include diff_background(rgba(220, 50, 47, 0.3), rgba(220, 50, 47, 0.25), #113b46); } diff --git a/app/assets/stylesheets/highlight/solarized_light.scss b/app/assets/stylesheets/highlight/solarized_light.scss index 5956a28cafe..faff353ded7 100644 --- a/app/assets/stylesheets/highlight/solarized_light.scss +++ b/app/assets/stylesheets/highlight/solarized_light.scss @@ -7,20 +7,25 @@ .code.solarized-light { // Line numbers - .line-numbers, .diff-line-num { + .line-numbers, + .diff-line-num { background-color: #fdf6e3; } - .diff-line-num, .diff-line-num a { + .diff-line-num, + .diff-line-num a { color: $black-transparent; } // Code itself - pre.code, .diff-line-num { + pre.code, + .diff-line-num { border-color: #c5d0d4; } - &, pre.code, .line_holder .line_content { + &, + pre.code, + .line_holder .line_content { background-color: #fdf6e3; color: #586e75; } @@ -37,11 +42,13 @@ border-color: darken(#ddd8c5, 15%); } - .diff-line-num.new, .line_content.new { + .diff-line-num.new, + .line_content.new { @include diff_background(rgba(133, 153, 0, 0.2), rgba(133, 153, 0, 0.25), #c5d0d4); } - .diff-line-num.old, .line_content.old { + .diff-line-num.old, + .line_content.old { @include diff_background(rgba(220, 50, 47, 0.2), rgba(220, 50, 47, 0.25), #c5d0d4); } diff --git a/app/assets/stylesheets/highlight/white.scss b/app/assets/stylesheets/highlight/white.scss index 6f31a5235c0..d5367d5f3f0 100644 --- a/app/assets/stylesheets/highlight/white.scss +++ b/app/assets/stylesheets/highlight/white.scss @@ -7,20 +7,25 @@ .code.white { // Line numbers - .line-numbers, .diff-line-num { + .line-numbers, + .diff-line-num { background-color: $background-color; } - .diff-line-num, .diff-line-num a { + .diff-line-num, + .diff-line-num a { color: $black-transparent; } // Code itself - pre.code, .diff-line-num { + pre.code, + .diff-line-num { border-color: $table-border-gray; } - &, pre.code, .line_holder .line_content { + &, + pre.code, + .line_holder .line_content { background-color: #fff; color: #333; } diff --git a/app/assets/stylesheets/pages/admin.scss b/app/assets/stylesheets/pages/admin.scss index 140d589024b..63396a6bb29 100644 --- a/app/assets/stylesheets/pages/admin.scss +++ b/app/assets/stylesheets/pages/admin.scss @@ -56,7 +56,8 @@ padding: 10px; text-align: center; - > div, p { + > div, + p { display: inline; margin: 0; diff --git a/app/assets/stylesheets/pages/ci_projects.scss b/app/assets/stylesheets/pages/ci_projects.scss index 67a9d7d2cf7..87c453a7a27 100644 --- a/app/assets/stylesheets/pages/ci_projects.scss +++ b/app/assets/stylesheets/pages/ci_projects.scss @@ -12,7 +12,8 @@ border-color: $border-color; } - th, td { + th, + td { padding: 10px $gl-padding; } diff --git a/app/assets/stylesheets/pages/commit.scss b/app/assets/stylesheets/pages/commit.scss index 264e7e01a34..8ecac08137b 100644 --- a/app/assets/stylesheets/pages/commit.scss +++ b/app/assets/stylesheets/pages/commit.scss @@ -2,14 +2,16 @@ display: block; } -.commit-author, .commit-committer { +.commit-author, +.commit-committer { display: block; color: #999; font-weight: normal; font-style: italic; } -.commit-author strong, .commit-committer strong { +.commit-author strong, +.commit-committer strong { font-weight: bold; font-style: normal; } diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss index 2b5621e20d6..ad315cfae62 100644 --- a/app/assets/stylesheets/pages/commits.scss +++ b/app/assets/stylesheets/pages/commits.scss @@ -63,7 +63,8 @@ display: inline-block; } - .btn-clipboard, .btn-transparent { + .btn-clipboard, + .btn-transparent { padding-left: 0; padding-right: 0; } @@ -162,7 +163,8 @@ .branch-commit { color: $gl-gray; - .commit-id, .commit-row-message { + .commit-id, + .commit-row-message { color: $gl-gray; } } diff --git a/app/assets/stylesheets/pages/confirmation.scss b/app/assets/stylesheets/pages/confirmation.scss index 292225c5261..81e5cee240d 100644 --- a/app/assets/stylesheets/pages/confirmation.scss +++ b/app/assets/stylesheets/pages/confirmation.scss @@ -2,7 +2,12 @@ margin-bottom: 20px; border-bottom: 1px solid #eee; - > h1, h2, h3, h4, h5, h6 { + > h1, + h2, + h3, + h4, + h5, + h6 { font-weight: 400; } @@ -10,7 +15,8 @@ margin-bottom: 20px; } - ul, ol { + ul, + ol { padding-left: 0; } diff --git a/app/assets/stylesheets/pages/detail_page.scss b/app/assets/stylesheets/pages/detail_page.scss index 2357671c2ae..0f0c0abe7ae 100644 --- a/app/assets/stylesheets/pages/detail_page.scss +++ b/app/assets/stylesheets/pages/detail_page.scss @@ -13,7 +13,8 @@ color: #5c5d5e; } - .issue_created_ago, .author_link { + .issue_created_ago, + .author_link { white-space: nowrap; } } diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index fe6421f8b3f..9627d1c841b 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -124,7 +124,8 @@ } } - .old_line, .new_line { + .old_line, + .new_line { margin: 0; padding: 0; border: none; @@ -281,7 +282,8 @@ position: relative; } - .frame.added, .frame.deleted { + .frame.added, + .frame.deleted { position: absolute; display: block; top: 0; @@ -347,7 +349,8 @@ text-align: center; background: #eee; - ul, li { + ul, + li { list-style: none; margin: 0; padding: 0; diff --git a/app/assets/stylesheets/pages/editor.scss b/app/assets/stylesheets/pages/editor.scss index 029dabd2138..cb8cefaca97 100644 --- a/app/assets/stylesheets/pages/editor.scss +++ b/app/assets/stylesheets/pages/editor.scss @@ -91,7 +91,9 @@ } } - .gitignore-selector, .license-selector, .gitlab-ci-yml-selector { + .gitignore-selector, + .license-selector, + .gitlab-ci-yml-selector { .dropdown { line-height: 21px; } diff --git a/app/assets/stylesheets/pages/errors.scss b/app/assets/stylesheets/pages/errors.scss index 32d2d7b1dbf..11309817d31 100644 --- a/app/assets/stylesheets/pages/errors.scss +++ b/app/assets/stylesheets/pages/errors.scss @@ -2,7 +2,9 @@ max-width: 400px; margin: 0 auto; - h1, h2, h3 { + h1, + h2, + h3 { text-align: center; } diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index 623da67a239..3e7fc3fa52c 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -43,7 +43,8 @@ ul.related-merge-requests > li { } } -.merge-requests-title, .related-branches-title { +.merge-requests-title, +.related-branches-title { font-size: 16px; font-weight: 600; } diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss index 8dac6ab999e..82e46377308 100644 --- a/app/assets/stylesheets/pages/login.scss +++ b/app/assets/stylesheets/pages/login.scss @@ -41,7 +41,8 @@ font-size: 13px; } - .login-box, .omniauth-container { + .login-box, + .omniauth-container { box-shadow: 0 0 0 1px $border-color; border-bottom-right-radius: 2px; border-bottom-left-radius: 2px; @@ -198,7 +199,8 @@ .form-control { - &:active, &:focus { + &:active, + &:focus { background-color: #fff; } } @@ -261,7 +263,8 @@ position: relative; } - .footer-container, hr.footer-fixed { + .footer-container, + hr.footer-fixed { position: absolute; bottom: 0; left: 0; diff --git a/app/assets/stylesheets/pages/merge_conflicts.scss b/app/assets/stylesheets/pages/merge_conflicts.scss index eed2b0ab7cc..aa8057e4b9d 100644 --- a/app/assets/stylesheets/pages/merge_conflicts.scss +++ b/app/assets/stylesheets/pages/merge_conflicts.scss @@ -101,7 +101,8 @@ $colors: ( @mixin color-scheme($color) { - .header.line_content, .diff-line-num { + .header.line_content, + .diff-line-num { &.origin { background-color: map-get($colors, #{$color}_header_origin_neutral); border-color: map-get($colors, #{$color}_header_origin_neutral); diff --git a/app/assets/stylesheets/pages/milestone.scss b/app/assets/stylesheets/pages/milestone.scss index dd6d1783667..13402acd8e1 100644 --- a/app/assets/stylesheets/pages/milestone.scss +++ b/app/assets/stylesheets/pages/milestone.scss @@ -50,7 +50,8 @@ } } -.issues-sortable-list, .merge_requests-sortable-list { +.issues-sortable-list, +.merge_requests-sortable-list { .issuable-detail { display: block; margin-top: 7px; diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 17f28959414..c1a3c082cfa 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -24,7 +24,8 @@ display: none; } -.new-note, .note-edit-form { +.new-note, +.note-edit-form { .note-form-actions { margin-top: $gl-padding; } diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index fffcdc812a7..586f21821f7 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -28,7 +28,8 @@ ul.notes { } } - .note-created-ago, .note-updated-at { + .note-created-ago, + .note-updated-at { white-space: nowrap; } diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 5b8dc7f8c40..f88175365c6 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -248,7 +248,8 @@ font-size: 14px; } - svg, .fa { + svg, + .fa { margin-right: 0; } } @@ -529,7 +530,8 @@ // Connect each build (except for first) with curved lines &:not(:first-child) { - &::after, &::before { + &::after, + &::before { content: ''; top: -49px; position: absolute; @@ -555,7 +557,8 @@ // Connect second build to first build with smaller curved line &:nth-child(2) { - &::after, &::before { + &::after, + &::before { height: 29px; top: -9px; } @@ -570,7 +573,8 @@ .build { // Remove right connecting horizontal line from first build in last stage &:first-child { - &::after, &::before { + &::after, + &::before { border: none; } } diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index ed80d2beec2..3f6fdaebc1d 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -253,7 +253,8 @@ } table.u2f-registrations { - th:not(:last-child), td:not(:last-child) { + th:not(:last-child), + td:not(:last-child) { border-right: solid 1px transparent; } } \ No newline at end of file diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index fe7cf3c87e3..f6355941837 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -6,7 +6,8 @@ } } -.no-ssh-key-message, .project-limit-message { +.no-ssh-key-message, +.project-limit-message { background-color: #f28d35; margin-bottom: 0; } @@ -385,7 +386,8 @@ a.deploy-project-label { text-align: center; width: 169px; - &:hover, &.forked { + &:hover, + &.forked { background-color: $row-hover; border-color: $row-hover-border; } @@ -734,7 +736,8 @@ pre.light-well { .table-bordered { border-radius: 1px; - th:not(:last-child), td:not(:last-child) { + th:not(:last-child), + td:not(:last-child) { border-right: solid 1px transparent; } } @@ -757,7 +760,8 @@ pre.light-well { } } -.project-refs-form .dropdown-menu, .dropdown-menu-projects { +.project-refs-form .dropdown-menu, +.dropdown-menu-projects { width: 300px; @media (min-width: $screen-sm-min) { diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index e77f9816d8a..6d472e8293f 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -65,7 +65,8 @@ .search-input-wrap { width: 100%; - .search-icon, .clear-icon { + .search-icon, + .clear-icon { position: absolute; right: 5px; top: 0; @@ -185,7 +186,8 @@ padding-right: $gl-padding + 15px; } - .btn-search, .btn-new { + .btn-search, + .btn-new { width: 100%; margin-top: 5px; diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index 6ea7a2b5498..99d53d52119 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -23,7 +23,8 @@ border-bottom: 1px solid $table-border-gray; border-top: 1px solid $table-border-gray; - td, th { + td, + th { line-height: 21px; } @@ -74,7 +75,8 @@ max-width: 320px; vertical-align: middle; - i, a { + i, + a { color: $gl-dark-link-color; } diff --git a/app/assets/stylesheets/print.scss b/app/assets/stylesheets/print.scss index a30b6492572..8239b7e6879 100644 --- a/app/assets/stylesheets/print.scss +++ b/app/assets/stylesheets/print.scss @@ -1,7 +1,24 @@ -.wiki h1, .wiki h2, .wiki h3, .wiki h4, .wiki h5, .wiki h6 {margin-top: 17px; } -.wiki h1 {font-size: 30px;} -.wiki h2 {font-size: 22px;} -.wiki h3 {font-size: 18px; font-weight: bold; } +.wiki h1, +.wiki h2, +.wiki h3, +.wiki h4, +.wiki h5, +.wiki h6 { + margin-top: 17px; +} + +.wiki h1 { + font-size: 30px; +} + +.wiki h2 { + font-size: 22px; +} + +.wiki h3 { + font-size: 18px; + font-weight: bold; +} header, nav, From 03b6108f6f293830e50d113068877be155549fdd Mon Sep 17 00:00:00 2001 From: David Wagner Date: Tue, 18 Oct 2016 23:20:36 +0200 Subject: [PATCH 62/73] Remove redundant class_name and foreign_key overrides They were Rails' default and are unnecessarily overridden. Signed-off-by: David Wagner --- app/models/ci/build.rb | 4 ++-- app/models/ci/pipeline.rb | 6 +++--- app/models/ci/runner.rb | 6 +++--- app/models/ci/runner_project.rb | 4 ++-- app/models/ci/trigger.rb | 4 ++-- app/models/ci/trigger_request.rb | 6 +++--- app/models/ci/variable.rb | 2 +- app/models/commit_status.rb | 2 +- app/models/group.rb | 2 +- app/models/members/group_member.rb | 2 +- app/models/members/project_member.rb | 2 +- app/models/merge_request.rb | 4 ++-- app/models/project.rb | 8 ++++---- app/models/user.rb | 8 ++++---- spec/models/members/project_member_spec.rb | 2 +- spec/models/merge_request_spec.rb | 4 ++-- spec/models/user_spec.rb | 4 ++-- 17 files changed, 35 insertions(+), 35 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index a6b606d13de..bf5f92f8462 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -3,8 +3,8 @@ module Ci include TokenAuthenticatable include AfterCommitQueue - belongs_to :runner, class_name: 'Ci::Runner' - belongs_to :trigger_request, class_name: 'Ci::TriggerRequest' + belongs_to :runner + belongs_to :trigger_request belongs_to :erased_by, class_name: 'User' serialize :options diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index d5c1e03b461..adda3b8f40c 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -7,12 +7,12 @@ module Ci self.table_name = 'ci_commits' - belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id + belongs_to :project, foreign_key: :gl_project_id belongs_to :user has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id - has_many :builds, class_name: 'Ci::Build', foreign_key: :commit_id - has_many :trigger_requests, dependent: :destroy, class_name: 'Ci::TriggerRequest', foreign_key: :commit_id + has_many :builds, foreign_key: :commit_id + has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id validates_presence_of :sha, unless: :importing? validates_presence_of :ref, unless: :importing? diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 44cb19ece3b..123930273e0 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -6,9 +6,9 @@ module Ci AVAILABLE_SCOPES = %w[specific shared active paused online] FORM_EDITABLE = %i[description tag_list active run_untagged locked] - has_many :builds, class_name: 'Ci::Build' - has_many :runner_projects, dependent: :destroy, class_name: 'Ci::RunnerProject' - has_many :projects, through: :runner_projects, class_name: '::Project', foreign_key: :gl_project_id + has_many :builds + has_many :runner_projects, dependent: :destroy + has_many :projects, through: :runner_projects, foreign_key: :gl_project_id has_one :last_build, ->() { order('id DESC') }, class_name: 'Ci::Build' diff --git a/app/models/ci/runner_project.rb b/app/models/ci/runner_project.rb index 4b44ffa886e..1f9baeca5b1 100644 --- a/app/models/ci/runner_project.rb +++ b/app/models/ci/runner_project.rb @@ -2,8 +2,8 @@ module Ci class RunnerProject < ActiveRecord::Base extend Ci::Model - belongs_to :runner, class_name: 'Ci::Runner' - belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id + belongs_to :runner + belongs_to :project, foreign_key: :gl_project_id validates_uniqueness_of :runner_id, scope: :gl_project_id end diff --git a/app/models/ci/trigger.rb b/app/models/ci/trigger.rb index a0b19b51a12..62889fe80d8 100644 --- a/app/models/ci/trigger.rb +++ b/app/models/ci/trigger.rb @@ -4,8 +4,8 @@ module Ci acts_as_paranoid - belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id - has_many :trigger_requests, dependent: :destroy, class_name: 'Ci::TriggerRequest' + belongs_to :project, foreign_key: :gl_project_id + has_many :trigger_requests, dependent: :destroy validates_presence_of :token validates_uniqueness_of :token diff --git a/app/models/ci/trigger_request.rb b/app/models/ci/trigger_request.rb index fc674871743..2b807731d0d 100644 --- a/app/models/ci/trigger_request.rb +++ b/app/models/ci/trigger_request.rb @@ -2,9 +2,9 @@ module Ci class TriggerRequest < ActiveRecord::Base extend Ci::Model - belongs_to :trigger, class_name: 'Ci::Trigger' - belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id - has_many :builds, class_name: 'Ci::Build' + belongs_to :trigger + belongs_to :pipeline, foreign_key: :commit_id + has_many :builds serialize :variables diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb index 6959223aed9..94d9e2b3208 100644 --- a/app/models/ci/variable.rb +++ b/app/models/ci/variable.rb @@ -2,7 +2,7 @@ module Ci class Variable < ActiveRecord::Base extend Ci::Model - belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id + belongs_to :project, foreign_key: :gl_project_id validates_uniqueness_of :key, scope: :gl_project_id validates :key, diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 7b554be4f9a..4cb3a69416e 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -5,7 +5,7 @@ class CommitStatus < ActiveRecord::Base self.table_name = 'ci_builds' - belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id + belongs_to :project, foreign_key: :gl_project_id belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id belongs_to :user diff --git a/app/models/group.rb b/app/models/group.rb index 00a595d2705..552e1154df6 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -6,7 +6,7 @@ class Group < Namespace include AccessRequestable include Referable - has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'GroupMember' + has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source alias_method :members, :group_members has_many :users, through: :group_members has_many :owners, diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index 1b54a85d064..204f34f0269 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -1,7 +1,7 @@ class GroupMember < Member SOURCE_TYPE = 'Namespace' - belongs_to :group, class_name: 'Group', foreign_key: 'source_id' + belongs_to :group, foreign_key: 'source_id' # Make sure group member points only to group as it source default_value_for :source_type, SOURCE_TYPE diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index e4880973117..008fff0857c 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -3,7 +3,7 @@ class ProjectMember < Member include Gitlab::ShellAdapter - belongs_to :project, class_name: 'Project', foreign_key: 'source_id' + belongs_to :project, foreign_key: 'source_id' # Make sure project member points only to project as it source default_value_for :source_type, SOURCE_TYPE diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index c476a3bb14e..4872f8b8649 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -6,8 +6,8 @@ class MergeRequest < ActiveRecord::Base include Taskable include Importable - belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project" - belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project" + belongs_to :target_project, class_name: "Project" + belongs_to :source_project, class_name: "Project" belongs_to :merge_user, class_name: "User" has_many :merge_request_diffs, dependent: :destroy diff --git a/app/models/project.rb b/app/models/project.rb index af117f0acb0..fbf7012972e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -63,11 +63,11 @@ class Project < ActiveRecord::Base alias_attribute :title, :name # Relations - belongs_to :creator, foreign_key: 'creator_id', class_name: 'User' + belongs_to :creator, class_name: 'User' belongs_to :group, -> { where(type: 'Group') }, foreign_key: 'namespace_id' belongs_to :namespace - has_one :last_event, -> {order 'events.created_at DESC'}, class_name: 'Event', foreign_key: 'project_id' + has_one :last_event, -> {order 'events.created_at DESC'}, class_name: 'Event' has_many :boards, before_add: :validate_board_limit, dependent: :destroy # Project services @@ -116,7 +116,7 @@ class Project < ActiveRecord::Base has_many :hooks, dependent: :destroy, class_name: 'ProjectHook' has_many :protected_branches, dependent: :destroy - has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'ProjectMember' + has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source alias_method :members, :project_members has_many :users, through: :project_members @@ -137,7 +137,7 @@ class Project < ActiveRecord::Base has_one :import_data, dependent: :destroy, class_name: "ProjectImportData" has_one :project_feature, dependent: :destroy - has_many :commit_statuses, dependent: :destroy, class_name: 'CommitStatus', foreign_key: :gl_project_id + has_many :commit_statuses, dependent: :destroy, foreign_key: :gl_project_id has_many :pipelines, dependent: :destroy, class_name: 'Ci::Pipeline', foreign_key: :gl_project_id has_many :builds, class_name: 'Ci::Build', foreign_key: :gl_project_id # the builds are created from the commit_statuses has_many :runner_projects, dependent: :destroy, class_name: 'Ci::RunnerProject', foreign_key: :gl_project_id diff --git a/app/models/user.rb b/app/models/user.rb index f367f4616fb..521879444d4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -47,7 +47,7 @@ class User < ActiveRecord::Base # # Namespace for personal projects - has_one :namespace, -> { where type: nil }, dependent: :destroy, foreign_key: :owner_id, class_name: "Namespace" + has_one :namespace, -> { where type: nil }, dependent: :destroy, foreign_key: :owner_id # Profile has_many :keys, dependent: :destroy @@ -66,17 +66,17 @@ class User < ActiveRecord::Base # Projects has_many :groups_projects, through: :groups, source: :projects has_many :personal_projects, through: :namespace, source: :projects - has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy, class_name: 'ProjectMember' + has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy has_many :projects, through: :project_members has_many :created_projects, foreign_key: :creator_id, class_name: 'Project' has_many :users_star_projects, dependent: :destroy has_many :starred_projects, through: :users_star_projects, source: :project - has_many :snippets, dependent: :destroy, foreign_key: :author_id, class_name: "Snippet" + has_many :snippets, dependent: :destroy, foreign_key: :author_id has_many :issues, dependent: :destroy, foreign_key: :author_id has_many :notes, dependent: :destroy, foreign_key: :author_id has_many :merge_requests, dependent: :destroy, foreign_key: :author_id - has_many :events, dependent: :destroy, foreign_key: :author_id, class_name: "Event" + has_many :events, dependent: :destroy, foreign_key: :author_id has_many :subscriptions, dependent: :destroy has_many :recent_events, -> { order "id DESC" }, foreign_key: :author_id, class_name: "Event" has_many :assigned_issues, dependent: :destroy, foreign_key: :assignee_id, class_name: "Issue" diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index b2fe96e2e02..f6b2ec5ae31 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe ProjectMember, models: true do describe 'associations' do - it { is_expected.to belong_to(:project).class_name('Project').with_foreign_key(:source_id) } + it { is_expected.to belong_to(:project).with_foreign_key(:source_id) } end describe 'validations' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 6e5137602aa..3155eff9ee1 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -6,8 +6,8 @@ describe MergeRequest, models: true do subject { create(:merge_request) } describe 'associations' do - it { is_expected.to belong_to(:target_project).with_foreign_key(:target_project_id).class_name('Project') } - it { is_expected.to belong_to(:source_project).with_foreign_key(:source_project_id).class_name('Project') } + it { is_expected.to belong_to(:target_project).class_name('Project') } + it { is_expected.to belong_to(:source_project).class_name('Project') } it { is_expected.to belong_to(:merge_user).class_name("User") } it { is_expected.to have_many(:merge_request_diffs).dependent(:destroy) } end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 65b2896930a..10c39b90212 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -15,11 +15,11 @@ describe User, models: true do describe 'associations' do it { is_expected.to have_one(:namespace) } - it { is_expected.to have_many(:snippets).class_name('Snippet').dependent(:destroy) } + it { is_expected.to have_many(:snippets).dependent(:destroy) } it { is_expected.to have_many(:project_members).dependent(:destroy) } it { is_expected.to have_many(:groups) } it { is_expected.to have_many(:keys).dependent(:destroy) } - it { is_expected.to have_many(:events).class_name('Event').dependent(:destroy) } + it { is_expected.to have_many(:events).dependent(:destroy) } it { is_expected.to have_many(:recent_events).class_name('Event') } it { is_expected.to have_many(:issues).dependent(:destroy) } it { is_expected.to have_many(:notes).dependent(:destroy) } From 541b9eb65828719c6dea20bc750873f2882d9d07 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Mon, 24 Oct 2016 21:44:24 -0400 Subject: [PATCH 63/73] Fix rubocop build error --- spec/features/login_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb index a3e1ca923ef..76bcfbe523a 100644 --- a/spec/features/login_spec.rb +++ b/spec/features/login_spec.rb @@ -257,7 +257,7 @@ feature 'Login', feature: true do end end - def ensure_tab_pane_correctness(visit_path=true) + def ensure_tab_pane_correctness(visit_path = true) if visit_path visit new_user_session_path end From fed3f718d8e2223305fb1c0ab4e72d514f50a8f6 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 25 Oct 2016 11:03:49 +0530 Subject: [PATCH 64/73] Fix `User#to_reference` 1. Changes in 8.13 require `Referable`s that don't have a project reference to accept two arguments - `from_project` and `target_project`. 2. `User#to_reference` was not changed to accept the `target_project` (even though it is not used). Moving an issue containing a user reference would throw a "invalid number of arguments" exception. Fixes #23662 --- app/models/user.rb | 2 +- spec/services/issues/move_service_spec.rb | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index f367f4616fb..9181db40eb4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -309,7 +309,7 @@ class User < ActiveRecord::Base username end - def to_reference(_from_project = nil) + def to_reference(_from_project = nil, _target_project = nil) "#{self.class.reference_prefix}#{username}" end diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 93bf0f64963..34ec13c43c6 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -207,10 +207,10 @@ describe Issues::MoveService, services: true do end end - describe 'rewritting references' do + describe 'rewriting references' do include_context 'issue move executed' - context 'issue reference' do + context 'issue references' do let(:another_issue) { create(:issue, project: old_project) } let(:description) { "Some description #{another_issue.to_reference}" } @@ -219,6 +219,16 @@ describe Issues::MoveService, services: true do .to eq "Some description #{old_project.to_reference}#{another_issue.to_reference}" end end + + context "user references" do + let(:another_issue) { create(:issue, project: old_project) } + let(:description) { "Some description #{user.to_reference}" } + + it "doesn't throw any errors for issues containing user references" do + expect(new_issue.description) + .to eq "Some description #{user.to_reference}" + end + end end context 'moving to same project' do From ddafea060d4b607cd3f5c29e947cdbf6483dcd5d Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 23 Oct 2016 16:46:21 -0700 Subject: [PATCH 65/73] Fix bug where labels would be assigned to issues that were moved If you attempt to move an issue from one project to another and leave labels blank, LabelsFinder would assign all labels in the new project to that issue. The issue is that :title is passed along to the Finder, but since it appears empty no filtering is done. As a result, all labels in the group are returned. This fix handles that case. Closes #23668 --- CHANGELOG.md | 1 + app/finders/labels_finder.rb | 4 ++++ spec/finders/labels_finder_spec.rb | 6 ++++++ 3 files changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86878e5af6c..1e2150b9ca7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov) ## 8.13.1 (unreleased) + - Fix bug where labels would be assigned to issues that were moved - Fix error in generating labels - Fix reply-by-email not working due to queue name mismatch - Fixed hidden pipeline graph on commit and MR page !6895 diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 6ace14a4bb5..2291c64b84d 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -35,6 +35,10 @@ class LabelsFinder < UnionFinder end def with_title(items) + # Match no labels if an empty title is supplied to avoid matching all + # labels (e.g. when an issue is moved) + return Label.none if params[:title] && params[:title].empty? + items = items.where(title: title) if title items end diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb index 27acc464ea2..114399ea3dc 100644 --- a/spec/finders/labels_finder_spec.rb +++ b/spec/finders/labels_finder_spec.rb @@ -64,6 +64,12 @@ describe LabelsFinder do expect(finder.execute).to eq [group_label_2] end + + it 'returns no labels if empty titles are supplied' do + finder = described_class.new(user, title: []) + + expect(finder.execute).to be_empty + end end end end From 3e941b136d335b789c7efea46fc3b8af294d3f47 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 23 Oct 2016 20:57:06 -0700 Subject: [PATCH 66/73] Add spec in Issues::MoveService to fix label assignment regression --- spec/services/issues/move_service_spec.rb | 31 +++++++++++++++++++---- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 93bf0f64963..8c33389003a 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -23,15 +23,16 @@ describe Issues::MoveService, services: true do old_project.team << [user, :reporter] new_project.team << [user, :reporter] - ['label1', 'label2'].each do |label| + labels = 2.times.map { |x| "label%d" % (x + 1) } + + labels.each do |label| old_issue.labels << create(:label, project_id: old_project.id, title: label) - end - new_project.labels << create(:label, title: 'label1') - new_project.labels << create(:label, title: 'label2') - end + new_project.labels << create(:label, title: label) + end + end end describe '#execute' do @@ -277,5 +278,25 @@ describe Issues::MoveService, services: true do it { expect { move }.to raise_error(StandardError, /permissions/) } end end + + context 'movable issue with no assigned labels' do + before do + old_project.team << [user, :reporter] + new_project.team << [user, :reporter] + + labels = 2.times.map { |x| "label%d" % (x + 1) } + + labels.each do |label| + new_project.labels << create(:label, title: label) + end + end + + include_context 'issue move executed' + + it 'does not assign labels to new issue' do + expected_label_titles = new_issue.reload.labels.map(&:title) + expect(expected_label_titles.size).to eq 0 + end + end end end From 7c0ccbaac4aad5057f76d4f62b3a892aae64e190 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 24 Oct 2016 10:05:21 +0200 Subject: [PATCH 67/73] Fix Rubocop offenses in issue move specs --- spec/services/issues/move_service_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 8c33389003a..302eef8bf7e 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -23,7 +23,7 @@ describe Issues::MoveService, services: true do old_project.team << [user, :reporter] new_project.team << [user, :reporter] - labels = 2.times.map { |x| "label%d" % (x + 1) } + labels = Array.new(2) { |x| "label%d" % (x + 1) } labels.each do |label| old_issue.labels << create(:label, @@ -32,7 +32,7 @@ describe Issues::MoveService, services: true do new_project.labels << create(:label, title: label) end - end + end end describe '#execute' do @@ -284,7 +284,7 @@ describe Issues::MoveService, services: true do old_project.team << [user, :reporter] new_project.team << [user, :reporter] - labels = 2.times.map { |x| "label%d" % (x + 1) } + labels = Array.new(2) { |x| "label%d" % (x + 1) } labels.each do |label| new_project.labels << create(:label, title: label) From af4d16d9b8eada31be308f87ab596e34e9907e73 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 24 Oct 2016 06:25:18 -0700 Subject: [PATCH 68/73] Allow the use of params[:name] when filtering labels --- app/finders/labels_finder.rb | 13 ++++++++++--- spec/finders/labels_finder_spec.rb | 12 ++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 2291c64b84d..032172fdfa8 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -37,10 +37,13 @@ class LabelsFinder < UnionFinder def with_title(items) # Match no labels if an empty title is supplied to avoid matching all # labels (e.g. when an issue is moved) - return Label.none if params[:title] && params[:title].empty? + return items.none if raw_title && raw_title.empty? - items = items.where(title: title) if title - items + if title + items = items.where(title: title) + else + items + end end def group_id @@ -59,6 +62,10 @@ class LabelsFinder < UnionFinder params[:title].presence || params[:name].presence end + def raw_title + params[:title] || params[:name] + end + def project return @project if defined?(@project) diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb index 114399ea3dc..eb8df8e2bb2 100644 --- a/spec/finders/labels_finder_spec.rb +++ b/spec/finders/labels_finder_spec.rb @@ -65,11 +65,23 @@ describe LabelsFinder do expect(finder.execute).to eq [group_label_2] end + it 'returns label with title alias' do + finder = described_class.new(user, name: 'Group Label 2') + + expect(finder.execute).to eq [group_label_2] + end + it 'returns no labels if empty titles are supplied' do finder = described_class.new(user, title: []) expect(finder.execute).to be_empty end + + it 'returns no labels if empty names are supplied' do + finder = described_class.new(user, name: []) + + expect(finder.execute).to be_empty + end end end end From ce256c28f2012a9c20fd1872fa91214b402528bf Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 24 Oct 2016 06:43:13 -0700 Subject: [PATCH 69/73] Improve label filtering implementation --- app/finders/labels_finder.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 032172fdfa8..8a85f7a2952 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -35,13 +35,11 @@ class LabelsFinder < UnionFinder end def with_title(items) - # Match no labels if an empty title is supplied to avoid matching all - # labels (e.g. when an issue is moved) - return items.none if raw_title && raw_title.empty? - if title - items = items.where(title: title) - else + items.where(title: title) + elsif params[:title] || params[:name] # empty input, should match nothing + items.none + else # not filtering items end end @@ -62,10 +60,6 @@ class LabelsFinder < UnionFinder params[:title].presence || params[:name].presence end - def raw_title - params[:title] || params[:name] - end - def project return @project if defined?(@project) From 02f835c105df6c46d5c73468eedf1e2b0a0793b5 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 24 Oct 2016 23:04:38 -0700 Subject: [PATCH 70/73] Improve readability and add specs for label filtering --- app/finders/labels_finder.rb | 13 +++++-------- spec/finders/labels_finder_spec.rb | 18 ++++++++++++++++-- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 8a85f7a2952..95e62cdb02a 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -35,13 +35,10 @@ class LabelsFinder < UnionFinder end def with_title(items) - if title - items.where(title: title) - elsif params[:title] || params[:name] # empty input, should match nothing - items.none - else # not filtering - items - end + return items if title.nil? + return items.none if title.blank? + + items.where(title: title) end def group_id @@ -57,7 +54,7 @@ class LabelsFinder < UnionFinder end def title - params[:title].presence || params[:name].presence + params[:title] || params[:name] end def project diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb index eb8df8e2bb2..10cfb66ec1c 100644 --- a/spec/finders/labels_finder_spec.rb +++ b/spec/finders/labels_finder_spec.rb @@ -38,6 +38,14 @@ describe LabelsFinder do expect(finder.execute).to eq [group_label_2, group_label_3, project_label_1, group_label_1, project_label_2, project_label_4] end + + it 'returns labels available if nil title is supplied' do + group_2.add_developer(user) + # params[:title] will return `nil` regardless whether it is specified + finder = described_class.new(user, title: nil) + + expect(finder.execute).to eq [group_label_2, group_label_3, project_label_1, group_label_1, project_label_2, project_label_4] + end end context 'filtering by group_id' do @@ -71,13 +79,19 @@ describe LabelsFinder do expect(finder.execute).to eq [group_label_2] end - it 'returns no labels if empty titles are supplied' do + it 'returns no labels if empty title is supplied' do finder = described_class.new(user, title: []) expect(finder.execute).to be_empty end - it 'returns no labels if empty names are supplied' do + it 'returns no labels if blank title is supplied' do + finder = described_class.new(user, title: '') + + expect(finder.execute).to be_empty + end + + it 'returns no labels if empty name is supplied' do finder = described_class.new(user, name: []) expect(finder.execute).to be_empty From e805253d4e2132827f45ce82c99b50c1e1314cfc Mon Sep 17 00:00:00 2001 From: Sonmez Kartal Date: Tue, 25 Oct 2016 09:41:29 +0300 Subject: [PATCH 71/73] Add docker-compose environment initialization command Signed-off-by: Sonmez Kartal --- doc/administration/integration/koding.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/administration/integration/koding.md b/doc/administration/integration/koding.md index a2c358af095..b95c425842c 100644 --- a/doc/administration/integration/koding.md +++ b/doc/administration/integration/koding.md @@ -61,6 +61,7 @@ executing commands in the following snippet. ```bash git clone https://github.com/koding/koding.git cd koding +docker-compose -f docker-compose-init.yml run init docker-compose up ``` From 692eb3f84e912bd586f923bd87737bd15ae3b750 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Tue, 25 Oct 2016 10:50:29 +0200 Subject: [PATCH 72/73] Capitalize Git [ci skip] --- doc/user/project/new_ci_build_permissions_model.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/project/new_ci_build_permissions_model.md b/doc/user/project/new_ci_build_permissions_model.md index 608475116a1..60b7bec2ba7 100644 --- a/doc/user/project/new_ci_build_permissions_model.md +++ b/doc/user/project/new_ci_build_permissions_model.md @@ -254,7 +254,7 @@ test: This will make GitLab CI initialize (fetch) and update (checkout) all your submodules recursively. -If git does not use the newly added relative URLs but still uses your old URLs, +If Git does not use the newly added relative URLs but still uses your old URLs, you might need to add `git submodule sync --recursive` to your `.gitlab-ci.yml`, prior to running `git submodule update --init --recursive`. This transfers the changes from your `.gitmodules` file into the `.git` folder, which is kept by From cabc131cfbee72e3a1eaae94619dcf1e3cc59d5a Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 25 Oct 2016 12:57:56 +0100 Subject: [PATCH 73/73] Stop unauthized users dragging on issue boards Closes #23763 --- CHANGELOG.md | 3 ++- app/helpers/boards_helper.rb | 2 +- spec/features/boards/boards_spec.rb | 4 ++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21f2bec867f..a8603170355 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,8 @@ Please view this file on the master branch, on stable branches it's out of date. - Fixed hidden pipeline graph on commit and MR page !6895 - Expire and build repository cache after project import - Fix 404 for group pages when GitLab setup uses relative url - - Simpler arguments passed to named_route on toggle_award_url helper method + - Simpler arguments passed to named_route on toggle_award_url helper method + - Fix unauthorized users dragging on issue boards - Better handle when no users were selected for adding to group or project. (Linus Thiel) - Only show register tab if signup enabled. diff --git a/app/helpers/boards_helper.rb b/app/helpers/boards_helper.rb index b7247ffa8b2..38c586ccd31 100644 --- a/app/helpers/boards_helper.rb +++ b/app/helpers/boards_helper.rb @@ -5,7 +5,7 @@ module BoardsHelper { endpoint: namespace_project_boards_path(@project.namespace, @project), board_id: board.id, - disabled: !can?(current_user, :admin_list, @project), + disabled: "#{!can?(current_user, :admin_list, @project)}", issue_link_base: namespace_project_issues_path(@project.namespace, @project) } end diff --git a/spec/features/boards/boards_spec.rb b/spec/features/boards/boards_spec.rb index 0fb1608a0a3..c533ce1d87f 100644 --- a/spec/features/boards/boards_spec.rb +++ b/spec/features/boards/boards_spec.rb @@ -624,6 +624,10 @@ describe 'Issue Boards', feature: true, js: true do it 'does not show create new list' do expect(page).not_to have_selector('.js-new-board-list') end + + it 'does not allow dragging' do + expect(page).not_to have_selector('.user-can-drag') + end end context 'as guest user' do