From 8acad49fe0a492c2f4e16756135679b3a190543b Mon Sep 17 00:00:00 2001 From: Hiroyuki Sato Date: Sun, 28 Feb 2016 21:11:43 +0900 Subject: [PATCH 01/14] Support Wiki with UTF-8 page name --- CHANGELOG | 1 + Gemfile | 4 +++- Gemfile.lock | 6 +++++- app/models/project_wiki.rb | 8 ++++---- app/models/wiki_page.rb | 2 +- config/initializers/gollum.rb | 13 +++++++++++++ 6 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 config/initializers/gollum.rb diff --git a/CHANGELOG b/CHANGELOG index 015efa05c6..d19c694863 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -10,6 +10,7 @@ v 8.6.0 (unreleased) setup. A password can be provided during setup (see installation docs), or GitLab will ask the user to create a new one upon first visit. - Fix issue when pushing to projects ending in .wiki + - Add support for wiki with UTF-8 page names (Hiroyuki Sato) - Don't load all of GitLab in mail_room - Update `omniauth-saml` to 1.5.0 to allow for custom response attributes to be set - Memoize @group in Admin::GroupsController (Yatish Mehta) diff --git a/Gemfile b/Gemfile index a0e8e79662..a849d7493a 100644 --- a/Gemfile +++ b/Gemfile @@ -58,7 +58,9 @@ gem "gitlab_git", '~> 9.0' gem 'gitlab_omniauth-ldap', '~> 1.2.1', require: "omniauth-ldap" # Git Wiki -gem 'gollum-lib', '~> 4.1.0' +# Required manually in config/initializers/gollum.rb to control load order +gem 'gollum-lib', '~> 4.1.0', require: false +gem 'gollum-rugged_adapter', '~> 0.4.2', require: false # Language detection gem "github-linguist", "~> 4.7.0", require: "linguist" diff --git a/Gemfile.lock b/Gemfile.lock index f4f5649eb7..669bfcf4d6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -381,6 +381,9 @@ GEM rouge (~> 1.9) sanitize (~> 2.1.0) stringex (~> 2.5.1) + gollum-rugged_adapter (0.4.2) + mime-types (>= 1.15) + rugged (~> 0.24.0, >= 0.21.3) gon (6.0.1) actionpack (>= 3.0) json @@ -703,7 +706,7 @@ GEM rubyntlm (0.5.2) rubypants (0.2.0) rufus-scheduler (3.1.10) - rugged (0.24.0b13) + rugged (0.24.0) safe_yaml (1.0.4) sanitize (2.1.0) nokogiri (>= 1.4.4) @@ -941,6 +944,7 @@ DEPENDENCIES gitlab_meta (= 7.0) gitlab_omniauth-ldap (~> 1.2.1) gollum-lib (~> 4.1.0) + gollum-rugged_adapter (~> 0.4.2) gon (~> 6.0.1) grape (~> 0.13.0) grape-entity (~> 0.4.2) diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index c96e6f0b8e..59b1b86d1f 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -2,7 +2,7 @@ class ProjectWiki include Gitlab::ShellAdapter MARKUPS = { - 'Markdown' => :md, + 'Markdown' => :markdown, 'RDoc' => :rdoc, 'AsciiDoc' => :asciidoc } unless defined?(MARKUPS) @@ -47,7 +47,7 @@ class ProjectWiki def wiki @wiki ||= begin Gollum::Wiki.new(path_to_repo) - rescue Gollum::NoSuchPathError + rescue Rugged::OSError create_repo! end end @@ -90,7 +90,7 @@ class ProjectWiki def create_page(title, content, format = :markdown, message = nil) commit = commit_details(:created, message, title) - wiki.write_page(title, format, content, commit) + wiki.write_page(title, format.to_sym, content, commit) update_project_activity rescue Gollum::DuplicatePageError => e @@ -101,7 +101,7 @@ class ProjectWiki def update_page(page, content, format = :markdown, message = nil) commit = commit_details(:updated, message, page.title) - wiki.update_page(page, page.name, format, content, commit) + wiki.update_page(page, page.name, format.to_sym, content, commit) update_project_activity end diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index dbd70dc5a4..526760779a 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -62,7 +62,7 @@ class WikiPage # The raw content of this page. def content @attributes[:content] ||= if @page - @page.raw_data + @page.text_data end end diff --git a/config/initializers/gollum.rb b/config/initializers/gollum.rb new file mode 100644 index 0000000000..703f24f93b --- /dev/null +++ b/config/initializers/gollum.rb @@ -0,0 +1,13 @@ +module Gollum + GIT_ADAPTER = "rugged" +end +require "gollum-lib" + +module Gollum + class Committer + # Patch for UTF-8 path + def method_missing(name, *args) + index.send(name, *args) + end + end +end From b24f9d66bc260dbfc04e63062b06bef390a4ba97 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 15 Mar 2016 17:38:45 +0000 Subject: [PATCH 02/14] Fixed overflow on users name in dropdowns Closes #14289 --- app/assets/stylesheets/framework/dropdowns.scss | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 5b647fc617..309da34da0 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -161,9 +161,8 @@ .dropdown-menu-user-full-name { display: block; - margin-bottom: 2px; font-weight: 600; - line-height: 1; + line-height: 16px; text-overflow: ellipsis; overflow: hidden; white-space: nowrap; @@ -171,7 +170,7 @@ .dropdown-menu-user-username { display: block; - line-height: 1; + line-height: 16px; text-overflow: ellipsis; overflow: hidden; white-space: nowrap; From 7c61bdcf6f7383ad26ea5b2c59bc8bc72096a8e6 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 15 Mar 2016 11:03:17 -0700 Subject: [PATCH 03/14] Fix Capybara 2.6.2 deprecation warnings --- features/support/capybara.rb | 2 +- spec/support/capybara.rb | 2 +- spec/support/wait_for_ajax.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/features/support/capybara.rb b/features/support/capybara.rb index f33379f76c..fe9e39cf50 100644 --- a/features/support/capybara.rb +++ b/features/support/capybara.rb @@ -9,7 +9,7 @@ Capybara.register_driver :poltergeist do |app| Capybara::Poltergeist::Driver.new(app, js_errors: true, timeout: timeout, window_size: [1366, 768]) end -Capybara.default_wait_time = timeout +Capybara.default_max_wait_time = timeout Capybara.ignore_hidden_elements = false unless ENV['CI'] || ENV['CI_SERVER'] diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 65d59e6813..e1f90e17cc 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -10,7 +10,7 @@ Capybara.register_driver :poltergeist do |app| Capybara::Poltergeist::Driver.new(app, js_errors: true, timeout: timeout, window_size: [1366, 768]) end -Capybara.default_wait_time = timeout +Capybara.default_max_wait_time = timeout Capybara.ignore_hidden_elements = true unless ENV['CI'] || ENV['CI_SERVER'] diff --git a/spec/support/wait_for_ajax.rb b/spec/support/wait_for_ajax.rb index 692d219e9f..b90fc11267 100644 --- a/spec/support/wait_for_ajax.rb +++ b/spec/support/wait_for_ajax.rb @@ -1,6 +1,6 @@ module WaitForAjax def wait_for_ajax - Timeout.timeout(Capybara.default_wait_time) do + Timeout.timeout(Capybara.default_max_wait_time) do loop until finished_all_ajax_requests? end end From 4bcc0977500af7af070296e2faf817eed10aafb9 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 17 Feb 2016 14:41:33 +0100 Subject: [PATCH 04/14] A worker deletes a user, so the request doesn't time out Fixes #13261 --- CHANGELOG | 1 + app/controllers/admin/users_controller.rb | 2 +- app/workers/delete_user_worker.rb | 10 ++++++++++ spec/workers/delete_user_worker_spec.rb | 22 ++++++++++++++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 app/workers/delete_user_worker.rb create mode 100644 spec/workers/delete_user_worker_spec.rb diff --git a/CHANGELOG b/CHANGELOG index 7f076f70c7..6de17a4a09 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -178,6 +178,7 @@ v 8.5.0 v 8.4.5 - No CE-specific changes + - User deletion is now done in the background so the request can not time out v 8.4.4 - Update omniauth-saml gem to 1.4.2 diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 87f4fb455b..3fe33018d7 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -122,7 +122,7 @@ class Admin::UsersController < Admin::ApplicationController DeleteUserService.new(current_user).execute(user) respond_to do |format| - format.html { redirect_to admin_users_path } + format.html { redirect_to admin_users_path, notice: "The user is being deleted." } format.json { head :ok } end end diff --git a/app/workers/delete_user_worker.rb b/app/workers/delete_user_worker.rb new file mode 100644 index 0000000000..bcbb6d58b4 --- /dev/null +++ b/app/workers/delete_user_worker.rb @@ -0,0 +1,10 @@ +class DeleteUserWorker + include Sidekiq::Worker + + def perform(current_user_id, delete_user_id) + delete_user = User.find(delete_user_id) + current_user = User.find(current_user_id) + + DeleteUserService.new(current_user).execute(delete_user) + end +end diff --git a/spec/workers/delete_user_worker_spec.rb b/spec/workers/delete_user_worker_spec.rb new file mode 100644 index 0000000000..73005c507c --- /dev/null +++ b/spec/workers/delete_user_worker_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe DeleteUserWorker do + describe "Deletes a user and all their personal projects" do + let!(:user) { create(:user) } + let!(:current_user) { create(:user) } + let!(:namespace) { create(:namespace, owner: user) } + let!(:project) { create(:project, namespace: namespace) } + + before do + DeleteUserWorker.new.perform(current_user.id, user.id) + end + + it 'deletes all personal projects' do + expect { Project.find(project.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'deletes the user' do + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end +end From b221d11a25218d3c2ad5304be91c849e1872be6c Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 25 Feb 2016 09:20:28 +0100 Subject: [PATCH 05/14] Add ability to delete a user with force --- CHANGELOG | 2 +- app/controllers/admin/users_controller.rb | 2 +- app/models/abuse_report.rb | 4 ++-- app/services/delete_user_service.rb | 26 +++++++++++++---------- app/services/destroy_group_service.rb | 4 ++-- app/workers/delete_user_worker.rb | 4 ++-- 6 files changed, 23 insertions(+), 19 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 6de17a4a09..bdeb9b68bd 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -59,6 +59,7 @@ v 8.5.3 - Show commit message in JIRA mention comment - Makes issue page and merge request page usable on mobile browsers. - Improved UI for profile settings + - User deletion is now done in the background so the request can not time out v 8.5.2 - Fix sidebar overlapping content when screen width was below 1200px @@ -178,7 +179,6 @@ v 8.5.0 v 8.4.5 - No CE-specific changes - - User deletion is now done in the background so the request can not time out v 8.4.4 - Update omniauth-saml gem to 1.4.2 diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 3fe33018d7..3063d299b1 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -119,7 +119,7 @@ class Admin::UsersController < Admin::ApplicationController end def destroy - DeleteUserService.new(current_user).execute(user) + DeleteUserWorker.perform_async(current_user.id, user.id) respond_to do |format| format.html { redirect_to admin_users_path, notice: "The user is being deleted." } diff --git a/app/models/abuse_report.rb b/app/models/abuse_report.rb index cc59aa4e91..155b5f327c 100644 --- a/app/models/abuse_report.rb +++ b/app/models/abuse_report.rb @@ -19,9 +19,9 @@ class AbuseReport < ActiveRecord::Base validates :message, presence: true validates :user_id, uniqueness: { message: 'has already been reported' } - def remove_user + def remove_user(current_user) user.block - user.destroy + DeleteUserWorker.perform_async(current_user.id, user.id, force: true) end def notify diff --git a/app/services/delete_user_service.rb b/app/services/delete_user_service.rb index 173e50c920..390960257c 100644 --- a/app/services/delete_user_service.rb +++ b/app/services/delete_user_service.rb @@ -5,18 +5,22 @@ class DeleteUserService @current_user = current_user end - def execute(user) - if user.solo_owned_groups.present? + def execute(user, options = {}) + if !options[:force] && user.solo_owned_groups.present? user.errors[:base] << 'You must transfer ownership or delete groups before you can remove user' - user - else - user.personal_projects.each do |project| - # Skip repository removal because we remove directory with namespace - # that contain all this repositories - ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete! - end - - user.destroy + return user end + + user.solo_owned_groups.each do |group| + DeleteGroupService.new(group, current_user).execute + end + + user.personal_projects.each do |project| + # Skip repository removal because we remove directory with namespace + # that contain all this repositories + ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete! + end + + user.destroy end end diff --git a/app/services/destroy_group_service.rb b/app/services/destroy_group_service.rb index 9189de390a..3c42ac61be 100644 --- a/app/services/destroy_group_service.rb +++ b/app/services/destroy_group_service.rb @@ -6,12 +6,12 @@ class DestroyGroupService end def execute - @group.projects.each do |project| + group.projects.each do |project| # Skip repository removal because we remove directory with namespace # that contain all this repositories ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete! end - @group.destroy + group.destroy end end diff --git a/app/workers/delete_user_worker.rb b/app/workers/delete_user_worker.rb index bcbb6d58b4..6ff361e4d8 100644 --- a/app/workers/delete_user_worker.rb +++ b/app/workers/delete_user_worker.rb @@ -1,10 +1,10 @@ class DeleteUserWorker include Sidekiq::Worker - def perform(current_user_id, delete_user_id) + def perform(current_user_id, delete_user_id, options = {}) delete_user = User.find(delete_user_id) current_user = User.find(current_user_id) - DeleteUserService.new(current_user).execute(delete_user) + DeleteUserService.new(current_user).execute(delete_user, options.symbolize_keys) end end From d249da1c16bea8f9d340520168c64794c5f78928 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Fri, 4 Mar 2016 09:49:23 +0100 Subject: [PATCH 06/14] Specs on force deleting of a user --- app/services/delete_user_service.rb | 2 +- spec/workers/delete_user_worker_spec.rb | 48 +++++++++++++++++++++---- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/app/services/delete_user_service.rb b/app/services/delete_user_service.rb index 390960257c..5f2e4f75b7 100644 --- a/app/services/delete_user_service.rb +++ b/app/services/delete_user_service.rb @@ -12,7 +12,7 @@ class DeleteUserService end user.solo_owned_groups.each do |group| - DeleteGroupService.new(group, current_user).execute + DestroyGroupService.new(group, current_user).execute end user.personal_projects.each do |project| diff --git a/spec/workers/delete_user_worker_spec.rb b/spec/workers/delete_user_worker_spec.rb index 73005c507c..bec9aecb48 100644 --- a/spec/workers/delete_user_worker_spec.rb +++ b/spec/workers/delete_user_worker_spec.rb @@ -7,16 +7,52 @@ describe DeleteUserWorker do let!(:namespace) { create(:namespace, owner: user) } let!(:project) { create(:project, namespace: namespace) } - before do - DeleteUserWorker.new.perform(current_user.id, user.id) + context 'no force flag given' do + before do + DeleteUserWorker.new.perform(current_user.id, user.id) + end + + it 'deletes all personal projects' do + expect { Project.find(project.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'deletes the user' do + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end end - it 'deletes all personal projects' do - expect { Project.find(project.id) }.to raise_error(ActiveRecord::RecordNotFound) + context "solo owned groups present" do + let(:solo_owned) { create(:group) } + let(:member) { create(:group_member) } + let(:user) { user = member.user } + + before do + solo_owned.group_members = [member] + DeleteUserWorker.new.perform(current_user.id, user.id) + end + + it 'does not delete the user' do + expect(User.find(user.id)).to eq user + end end - it 'deletes the user' do - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + context "deletions with force" do + let(:solo_owned) { create(:group) } + let(:member) { create(:group_member) } + let(:user) { user = member.user } + + before do + solo_owned.group_members = [member] + DeleteUserWorker.new.perform(current_user.id, user.id, "force" => true) + end + + it 'deletes solo owned groups' do + expect { Project.find(solo_owned.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'deletes the user' do + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end end end end From 4231cfeba5f6de519a8609b8aa8a6880e38c2414 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Mon, 7 Mar 2016 12:40:21 +0100 Subject: [PATCH 07/14] Target right release, rebase got it wrong --- CHANGELOG | 1 + spec/workers/delete_user_worker_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index bdeb9b68bd..ee9d3cebf4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -49,6 +49,7 @@ v 8.5.5 - Fix pagination for filtered dashboard and explore pages - Fix "Show all" link behavior - Add #upcoming filter to Milestone filter (Tiago Botelho) + - User deletion is now done in the background so the request can not time out v 8.5.4 - Do not cache requests for badges (including builds badge) diff --git a/spec/workers/delete_user_worker_spec.rb b/spec/workers/delete_user_worker_spec.rb index bec9aecb48..a98ce6e599 100644 --- a/spec/workers/delete_user_worker_spec.rb +++ b/spec/workers/delete_user_worker_spec.rb @@ -24,7 +24,7 @@ describe DeleteUserWorker do context "solo owned groups present" do let(:solo_owned) { create(:group) } let(:member) { create(:group_member) } - let(:user) { user = member.user } + let(:user) { member.user } before do solo_owned.group_members = [member] @@ -39,7 +39,7 @@ describe DeleteUserWorker do context "deletions with force" do let(:solo_owned) { create(:group) } let(:member) { create(:group_member) } - let(:user) { user = member.user } + let(:user) { member.user } before do solo_owned.group_members = [member] From a09323c9f5df69a60d00278dec64775821751711 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Mon, 7 Mar 2016 13:46:37 +0100 Subject: [PATCH 08/14] Fix broken specs --- app/controllers/admin/abuse_reports_controller.rb | 2 +- spec/models/abuse_report_spec.rb | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/app/controllers/admin/abuse_reports_controller.rb b/app/controllers/admin/abuse_reports_controller.rb index 2463cfa87b..a3f78e3477 100644 --- a/app/controllers/admin/abuse_reports_controller.rb +++ b/app/controllers/admin/abuse_reports_controller.rb @@ -6,7 +6,7 @@ class Admin::AbuseReportsController < Admin::ApplicationController def destroy abuse_report = AbuseReport.find(params[:id]) - abuse_report.remove_user if params[:remove_user] + abuse_report.remove_user(current_user) if params[:remove_user] abuse_report.destroy render nothing: true diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 4799bbaa57..8d16239bcb 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -13,7 +13,8 @@ require 'rails_helper' RSpec.describe AbuseReport, type: :model do - subject { create(:abuse_report) } + subject { create(:abuse_report) } + let(:user) { create(:user) } it { expect(subject).to be_valid } @@ -31,17 +32,13 @@ RSpec.describe AbuseReport, type: :model do describe '#remove_user' do it 'blocks the user' do - report = build(:abuse_report) - - allow(report.user).to receive(:destroy) - - expect { report.remove_user }.to change { report.user.blocked? }.to(true) + expect { subject.remove_user(user) }.to change { subject.user.blocked? }.to(true) end - it 'removes the user' do - report = build(:abuse_report) + it 'lets a worker delete the user' do + expect(DeleteUserWorker).to receive(:perform_async).with(user.id, subject.user.id, force: true) - expect { report.remove_user }.to change { User.count }.by(-1) + subject.remove_user(user) end end From 8077a5f9589e365f58f22015819bff7debcd7299 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Sat, 12 Mar 2016 13:11:30 +0100 Subject: [PATCH 09/14] Clearify who deletes the user --- CHANGELOG | 2 +- app/controllers/admin/abuse_reports_controller.rb | 2 +- app/models/abuse_report.rb | 4 ++-- app/services/delete_user_service.rb | 2 +- spec/models/abuse_report_spec.rb | 7 ++++--- spec/workers/delete_user_worker_spec.rb | 2 +- 6 files changed, 10 insertions(+), 9 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index ee9d3cebf4..fd7c7e7bc9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -39,6 +39,7 @@ v 8.6.0 (unreleased) - Add ability to show archived projects on dashboard, explore and group pages - Move group activity to separate page - Continue parameters are checked to ensure redirection goes to the same instance + - User deletion is now done in the background so the request can not time out v 8.5.6 - Obtain a lease before querying LDAP @@ -60,7 +61,6 @@ v 8.5.3 - Show commit message in JIRA mention comment - Makes issue page and merge request page usable on mobile browsers. - Improved UI for profile settings - - User deletion is now done in the background so the request can not time out v 8.5.2 - Fix sidebar overlapping content when screen width was below 1200px diff --git a/app/controllers/admin/abuse_reports_controller.rb b/app/controllers/admin/abuse_reports_controller.rb index a3f78e3477..e9b0972bdd 100644 --- a/app/controllers/admin/abuse_reports_controller.rb +++ b/app/controllers/admin/abuse_reports_controller.rb @@ -6,7 +6,7 @@ class Admin::AbuseReportsController < Admin::ApplicationController def destroy abuse_report = AbuseReport.find(params[:id]) - abuse_report.remove_user(current_user) if params[:remove_user] + abuse_report.remove_user(deleted_by: current_user) if params[:remove_user] abuse_report.destroy render nothing: true diff --git a/app/models/abuse_report.rb b/app/models/abuse_report.rb index 155b5f327c..b61f512312 100644 --- a/app/models/abuse_report.rb +++ b/app/models/abuse_report.rb @@ -19,9 +19,9 @@ class AbuseReport < ActiveRecord::Base validates :message, presence: true validates :user_id, uniqueness: { message: 'has already been reported' } - def remove_user(current_user) + def remove_user(deleted_by:) user.block - DeleteUserWorker.perform_async(current_user.id, user.id, force: true) + DeleteUserWorker.perform_async(deleted_by.id, user.id, delete_solo_owned_groups: true) end def notify diff --git a/app/services/delete_user_service.rb b/app/services/delete_user_service.rb index 5f2e4f75b7..ce79287e35 100644 --- a/app/services/delete_user_service.rb +++ b/app/services/delete_user_service.rb @@ -6,7 +6,7 @@ class DeleteUserService end def execute(user, options = {}) - if !options[:force] && user.solo_owned_groups.present? + if !options[:delete_solo_owned_groups] && user.solo_owned_groups.present? user.errors[:base] << 'You must transfer ownership or delete groups before you can remove user' return user end diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 8d16239bcb..ac12ab6c75 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -32,13 +32,14 @@ RSpec.describe AbuseReport, type: :model do describe '#remove_user' do it 'blocks the user' do - expect { subject.remove_user(user) }.to change { subject.user.blocked? }.to(true) + expect { subject.remove_user(deleted_by: user) }.to change { subject.user.blocked? }.to(true) end it 'lets a worker delete the user' do - expect(DeleteUserWorker).to receive(:perform_async).with(user.id, subject.user.id, force: true) + expect(DeleteUserWorker).to receive(:perform_async).with(user.id, subject.user.id, + delete_solo_owned_groups: true) - subject.remove_user(user) + subject.remove_user(deleted_by: user) end end diff --git a/spec/workers/delete_user_worker_spec.rb b/spec/workers/delete_user_worker_spec.rb index a98ce6e599..975b0470cb 100644 --- a/spec/workers/delete_user_worker_spec.rb +++ b/spec/workers/delete_user_worker_spec.rb @@ -43,7 +43,7 @@ describe DeleteUserWorker do before do solo_owned.group_members = [member] - DeleteUserWorker.new.perform(current_user.id, user.id, "force" => true) + DeleteUserWorker.new.perform(current_user.id, user.id, "delete_solo_owned_groups" => true) end it 'deletes solo owned groups' do From aff7c92596df8a277bd59411fb77425c91a403ba Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Tue, 15 Mar 2016 15:09:55 -0500 Subject: [PATCH 10/14] Typo --- app/views/shared/issuable/_filter.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 42a3c2c3f0..3eb0db276b 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -16,7 +16,7 @@ - if params[:assignee_id] = hidden_field_tag(:assignee_id, params[:assignee_id]) = dropdown_tag("Assignee", options: { toggle_class: "js-user-search js-filter-submit js-assignee-search", title: "Filter by assignee", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-assignee", - placeholder: "Search assignee", data: { any_user: "Any Author", first_user: (current_user.username if current_user), null_user: true, current_user: true, project_id: (@project.id if @project), selected: params[:assignee_id], field_name: "assignee_id" } }) + placeholder: "Search assignee", data: { any_user: "Any Assignee", first_user: (current_user.username if current_user), null_user: true, current_user: true, project_id: (@project.id if @project), selected: params[:assignee_id], field_name: "assignee_id" } }) .filter-item.inline.milestone-filter - if params[:milestone_title] From 29a43373ce9b3676ae7baea61b0ecaf67db67445 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Mon, 14 Mar 2016 11:18:03 +0100 Subject: [PATCH 11/14] Specs for the DeleteUserService --- CHANGELOG | 1 - spec/services/delete_user_service_spec.rb | 58 ++++++++++++++++++++++ spec/workers/delete_user_worker_spec.rb | 60 +++++------------------ 3 files changed, 69 insertions(+), 50 deletions(-) create mode 100644 spec/services/delete_user_service_spec.rb diff --git a/CHANGELOG b/CHANGELOG index fd7c7e7bc9..2b5c03b875 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -50,7 +50,6 @@ v 8.5.5 - Fix pagination for filtered dashboard and explore pages - Fix "Show all" link behavior - Add #upcoming filter to Milestone filter (Tiago Botelho) - - User deletion is now done in the background so the request can not time out v 8.5.4 - Do not cache requests for badges (including builds badge) diff --git a/spec/services/delete_user_service_spec.rb b/spec/services/delete_user_service_spec.rb new file mode 100644 index 0000000000..a65938fa03 --- /dev/null +++ b/spec/services/delete_user_service_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe DeleteUserService, services: true do + describe "Deletes a user and all their personal projects" do + let!(:user) { create(:user) } + let!(:current_user) { create(:user) } + let!(:namespace) { create(:namespace, owner: user) } + let!(:project) { create(:project, namespace: namespace) } + + context 'no options are given' do + it 'deletes the user' do + DeleteUserService.new(current_user).execute(user) + + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'will delete the project in the near future' do + expect_any_instance_of(Projects::DestroyService).to receive(:pending_delete!).once + + DeleteUserService.new(current_user).execute(user) + end + end + + context "solo owned groups present" do + let(:solo_owned) { create(:group) } + let(:member) { create(:group_member) } + let(:user) { member.user } + + before do + solo_owned.group_members = [member] + DeleteUserService.new(current_user).execute(user) + end + + it 'does not delete the user' do + expect(User.find(user.id)).to eq user + end + end + + context "deletions with solo owned groups" do + let(:solo_owned) { create(:group) } + let(:member) { create(:group_member) } + let(:user) { member.user } + + before do + solo_owned.group_members = [member] + DeleteUserService.new(current_user).execute(user, delete_solo_owned_groups: true) + end + + it 'deletes solo owned groups' do + expect { Project.find(solo_owned.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'deletes the user' do + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end +end diff --git a/spec/workers/delete_user_worker_spec.rb b/spec/workers/delete_user_worker_spec.rb index 975b0470cb..14c5652128 100644 --- a/spec/workers/delete_user_worker_spec.rb +++ b/spec/workers/delete_user_worker_spec.rb @@ -1,58 +1,20 @@ require 'spec_helper' describe DeleteUserWorker do - describe "Deletes a user and all their personal projects" do - let!(:user) { create(:user) } - let!(:current_user) { create(:user) } - let!(:namespace) { create(:namespace, owner: user) } - let!(:project) { create(:project, namespace: namespace) } + let!(:user) { create(:user) } + let!(:current_user) { create(:user) } - context 'no force flag given' do - before do - DeleteUserWorker.new.perform(current_user.id, user.id) - end + it "calls the DeleteUserWorker with the params it was given" do + expect_any_instance_of(DeleteUserService).to receive(:execute). + with(user, {}) - it 'deletes all personal projects' do - expect { Project.find(project.id) }.to raise_error(ActiveRecord::RecordNotFound) - end + DeleteUserWorker.new.perform(current_user.id, user.id) + end - it 'deletes the user' do - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - end - end + it "uses symbolized keys" do + expect_any_instance_of(DeleteUserService).to receive(:execute). + with(user, test: "test") - context "solo owned groups present" do - let(:solo_owned) { create(:group) } - let(:member) { create(:group_member) } - let(:user) { member.user } - - before do - solo_owned.group_members = [member] - DeleteUserWorker.new.perform(current_user.id, user.id) - end - - it 'does not delete the user' do - expect(User.find(user.id)).to eq user - end - end - - context "deletions with force" do - let(:solo_owned) { create(:group) } - let(:member) { create(:group_member) } - let(:user) { member.user } - - before do - solo_owned.group_members = [member] - DeleteUserWorker.new.perform(current_user.id, user.id, "delete_solo_owned_groups" => true) - end - - it 'deletes solo owned groups' do - expect { Project.find(solo_owned.id) }.to raise_error(ActiveRecord::RecordNotFound) - end - - it 'deletes the user' do - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - end - end + DeleteUserWorker.new.perform(current_user.id, user.id, "test" => "test") end end From 7ae573c75ac51413c04249f77ed8ca4e144b7549 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 15 Mar 2016 23:22:05 +0100 Subject: [PATCH 12/14] Bump Git version requirement to 2.7.3 --- CHANGELOG | 3 +++ README.md | 2 +- doc/install/installation.md | 2 +- lib/tasks/gitlab/check.rake | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index b120810ebd..6854e7cab8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -32,6 +32,9 @@ v 8.6.0 (unreleased) - Move group activity to separate page - Continue parameters are checked to ensure redirection goes to the same instance +v 8.5.6 + - Bump Git version requirement to 2.7.3 + v 8.5.5 - Ensure removing a project removes associated Todo entries - Prevent a 500 error in Todos when author was removed diff --git a/README.md b/README.md index 3ec1d4a776..208427fcf8 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ GitLab is a Ruby on Rails application that runs on the following software: - Ubuntu/Debian/CentOS/RHEL - Ruby (MRI) 2.1 -- Git 1.7.10+ +- Git 2.7.3+ - Redis 2.8+ - MySQL or PostgreSQL diff --git a/doc/install/installation.md b/doc/install/installation.md index 4f01139726..362b27350c 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -76,7 +76,7 @@ Make sure you have the right version of Git installed # Install Git sudo apt-get install -y git-core - # Make sure Git is version 1.7.10 or higher, for example 1.7.12 or 2.0.0 + # Make sure Git is version 2.7.3 or higher git --version Is the system packaged Git too old? Remove it and compile from source. diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 581ab26db7..27ed57efe5 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -913,7 +913,7 @@ namespace :gitlab do end def check_git_version - required_version = Gitlab::VersionInfo.new(1, 7, 10) + required_version = Gitlab::VersionInfo.new(2, 7, 3) current_version = Gitlab::VersionInfo.parse(run(%W(#{Gitlab.config.git.bin_path} --version))) puts "Your git bin path is \"#{Gitlab.config.git.bin_path}\"" From cd683714eb6240bb3d42dabb3f6dc470fc930fb4 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 15 Mar 2016 23:24:27 +0100 Subject: [PATCH 13/14] Fix changelog --- CHANGELOG | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 12f44a4e68..b83940d2f1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -40,8 +40,10 @@ v 8.6.0 (unreleased) - Move group activity to separate page - Continue parameters are checked to ensure redirection goes to the same instance -v 8.5.6 +v 8.5.7 - Bump Git version requirement to 2.7.3 + +v 8.5.6 - Obtain a lease before querying LDAP v 8.5.5 From 7e03b40221388bd0567be6609f2ededa7f934179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Tue, 15 Mar 2016 19:51:27 -0500 Subject: [PATCH 14/14] Return an empty Array when there aren't lines to parse. --- lib/gitlab/diff/parser.rb | 2 ++ spec/lib/gitlab/diff/parser_spec.rb | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/lib/gitlab/diff/parser.rb b/lib/gitlab/diff/parser.rb index d0f6ba23ab..d0815fc7ee 100644 --- a/lib/gitlab/diff/parser.rb +++ b/lib/gitlab/diff/parser.rb @@ -4,6 +4,8 @@ module Gitlab include Enumerable def parse(lines) + return [] if lines.blank? + @lines = lines line_obj_index = 0 line_old = 1 diff --git a/spec/lib/gitlab/diff/parser_spec.rb b/spec/lib/gitlab/diff/parser_spec.rb index f576c39284..cdff063a9e 100644 --- a/spec/lib/gitlab/diff/parser_spec.rb +++ b/spec/lib/gitlab/diff/parser_spec.rb @@ -90,4 +90,9 @@ eos end end end + + context 'when lines is empty' do + it { expect(parser.parse([])).to eq([]) } + it { expect(parser.parse(nil)).to eq([]) } + end end