From 22a2559412027e6125835a8d49e5b25230b7a933 Mon Sep 17 00:00:00 2001 From: Mauro Verrocchio Date: Tue, 21 Jun 2016 23:10:09 +0200 Subject: [PATCH 01/38] Apply responsive design for labels on graphs Labels Commits and Continuous Integration graphs now scales if on mobile. --- app/views/projects/graphs/ci/_build_times.haml | 7 ++++++- app/views/projects/graphs/ci/_builds.haml | 7 ++++++- app/views/projects/graphs/commits.html.haml | 4 ++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/views/projects/graphs/ci/_build_times.haml b/app/views/projects/graphs/ci/_build_times.haml index c58223fd39e..195f18afc76 100644 --- a/app/views/projects/graphs/ci/_build_times.haml +++ b/app/views/projects/graphs/ci/_build_times.haml @@ -19,4 +19,9 @@ ] } var ctx = $("#build_timesChart").get(0).getContext("2d"); - new Chart(ctx).Bar(data,{"scaleOverlay": true, responsive: true, maintainAspectRatio: false}); + var options = { scaleOverlay: true, responsive: true, maintainAspectRatio: false }; + if (window.innerWidth < 768) { + // Scale fonts if window width lower than 768px (iPad portrait) + options.scaleFontSize = 8 + } + new Chart(ctx).Bar(data, options); diff --git a/app/views/projects/graphs/ci/_builds.haml b/app/views/projects/graphs/ci/_builds.haml index 8fca07114fa..1fbf6ca2c1c 100644 --- a/app/views/projects/graphs/ci/_builds.haml +++ b/app/views/projects/graphs/ci/_builds.haml @@ -48,4 +48,9 @@ ] } var ctx = $("##{scope}Chart").get(0).getContext("2d"); - new Chart(ctx).Line(data,{"scaleOverlay": true, responsive: true, maintainAspectRatio: false}); + var options = { scaleOverlay: true, responsive: true, maintainAspectRatio: false }; + if (window.innerWidth < 768) { + // Scale fonts if window width lower than 768px (iPad portrait) + options.scaleFontSize = 8 + } + new Chart(ctx).Line(data, options); diff --git a/app/views/projects/graphs/commits.html.haml b/app/views/projects/graphs/commits.html.haml index 65db8af494d..7e34a89f9ae 100644 --- a/app/views/projects/graphs/commits.html.haml +++ b/app/views/projects/graphs/commits.html.haml @@ -59,6 +59,10 @@ var container = $(selector).parent(); var generateChart = function() { selector.attr('width', $(container).width()); + if (window.innerWidth < 768) { + // Scale fonts if window width lower than 768px (iPad portrait) + options.scaleFontSize = 8 + } return new Chart(ctx).Bar(data, options); }; // enabling auto-resizing From 8e096e724c4938b9112eb4d8a1892a9b8962ef3e Mon Sep 17 00:00:00 2001 From: Aurelio Jargas Date: Fri, 22 Jul 2016 10:10:53 +0000 Subject: [PATCH 02/38] Add missing escape at line end --- doc/ci/docker/using_docker_build.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md index 7f83f846454..0f64137a8a9 100644 --- a/doc/ci/docker/using_docker_build.md +++ b/doc/ci/docker/using_docker_build.md @@ -38,7 +38,7 @@ GitLab Runner then executes build scripts as the `gitlab-runner` user. $ sudo gitlab-ci-multi-runner register -n \ --url https://gitlab.com/ci \ --registration-token REGISTRATION_TOKEN \ - --executor shell + --executor shell \ --description "My Runner" ``` From d19e53ba9409b17144c4c9e041d1528f5ae07961 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Fri, 22 Jul 2016 09:15:14 -0700 Subject: [PATCH 03/38] Add branch or tag icon to ref in builds page --- app/views/projects/ci/builds/_build.html.haml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index a9fb3c58431..a3114771a42 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -22,6 +22,8 @@ - if defined?(ref) && ref - if build.ref + .icon-container + = build.tag? ? icon('tag') : icon('code-fork') = link_to build.ref, namespace_project_commits_path(build.project.namespace, build.project, build.ref), class: "monospace branch-name" - else .light none From 2b7e6e99ed78b525eaa7f9e034b9bcf1e4fc5746 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 25 Jul 2016 13:48:40 -0400 Subject: [PATCH 04/38] Add ENV variable to skip repository storages validations --- CHANGELOG | 1 + config/initializers/6_validations.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 4d2c281b82d..bddfe3ae102 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -13,6 +13,7 @@ v 8.11.0 (unreleased) v 8.10.2 (unreleased) - User can now search branches by name. !5144 + - Add ENV variable to skip repository storages validations - Fix backup restore. !5459 - Use project ID in repository cache to prevent stale data from persisting across projects. !5460 diff --git a/config/initializers/6_validations.rb b/config/initializers/6_validations.rb index 37746968675..d92f64e1647 100644 --- a/config/initializers/6_validations.rb +++ b/config/initializers/6_validations.rb @@ -26,4 +26,4 @@ def validate_storages end end -validate_storages unless Rails.env.test? +validate_storages unless Rails.env.test? || ENV['SKIP_STORAGE_VALIDATION'] == 'true' From f4aac773894bb17f550fff8ee548f2a18cd18b64 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 1 Jul 2016 12:51:21 -0700 Subject: [PATCH 05/38] Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable This significantly reduces the DB churn in the PostReceive task when it performs reference extraction. See #18663 --- CHANGELOG | 1 + config/initializers/sidekiq.rb | 1 + .../sidekiq_middleware/request_store_middleware.rb | 13 +++++++++++++ 3 files changed, 15 insertions(+) create mode 100644 lib/gitlab/sidekiq_middleware/request_store_middleware.rb diff --git a/CHANGELOG b/CHANGELOG index 534f57cb08e..0b796cc66a1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.11.0 (unreleased) - Remove magic comments (`# encoding: UTF-8`) from Ruby files !5456 (winniehell) - Fix CI status icon link underline (ClemMakesApps) - Fix of 'Commits being passed to custom hooks are already reachable when using the UI' + - Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable - Limit git rev-list output count to one in forced push check - Add green outline to New Branch button !5447 (winniehell) - Retrieve rendered HTML from cache in one request diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 5e839327e7a..48b6075ba78 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -7,6 +7,7 @@ Sidekiq.configure_server do |config| config.server_middleware do |chain| chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS'] chain.add Gitlab::SidekiqMiddleware::MemoryKiller if ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] + chain.add Gitlab::SidekiqMiddleware::RequestStoreMiddleware if ENV['SIDEKIQ_REQUEST_STORE'] end # Sidekiq-cron: load recurring jobs from gitlab.yml diff --git a/lib/gitlab/sidekiq_middleware/request_store_middleware.rb b/lib/gitlab/sidekiq_middleware/request_store_middleware.rb new file mode 100644 index 00000000000..b1fa0e3cb4e --- /dev/null +++ b/lib/gitlab/sidekiq_middleware/request_store_middleware.rb @@ -0,0 +1,13 @@ +module Gitlab + module SidekiqMiddleware + class RequestStoreMiddleware + def call(worker, job, queue) + RequestStore.begin! + yield + ensure + RequestStore.end! + RequestStore.clear! + end + end + end +end From 22386040fe5c1cd5b3ab163211b77c6ff254e88e Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 25 Jul 2016 18:05:14 -0700 Subject: [PATCH 06/38] Enable SIDEKIQ_REQUEST_STORE by default --- config/initializers/sidekiq.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 48b6075ba78..cf49ec2194c 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -7,7 +7,7 @@ Sidekiq.configure_server do |config| config.server_middleware do |chain| chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS'] chain.add Gitlab::SidekiqMiddleware::MemoryKiller if ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] - chain.add Gitlab::SidekiqMiddleware::RequestStoreMiddleware if ENV['SIDEKIQ_REQUEST_STORE'] + chain.add Gitlab::SidekiqMiddleware::RequestStoreMiddleware unless ENV['SIDEKIQ_REQUEST_STORE'] == '0' end # Sidekiq-cron: load recurring jobs from gitlab.yml From cfd103dbb55a37393966b764a55e0fe67b0232c3 Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Mon, 25 Jul 2016 10:01:02 -0500 Subject: [PATCH 07/38] Disable MySQL foreign key checks before dropping all tables --- CHANGELOG | 1 + lib/tasks/gitlab/db.rake | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 4d2c281b82d..02a7e2519d1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -14,6 +14,7 @@ v 8.11.0 (unreleased) v 8.10.2 (unreleased) - User can now search branches by name. !5144 - Fix backup restore. !5459 + - Disable MySQL foreign key checks before dropping all tables. !5472 - Use project ID in repository cache to prevent stale data from persisting across projects. !5460 v 8.10.1 diff --git a/lib/tasks/gitlab/db.rake b/lib/tasks/gitlab/db.rake index 0ec19e1a625..7c96bc864ce 100644 --- a/lib/tasks/gitlab/db.rake +++ b/lib/tasks/gitlab/db.rake @@ -25,6 +25,10 @@ namespace :gitlab do desc 'Drop all tables' task :drop_tables => :environment do connection = ActiveRecord::Base.connection + + # If MySQL, turn off foreign key checks + connection.execute('SET FOREIGN_KEY_CHECKS=0') if Gitlab::Database.mysql? + tables = connection.tables tables.delete 'schema_migrations' # Truncate schema_migrations to ensure migrations re-run @@ -35,6 +39,9 @@ namespace :gitlab do # MySQL: http://dev.mysql.com/doc/refman/5.7/en/drop-table.html # Add `IF EXISTS` because cascade could have already deleted a table. tables.each { |t| connection.execute("DROP TABLE IF EXISTS #{connection.quote_table_name(t)} CASCADE") } + + # If MySQL, re-enable foreign key checks + connection.execute('SET FOREIGN_KEY_CHECKS=1') if Gitlab::Database.mysql? end desc 'Configures the database by running migrate, or by loading the schema and seeding if needed' From 53b6e3b8d337142ba3c9241bd2041401f82b8b01 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 26 Jul 2016 00:34:13 -0300 Subject: [PATCH 08/38] Remove spec/features/projects/branches_spec.rb HEAD file --- spec/features/projects/branches_spec.rb~HEAD | 32 -------------------- 1 file changed, 32 deletions(-) delete mode 100644 spec/features/projects/branches_spec.rb~HEAD diff --git a/spec/features/projects/branches_spec.rb~HEAD b/spec/features/projects/branches_spec.rb~HEAD deleted file mode 100644 index 79abba21854..00000000000 --- a/spec/features/projects/branches_spec.rb~HEAD +++ /dev/null @@ -1,32 +0,0 @@ -require 'spec_helper' - -describe 'Branches', feature: true do - let(:project) { create(:project) } - let(:repository) { project.repository } - - before do - login_as :user - project.team << [@user, :developer] - end - - describe 'Initial branches page' do - it 'shows all the branches' do - visit namespace_project_branches_path(project.namespace, project) - - repository.branches { |branch| expect(page).to have_content("#{branch.name}") } - expect(page).to have_content("Protected branches can be managed in project settings") - end - end - - describe 'Find branches' do - it 'shows filtered branches', js: true do - visit namespace_project_branches_path(project.namespace, project, project.id) - - fill_in 'branch-search', with: 'fix' - find('#branch-search').native.send_keys(:enter) - - expect(page).to have_content('fix') - expect(find('.all-branches')).to have_selector('li', count: 1) - end - end -end From 939436480c8c6bd524d3c4c78674e81f90861d06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 25 Jul 2016 17:08:36 +0200 Subject: [PATCH 09/38] Ensure relative paths for video are rewritten as we do for images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG | 1 + app/models/blob.rb | 4 +++ app/models/commit.rb | 4 +-- lib/banzai/filter/relative_link_filter.rb | 2 +- spec/finders/branches_finder_spec.rb | 2 +- .../filter/relative_link_filter_spec.rb | 25 +++++++++++++++++++ spec/models/blob_spec.rb | 16 ++++++++++++ spec/models/commit_spec.rb | 1 + spec/support/test_env.rb | 3 ++- 9 files changed, 53 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4f542a86ce8..2cd4a173301 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -18,6 +18,7 @@ v 8.10.2 (unreleased) - Fix backup restore. !5459 - Disable MySQL foreign key checks before dropping all tables. !5472 - Use project ID in repository cache to prevent stale data from persisting across projects. !5460 + - Ensure relative paths for video are rewritten as we do for images. !5474 v 8.10.1 - Refactor repository storages documentation. !5428 diff --git a/app/models/blob.rb b/app/models/blob.rb index 4279ea2ce57..0df2805e448 100644 --- a/app/models/blob.rb +++ b/app/models/blob.rb @@ -31,6 +31,10 @@ class Blob < SimpleDelegator text? && language && language.name == 'SVG' end + def video? + UploaderHelper::VIDEO_EXT.include?(extname.downcase.delete('.')) + end + def to_partial_path if lfs_pointer? 'download' diff --git a/app/models/commit.rb b/app/models/commit.rb index 2ef3973c160..f80f1063406 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -295,8 +295,8 @@ class Commit def uri_type(path) entry = @raw.tree.path(path) if entry[:type] == :blob - blob = Gitlab::Git::Blob.new(name: entry[:name]) - blob.image? ? :raw : :blob + blob = ::Blob.decorate(Gitlab::Git::Blob.new(name: entry[:name])) + blob.image? || blob.video? ? :raw : :blob else entry[:type] end diff --git a/lib/banzai/filter/relative_link_filter.rb b/lib/banzai/filter/relative_link_filter.rb index 21ed0410f7f..337fb50317d 100644 --- a/lib/banzai/filter/relative_link_filter.rb +++ b/lib/banzai/filter/relative_link_filter.rb @@ -20,7 +20,7 @@ module Banzai process_link_attr el.attribute('href') end - doc.search('img').each do |el| + doc.css('img, video').each do |el| process_link_attr el.attribute('src') end diff --git a/spec/finders/branches_finder_spec.rb b/spec/finders/branches_finder_spec.rb index 9c9763d746b..dd85203a038 100644 --- a/spec/finders/branches_finder_spec.rb +++ b/spec/finders/branches_finder_spec.rb @@ -20,7 +20,7 @@ describe BranchesFinder do result = branches_finder.execute - expect(result.first.name).to eq('expand-collapse-lines') + expect(result.first.name).to eq('video') end it 'sorts by last_updated' do diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb index 2401875a057..9921171f2aa 100644 --- a/spec/lib/banzai/filter/relative_link_filter_spec.rb +++ b/spec/lib/banzai/filter/relative_link_filter_spec.rb @@ -17,6 +17,10 @@ describe Banzai::Filter::RelativeLinkFilter, lib: true do %() end + def video(path) + %() + end + def link(path) %(#{path}) end @@ -37,6 +41,12 @@ describe Banzai::Filter::RelativeLinkFilter, lib: true do doc = filter(image('files/images/logo-black.png')) expect(doc.at_css('img')['src']).to eq 'files/images/logo-black.png' end + + it 'does not modify any relative URL in video' do + doc = filter(video('files/videos/intro.mp4'), commit: project.commit('video'), ref: 'video') + + expect(doc.at_css('video')['src']).to eq 'files/videos/intro.mp4' + end end shared_examples :relative_to_requested do @@ -111,11 +121,26 @@ describe Banzai::Filter::RelativeLinkFilter, lib: true do end it 'rebuilds relative URL for an image in the repo' do + doc = filter(image('files/images/logo-black.png')) + + expect(doc.at_css('img')['src']). + to eq "/#{project_path}/raw/#{ref}/files/images/logo-black.png" + end + + it 'rebuilds relative URL for link to an image in the repo' do doc = filter(link('files/images/logo-black.png')) + expect(doc.at_css('a')['href']). to eq "/#{project_path}/raw/#{ref}/files/images/logo-black.png" end + it 'rebuilds relative URL for a video in the repo' do + doc = filter(video('files/videos/intro.mp4'), commit: project.commit('video'), ref: 'video') + + expect(doc.at_css('video')['src']). + to eq "/#{project_path}/raw/video/files/videos/intro.mp4" + end + it 'does not modify relative URL with an anchor only' do doc = filter(link('#section-1')) expect(doc.at_css('a')['href']).to eq '#section-1' diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 78e95c8fac5..1e5d6a34f83 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -33,6 +33,22 @@ describe Blob do end end + describe '#video?' do + it 'is falsey with image extension' do + git_blob = Gitlab::Git::Blob.new(name: 'image.png') + + expect(described_class.decorate(git_blob)).not_to be_video + end + + UploaderHelper::VIDEO_EXT.each do |ext| + it "is truthy when extension is .#{ext}" do + git_blob = Gitlab::Git::Blob.new(name: "video.#{ext}") + + expect(described_class.decorate(git_blob)).to be_video + end + end + end + describe '#to_partial_path' do def stubbed_blob(overrides = {}) overrides.reverse_merge!( diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ba02d5fe977..ec1544bf815 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -212,6 +212,7 @@ eos it 'returns the URI type at the given path' do expect(commit.uri_type('files/html')).to be(:tree) expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) + expect(project.commit('video').uri_type('files/videos/intro.mp4')).to be(:raw) expect(commit.uri_type('files/js/application.js')).to be(:blob) end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 83f2ad96fd8..3735abe2302 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -20,7 +20,8 @@ module TestEnv 'gitattributes' => '5a62481', 'expand-collapse-diffs' => '4842455', 'expand-collapse-files' => '025db92', - 'expand-collapse-lines' => '238e82d' + 'expand-collapse-lines' => '238e82d', + 'video' => '8879059' } # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily From b203256eb9b1037adbe86ff7ef128b4410a3a8df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 26 Jul 2016 10:28:50 +0200 Subject: [PATCH 10/38] Fix CHANGELOG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4f542a86ce8..233c355158b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,17 +1,18 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.11.0 (unreleased) - - Remove magic comments (`# encoding: UTF-8`) from Ruby files !5456 (winniehell) + - Remove magic comments (`# encoding: UTF-8`) from Ruby files. !5456 (winniehell) - Fix CI status icon link underline (ClemMakesApps) - Fix of 'Commits being passed to custom hooks are already reachable when using the UI' - Limit git rev-list output count to one in forced push check - - Add green outline to New Branch button !5447 (winniehell) + - Add green outline to New Branch button. !5447 (winniehell) - Retrieve rendered HTML from cache in one request - Nokogiri's various parsing methods are now instrumented - - Make fork counter always clickable !5463 (winniehell) - - Load project invited groups and members eagerly in ProjectTeam#fetch_members + - Make fork counter always clickable. !5463 (winniehell) + - Remove `search_id` of labels dropdown filter to fix 'Missleading URI for labels in Merge Requests and Issues view'. !5368 (Scott Le) + - Load project invited groups and members eagerly in `ProjectTeam#fetch_members` - Add GitLab Workhorse version to admin dashboard (Katarzyna Kobierska Ula Budziszewska) - - Add ES6 gem + - Add the `sprockets-es6` gem v 8.10.2 (unreleased) - User can now search branches by name. !5144 @@ -27,10 +28,6 @@ v 8.10.1 - Fix bug where replies to commit notes displayed in the MR discussion tab wouldn't show up on the commit page. !5446 - Ignore invalid trusted proxies in X-Forwarded-For header. !5454 - Add links to the real markdown.md file for all GFM examples. !5458 - - Remove `search_id` of labels dropdown filter to fix 'Missleading URI for labels in Merge Requests and Issues view' !5368 (Scott Le) - -v 8.10.1 (unreleased) - - Fix bug where replies to commit notes displayed in the MR discussion tab wouldn't show up on the commit page v 8.10.0 - Fix profile activity heatmap to show correct day name (eanplatter) From d06df33daa0013bfec624ee0ab3ef72e92b0f1cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 25 Jul 2016 18:35:00 +0200 Subject: [PATCH 11/38] Ensure current user can retry a build before showing the 'Retry' button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG | 1 + app/views/projects/builds/_sidebar.html.haml | 2 +- spec/features/builds_spec.rb | 30 +++++++++++++++++--- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index ef8c72f4aa2..3a61ce4c0d6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -20,6 +20,7 @@ v 8.10.2 (unreleased) - Disable MySQL foreign key checks before dropping all tables. !5472 - Use project ID in repository cache to prevent stale data from persisting across projects. !5460 - Ensure relative paths for video are rewritten as we do for images. !5474 + - Ensure current user can retry a build before showing the 'Retry' button. !5476 v 8.10.1 - Refactor repository storages documentation. !5428 diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index dc57b49f27a..b89183c40dc 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -40,7 +40,7 @@ .block{ class: ("block-first" if !@build.coverage && !(can?(current_user, :read_build, @project) && (@build.artifacts? || @build.artifacts_expired?))) } .title Build details - - if @build.retryable? + - if can?(current_user, :update_build, @build) && @build.retryable? = link_to "Retry", retry_namespace_project_build_path(@project.namespace, @project, @build), class: 'pull-right', method: :post - if @build.merge_request %p.build-detail-row diff --git a/spec/features/builds_spec.rb b/spec/features/builds_spec.rb index cab3dc1d167..0cfeb2e57d8 100644 --- a/spec/features/builds_spec.rb +++ b/spec/features/builds_spec.rb @@ -199,9 +199,13 @@ describe "Builds" do click_link 'Retry' end - it { expect(page.status_code).to eq(200) } - it { expect(page).to have_content 'pending' } - it { expect(page).to have_content 'Cancel' } + it 'shows the right status and buttons' do + expect(page).to have_http_status(200) + expect(page).to have_content 'pending' + page.within('aside.right-sidebar') do + expect(page).to have_content 'Cancel' + end + end end context "Build from other project" do @@ -212,7 +216,25 @@ describe "Builds" do page.driver.post(retry_namespace_project_build_path(@project.namespace, @project, @build2)) end - it { expect(page.status_code).to eq(404) } + it { expect(page).to have_http_status(404) } + end + + context "Build that current user is not allowed to retry" do + before do + @build.run! + @build.cancel! + @project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + + logout_direct + login_with(create(:user)) + visit namespace_project_build_path(@project.namespace, @project, @build) + end + + it 'does not show the Retry button' do + page.within('aside.right-sidebar') do + expect(page).not_to have_content 'Retry' + end + end end end From 226c224234d8ac67f22fb162953e7ac905d42ad8 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Tue, 26 Jul 2016 10:17:28 +0200 Subject: [PATCH 12/38] Rescue Rugged::OSError (lock exists) when creating references. --- CHANGELOG | 1 + app/models/repository.rb | 3 +++ 2 files changed, 4 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index ef8c72f4aa2..7821ca7b4b0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -17,6 +17,7 @@ v 8.11.0 (unreleased) v 8.10.2 (unreleased) - User can now search branches by name. !5144 - Fix backup restore. !5459 + - Rescue Rugged::OSError (lock exists) when creating references. !5497 - Disable MySQL foreign key checks before dropping all tables. !5472 - Use project ID in repository cache to prevent stale data from persisting across projects. !5460 - Ensure relative paths for video are rewritten as we do for images. !5474 diff --git a/app/models/repository.rb b/app/models/repository.rb index e9d5f4c91f8..d8775ecbd6c 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -211,6 +211,9 @@ class Repository rugged.references.create(keep_around_ref_name(sha), sha, force: true) rescue Rugged::ReferenceError => ex Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}" + rescue Rugged::OSError => ex + raise unless ex.message =~ /Failed to create locked file/ && ex.message =~ /File exists/ + Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}" end end From 4f2ef9a6e5e6997ef4bc70e435b9d0011b46f509 Mon Sep 17 00:00:00 2001 From: Katarzyna Kobierska Date: Wed, 20 Jul 2016 09:19:07 +0200 Subject: [PATCH 13/38] Multiple trigger variables show in separate line --- CHANGELOG | 6 ++++++ app/views/projects/builds/_sidebar.html.haml | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index ef8c72f4aa2..858e8c268d2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -29,6 +29,12 @@ v 8.10.1 - Fix bug where replies to commit notes displayed in the MR discussion tab wouldn't show up on the commit page. !5446 - Ignore invalid trusted proxies in X-Forwarded-For header. !5454 - Add links to the real markdown.md file for all GFM examples. !5458 + - Multiple trigger variables show in separate lines (Katarzyna Kobierska Ula Budziszewska) + +v 8.10.1 (unreleased) + - Fix Error 500 when creating Wiki pages with hyphens or spaces + - Ignore invalid trusted proxies in X-Forwarded-For header + - Fix bug where replies to commit notes displayed in the MR discussion tab wouldn't show up on the commit page v 8.10.0 - Fix profile activity heatmap to show correct day name (eanplatter) diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index dc57b49f27a..4794548e169 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -88,8 +88,9 @@ %p %span.build-light-text Variables: - %code - - @build.trigger_request.variables.each do |key, value| + + - @build.trigger_request.variables.each do |key, value| + %code #{key}=#{value} .block From 79c29a67caa56fdff888221a71a446a3c4e7ac5e Mon Sep 17 00:00:00 2001 From: Katarzyna Kobierska Date: Thu, 21 Jul 2016 11:23:21 +0200 Subject: [PATCH 14/38] Add test for multiple trigger variables view --- .../projects/builds/show.html.haml_spec.rb | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/spec/views/projects/builds/show.html.haml_spec.rb b/spec/views/projects/builds/show.html.haml_spec.rb index 42220a20c75..bc433a15a7c 100644 --- a/spec/views/projects/builds/show.html.haml_spec.rb +++ b/spec/views/projects/builds/show.html.haml_spec.rb @@ -44,9 +44,26 @@ describe 'projects/builds/show' do it 'shows commit title and not show commit message' do render - + expect(rendered).to have_css('p.build-light-text.append-bottom-0', text: /\A\n#{Regexp.escape(commit_title)}\n\Z/) end end + + describe 'shows trigger variables in sidebar' do + let(:trigger) { create(:ci_trigger, project: project) } + let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) } + + before do + build.trigger_request = trigger_request + end + + it 'shows trigger variables in separate lines' do + trigger_request.update_attributes(variables: { TRIGGER_KEY: 'TRIGGER_VALUE', TRIGGER_KEY_1: 'TRIGGER_VALUE_1' }) + render + + expect(rendered).to have_css('code', text: /\A#{Regexp.escape('TRIGGER_KEY=TRIGGER_VALUE')}\Z/) + expect(rendered).to have_css('code', text: /\A#{Regexp.escape('TRIGGER_KEY_1=TRIGGER_VALUE_1')}\Z/) + end + end end From 842de4efc082c4c5ff6dcbe1869e1046569e7807 Mon Sep 17 00:00:00 2001 From: Katarzyna Kobierska Date: Thu, 21 Jul 2016 13:50:01 +0200 Subject: [PATCH 15/38] Tests refactoring for trigger variables --- spec/factories/ci/trigger_requests.rb | 3 ++- .../projects/builds/show.html.haml_spec.rb | 17 ++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/spec/factories/ci/trigger_requests.rb b/spec/factories/ci/trigger_requests.rb index 6d47d05f8ad..b8d8fab0e0b 100644 --- a/spec/factories/ci/trigger_requests.rb +++ b/spec/factories/ci/trigger_requests.rb @@ -5,7 +5,8 @@ FactoryGirl.define do variables do { - TRIGGER_KEY: 'TRIGGER_VALUE' + TRIGGER_KEY_1: 'TRIGGER_VALUE_1', + TRIGGER_KEY_2: 'TRIGGER_VALUE_2' } end end diff --git a/spec/views/projects/builds/show.html.haml_spec.rb b/spec/views/projects/builds/show.html.haml_spec.rb index bc433a15a7c..88b96939720 100644 --- a/spec/views/projects/builds/show.html.haml_spec.rb +++ b/spec/views/projects/builds/show.html.haml_spec.rb @@ -51,19 +51,22 @@ describe 'projects/builds/show' do end describe 'shows trigger variables in sidebar' do - let(:trigger) { create(:ci_trigger, project: project) } - let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) } + let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline) } before do build.trigger_request = trigger_request + render end it 'shows trigger variables in separate lines' do - trigger_request.update_attributes(variables: { TRIGGER_KEY: 'TRIGGER_VALUE', TRIGGER_KEY_1: 'TRIGGER_VALUE_1' }) - render - - expect(rendered).to have_css('code', text: /\A#{Regexp.escape('TRIGGER_KEY=TRIGGER_VALUE')}\Z/) - expect(rendered).to have_css('code', text: /\A#{Regexp.escape('TRIGGER_KEY_1=TRIGGER_VALUE_1')}\Z/) + expect(rendered).to have_css('code', text: variable_regexp('TRIGGER_KEY_1','TRIGGER_VALUE_1')) + expect(rendered).to have_css('code', text: variable_regexp('TRIGGER_KEY_2', 'TRIGGER_VALUE_2')) end end + + private + + def variable_regexp(key, value) + /\A#{Regexp.escape("#{key}=#{value}")}\Z/ + end end From 940382793384547ad87baf8b38a16bc501243a88 Mon Sep 17 00:00:00 2001 From: Katarzyna Kobierska Date: Mon, 25 Jul 2016 11:21:20 +0200 Subject: [PATCH 16/38] Fix tests --- spec/models/build_spec.rb | 2 +- spec/requests/ci/api/builds_spec.rb | 2 +- spec/views/projects/builds/show.html.haml_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 978ad9c52d5..dc88697199b 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -259,7 +259,7 @@ describe Ci::Build, models: true do let(:trigger) { create(:ci_trigger, project: project) } let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) } let(:user_trigger_variable) do - { key: :TRIGGER_KEY, value: 'TRIGGER_VALUE', public: false } + { key: :TRIGGER_KEY_1, value: 'TRIGGER_VALUE_1', public: false } end let(:predefined_trigger_variable) do { key: 'CI_BUILD_TRIGGERED', value: 'true', public: true } diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 1c7c60ec644..cf1e8d9b514 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -98,7 +98,7 @@ describe Ci::API::API do { "key" => "CI_BUILD_TRIGGERED", "value" => "true", "public" => true }, { "key" => "DB_NAME", "value" => "postgres", "public" => true }, { "key" => "SECRET_KEY", "value" => "secret_value", "public" => false }, - { "key" => "TRIGGER_KEY", "value" => "TRIGGER_VALUE", "public" => false } + { "key" => "TRIGGER_KEY_1", "value" => "TRIGGER_VALUE_1", "public" => false } ) end diff --git a/spec/views/projects/builds/show.html.haml_spec.rb b/spec/views/projects/builds/show.html.haml_spec.rb index 88b96939720..464051063d8 100644 --- a/spec/views/projects/builds/show.html.haml_spec.rb +++ b/spec/views/projects/builds/show.html.haml_spec.rb @@ -59,7 +59,7 @@ describe 'projects/builds/show' do end it 'shows trigger variables in separate lines' do - expect(rendered).to have_css('code', text: variable_regexp('TRIGGER_KEY_1','TRIGGER_VALUE_1')) + expect(rendered).to have_css('code', text: variable_regexp('TRIGGER_KEY_1', 'TRIGGER_VALUE_1')) expect(rendered).to have_css('code', text: variable_regexp('TRIGGER_KEY_2', 'TRIGGER_VALUE_2')) end end From 34a1e9633fa74c509a408c1ab8cf80de9e2e125c Mon Sep 17 00:00:00 2001 From: Josef Strzibny Date: Wed, 4 May 2016 00:32:14 +0200 Subject: [PATCH 17/38] Clean up unused routes --- config/routes.rb | 26 ++++++++++++-------------- spec/routing/admin_routing_spec.rb | 5 ----- spec/routing/project_routing_spec.rb | 4 ---- spec/routing/routing_spec.rb | 8 -------- 4 files changed, 12 insertions(+), 31 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 21f3585bacd..4047bfecdc5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -42,10 +42,9 @@ Rails.application.routes.draw do resource :lint, only: [:show, :create] - resources :projects do + resources :projects, only: [:index, :show] do member do get :status, to: 'projects#badge' - get :integration end end @@ -137,20 +136,20 @@ Rails.application.routes.draw do # Import # namespace :import do - resource :github, only: [:create, :new], controller: :github do + resource :github, only: [:create], controller: :github do post :personal_access_token get :status get :callback get :jobs end - resource :gitlab, only: [:create, :new], controller: :gitlab do + resource :gitlab, only: [:create], controller: :gitlab do get :status get :callback get :jobs end - resource :bitbucket, only: [:create, :new], controller: :bitbucket do + resource :bitbucket, only: [:create], controller: :bitbucket do get :status get :callback get :jobs @@ -243,7 +242,6 @@ Rails.application.routes.draw do get :projects get :keys get :groups - put :team_update put :block put :unblock put :unlock @@ -296,11 +294,11 @@ Rails.application.routes.draw do post :repository_check end - resources :runner_projects, only: [:create, :destroy] + resources :runner_projects, only: [:index, :create, :destroy] end end - resource :appearances, path: 'appearance' do + resource :appearances, only: [:show, :create, :update], path: 'appearance' do member do get :preview delete :logo @@ -309,7 +307,7 @@ Rails.application.routes.draw do end resource :application_settings, only: [:show, :update] do - resources :services + resources :services, only: [:index, :edit, :update] put :reset_runners_token put :reset_health_check_token put :clear_repository_check_states @@ -346,7 +344,7 @@ Rails.application.routes.draw do end scope module: :profiles do - resource :account, only: [:show, :update] do + resource :account, only: [:show] do member do delete :unlink end @@ -358,7 +356,7 @@ Rails.application.routes.draw do end end resource :preferences, only: [:show, :update] - resources :keys + resources :keys, only: [:index, :show, :new, :create, :destroy] resources :emails, only: [:index, :create, :destroy] resource :avatar, only: [:destroy] @@ -660,7 +658,7 @@ Rails.application.routes.draw do post '/wikis/*id/markdown_preview', to: 'wikis#markdown_preview', constraints: WIKI_SLUG_ID, as: 'wiki_markdown_preview' end - resource :repository, only: [:show, :create] do + resource :repository, only: [:create] do member do get 'archive', constraints: { format: Gitlab::Regex.archive_formats_regex } end @@ -782,7 +780,7 @@ Rails.application.routes.draw do end end - resources :labels, constraints: { id: /\d+/ } do + resources :labels, except: [:show], constraints: { id: /\d+/ } do collection do post :generate post :set_priorities @@ -807,7 +805,7 @@ Rails.application.routes.draw do end end - resources :project_members, except: [:new, :edit], constraints: { id: /[a-zA-Z.\/0-9_\-#%+]+/ }, concerns: :access_requestable do + resources :project_members, except: [:show, :new, :edit], constraints: { id: /[a-zA-Z.\/0-9_\-#%+]+/ }, concerns: :access_requestable do collection do delete :leave diff --git a/spec/routing/admin_routing_spec.rb b/spec/routing/admin_routing_spec.rb index 8b19936ae6d..69eeb45ed71 100644 --- a/spec/routing/admin_routing_spec.rb +++ b/spec/routing/admin_routing_spec.rb @@ -1,6 +1,5 @@ require 'spec_helper' -# team_update_admin_user PUT /admin/users/:id/team_update(.:format) admin/users#team_update # block_admin_user PUT /admin/users/:id/block(.:format) admin/users#block # unblock_admin_user PUT /admin/users/:id/unblock(.:format) admin/users#unblock # admin_users GET /admin/users(.:format) admin/users#index @@ -11,10 +10,6 @@ require 'spec_helper' # PUT /admin/users/:id(.:format) admin/users#update # DELETE /admin/users/:id(.:format) admin/users#destroy describe Admin::UsersController, "routing" do - it "to #team_update" do - expect(put("/admin/users/1/team_update")).to route_to('admin/users#team_update', id: '1') - end - it "to #block" do expect(put("/admin/users/1/block")).to route_to('admin/users#block', id: '1') end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 620f328a114..9151cd3aefe 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -135,10 +135,6 @@ describe Projects::RepositoriesController, 'routing' do it 'to #archive format:tar.bz2' do expect(get('/gitlab/gitlabhq/repository/archive.tar.bz2')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.bz2') end - - it 'to #show' do - expect(get('/gitlab/gitlabhq/repository')).to route_to('projects/repositories#show', namespace_id: 'gitlab', project_id: 'gitlabhq') - end end describe Projects::BranchesController, 'routing' do diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 0a52c1ab933..1d4df9197f6 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -176,18 +176,10 @@ describe Profiles::KeysController, "routing" do expect(post("/profile/keys")).to route_to('profiles/keys#create') end - it "to #edit" do - expect(get("/profile/keys/1/edit")).to route_to('profiles/keys#edit', id: '1') - end - it "to #show" do expect(get("/profile/keys/1")).to route_to('profiles/keys#show', id: '1') end - it "to #update" do - expect(put("/profile/keys/1")).to route_to('profiles/keys#update', id: '1') - end - it "to #destroy" do expect(delete("/profile/keys/1")).to route_to('profiles/keys#destroy', id: '1') end From e9f132eb7f859360d1cc4da342b81ea4175c6eb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 26 Jul 2016 12:07:49 +0200 Subject: [PATCH 18/38] Add a CHANGELOG entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 181829a86a5..3613c1276f4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,7 @@ v 8.11.0 (unreleased) - Fix of 'Commits being passed to custom hooks are already reachable when using the UI' - Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable - Limit git rev-list output count to one in forced push check + - Clean up unused routes (Josef Strzibny) - Add green outline to New Branch button. !5447 (winniehell) - Retrieve rendered HTML from cache in one request - Nokogiri's various parsing methods are now instrumented From ea69ee1942c2e983cb91c197b0df82b7783badf1 Mon Sep 17 00:00:00 2001 From: Katarzyna Kobierska Date: Tue, 26 Jul 2016 12:38:35 +0200 Subject: [PATCH 19/38] Update CHANGELOG --- CHANGELOG | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 858e8c268d2..1db3465c182 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -13,6 +13,7 @@ v 8.11.0 (unreleased) - Load project invited groups and members eagerly in `ProjectTeam#fetch_members` - Add GitLab Workhorse version to admin dashboard (Katarzyna Kobierska Ula Budziszewska) - Add the `sprockets-es6` gem + - Multiple trigger variables show in separate lines (Katarzyna Kobierska Ula Budziszewska) v 8.10.2 (unreleased) - User can now search branches by name. !5144 @@ -29,12 +30,6 @@ v 8.10.1 - Fix bug where replies to commit notes displayed in the MR discussion tab wouldn't show up on the commit page. !5446 - Ignore invalid trusted proxies in X-Forwarded-For header. !5454 - Add links to the real markdown.md file for all GFM examples. !5458 - - Multiple trigger variables show in separate lines (Katarzyna Kobierska Ula Budziszewska) - -v 8.10.1 (unreleased) - - Fix Error 500 when creating Wiki pages with hyphens or spaces - - Ignore invalid trusted proxies in X-Forwarded-For header - - Fix bug where replies to commit notes displayed in the MR discussion tab wouldn't show up on the commit page v 8.10.0 - Fix profile activity heatmap to show correct day name (eanplatter) From 065c428a8bcf13584c4df9aefc7e39a23ea8115f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 26 Jul 2016 12:45:03 +0200 Subject: [PATCH 20/38] Add route for Import::GithubController#new MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 4047bfecdc5..0e03d2b4c32 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -136,7 +136,7 @@ Rails.application.routes.draw do # Import # namespace :import do - resource :github, only: [:create], controller: :github do + resource :github, only: [:create, :new], controller: :github do post :personal_access_token get :status get :callback From 3abffb2be25844920d6e0b2ac9735ebb38b78ef5 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 26 Jul 2016 12:19:02 +0100 Subject: [PATCH 21/38] Fix expand all diffs button in compare view We can't reuse the existing value of the format parameter, because on the merge request page that's 'json', so the link would go to a JSON file. We can't set it to HTML, because that adds '.html' the URL, which breaks the compare view (and looks bad). Setting it to nil uses the default format, which in all of these cases is HTML anyway. --- CHANGELOG | 1 + app/views/projects/diffs/_diffs.html.haml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 181829a86a5..f3abe9f9a59 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -22,6 +22,7 @@ v 8.10.2 (unreleased) - Use project ID in repository cache to prevent stale data from persisting across projects. !5460 - Ensure relative paths for video are rewritten as we do for images. !5474 - Ensure current user can retry a build before showing the 'Retry' button. !5476 + - Fix expand all diffs button in compare view v 8.10.1 - Refactor repository storages documentation. !5428 diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index 8ae433b4823..4bf3ccace20 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -7,7 +7,7 @@ .content-block.oneline-block.files-changed .inline-parallel-buttons - if !expand_all_diffs? && diff_files.any? { |diff_file| diff_file.collapsed? } - = link_to 'Expand all', url_for(params.merge(expand_all_diffs: 1, format: 'html')), class: 'btn btn-default' + = link_to 'Expand all', url_for(params.merge(expand_all_diffs: 1, format: nil)), class: 'btn btn-default' - if show_whitespace_toggle - if current_controller?(:commit) = commit_diff_whitespace_link(@project, @commit, class: 'hidden-xs') From e44bbcb99419357adbab798e7435f61e1c8047d7 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 26 Jul 2016 16:31:25 +0100 Subject: [PATCH 22/38] Show release notes in tag list A release's tag reference is just the name of the tag, not the entire tag object. This also fixes the tags index if a tag's message contains non-UTF8 byte sequences. --- CHANGELOG | 1 + app/controllers/projects/tags_controller.rb | 3 ++- .../projects/tags_controller_spec.rb | 20 +++++++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 spec/controllers/projects/tags_controller_spec.rb diff --git a/CHANGELOG b/CHANGELOG index dca9b209582..432d251dfc6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -21,6 +21,7 @@ v 8.10.2 (unreleased) - Fix backup restore. !5459 - Rescue Rugged::OSError (lock exists) when creating references. !5497 - Disable MySQL foreign key checks before dropping all tables. !5472 + - Show release notes in tags list - Use project ID in repository cache to prevent stale data from persisting across projects. !5460 - Ensure relative paths for video are rewritten as we do for images. !5474 - Ensure current user can retry a build before showing the 'Retry' button. !5476 diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index 6dc495247c8..8592579abbd 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -10,11 +10,12 @@ class Projects::TagsController < Projects::ApplicationController @tags = @repository.tags_sorted_by(@sort) @tags = Kaminari.paginate_array(@tags).page(params[:page]) - @releases = project.releases.where(tag: @tags) + @releases = project.releases.where(tag: @tags.map(&:name)) end def show @tag = @repository.find_tag(params[:id]) + @release = @project.releases.find_or_initialize_by(tag: @tag.name) @commit = @repository.commit(@tag.target) end diff --git a/spec/controllers/projects/tags_controller_spec.rb b/spec/controllers/projects/tags_controller_spec.rb new file mode 100644 index 00000000000..a6995145cc1 --- /dev/null +++ b/spec/controllers/projects/tags_controller_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Projects::TagsController do + let(:project) { create(:project, :public) } + let!(:release) { create(:release, project: project) } + let!(:invalid_release) { create(:release, project: project, tag: 'does-not-exist') } + + describe 'GET index' do + before { get :index, namespace_id: project.namespace.to_param, project_id: project.to_param } + + it 'returns the tags for the page' do + expect(assigns(:tags).map(&:name)).to eq(['v1.1.0', 'v1.0.0']) + end + + it 'returns releases matching those tags' do + expect(assigns(:releases)).to include(release) + expect(assigns(:releases)).not_to include(invalid_release) + end + end +end From 0187c85b780390ee8a119091e98a2bea18a43019 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Tue, 26 Jul 2016 11:21:07 -0600 Subject: [PATCH 23/38] Upgrade database_cleaner from 1.4.1 to 1.5.3. This includes support for Rails 5 without deprecations warnings. Changelog: https://github.com/DatabaseCleaner/database_cleaner/blob/master/History.rdoc#153-2016-04-22 --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index f2ac74a5976..8e5757eb2db 100644 --- a/Gemfile +++ b/Gemfile @@ -275,7 +275,7 @@ group :development, :test do gem 'awesome_print', '~> 1.2.0', require: false gem 'fuubar', '~> 2.0.0' - gem 'database_cleaner', '~> 1.4.0' + gem 'database_cleaner', '~> 1.5.0' gem 'factory_girl_rails', '~> 4.6.0' gem 'rspec-rails', '~> 3.5.0' gem 'rspec-retry', '~> 0.4.5' diff --git a/Gemfile.lock b/Gemfile.lock index bfa7e38da85..b6379b52f34 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -153,7 +153,7 @@ GEM d3_rails (3.5.11) railties (>= 3.1.0) daemons (1.2.3) - database_cleaner (1.4.1) + database_cleaner (1.5.3) debug_inspector (0.0.2) debugger-ruby_core_source (1.3.8) default_value_for (3.0.1) @@ -841,7 +841,7 @@ DEPENDENCIES connection_pool (~> 2.0) creole (~> 0.5.0) d3_rails (~> 3.5.0) - database_cleaner (~> 1.4.0) + database_cleaner (~> 1.5.0) default_value_for (~> 3.0.0) devise (~> 4.0) devise-two-factor (~> 3.0.0) From 345cd22f21e4e5a6e340c35e50b43105ee107570 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Fri, 15 Jul 2016 17:46:39 +0200 Subject: [PATCH 24/38] Profile requests when a header is passed --- CHANGELOG | 1 + Gemfile | 2 + Gemfile.lock | 2 + .../admin/requests_profiles_controller.rb | 17 +++++++ .../admin/background_jobs/_head.html.haml | 4 ++ .../admin/requests_profiles/index.html.haml | 26 ++++++++++ app/views/layouts/nav/_admin.html.haml | 2 +- app/workers/requests_profiles_worker.rb | 9 ++++ config/initializers/1_settings.rb | 3 ++ config/initializers/request_profiler.rb | 3 ++ config/routes.rb | 1 + lib/gitlab/request_profiler.rb | 19 ++++++++ lib/gitlab/request_profiler/middleware.rb | 47 +++++++++++++++++++ lib/gitlab/request_profiler/profile.rb | 43 +++++++++++++++++ 14 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 app/controllers/admin/requests_profiles_controller.rb create mode 100644 app/views/admin/requests_profiles/index.html.haml create mode 100644 app/workers/requests_profiles_worker.rb create mode 100644 config/initializers/request_profiler.rb create mode 100644 lib/gitlab/request_profiler.rb create mode 100644 lib/gitlab/request_profiler/middleware.rb create mode 100644 lib/gitlab/request_profiler/profile.rb diff --git a/CHANGELOG b/CHANGELOG index 432d251dfc6..3bf20b639fe 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ v 8.11.0 (unreleased) - Add GitLab Workhorse version to admin dashboard (Katarzyna Kobierska Ula Budziszewska) - Add the `sprockets-es6` gem - Multiple trigger variables show in separate lines (Katarzyna Kobierska Ula Budziszewska) + - Profile requests when a header is passed v 8.10.2 (unreleased) - User can now search branches by name. !5144 diff --git a/Gemfile b/Gemfile index 8e5757eb2db..85e30a0ee6f 100644 --- a/Gemfile +++ b/Gemfile @@ -334,6 +334,8 @@ gem 'mail_room', '~> 0.8' gem 'email_reply_parser', '~> 0.5.8' +gem 'ruby-prof', '~> 0.15.9' + ## CI gem 'activerecord-session_store', '~> 1.0.0' gem 'nested_form', '~> 0.3.2' diff --git a/Gemfile.lock b/Gemfile.lock index b6379b52f34..2039a0bb421 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -620,6 +620,7 @@ GEM rubocop (>= 0.40.0) ruby-fogbugz (0.2.1) crack (~> 0.4) + ruby-prof (0.15.9) ruby-progressbar (1.8.1) ruby-saml (1.3.0) nokogiri (>= 1.5.10) @@ -948,6 +949,7 @@ DEPENDENCIES rubocop (~> 0.41.2) rubocop-rspec (~> 1.5.0) ruby-fogbugz (~> 0.2.1) + ruby-prof (~> 0.15.9) sanitize (~> 2.0) sass-rails (~> 5.0.0) scss_lint (~> 0.47.0) diff --git a/app/controllers/admin/requests_profiles_controller.rb b/app/controllers/admin/requests_profiles_controller.rb new file mode 100644 index 00000000000..a478176e138 --- /dev/null +++ b/app/controllers/admin/requests_profiles_controller.rb @@ -0,0 +1,17 @@ +class Admin::RequestsProfilesController < Admin::ApplicationController + def index + @profile_token = Gitlab::RequestProfiler.profile_token + @profiles = Gitlab::RequestProfiler::Profile.all.group_by(&:request_path) + end + + def show + clean_name = Rack::Utils.clean_path_info(params[:name]) + profile = Gitlab::RequestProfiler::Profile.find(clean_name) + + if profile + render text: profile.content + else + redirect_to admin_requests_profiles_path, alert: 'Profile not found' + end + end +end diff --git a/app/views/admin/background_jobs/_head.html.haml b/app/views/admin/background_jobs/_head.html.haml index 9d722bd7382..89d7a40d6b0 100644 --- a/app/views/admin/background_jobs/_head.html.haml +++ b/app/views/admin/background_jobs/_head.html.haml @@ -16,3 +16,7 @@ = link_to admin_health_check_path, title: 'Health Check' do %span Health Check + = nav_link(controller: :requests_profiles) do + = link_to admin_requests_profiles_path, title: 'Requests Profiles' do + %span + Requests Profiles diff --git a/app/views/admin/requests_profiles/index.html.haml b/app/views/admin/requests_profiles/index.html.haml new file mode 100644 index 00000000000..ae918086a57 --- /dev/null +++ b/app/views/admin/requests_profiles/index.html.haml @@ -0,0 +1,26 @@ +- @no_container = true +- page_title 'Requests Profiles' += render 'admin/background_jobs/head' + +%div{ class: container_class } + %h3.page-title + = page_title + + .bs-callout.clearfix + Pass the header + %code X-Profile-Token: #{@profile_token} + to profile the request + + - if @profiles.present? + .prepend-top-default + - @profiles.each do |path, profiles| + .panel.panel-default.panel-small + .panel-heading + %code= path + %ul.content-list + - profiles.each do |profile| + %li + = link_to profile.time.to_s(:long), admin_requests_profile_path(profile), data: {no_turbolink: true} + - else + %p + No profiles found diff --git a/app/views/layouts/nav/_admin.html.haml b/app/views/layouts/nav/_admin.html.haml index 5ee8772882e..ac04f57e217 100644 --- a/app/views/layouts/nav/_admin.html.haml +++ b/app/views/layouts/nav/_admin.html.haml @@ -9,7 +9,7 @@ = link_to admin_root_path, title: 'Overview', class: 'shortcuts-tree' do %span Overview - = nav_link(controller: %w(system_info background_jobs logs health_check)) do + = nav_link(controller: %w(system_info background_jobs logs health_check requests_profiles)) do = link_to admin_system_info_path, title: 'Monitoring' do %span Monitoring diff --git a/app/workers/requests_profiles_worker.rb b/app/workers/requests_profiles_worker.rb new file mode 100644 index 00000000000..9dd228a2483 --- /dev/null +++ b/app/workers/requests_profiles_worker.rb @@ -0,0 +1,9 @@ +class RequestsProfilesWorker + include Sidekiq::Worker + + sidekiq_options queue: :default + + def perform + Gitlab::RequestProfiler.remove_all_profiles + end +end diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 86f55210487..49130f37b31 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -290,6 +290,9 @@ Settings.cron_jobs['repository_archive_cache_worker']['job_class'] = 'Repository Settings.cron_jobs['gitlab_remove_project_export_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['gitlab_remove_project_export_worker']['cron'] ||= '0 * * * *' Settings.cron_jobs['gitlab_remove_project_export_worker']['job_class'] = 'GitlabRemoveProjectExportWorker' +Settings.cron_jobs['requests_profiles_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['requests_profiles_worker']['cron'] ||= '0 0 * * *' +Settings.cron_jobs['requests_profiles_worker']['job_class'] = 'RequestsProfilesWorker' # # GitLab Shell diff --git a/config/initializers/request_profiler.rb b/config/initializers/request_profiler.rb new file mode 100644 index 00000000000..fb5a7b8372e --- /dev/null +++ b/config/initializers/request_profiler.rb @@ -0,0 +1,3 @@ +Rails.application.configure do |config| + config.middleware.use(Gitlab::RequestProfiler::Middleware) +end diff --git a/config/routes.rb b/config/routes.rb index 21f3585bacd..a41a04a0b38 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -281,6 +281,7 @@ Rails.application.routes.draw do resource :health_check, controller: 'health_check', only: [:show] resource :background_jobs, controller: 'background_jobs', only: [:show] resource :system_info, controller: 'system_info', only: [:show] + resources :requests_profiles, only: [:index, :show], param: :name resources :namespaces, path: '/projects', constraints: { id: /[a-zA-Z.0-9_\-]+/ }, only: [] do root to: 'projects#index', as: :projects diff --git a/lib/gitlab/request_profiler.rb b/lib/gitlab/request_profiler.rb new file mode 100644 index 00000000000..8130e55351e --- /dev/null +++ b/lib/gitlab/request_profiler.rb @@ -0,0 +1,19 @@ +require 'fileutils' + +module Gitlab + module RequestProfiler + PROFILES_DIR = "#{Gitlab.config.shared.path}/tmp/requests_profiles" + + def profile_token + Rails.cache.fetch('profile-token') do + Devise.friendly_token + end + end + module_function :profile_token + + def remove_all_profiles + FileUtils.rm_rf(PROFILES_DIR) + end + module_function :remove_all_profiles + end +end diff --git a/lib/gitlab/request_profiler/middleware.rb b/lib/gitlab/request_profiler/middleware.rb new file mode 100644 index 00000000000..8da8b754975 --- /dev/null +++ b/lib/gitlab/request_profiler/middleware.rb @@ -0,0 +1,47 @@ +require 'ruby-prof' + +module Gitlab + module RequestProfiler + class Middleware + def initialize(app) + @app = app + end + + def call(env) + if profile?(env) + call_with_profiling(env) + else + @app.call(env) + end + end + + def profile?(env) + header_token = env['HTTP_X_PROFILE_TOKEN'] + return unless header_token.present? + + profile_token = RequestProfiler.profile_token + return unless profile_token.present? + + header_token == profile_token + end + + def call_with_profiling(env) + ret = nil + result = RubyProf::Profile.profile do + ret = @app.call(env) + end + + printer = RubyProf::CallStackPrinter.new(result) + file_name = "#{env['PATH_INFO'].tr('/', '|')}_#{Time.current.to_i}.html" + file_path = "#{PROFILES_DIR}/#{file_name}" + + FileUtils.mkdir_p(PROFILES_DIR) + File.open(file_path, 'wb') do |file| + printer.print(file) + end + + ret + end + end + end +end diff --git a/lib/gitlab/request_profiler/profile.rb b/lib/gitlab/request_profiler/profile.rb new file mode 100644 index 00000000000..f89d56903ef --- /dev/null +++ b/lib/gitlab/request_profiler/profile.rb @@ -0,0 +1,43 @@ +module Gitlab + module RequestProfiler + class Profile + attr_reader :name, :time, :request_path + + alias_method :to_param, :name + + def self.all + Dir["#{PROFILES_DIR}/*.html"].map do |path| + new(File.basename(path)) + end + end + + def self.find(name) + name_dup = name.dup + name_dup << '.html' unless name.end_with?('.html') + + file_path = "#{PROFILES_DIR}/#{name_dup}" + return unless File.exist?(file_path) + + new(name_dup) + end + + def initialize(name) + @name = name + + set_attributes + end + + def content + File.read("#{PROFILES_DIR}/#{name}") + end + + private + + def set_attributes + _, path, timestamp = name.split(/(.*)_(\d+)\.html$/) + @request_path = path.tr('|', '/') + @time = Time.at(timestamp.to_i).utc + end + end + end +end From 21209a87df975deec3f1184c7622df7b57ad1e13 Mon Sep 17 00:00:00 2001 From: winniehell Date: Sun, 24 Jul 2016 22:13:46 +0200 Subject: [PATCH 25/38] Make branches sortable without push permission (!5462) --- CHANGELOG | 1 + app/views/projects/branches/index.html.haml | 43 +++++++++++---------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 432d251dfc6..93af465e2a0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -12,6 +12,7 @@ v 8.11.0 (unreleased) - Make fork counter always clickable. !5463 (winniehell) - Remove `search_id` of labels dropdown filter to fix 'Missleading URI for labels in Merge Requests and Issues view'. !5368 (Scott Le) - Load project invited groups and members eagerly in `ProjectTeam#fetch_members` + - Make branches sortable without push permission !5462 (winniehell) - Add GitLab Workhorse version to admin dashboard (Katarzyna Kobierska Ula Budziszewska) - Add the `sprockets-es6` gem - Multiple trigger variables show in separate lines (Katarzyna Kobierska Ula Budziszewska) diff --git a/app/views/projects/branches/index.html.haml b/app/views/projects/branches/index.html.haml index 6f806e3ce53..cb190d121c2 100644 --- a/app/views/projects/branches/index.html.haml +++ b/app/views/projects/branches/index.html.haml @@ -7,28 +7,31 @@ .nav-text Protected branches can be managed in project settings - - if can? current_user, :push_code, @project - .nav-controls - = form_tag(filter_branches_path, method: :get) do - = search_field_tag :search, params[:search], { placeholder: 'Filter by branch name', id: 'branch-search', class: 'form-control search-text-input input-short', spellcheck: false } - .dropdown.inline - %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} - %span.light - - if params[:sort].present? - = params[:sort].humanize - - else - Name - %b.caret - %ul.dropdown-menu.dropdown-menu-align-right - %li - = link_to filter_branches_path(sort: nil) do - = sort_title_name - = link_to filter_branches_path(sort: 'recently_updated') do - = sort_title_recently_updated - = link_to filter_branches_path(sort: 'last_updated') do - = sort_title_oldest_updated + .nav-controls + = form_tag(filter_branches_path, method: :get) do + = search_field_tag :search, params[:search], { placeholder: 'Filter by branch name', id: 'branch-search', class: 'form-control search-text-input input-short', spellcheck: false } + + .dropdown.inline + %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} + %span.light + - if params[:sort].present? + = params[:sort].humanize + - else + Name + %b.caret + %ul.dropdown-menu.dropdown-menu-align-right + %li + = link_to filter_branches_path(sort: nil) do + = sort_title_name + = link_to filter_branches_path(sort: 'recently_updated') do + = sort_title_recently_updated + = link_to filter_branches_path(sort: 'last_updated') do + = sort_title_oldest_updated + + - if can? current_user, :push_code, @project = link_to new_namespace_project_branch_path(@project.namespace, @project), class: 'btn btn-create' do New branch + - if @branches.any? %ul.content-list.all-branches - @branches.each do |branch| From 92cd19709b5abb3af8852c496ce65adc85e94043 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 15 Jul 2016 10:20:29 -0600 Subject: [PATCH 26/38] Remove inline scripts from import pages. Instead add data attributes to a JS hook element on every import status page. --- app/assets/javascripts/importer_status.js | 8 ++++++++ app/views/import/bitbucket/status.html.haml | 4 +--- app/views/import/fogbugz/status.html.haml | 3 +-- app/views/import/github/status.html.haml | 3 +-- app/views/import/gitlab/status.html.haml | 3 +-- app/views/import/gitorious/status.html.haml | 3 +-- app/views/import/google_code/status.html.haml | 3 +-- 7 files changed, 14 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/importer_status.js b/app/assets/javascripts/importer_status.js index 55b6f132bab..0f840821f53 100644 --- a/app/assets/javascripts/importer_status.js +++ b/app/assets/javascripts/importer_status.js @@ -66,4 +66,12 @@ })(); + $(function() { + if ($('.js-importer-status').length) { + var jobsImportPath = $('.js-importer-status').data('jobs-import-path'); + var importPath = $('.js-importer-status').data('import-path'); + + new ImporterStatus(jobsImportPath, importPath); + } + }); }).call(this); diff --git a/app/views/import/bitbucket/status.html.haml b/app/views/import/bitbucket/status.html.haml index 6e993e58f0d..15dd98077c8 100644 --- a/app/views/import/bitbucket/status.html.haml +++ b/app/views/import/bitbucket/status.html.haml @@ -74,6 +74,4 @@ = link_to "import flow", status_import_bitbucket_path, "data-no-turbolink" => "true" again. - -:javascript - new ImporterStatus("#{jobs_import_bitbucket_path}", "#{import_bitbucket_path}"); +.js-importer-status{ data: { jobs_import_path: "#{jobs_import_bitbucket_path}", import_path: "#{import_bitbucket_path}" } } diff --git a/app/views/import/fogbugz/status.html.haml b/app/views/import/fogbugz/status.html.haml index d3d3c595c17..c8a6fa1aa9e 100644 --- a/app/views/import/fogbugz/status.html.haml +++ b/app/views/import/fogbugz/status.html.haml @@ -56,5 +56,4 @@ Import = icon("spinner spin", class: "loading-icon") -:javascript - new ImporterStatus("#{jobs_import_fogbugz_path}", "#{import_fogbugz_path}"); +.js-importer-status{ data: { jobs_import_path: "#{jobs_import_fogbugz_path}", import_path: "#{import_fogbugz_path}" } } diff --git a/app/views/import/github/status.html.haml b/app/views/import/github/status.html.haml index 7486b1423e2..deaaf9af875 100644 --- a/app/views/import/github/status.html.haml +++ b/app/views/import/github/status.html.haml @@ -55,5 +55,4 @@ Import = icon("spinner spin", class: "loading-icon") -:javascript - new ImporterStatus("#{jobs_import_github_path}", "#{import_github_path}"); +.js-importer-status{ data: { jobs_import_path: "#{jobs_import_github_path}", import_path: "#{import_github_path}" } } diff --git a/app/views/import/gitlab/status.html.haml b/app/views/import/gitlab/status.html.haml index aedb8468eca..fcfc6fd37f4 100644 --- a/app/views/import/gitlab/status.html.haml +++ b/app/views/import/gitlab/status.html.haml @@ -51,5 +51,4 @@ Import = icon("spinner spin", class: "loading-icon") -:javascript - new ImporterStatus("#{jobs_import_gitlab_path}", "#{import_gitlab_path}"); +.js-importer-status{ data: { jobs_import_path: "#{jobs_import_gitlab_path}", import_path: "#{import_gitlab_path}" } } diff --git a/app/views/import/gitorious/status.html.haml b/app/views/import/gitorious/status.html.haml index 267eee4f262..ed3afb0ce33 100644 --- a/app/views/import/gitorious/status.html.haml +++ b/app/views/import/gitorious/status.html.haml @@ -51,5 +51,4 @@ Import = icon("spinner spin", class: "loading-icon") -:javascript - new ImporterStatus("#{jobs_import_gitorious_path}", "#{import_gitorious_path}"); +.js-importer-status{ data: { jobs_import_path: "#{jobs_import_gitorious_path}", import_path: "#{import_gitorious_path}" } } diff --git a/app/views/import/google_code/status.html.haml b/app/views/import/google_code/status.html.haml index 5ada6b174eb..e79f122940a 100644 --- a/app/views/import/google_code/status.html.haml +++ b/app/views/import/google_code/status.html.haml @@ -77,5 +77,4 @@ = link_to "import flow", new_import_google_code_path again. -:javascript - new ImporterStatus("#{jobs_import_google_code_path}", "#{import_google_code_path}"); +.js-importer-status{ data: { jobs_import_path: "#{jobs_import_google_code_path}", import_path: "#{import_google_code_path}" } } From 1dcfb1d3a7b6b8002b1f25e8cf463617acbc1299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 26 Jul 2016 17:22:13 -0400 Subject: [PATCH 27/38] Fix a bug where forking a project from a repository storage to another would fail --- CHANGELOG | 1 + GITLAB_SHELL_VERSION | 2 +- app/models/project.rb | 4 +++- app/workers/repository_fork_worker.rb | 5 ++-- lib/gitlab/backend/shell.rb | 10 ++++---- spec/models/project_spec.rb | 26 +++++++++++++++++++++ spec/workers/repository_fork_worker_spec.rb | 10 +++++--- 7 files changed, 47 insertions(+), 11 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 432d251dfc6..6266161a3a8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -21,6 +21,7 @@ v 8.10.2 (unreleased) - Fix backup restore. !5459 - Rescue Rugged::OSError (lock exists) when creating references. !5497 - Disable MySQL foreign key checks before dropping all tables. !5472 + - Fix a bug where forking a project from a repository storage to another would fail - Show release notes in tags list - Use project ID in repository cache to prevent stale data from persisting across projects. !5460 - Ensure relative paths for video are rewritten as we do for images. !5474 diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 944880fa15e..e4604e3afd0 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -3.2.0 +3.2.1 diff --git a/app/models/project.rb b/app/models/project.rb index 023b1dc3725..1616175709f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -451,7 +451,9 @@ class Project < ActiveRecord::Base def add_import_job if forked? - job_id = RepositoryForkWorker.perform_async(self.id, forked_from_project.path_with_namespace, self.namespace.path) + job_id = RepositoryForkWorker.perform_async(id, forked_from_project.repository_storage_path, + forked_from_project.path_with_namespace, + self.namespace.path) else job_id = RepositoryImportWorker.perform_async(self.id) end diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index f7604e48f83..d69d6037053 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -4,7 +4,7 @@ class RepositoryForkWorker sidekiq_options queue: :gitlab_shell - def perform(project_id, source_path, target_path) + def perform(project_id, forked_from_repository_storage_path, source_path, target_path) project = Project.find_by_id(project_id) unless project.present? @@ -12,7 +12,8 @@ class RepositoryForkWorker return end - result = gitlab_shell.fork_repository(project.repository_storage_path, source_path, target_path) + result = gitlab_shell.fork_repository(forked_from_repository_storage_path, source_path, + project.repository_storage_path, target_path) unless result logger.error("Unable to fork project #{project_id} for repository #{source_path} -> #{target_path}") project.mark_import_as_failed('The project could not be forked.') diff --git a/lib/gitlab/backend/shell.rb b/lib/gitlab/backend/shell.rb index 34e0143a82e..839a4fa30d5 100644 --- a/lib/gitlab/backend/shell.rb +++ b/lib/gitlab/backend/shell.rb @@ -60,16 +60,18 @@ module Gitlab end # Fork repository to new namespace - # storage - project's storage path + # forked_from_storage - forked-from project's storage path # path - project path with namespace + # forked_to_storage - forked-to project's storage path # fork_namespace - namespace for forked project # # Ex. - # fork_repository("/path/to/storage", "gitlab/gitlab-ci", "randx") + # fork_repository("/path/to/forked_from/storage", "gitlab/gitlab-ci", "/path/to/forked_to/storage", "randx") # - def fork_repository(storage, path, fork_namespace) + def fork_repository(forked_from_storage, path, forked_to_storage, fork_namespace) Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'fork-project', - storage, "#{path}.git", fork_namespace]) + forked_from_storage, "#{path}.git", forked_to_storage, + fork_namespace]) end # Remove repository from file system diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9b017288488..c884bb31199 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1244,6 +1244,32 @@ describe Project, models: true do end end + describe '#add_import_job' do + context 'forked' do + let(:forked_project_link) { create(:forked_project_link) } + let(:forked_from_project) { forked_project_link.forked_from_project } + let(:project) { forked_project_link.forked_to_project } + + it 'schedules a RepositoryForkWorker job' do + expect(RepositoryForkWorker).to receive(:perform_async). + with(project.id, forked_from_project.repository_storage_path, + forked_from_project.path_with_namespace, project.namespace.path) + + project.add_import_job + end + end + + context 'not forked' do + let(:project) { create(:project) } + + it 'schedules a RepositoryImportWorker job' do + expect(RepositoryImportWorker).to receive(:perform_async).with(project.id) + + project.add_import_job + end + end + end + describe '.where_paths_in' do context 'without any paths' do it 'returns an empty relation' do diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index 5f762282b5e..60605460adb 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -14,21 +14,24 @@ describe RepositoryForkWorker do describe "#perform" do it "creates a new repository from a fork" do expect(shell).to receive(:fork_repository).with( - project.repository_storage_path, + '/test/path', project.path_with_namespace, + project.repository_storage_path, fork_project.namespace.path ).and_return(true) subject.perform( project.id, + '/test/path', project.path_with_namespace, fork_project.namespace.path) end it 'flushes various caches' do expect(shell).to receive(:fork_repository).with( - project.repository_storage_path, + '/test/path', project.path_with_namespace, + project.repository_storage_path, fork_project.namespace.path ).and_return(true) @@ -38,7 +41,7 @@ describe RepositoryForkWorker do expect_any_instance_of(Repository).to receive(:expire_exists_cache). and_call_original - subject.perform(project.id, project.path_with_namespace, + subject.perform(project.id, '/test/path', project.path_with_namespace, fork_project.namespace.path) end @@ -49,6 +52,7 @@ describe RepositoryForkWorker do subject.perform( project.id, + '/test/path', project.path_with_namespace, fork_project.namespace.path) end From ef8d9c269a8383ba2e3766b13b44b655e0588609 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 26 Jul 2016 15:25:15 -0600 Subject: [PATCH 28/38] Whitelist 'Simplified BSD' license --- config/dependency_decisions.yml | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/config/dependency_decisions.yml b/config/dependency_decisions.yml index 293f2b71d65..74325872b09 100644 --- a/config/dependency_decisions.yml +++ b/config/dependency_decisions.yml @@ -68,6 +68,25 @@ :why: https://opensource.org/licenses/BSD-2-Clause :versions: [] :when: 2016-05-02 05:55:09.796363000 Z +- - :whitelist + - LGPLv2+ + - :who: Stan Hu + :why: Equivalent to LGPLv2 + :versions: [] + :when: 2016-06-07 17:14:10.907682000 Z +- - :whitelist + - Artistic 2.0 + - :who: Josh Frye + :why: Disk/mount information display on Admin pages + :versions: [] + :when: 2016-06-29 16:32:45.432113000 Z +- - :whitelist + - Simplified BSD + - :who: Douwe Maan + :why: https://opensource.org/licenses/BSD-2-Clause + :versions: [] + :when: 2016-07-26 21:24:07.248480000 Z + # LICENSE BLACKLIST - - :blacklist @@ -175,15 +194,3 @@ :why: https://github.com/jmcnevin/rubypants/blob/master/LICENSE.rdoc :versions: [] :when: 2016-05-02 05:56:50.696858000 Z -- - :whitelist - - LGPLv2+ - - :who: Stan Hu - :why: Equivalent to LGPLv2 - :versions: [] - :when: 2016-06-07 17:14:10.907682000 Z -- - :whitelist - - Artistic 2.0 - - :who: Josh Frye - :why: Disk/mount information display on Admin pages - :versions: [] - :when: 2016-06-29 16:32:45.432113000 Z From 61fac97f4feb8e0c4d5a3e1781a136a4c4788dba Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 26 Jul 2016 15:18:26 -0700 Subject: [PATCH 29/38] Fix missing schema update for 20160722221922 --- db/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index b87f8108bb2..15cee55a7bf 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: 20160721081015) do +ActiveRecord::Schema.define(version: 20160722221922) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" From d1ea2bca61dff21948024d897e1d4475123a10e8 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 19 Jul 2016 21:52:31 -0700 Subject: [PATCH 30/38] Optimize maximum user access level lookup in loading of notes NotesHelper#note_editable? and ProjectTeam#human_max_access currently take about 16% of the load time of an issue page. This MR preloads the maximum access level of users for all notes in issues and merge requests with several queries instead of one per user and caches the result in RequestStore. --- CHANGELOG | 1 + app/controllers/projects/issues_controller.rb | 3 + .../projects/merge_requests_controller.rb | 3 + app/helpers/notes_helper.rb | 15 +++-- app/models/ability.rb | 14 +++++ app/models/project_team.rb | 54 +++++++++++++----- lib/gitlab/access.rb | 1 + spec/helpers/notes_helper_spec.rb | 57 ++++++++++--------- spec/models/ability_spec.rb | 56 ++++++++++++++++++ spec/models/project_team_spec.rb | 51 ++++++++++++++--- 10 files changed, 198 insertions(+), 57 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 5ff0cb42ccc..fb38db15630 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ v 8.11.0 (unreleased) - Fix CI status icon link underline (ClemMakesApps) - Fix of 'Commits being passed to custom hooks are already reachable when using the UI' - Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable + - Optimize maximum user access level lookup in loading of notes - Limit git rev-list output count to one in forced push check - Add green outline to New Branch button. !5447 (winniehell) - Retrieve rendered HTML from cache in one request diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index fa663c9bda4..16ed7c2b6b4 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -1,4 +1,5 @@ class Projects::IssuesController < Projects::ApplicationController + include NotesHelper include ToggleSubscriptionAction include IssuableActions include ToggleAwardEmoji @@ -70,6 +71,8 @@ class Projects::IssuesController < Projects::ApplicationController @note = @project.notes.new(noteable: @issue) @noteable = @issue + preload_max_access_for_authors(@notes, @project) if @notes + respond_to do |format| format.html format.json do diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 594a61464b9..da1b9c3e48a 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -3,6 +3,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController include DiffForPath include DiffHelper include IssuableActions + include NotesHelper include ToggleAwardEmoji before_action :module_enabled @@ -385,6 +386,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController @project_wiki, @ref ) + + preload_max_access_for_authors(@notes, @project) if @notes end def define_widget_vars diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 0f60dd828ab..0c47abe0fba 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -7,7 +7,7 @@ module NotesHelper end def note_editable?(note) - note.editable? && can?(current_user, :admin_note, note) + Ability.can_edit_note?(current_user, note) end def noteable_json(noteable) @@ -87,14 +87,13 @@ module NotesHelper end end - def note_max_access_for_user(note) - @max_access_by_user_id ||= Hash.new do |hash, key| - project = key[:project] - hash[key] = project.team.human_max_access(key[:user_id]) - end + def preload_max_access_for_authors(notes, project) + user_ids = notes.map(&:author_id) + project.team.max_member_access_for_user_ids(user_ids) + end - full_key = { project: note.project, user_id: note.author_id } - @max_access_by_user_id[full_key] + def note_max_access_for_user(note) + note.project.team.human_max_access(note.author_id) end def discussion_diff_path(discussion) diff --git a/app/models/ability.rb b/app/models/ability.rb index f33c8d61d3f..6884d99c5a6 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -388,6 +388,20 @@ class Ability GroupProjectsFinder.new(group).execute(user).any? end + def can_edit_note?(user, note) + return false unless note.editable? + return false unless user.present? + return true if note.author == user + return true if user.admin? + + if note.project + max_access_level = note.project.team.max_member_access(user.id) + max_access_level >= Gitlab::Access::MASTER + else + false + end + end + def namespace_abilities(user, namespace) rules = [] diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 9d312a53790..67faea1f9f3 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -132,22 +132,41 @@ class ProjectTeam Gitlab::Access.options_with_owner.key(max_member_access(user_id)) end - # This method assumes project and group members are eager loaded for optimal - # performance. + # Determine the maximum access level for a group of users in bulk. + # + # Returns a Hash mapping user ID -> maxmum access level. + def max_member_access_for_user_ids(user_ids) + user_ids = user_ids.uniq + key = "max_member_access:#{project.id}" + RequestStore.store[key] ||= Hash.new + access = RequestStore.store[key] + + # Lookup only the IDs we need + user_ids = user_ids - access.keys + + if user_ids.present? + user_ids.map { |id| access[id] = Gitlab::Access::NO_ACCESS } + + member_access = project.members.where(user_id: user_ids).has_access.pluck(:user_id, :access_level).to_h + merge_max!(access, member_access) + + if group + group_access = group.members.where(user_id: user_ids).has_access.pluck(:user_id, :access_level).to_h + merge_max!(access, group_access) + end + + if project.invited_groups.any? && project.allowed_to_share_with_group? + # Not optimized + invited_levels = user_ids.map { |id| [id, max_invited_level(id)] }.to_h + merge_max!(access, invited_levels) + end + end + + access + end + def max_member_access(user_id) - access = [] - - access += project.members.where(user_id: user_id).has_access.pluck(:access_level) - - if group - access += group.members.where(user_id: user_id).has_access.pluck(:access_level) - end - - if project.invited_groups.any? && project.allowed_to_share_with_group? - access << max_invited_level(user_id) - end - - access.compact.max + max_member_access_for_user_ids([user_id])[user_id] end private @@ -156,6 +175,7 @@ class ProjectTeam project.project_group_links.map do |group_link| invited_group = group_link.group access = invited_group.group_members.find_by(user_id: user_id).try(:access_field) + access = Gitlab::Access::NO_ACCESS unless access.present? # If group member has higher access level we should restrict it # to max allowed access level @@ -215,4 +235,8 @@ class ProjectTeam def group project.group end + + def merge_max!(first_hash, second_hash) + first_hash.merge!(second_hash) { |_key, old, new| old > new ? old : new } + end end diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index de41ea415a6..a533bac2692 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -7,6 +7,7 @@ module Gitlab module Access class AccessDeniedError < StandardError; end + NO_ACCESS = 0 GUEST = 10 REPORTER = 20 DEVELOPER = 30 diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb index 08a93503258..af371248ae9 100644 --- a/spec/helpers/notes_helper_spec.rb +++ b/spec/helpers/notes_helper_spec.rb @@ -1,37 +1,30 @@ require "spec_helper" describe NotesHelper do + let(:owner) { create(:owner) } + let(:group) { create(:group) } + let(:project) { create(:empty_project, namespace: group) } + let(:master) { create(:user) } + let(:reporter) { create(:user) } + let(:guest) { create(:user) } + + let(:owner_note) { create(:note, author: owner, project: project) } + let(:master_note) { create(:note, author: master, project: project) } + let(:reporter_note) { create(:note, author: reporter, project: project) } + let!(:notes) { [owner_note, master_note, reporter_note] } + + before do + group.add_owner(owner) + project.team << [master, :master] + project.team << [reporter, :reporter] + project.team << [guest, :guest] + end + describe "#notes_max_access_for_users" do - let(:owner) { create(:owner) } - let(:group) { create(:group) } - let(:project) { create(:empty_project, namespace: group) } - let(:master) { create(:user) } - let(:reporter) { create(:user) } - let(:guest) { create(:user) } - - let(:owner_note) { create(:note, author: owner, project: project) } - let(:master_note) { create(:note, author: master, project: project) } - let(:reporter_note) { create(:note, author: reporter, project: project) } - let!(:notes) { [owner_note, master_note, reporter_note] } - - before do - group.add_owner(owner) - project.team << [master, :master] - project.team << [reporter, :reporter] - project.team << [guest, :guest] - end - it 'return human access levels' do - original_method = project.team.method(:human_max_access) - expect_any_instance_of(ProjectTeam).to receive(:human_max_access).exactly(3).times do |*args| - original_method.call(args[1]) - end - expect(helper.note_max_access_for_user(owner_note)).to eq('Owner') expect(helper.note_max_access_for_user(master_note)).to eq('Master') expect(helper.note_max_access_for_user(reporter_note)).to eq('Reporter') - # Call it again to ensure value is cached - expect(helper.note_max_access_for_user(owner_note)).to eq('Owner') end it 'handles access in different projects' do @@ -43,4 +36,16 @@ describe NotesHelper do expect(helper.note_max_access_for_user(other_note)).to eq('Reporter') end end + + describe '#preload_max_access_for_authors' do + it 'loads multiple users' do + expected_access = { + owner.id => Gitlab::Access::OWNER, + master.id => Gitlab::Access::MASTER, + reporter.id => Gitlab::Access::REPORTER + } + + expect(helper.preload_max_access_for_authors(notes, project)).to eq(expected_access) + end + end end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 1acb5846fcf..cd5f40fe3d2 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -1,6 +1,62 @@ require 'spec_helper' describe Ability, lib: true do + describe '.can_edit_note?' do + let(:project) { create(:empty_project) } + let!(:note) { create(:note_on_issue, project: project) } + + context 'using an anonymous user' do + it 'returns false' do + expect(described_class.can_edit_note?(nil, note)).to be_falsy + end + end + + context 'using a system note' do + it 'returns false' do + system_note = create(:note, system: true) + user = create(:user) + + expect(described_class.can_edit_note?(user, system_note)).to be_falsy + end + end + + context 'using users with different access levels' do + let(:user) { create(:user) } + + it 'returns true for the author' do + expect(described_class.can_edit_note?(note.author, note)).to be_truthy + end + + it 'returns false for a guest user' do + project.team << [user, :guest] + + expect(described_class.can_edit_note?(user, note)).to be_falsy + end + + it 'returns false for a developer' do + project.team << [user, :developer] + + expect(described_class.can_edit_note?(user, note)).to be_falsy + end + + it 'returns true for a master' do + project.team << [user, :master] + + expect(described_class.can_edit_note?(user, note)).to be_truthy + end + + it 'returns true for a group owner' do + group = create(:group) + project.project_group_links.create( + group: group, + group_access: Gitlab::Access::MASTER) + group.add_owner(user) + + expect(described_class.can_edit_note?(user, note)).to be_truthy + end + end + end + describe '.users_that_can_read_project' do context 'using a public project' do it 'returns all the users' do diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 9262aeb6ed8..115fffd82d9 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -151,8 +151,8 @@ describe ProjectTeam, models: true do it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::MASTER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } it { expect(project.team.max_member_access(guest.id)).to eq(Gitlab::Access::GUEST) } - it { expect(project.team.max_member_access(nonmember.id)).to be_nil } - it { expect(project.team.max_member_access(requester.id)).to be_nil } + it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } end context 'when project is shared with group' do @@ -168,14 +168,14 @@ describe ProjectTeam, models: true do it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::DEVELOPER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } - it { expect(project.team.max_member_access(nonmember.id)).to be_nil } - it { expect(project.team.max_member_access(requester.id)).to be_nil } + it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } context 'but share_with_group_lock is true' do before { project.namespace.update(share_with_group_lock: true) } - it { expect(project.team.max_member_access(master.id)).to be_nil } - it { expect(project.team.max_member_access(reporter.id)).to be_nil } + it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::NO_ACCESS) } end end end @@ -194,8 +194,43 @@ describe ProjectTeam, models: true do it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::MASTER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } it { expect(project.team.max_member_access(guest.id)).to eq(Gitlab::Access::GUEST) } - it { expect(project.team.max_member_access(nonmember.id)).to be_nil } - it { expect(project.team.max_member_access(requester.id)).to be_nil } + it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } + end + end + + describe "#max_member_access_for_users" do + it 'returns correct roles for different users' do + master = create(:user) + reporter = create(:user) + promoted_guest = create(:user) + guest = create(:user) + project = create(:project) + + project.team << [master, :master] + project.team << [reporter, :reporter] + project.team << [promoted_guest, :guest] + project.team << [guest, :guest] + + group = create(:group) + group_developer = create(:user) + project.project_group_links.create( + group: group, + group_access: Gitlab::Access::DEVELOPER) + + group.add_master(promoted_guest) + group.add_developer(group_developer) + users = [master, reporter, promoted_guest, guest, group_developer].map(&:id) + + expected = { + master.id => Gitlab::Access::MASTER, + reporter.id => Gitlab::Access::REPORTER, + promoted_guest.id => Gitlab::Access::DEVELOPER, + guest.id => Gitlab::Access::GUEST, + group_developer.id => Gitlab::Access::DEVELOPER + } + + expect(project.team.max_member_access_for_user_ids(users)).to eq(expected) end end end From 871723da7fa6b341b64197e27c6bd99d52f2dcd8 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 25 Jul 2016 06:21:55 -0700 Subject: [PATCH 31/38] Incorporate review comments --- app/controllers/projects/issues_controller.rb | 2 +- app/controllers/projects/merge_requests_controller.rb | 2 +- app/models/ability.rb | 6 ++---- app/models/member.rb | 4 ++++ app/models/project_team.rb | 8 ++++---- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 16ed7c2b6b4..91ff9407216 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -71,7 +71,7 @@ class Projects::IssuesController < Projects::ApplicationController @note = @project.notes.new(noteable: @issue) @noteable = @issue - preload_max_access_for_authors(@notes, @project) if @notes + preload_max_access_for_authors(@notes, @project) respond_to do |format| format.html diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index da1b9c3e48a..23252fa59cc 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -387,7 +387,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @ref ) - preload_max_access_for_authors(@notes, @project) if @notes + preload_max_access_for_authors(@notes, @project) end def define_widget_vars diff --git a/app/models/ability.rb b/app/models/ability.rb index 6884d99c5a6..e47c5539f60 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -389,10 +389,8 @@ class Ability end def can_edit_note?(user, note) - return false unless note.editable? - return false unless user.present? - return true if note.author == user - return true if user.admin? + return false if !note.editable? || !user.present? + return true if note.author == user || user.admin? if note.project max_access_level = note.project.team.max_member_access(user.id) diff --git a/app/models/member.rb b/app/models/member.rb index 44db3d977fa..24ab1276ee9 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -53,6 +53,10 @@ class Member < ActiveRecord::Base default_value_for :notification_level, NotificationSetting.levels[:global] class << self + def access_for_user_ids(user_ids) + where(user_id: user_ids).has_access.pluck(:user_id, :access_level).to_h + end + def find_by_invite_token(invite_token) invite_token = Devise.token_generator.digest(self, :invite_token, invite_token) find_by(invite_token: invite_token) diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 67faea1f9f3..21b3a013673 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -138,20 +138,20 @@ class ProjectTeam def max_member_access_for_user_ids(user_ids) user_ids = user_ids.uniq key = "max_member_access:#{project.id}" - RequestStore.store[key] ||= Hash.new + RequestStore.store[key] ||= {} access = RequestStore.store[key] # Lookup only the IDs we need user_ids = user_ids - access.keys if user_ids.present? - user_ids.map { |id| access[id] = Gitlab::Access::NO_ACCESS } + user_ids.each { |id| access[id] = Gitlab::Access::NO_ACCESS } - member_access = project.members.where(user_id: user_ids).has_access.pluck(:user_id, :access_level).to_h + member_access = project.members.access_for_user_ids(user_ids) merge_max!(access, member_access) if group - group_access = group.members.where(user_id: user_ids).has_access.pluck(:user_id, :access_level).to_h + group_access = group.members.access_for_user_ids(user_ids) merge_max!(access, group_access) end From 1fba07109f44dfb69664e528d3aca409a2dbaff1 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 26 Jul 2016 16:56:06 -0700 Subject: [PATCH 32/38] Optimize the invited group link access level check --- app/models/project_team.rb | 34 ++++++++++++++++++-------------- spec/models/project_team_spec.rb | 14 +++++++++++-- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 21b3a013673..03ea8b8b851 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -155,10 +155,13 @@ class ProjectTeam merge_max!(access, group_access) end + # Each group produces a list of maximum access level per user. We take the + # max of the values produced by each group. if project.invited_groups.any? && project.allowed_to_share_with_group? - # Not optimized - invited_levels = user_ids.map { |id| [id, max_invited_level(id)] }.to_h - merge_max!(access, invited_levels) + project.project_group_links.each do |group_link| + invited_access = max_invited_level_for_users(group_link, user_ids) + merge_max!(access, invited_access) + end end end @@ -171,20 +174,21 @@ class ProjectTeam private - def max_invited_level(user_id) - project.project_group_links.map do |group_link| - invited_group = group_link.group - access = invited_group.group_members.find_by(user_id: user_id).try(:access_field) - access = Gitlab::Access::NO_ACCESS unless access.present? + # For a given group, return the maximum access level for the user. This is the min of + # the invited access level of the group and the access level of the user within the group. + # For example, if the group has been given DEVELOPER access but the member has MASTER access, + # the user should receive only DEVELOPER access. + def max_invited_level_for_users(group_link, user_ids) + invited_group = group_link.group + capped_access_level = group_link.group_access + access = invited_group.group_members.access_for_user_ids(user_ids) - # If group member has higher access level we should restrict it - # to max allowed access level - if access && access > group_link.group_access - access = group_link.group_access - end + # If the user is not in the list, assume he/she does not have access + missing_users = user_ids - access.keys + missing_users.each { |id| access[id] = Gitlab::Access::NO_ACCESS } - access - end.compact.max + # Cap the maximum access by the invited level access + access.each { |key, value| access[key] = [value, capped_access_level].min } end def fetch_members(level = nil) diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 115fffd82d9..f9b3e65745e 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -214,20 +214,30 @@ describe ProjectTeam, models: true do group = create(:group) group_developer = create(:user) + second_developer = create(:user) project.project_group_links.create( group: group, group_access: Gitlab::Access::DEVELOPER) group.add_master(promoted_guest) group.add_developer(group_developer) - users = [master, reporter, promoted_guest, guest, group_developer].map(&:id) + group.add_developer(second_developer) + + second_group = create(:group) + project.project_group_links.create( + group: second_group, + group_access: Gitlab::Access::MASTER) + second_group.add_master(second_developer) + + users = [master, reporter, promoted_guest, guest, group_developer, second_developer].map(&:id) expected = { master.id => Gitlab::Access::MASTER, reporter.id => Gitlab::Access::REPORTER, promoted_guest.id => Gitlab::Access::DEVELOPER, guest.id => Gitlab::Access::GUEST, - group_developer.id => Gitlab::Access::DEVELOPER + group_developer.id => Gitlab::Access::DEVELOPER, + second_developer.id => Gitlab::Access::MASTER } expect(project.team.max_member_access_for_user_ids(users)).to eq(expected) From ba192c73cdaeec7cec9079a7b5dec4787b15ebdf Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 26 Jul 2016 17:05:52 -0700 Subject: [PATCH 33/38] Rubocop offenses --- spec/models/project_team_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index f9b3e65745e..1f42fbd3385 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -225,19 +225,19 @@ describe ProjectTeam, models: true do second_group = create(:group) project.project_group_links.create( - group: second_group, - group_access: Gitlab::Access::MASTER) + group: second_group, + group_access: Gitlab::Access::MASTER) second_group.add_master(second_developer) users = [master, reporter, promoted_guest, guest, group_developer, second_developer].map(&:id) expected = { - master.id => Gitlab::Access::MASTER, - reporter.id => Gitlab::Access::REPORTER, - promoted_guest.id => Gitlab::Access::DEVELOPER, - guest.id => Gitlab::Access::GUEST, - group_developer.id => Gitlab::Access::DEVELOPER, - second_developer.id => Gitlab::Access::MASTER + master.id => Gitlab::Access::MASTER, + reporter.id => Gitlab::Access::REPORTER, + promoted_guest.id => Gitlab::Access::DEVELOPER, + guest.id => Gitlab::Access::GUEST, + group_developer.id => Gitlab::Access::DEVELOPER, + second_developer.id => Gitlab::Access::MASTER } expect(project.team.max_member_access_for_user_ids(users)).to eq(expected) From a85d24fcec41d305fdf0b6a8ee01f6e98d98e0fe Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 26 Jul 2016 17:07:51 -0700 Subject: [PATCH 34/38] Fix typo in comment --- app/models/project_team.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 03ea8b8b851..fdfaf052730 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -134,7 +134,7 @@ class ProjectTeam # Determine the maximum access level for a group of users in bulk. # - # Returns a Hash mapping user ID -> maxmum access level. + # Returns a Hash mapping user ID -> maximum access level. def max_member_access_for_user_ids(user_ids) user_ids = user_ids.uniq key = "max_member_access:#{project.id}" From 277e9e4ee84d6b72152ce55854b579c992f9db9c Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 26 Jul 2016 17:20:19 -0700 Subject: [PATCH 35/38] Add a spec for access_for_user_ids --- spec/models/member_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 40181a8b906..44cd3c08718 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -79,6 +79,18 @@ describe Member, models: true do @accepted_request_member = project.requesters.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request } end + describe '.access_for_user_ids' do + it 'returns the right access levels' do + users = [@owner_user.id, @master_user.id] + expected = { + @owner_user.id => Gitlab::Access::OWNER, + @master_user.id => Gitlab::Access::MASTER + } + + expect(described_class.access_for_user_ids(users)).to eq(expected) + end + end + describe '.invite' do it { expect(described_class.invite).not_to include @master } it { expect(described_class.invite).to include @invited_member } From 6e1353102d1255abbcc48779dd521e99e655a057 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 26 Jul 2016 20:59:24 -0600 Subject: [PATCH 36/38] Fix typo in Elixir CI template [ci skip] --- vendor/gitlab-ci-yml/Elixir.gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/gitlab-ci-yml/Elixir.gitlab-ci.yml b/vendor/gitlab-ci-yml/Elixir.gitlab-ci.yml index 0b329aaf1c4..00f9541e89b 100644 --- a/vendor/gitlab-ci-yml/Elixir.gitlab-ci.yml +++ b/vendor/gitlab-ci-yml/Elixir.gitlab-ci.yml @@ -2,7 +2,7 @@ # The image already has Hex installed. You might want to consider to use `elixir:latest` image: trenpixster/elixir:latest -# Pic zero or more services to be used on all builds. +# Pick zero or more services to be used on all builds. # Only needed when using a docker container to run your tests in. # Check out: http://docs.gitlab.com/ce/ci/docker/using_docker_images.html#what-is-service services: From 0fa020f1c2ed1cf3841141f35df3a8523301407a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 27 Jul 2016 10:52:09 +0200 Subject: [PATCH 37/38] Update gitlab-shell version to 3.2.1 in the 8.9->8.10 update guide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was missing from gitlab-org/gitlab-ce!5509. Signed-off-by: Rémy Coutable --- doc/update/8.9-to-8.10.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/update/8.9-to-8.10.md b/doc/update/8.9-to-8.10.md index 71cbe5c8ac6..a057a423e61 100644 --- a/doc/update/8.9-to-8.10.md +++ b/doc/update/8.9-to-8.10.md @@ -46,7 +46,7 @@ sudo -u git -H git checkout 8-10-stable-ee ```bash cd /home/git/gitlab-shell sudo -u git -H git fetch --all --tags -sudo -u git -H git checkout v3.2.0 +sudo -u git -H git checkout v3.2.1 ``` ### 5. Update gitlab-workhorse From 774997e964ca7724a37d3f17a75f3224f8ce896c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 27 Jul 2016 11:01:31 +0200 Subject: [PATCH 38/38] Remove useless new route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 0e03d2b4c32..418b3fb3902 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -294,7 +294,7 @@ Rails.application.routes.draw do post :repository_check end - resources :runner_projects, only: [:index, :create, :destroy] + resources :runner_projects, only: [:create, :destroy] end end