From 49c3cc85c4e45cad6afef74b6d220b270290c514 Mon Sep 17 00:00:00 2001 From: Vinnie Okada Date: Mon, 23 Mar 2015 22:06:19 -0600 Subject: [PATCH 01/57] Remove the email patches link for merge commits Rugged's `to_mbox` method doesn't work for merge commits, so don't display the option to download merge commits as email patches. --- CHANGELOG | 1 + app/views/projects/commit/_commit_box.html.haml | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 4c64496008..c7c1ae36b2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,7 @@ v 7.10.0 (unreleased) - enable line wrapping per default and remove the checkbox to toggle it (Hannes Rosenögger) - extend the commit calendar to show the actual commits made on a date (Hannes Rosenögger) - Add a service to support external wikis (Hannes Rosenögger) + - Omit the "email patches" link and fix plain diff view for merge commits - List new commits for newly pushed branch in activity view. - Add changelog, license and contribution guide links to project sidebar. - Improve diff UI diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 7409f702c5..6b1b9782f0 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -10,7 +10,8 @@ Download as %span.caret %ul.dropdown-menu - %li= link_to "Email Patches", namespace_project_commit_path(@project.namespace, @project, @commit, format: :patch) + - unless @commit.parents.length > 1 + %li= link_to "Email Patches", namespace_project_commit_path(@project.namespace, @project, @commit, format: :patch) %li= link_to "Plain Diff", namespace_project_commit_path(@project.namespace, @project, @commit, format: :diff) = link_to namespace_project_tree_path(@project.namespace, @project, @commit), class: "btn btn-primary btn-grouped" do %span Browse Code » From 61e8ca8ce020781c6c4685fe8dcc508a08f24bf8 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Mar 2015 15:55:12 +0100 Subject: [PATCH 02/57] Don't leak private group existence by redirecting from namespace controller to group controller. --- CHANGELOG | 1 + app/controllers/namespaces_controller.rb | 18 +++++++++++++----- app/models/concerns/mentionable.rb | 2 +- lib/gitlab/markdown.rb | 2 +- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 25936eb1e1..9f3abc9ba1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -31,6 +31,7 @@ v 7.10.0 (unreleased) - Replace commits calendar with faster contribution calendar that includes issues and merge requests - Add inifinite scroll to user page activity - Don't show commit comment button when user is not signed in. + - Don't leak private group existence by redirecting from namespace controller to group controller. v 7.9.0 - Send EmailsOnPush email when branch or tag is created or deleted. diff --git a/app/controllers/namespaces_controller.rb b/app/controllers/namespaces_controller.rb index b7a9d8c129..386d103ee5 100644 --- a/app/controllers/namespaces_controller.rb +++ b/app/controllers/namespaces_controller.rb @@ -4,14 +4,22 @@ class NamespacesController < ApplicationController def show namespace = Namespace.find_by(path: params[:id]) - unless namespace - return render_404 + if namespace + if namespace.is_a?(Group) + group = namespace + else + user = namespace.owner + end end - if namespace.type == "Group" - redirect_to group_path(namespace) + if user + redirect_to user_path(user) + elsif group && can?(current_user, :read_group, group) + redirect_to group_path(group) + elsif current_user.nil? + authenticate_user! else - redirect_to user_path(namespace.owner) + render_404 end end end diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index 74900d4675..d96e07034e 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -52,7 +52,7 @@ module Mentionable if identifier == "all" users.push(*project.team.members.flatten) elsif namespace = Namespace.find_by(path: identifier) - if namespace.type == "Group" + if namespace.is_a?(Group) users.push(*namespace.users) else users << namespace.owner diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index e02e5b9fc3..79e821d18e 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -221,7 +221,7 @@ module Gitlab link_to("@all", namespace_project_url(project.namespace, project), options) elsif namespace = Namespace.find_by(path: identifier) url = - if namespace.type == "Group" + if namespace.is_a?(Group) group_url(identifier) else user_url(identifier) From 3b3662da0a5f31dddbe19be7f8e787c1b90b1b22 Mon Sep 17 00:00:00 2001 From: Stephan van Leeuwen Date: Sun, 21 Dec 2014 13:35:11 +0100 Subject: [PATCH 03/57] Updated api method GET /projects/:id/events to use paginate instead of a self-implementation Also updated example request url Added changelog item --- CHANGELOG | 1 + lib/api/projects.rb | 7 ++----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 3ecc45cde0..297a8ee348 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -33,6 +33,7 @@ v 7.10.0 (unreleased) - Don't show commit comment button when user is not signed in. - Don't include system notes in issue/MR comment count. - Don't mark merge request as updated when merge status relative to target branch changes. + - API: Add pagination to project events v 7.9.0 - Send EmailsOnPush email when branch or tag is created or deleted. diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 83f65eec6c..e3fff79d68 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -88,17 +88,14 @@ module API present user_project, with: Entities::ProjectWithAccess, user: current_user end - # Get a single project events + # Get events for a single project # # Parameters: # id (required) - The ID of a project # Example Request: # GET /projects/:id/events get ":id/events" do - limit = (params[:per_page] || 20).to_i - offset = (params[:page] || 0).to_i * limit - events = user_project.events.recent.limit(limit).offset(offset) - + events = paginate user_project.events.recent present events, with: Entities::Event end From 00dd44455a499050cd225990ed3fb9954f333b4d Mon Sep 17 00:00:00 2001 From: Dan Tudor Date: Wed, 25 Mar 2015 15:15:26 +0000 Subject: [PATCH 04/57] Allow ability to delete branches with '/` in name --- lib/api/branches.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/api/branches.rb b/lib/api/branches.rb index edfdf842f8..431d52a98e 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -1,5 +1,4 @@ require 'mime/types' -require 'uri' module API # Projects API @@ -101,10 +100,11 @@ module API # branch (required) - The name of the branch # Example Request: # DELETE /projects/:id/repository/branches/:branch - delete ":id/repository/branches/:branch" do + delete ":id/repository/branches/:branch" + requirements: { branch: /.*/ } do authorize_push_project result = DeleteBranchService.new(user_project, current_user). - execute(URI.unescape(params[:branch])) + execute(params[:branch]) if result[:status] == :success { From 91b108192c84178ec100ece41f159b68738f1a32 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 26 Mar 2015 19:38:20 +0000 Subject: [PATCH 05/57] Fork/Star button re-design --- app/assets/stylesheets/generic/mobile.scss | 7 ----- app/assets/stylesheets/pages/projects.scss | 22 ++------------- app/helpers/projects_helper.rb | 4 +-- app/views/projects/_home_panel.html.haml | 32 ++++++++++------------ 4 files changed, 19 insertions(+), 46 deletions(-) diff --git a/app/assets/stylesheets/generic/mobile.scss b/app/assets/stylesheets/generic/mobile.scss index 1b0e056216..71a1fc4493 100644 --- a/app/assets/stylesheets/generic/mobile.scss +++ b/app/assets/stylesheets/generic/mobile.scss @@ -24,13 +24,6 @@ display: none !important; } - .project-home-panel { - .star-fork-buttons { - padding-top: 10px; - padding-right: 15px; - } - } - .project-home-links { display: none; } diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 6d55a5fa66..9ad1be4157 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -49,8 +49,7 @@ @extend .clearfix; margin-bottom: 15px; - .project-home-desc, - .star-fork-buttons { + .project-home-desc { font-size: 16px; line-height: 1.3; } @@ -60,23 +59,8 @@ color: #666; } - .star-fork-buttons { - float: right; - min-width: 200px; - font-weight: bold; - - .star-buttons, .fork-buttons { - float: right; - margin-left: 20px; - - a:hover { - text-decoration: none; - } - - .count { - margin-left: 5px; - } - } + .btn-action-count { + margin-left: 5px; } } diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 7bf51b5b8e..f535b01b2e 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -81,7 +81,7 @@ module ProjectsHelper end def link_to_toggle_star(title, starred, signed_in) - cls = 'star-btn' + cls = 'star-btn btn btn-primary' cls << ' disabled' unless signed_in toggle_html = content_tag('span', class: 'toggle') do @@ -120,7 +120,7 @@ module ProjectsHelper def link_to_toggle_fork out = icon('code-fork') out << ' Fork' - out << content_tag(:span, class: 'count') do + out << content_tag(:span, class: 'count btn-action-count') do @project.forks_count.to_s end end diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index a295a0d6cd..3bb70458b1 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -14,29 +14,25 @@ – = link_to namespace_project_blob_path(@project.namespace, @project, tree_join(@repository.root_ref, readme.name)) do = readme.name - .star-fork-buttons - - unless @project.empty_repo? - .fork-buttons - - if current_user && can?(current_user, :fork_project, @project) && @project.namespace != current_user.namespace - - if current_user.already_forked?(@project) && current_user.manageable_namespaces.size < 2 - = link_to namespace_project_path(current_user, current_user.fork_of(@project)), title: 'Go to your fork' do - = link_to_toggle_fork - - else - = link_to new_namespace_project_fork_path(@project.namespace, @project), title: "Fork project" do - = link_to_toggle_fork - - .star-buttons - %span.star.js-toggler-container{class: @show_star ? 'on' : ''} - - if current_user - = link_to_toggle_star('Star this project.', false, true) - = link_to_toggle_star('Unstar this project.', true, true) - - else - = link_to_toggle_star('You must sign in to star a project.', false, false) .project-home-row.hidden-xs - if current_user && !empty_repo .project-home-dropdown = render "dropdown" + %span.star.pull-right.prepend-left-10.js-toggler-container{class: @show_star ? 'on' : ''} + - if current_user + = link_to_toggle_star('Star this project.', false, true) + = link_to_toggle_star('Unstar this project.', true, true) + .pull-right.prepend-left-10 + - unless @project.empty_repo? + .fork-buttons + - if current_user && can?(current_user, :fork_project, @project) && @project.namespace != current_user.namespace + - if current_user.already_forked?(@project) && current_user.manageable_namespaces.size < 2 + = link_to namespace_project_path(current_user, current_user.fork_of(@project)), title: 'Go to your fork', class: 'btn btn-primary' do + = link_to_toggle_fork + - else + = link_to new_namespace_project_fork_path(@project.namespace, @project), title: "Fork project", class: 'btn btn-primary' do + = link_to_toggle_fork - unless @project.empty_repo? - if can? current_user, :download_code, @project .pull-right.prepend-left-10 From 79bd8ca9895e030b20bbb7756e1583486c908386 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 27 Mar 2015 07:06:38 +0000 Subject: [PATCH 06/57] Changed button type --- app/helpers/projects_helper.rb | 2 +- app/views/projects/_home_panel.html.haml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index f535b01b2e..3199ff3b99 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -81,7 +81,7 @@ module ProjectsHelper end def link_to_toggle_star(title, starred, signed_in) - cls = 'star-btn btn btn-primary' + cls = 'star-btn btn btn-default' cls << ' disabled' unless signed_in toggle_html = content_tag('span', class: 'toggle') do diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index 3bb70458b1..599afe6e77 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -28,10 +28,10 @@ .fork-buttons - if current_user && can?(current_user, :fork_project, @project) && @project.namespace != current_user.namespace - if current_user.already_forked?(@project) && current_user.manageable_namespaces.size < 2 - = link_to namespace_project_path(current_user, current_user.fork_of(@project)), title: 'Go to your fork', class: 'btn btn-primary' do + = link_to namespace_project_path(current_user, current_user.fork_of(@project)), title: 'Go to your fork', class: 'btn btn-default' do = link_to_toggle_fork - else - = link_to new_namespace_project_fork_path(@project.namespace, @project), title: "Fork project", class: 'btn btn-primary' do + = link_to new_namespace_project_fork_path(@project.namespace, @project), title: "Fork project", class: 'btn btn-default' do = link_to_toggle_fork - unless @project.empty_repo? - if can? current_user, :download_code, @project From a4608a8dbce1ba5b4c07a51f88d6a5fa60728dca Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 27 Mar 2015 10:27:13 +0100 Subject: [PATCH 07/57] Add tests. --- .../controllers/namespaces_controller_spec.rb | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 spec/controllers/namespaces_controller_spec.rb diff --git a/spec/controllers/namespaces_controller_spec.rb b/spec/controllers/namespaces_controller_spec.rb new file mode 100644 index 0000000000..9c8619722c --- /dev/null +++ b/spec/controllers/namespaces_controller_spec.rb @@ -0,0 +1,121 @@ +require 'spec_helper' + +describe NamespacesController do + let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } + + describe "GET show" do + context "when the namespace belongs to a user" do + let!(:other_user) { create(:user) } + + it "redirects to the user's page" do + get :show, id: other_user.username + + expect(response).to redirect_to(user_path(other_user)) + end + end + + context "when the namespace belongs to a group" do + let!(:group) { create(:group) } + let!(:project) { create(:project, namespace: group) } + + context "when the group has public projects" do + before do + project.update_attribute(:visibility_level, Project::PUBLIC) + end + + context "when not signed in" do + it "redirects to the group's page" do + get :show, id: group.path + + expect(response).to redirect_to(group_path(group)) + end + end + + context "when signed in" do + before do + sign_in(user) + end + + it "redirects to the group's page" do + get :show, id: group.path + + expect(response).to redirect_to(group_path(group)) + end + end + end + + context "when the project doesn't have public projects" do + context "when not signed in" do + it "redirects to the sign in page" do + get :show, id: group.path + + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when signed in" do + before do + sign_in(user) + end + + context "when the user has access to the project" do + before do + project.team << [user, :master] + end + + context "when the user is blocked" do + before do + user.block + project.team << [user, :master] + end + + it "redirects to the sign in page" do + get :show, id: group.path + + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when the user isn't blocked" do + it "redirects to the group's page" do + get :show, id: group.path + + expect(response).to redirect_to(group_path(group)) + end + end + end + + context "when the user doesn't have access to the project" do + it "responds with status 404" do + get :show, id: group.path + + expect(response.status).to eq(404) + end + end + end + end + end + + context "when the namespace doesn't exist" do + context "when signed in" do + before do + sign_in(user) + end + + it "responds with status 404" do + get :show, id: "doesntexist" + + expect(response.status).to eq(404) + end + end + + context "when not signed in" do + it "redirects to the sign in page" do + get :show, id: "doesntexist" + + expect(response).to redirect_to(new_user_session_path) + end + end + end + end +end From c5de2ce742f7f2a472eadcd9a2e0d93992930180 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Mar 2015 17:20:05 +0100 Subject: [PATCH 08/57] Return full URLs from GitLabIssueTrackerService. --- CHANGELOG | 1 + .../gitlab_issue_tracker_service.rb | 19 +++++++++---------- spec/helpers/gitlab_markdown_helper_spec.rb | 4 ++-- .../gitlab_issue_tracker_service_spec.rb | 14 +++++++------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 06b7413e61..d1470c45ba 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -46,6 +46,7 @@ v 7.10.0 (unreleased) - Refactor issue filtering - AJAX selectbox for issue assignee and author filters - Fix issue with missing options in issue filtering dropdown if selected one + - Get issue links in notification mail to work again. v 7.9.0 - Send EmailsOnPush email when branch or tag is created or deleted. diff --git a/app/models/project_services/gitlab_issue_tracker_service.rb b/app/models/project_services/gitlab_issue_tracker_service.rb index 84346350a6..4ff9f75fcc 100644 --- a/app/models/project_services/gitlab_issue_tracker_service.rb +++ b/app/models/project_services/gitlab_issue_tracker_service.rb @@ -20,8 +20,13 @@ class GitlabIssueTrackerService < IssueTrackerService include Rails.application.routes.url_helpers - prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url + default_url_options[:host] = Gitlab.config.gitlab.host + default_url_options[:protocol] = Gitlab.config.gitlab.protocol + default_url_options[:port] = Gitlab.config.gitlab.port unless Gitlab.config.gitlab_on_standard_port? + default_url_options[:script_name] = Gitlab.config.gitlab.relative_url_root + + prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url def default? true @@ -32,20 +37,14 @@ class GitlabIssueTrackerService < IssueTrackerService end def project_url - "#{gitlab_url}#{namespace_project_issues_path(project.namespace, project)}" + namespace_project_issues_url(project.namespace, project) end def new_issue_url - "#{gitlab_url}#{new_namespace_project_issue_path(namespace_id: project.namespace, project_id: project)}" + new_namespace_project_issue_url(namespace_id: project.namespace, project_id: project) end def issue_url(iid) - "#{gitlab_url}#{namespace_project_issue_path(namespace_id: project.namespace, project_id: project, id: iid)}" - end - - private - - def gitlab_url - Gitlab.config.gitlab.relative_url_root.chomp("/") if Gitlab.config.gitlab.relative_url_root + namespace_project_issue_url(namespace_id: project.namespace, project_id: project, id: iid) end end diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index c631acc591..f415f126ee 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -522,7 +522,7 @@ describe GitlabMarkdownHelper do # First issue link expect(groups[1]). - to match(/href="#{namespace_project_issue_path(project.namespace, project, issues[0])}"/) + to match(/href="#{namespace_project_issue_url(project.namespace, project, issues[0])}"/) expect(groups[1]).to match(/##{issues[0].iid}$/) # Internal commit link @@ -531,7 +531,7 @@ describe GitlabMarkdownHelper do # Second issue link expect(groups[3]). - to match(/href="#{namespace_project_issue_path(project.namespace, project, issues[1])}"/) + to match(/href="#{namespace_project_issue_url(project.namespace, project, issues[1])}"/) expect(groups[3]).to match(/##{issues[1].iid}$/) # Trailing commit link diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb index 959044dc72..bedb6ecf1b 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -39,15 +39,15 @@ describe GitlabIssueTrackerService do end it 'should give the correct path' do - expect(@service.project_url).to eq("/#{project.path_with_namespace}/issues") - expect(@service.new_issue_url).to eq("/#{project.path_with_namespace}/issues/new") - expect(@service.issue_url(432)).to eq("/#{project.path_with_namespace}/issues/432") + expect(@service.project_url).to eq("http://localhost/#{project.path_with_namespace}/issues") + expect(@service.new_issue_url).to eq("http://localhost/#{project.path_with_namespace}/issues/new") + expect(@service.issue_url(432)).to eq("http://localhost/#{project.path_with_namespace}/issues/432") end end context 'with enabled relative urls' do before do - Settings.gitlab.stub(:relative_url_root).and_return("/gitlab/root") + GitlabIssueTrackerService.default_url_options[:script_name] = "/gitlab/root" @service = project.create_gitlab_issue_tracker_service(active: true) end @@ -56,9 +56,9 @@ describe GitlabIssueTrackerService do end it 'should give the correct path' do - expect(@service.project_url).to eq("/gitlab/root/#{project.path_with_namespace}/issues") - expect(@service.new_issue_url).to eq("/gitlab/root/#{project.path_with_namespace}/issues/new") - expect(@service.issue_url(432)).to eq("/gitlab/root/#{project.path_with_namespace}/issues/432") + expect(@service.project_url).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues") + expect(@service.new_issue_url).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues/new") + expect(@service.issue_url(432)).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues/432") end end end From e08d947e77fa79b723bb1a32794f99d3c39ee463 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 25 Mar 2015 10:03:55 +0100 Subject: [PATCH 09/57] Use relative URL for Markdown references, except in mails. --- app/helpers/issues_helper.rb | 24 +++-- .../gitlab_issue_tracker_service.rb | 12 +++ .../project_services/issue_tracker_service.rb | 12 +++ app/views/layouts/nav/_project.html.haml | 2 +- app/views/notify/_note_message.html.haml | 2 +- app/views/notify/new_issue_email.html.haml | 2 +- .../notify/new_merge_request_email.html.haml | 2 +- app/views/projects/_dropdown.html.haml | 2 +- lib/gitlab/markdown.rb | 90 +++++++++++-------- spec/helpers/gitlab_markdown_helper_spec.rb | 6 +- .../gitlab_issue_tracker_service_spec.rb | 33 +++---- 11 files changed, 120 insertions(+), 67 deletions(-) diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index a4bd4d3021..ad4a761272 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -13,22 +13,34 @@ module IssuesHelper OpenStruct.new(id: 0, title: 'None (backlog)', name: 'Unassigned') end - def url_for_project_issues(project = @project) + def url_for_project_issues(project = @project, options = {}) return '' if project.nil? - project.issues_tracker.project_url + if options[:only_path] + project.issues_tracker.project_path + else + project.issues_tracker.project_url + end end - def url_for_new_issue(project = @project) + def url_for_new_issue(project = @project, options = {}) return '' if project.nil? - project.issues_tracker.new_issue_url + if options[:only_path] + project.issues_tracker.new_issue_path + else + project.issues_tracker.new_issue_url + end end - def url_for_issue(issue_iid, project = @project) + def url_for_issue(issue_iid, project = @project, options = {}) return '' if project.nil? - project.issues_tracker.issue_url(issue_iid) + if options[:only_path] + project.issues_tracker.issue_path(issue_iid) + else + project.issues_tracker.issue_url(issue_iid) + end end def title_for_issue(issue_iid, project = @project) diff --git a/app/models/project_services/gitlab_issue_tracker_service.rb b/app/models/project_services/gitlab_issue_tracker_service.rb index 4ff9f75fcc..5f0553f3b0 100644 --- a/app/models/project_services/gitlab_issue_tracker_service.rb +++ b/app/models/project_services/gitlab_issue_tracker_service.rb @@ -47,4 +47,16 @@ class GitlabIssueTrackerService < IssueTrackerService def issue_url(iid) namespace_project_issue_url(namespace_id: project.namespace, project_id: project, id: iid) end + + def project_path + namespace_project_issues_path(project.namespace, project) + end + + def new_issue_path + new_namespace_project_issue_path(namespace_id: project.namespace, project_id: project) + end + + def issue_path(iid) + namespace_project_issue_path(namespace_id: project.namespace, project_id: project, id: iid) + end end diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index 8e90c44d10..c8ab9d63b7 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -34,6 +34,18 @@ class IssueTrackerService < Service self.issues_url.gsub(':id', iid.to_s) end + def project_path + project_url + end + + def new_issue_path + new_issue_url + end + + def issue_path(iid) + issue_url(iid) + end + def fields [ { type: 'text', name: 'description', placeholder: description }, diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 52681865d6..6c13f30f62 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -53,7 +53,7 @@ - if project_nav_tab? :issues = nav_link(controller: :issues) do - = link_to url_for_project_issues, title: 'Issues', class: 'shortcuts-issues' do + = link_to url_for_project_issues(@project, only_path: true), title: 'Issues', class: 'shortcuts-issues' do %i.fa.fa-exclamation-circle %span Issues diff --git a/app/views/notify/_note_message.html.haml b/app/views/notify/_note_message.html.haml index 778a78acf5..e796353cec 100644 --- a/app/views/notify/_note_message.html.haml +++ b/app/views/notify/_note_message.html.haml @@ -1,2 +1,2 @@ %div - = replace_image_links_with_base64(markdown(@note.note), @note.project) + = replace_image_links_with_base64(markdown(@note.note, reference_only_path: false), @note.project) diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml index 03cbee9460..d4d413b5b4 100644 --- a/app/views/notify/new_issue_email.html.haml +++ b/app/views/notify/new_issue_email.html.haml @@ -1,5 +1,5 @@ -if @issue.description - = replace_image_links_with_base64(markdown(@issue.description), @issue.project) + = replace_image_links_with_base64(markdown(@issue.description, reference_only_path: false), @issue.project) - if @issue.assignee_id.present? %p diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 729a7bb505..60e33227e0 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -6,4 +6,4 @@ Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name} -if @merge_request.description - = replace_image_links_with_base64(markdown(@merge_request.description), @merge_request.project) + = replace_image_links_with_base64(markdown(@merge_request.description, reference_only_path: false), @merge_request.project) diff --git a/app/views/projects/_dropdown.html.haml b/app/views/projects/_dropdown.html.haml index f4f4c2662c..3adb308728 100644 --- a/app/views/projects/_dropdown.html.haml +++ b/app/views/projects/_dropdown.html.haml @@ -5,7 +5,7 @@ %ul.dropdown-menu - if @project.issues_enabled && can?(current_user, :write_issue, @project) %li - = link_to url_for_new_issue, title: "New Issue" do + = link_to url_for_new_issue(@project, only_path: true), title: "New Issue" do New issue - if @project.merge_requests_enabled && can?(current_user, :write_merge_request, @project) %li diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 41bb8d0892..dbc1025855 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -32,12 +32,12 @@ module Gitlab module Markdown include IssuesHelper - attr_reader :html_options + attr_reader :options, :html_options # Public: Parse the provided text with GitLab-Flavored Markdown # # text - the source text - # project - extra options for the reference links as given to link_to + # project - the project # html_options - extra options for the reference links as given to link_to def gfm(text, project = @project, html_options = {}) gfm_with_options(text, {}, project, html_options) @@ -46,9 +46,10 @@ module Gitlab # Public: Parse the provided text with GitLab-Flavored Markdown # # text - the source text - # options - parse_tasks: true - render tasks - # - xhtml: true - output XHTML instead of HTML - # project - extra options for the reference links as given to link_to + # options - parse_tasks - render tasks + # - xhtml - output XHTML instead of HTML + # - reference_only_path - Use relative path for reference links + # project - the project # html_options - extra options for the reference links as given to link_to def gfm_with_options(text, options = {}, project = @project, html_options = {}) return text if text.nil? @@ -58,6 +59,13 @@ module Gitlab # for gsub calls to work as we need them to. text = text.dup.to_str + options.reverse_merge!( + parse_tasks: false, + xhtml: false, + reference_only_path: true + ) + + @options = options @html_options = html_options # Extract pre blocks so they are not altered @@ -113,12 +121,13 @@ module Gitlab markdown_pipeline = HTML::Pipeline::Gitlab.new(filters).pipeline result = markdown_pipeline.call(text, markdown_context) - saveoptions = 0 + + save_options = 0 if options[:xhtml] - saveoptions |= Nokogiri::XML::Node::SaveOptions::AS_XHTML + save_options |= Nokogiri::XML::Node::SaveOptions::AS_XHTML end - text = result[:output].to_html(save_with: saveoptions) + text = result[:output].to_html(save_with: save_options) if options[:parse_tasks] text = parse_tasks(text) @@ -229,33 +238,37 @@ module Gitlab end def reference_user(identifier, project = @project, _ = nil) - options = html_options.merge( + link_options = html_options.merge( class: "gfm gfm-project_member #{html_options[:class]}" ) if identifier == "all" - link_to("@all", namespace_project_url(project.namespace, project), options) + link_to( + "@all", + namespace_project_url(project.namespace, project, only_path: options[:reference_only_path]), + link_options + ) elsif namespace = Namespace.find_by(path: identifier) url = if namespace.type == "Group" - group_url(identifier) + group_url(identifier, only_path: options[:reference_only_path]) else - user_url(identifier) + user_url(identifier, only_path: options[:reference_only_path]) end - link_to("@#{identifier}", url, options) + link_to("@#{identifier}", url, link_options) end end def reference_label(identifier, project = @project, _ = nil) if label = project.labels.find_by(id: identifier) - options = html_options.merge( + link_options = html_options.merge( class: "gfm gfm-label #{html_options[:class]}" ) link_to( render_colored_label(label), namespace_project_issues_path(project.namespace, project, label_name: label.name), - options + link_options ) end end @@ -263,14 +276,14 @@ module Gitlab def reference_issue(identifier, project = @project, prefix_text = nil) if project.default_issues_tracker? if project.issue_exists? identifier - url = url_for_issue(identifier, project) + url = url_for_issue(identifier, project, only_path: options[:reference_only_path]) title = title_for_issue(identifier, project) - options = html_options.merge( + link_options = html_options.merge( title: "Issue: #{title}", class: "gfm gfm-issue #{html_options[:class]}" ) - link_to("#{prefix_text}##{identifier}", url, options) + link_to("#{prefix_text}##{identifier}", url, link_options) end else if project.external_issue_tracker.present? @@ -280,44 +293,46 @@ module Gitlab end end - def reference_merge_request(identifier, project = @project, - prefix_text = nil) + def reference_merge_request(identifier, project = @project, prefix_text = nil) if merge_request = project.merge_requests.find_by(iid: identifier) - options = html_options.merge( + link_options = html_options.merge( title: "Merge Request: #{merge_request.title}", class: "gfm gfm-merge_request #{html_options[:class]}" ) url = namespace_project_merge_request_url(project.namespace, project, - merge_request) - link_to("#{prefix_text}!#{identifier}", url, options) + merge_request, + only_path: options[:reference_only_path]) + link_to("#{prefix_text}!#{identifier}", url, link_options) end end def reference_snippet(identifier, project = @project, _ = nil) if snippet = project.snippets.find_by(id: identifier) - options = html_options.merge( + link_options = html_options.merge( title: "Snippet: #{snippet.title}", class: "gfm gfm-snippet #{html_options[:class]}" ) link_to( "$#{identifier}", - namespace_project_snippet_url(project.namespace, project, snippet), - options + namespace_project_snippet_url(project.namespace, project, snippet, + only_path: options[:reference_only_path]), + link_options ) end end def reference_commit(identifier, project = @project, prefix_text = nil) if project.valid_repo? && commit = project.repository.commit(identifier) - options = html_options.merge( + link_options = html_options.merge( title: commit.link_title, class: "gfm gfm-commit #{html_options[:class]}" ) prefix_text = "#{prefix_text}@" if prefix_text link_to( "#{prefix_text}#{identifier}", - namespace_project_commit_url(project.namespace, project, commit), - options + namespace_project_commit_url( project.namespace, project, commit, + only_path: options[:reference_only_path]), + link_options ) end end @@ -332,7 +347,7 @@ module Gitlab from = project.repository.commit(from_id) && to = project.repository.commit(to_id) - options = html_options.merge( + link_options = html_options.merge( title: "Commits #{from_id} through #{to_id}", class: "gfm gfm-commit_range #{html_options[:class]}" ) @@ -340,22 +355,23 @@ module Gitlab link_to( "#{prefix_text}#{identifier}", - namespace_project_compare_url(project.namespace, project, from: from_id, to: to_id), - options + namespace_project_compare_url(project.namespace, project, + from: from_id, to: to_id, + only_path: options[:reference_only_path]), + link_options ) end end - def reference_external_issue(identifier, project = @project, - prefix_text = nil) - url = url_for_issue(identifier, project) + def reference_external_issue(identifier, project = @project, prefix_text = nil) + url = url_for_issue(identifier, project, only_path: options[:reference_only_path]) title = project.external_issue_tracker.title - options = html_options.merge( + link_options = html_options.merge( title: "Issue in #{title}", class: "gfm gfm-issue #{html_options[:class]}" ) - link_to("#{prefix_text}##{identifier}", url, options) + link_to("#{prefix_text}##{identifier}", url, link_options) end # Turn list items that start with "[ ]" into HTML checkbox inputs. diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index f415f126ee..0d06c6ffb8 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -522,7 +522,7 @@ describe GitlabMarkdownHelper do # First issue link expect(groups[1]). - to match(/href="#{namespace_project_issue_url(project.namespace, project, issues[0])}"/) + to match(/href="#{namespace_project_issue_path(project.namespace, project, issues[0])}"/) expect(groups[1]).to match(/##{issues[0].iid}$/) # Internal commit link @@ -531,7 +531,7 @@ describe GitlabMarkdownHelper do # Second issue link expect(groups[3]). - to match(/href="#{namespace_project_issue_url(project.namespace, project, issues[1])}"/) + to match(/href="#{namespace_project_issue_path(project.namespace, project, issues[1])}"/) expect(groups[3]).to match(/##{issues[1].iid}$/) # Trailing commit link @@ -651,7 +651,7 @@ describe GitlabMarkdownHelper do end it "should leave ref-like href of 'manual' links untouched" do - expect(markdown("why not [inspect !#{merge_request.iid}](http://example.tld/#!#{merge_request.iid})")).to eq("

why not inspect !#{merge_request.iid}

\n") + expect(markdown("why not [inspect !#{merge_request.iid}](http://example.tld/#!#{merge_request.iid})")).to eq("

why not inspect !#{merge_request.iid}

\n") end it "should leave ref-like src of images untouched" do diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb index bedb6ecf1b..f94bef5c36 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -30,22 +30,6 @@ describe GitlabIssueTrackerService do let(:project) { create(:project) } context 'with absolute urls' do - before do - @service = project.create_gitlab_issue_tracker_service(active: true) - end - - after do - @service.destroy! - end - - it 'should give the correct path' do - expect(@service.project_url).to eq("http://localhost/#{project.path_with_namespace}/issues") - expect(@service.new_issue_url).to eq("http://localhost/#{project.path_with_namespace}/issues/new") - expect(@service.issue_url(432)).to eq("http://localhost/#{project.path_with_namespace}/issues/432") - end - end - - context 'with enabled relative urls' do before do GitlabIssueTrackerService.default_url_options[:script_name] = "/gitlab/root" @service = project.create_gitlab_issue_tracker_service(active: true) @@ -61,5 +45,22 @@ describe GitlabIssueTrackerService do expect(@service.issue_url(432)).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues/432") end end + + context 'with relative urls' do + before do + GitlabIssueTrackerService.default_url_options[:script_name] = "/gitlab/root" + @service = project.create_gitlab_issue_tracker_service(active: true) + end + + after do + @service.destroy! + end + + it 'should give the correct path' do + expect(@service.project_path).to eq("/gitlab/root/#{project.path_with_namespace}/issues") + expect(@service.new_issue_path).to eq("/gitlab/root/#{project.path_with_namespace}/issues/new") + expect(@service.issue_path(432)).to eq("/gitlab/root/#{project.path_with_namespace}/issues/432") + end + end end end From 3f7531d6f2974ea75ab8e67bc93049f674ddb672 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Mar 2015 14:53:30 +0100 Subject: [PATCH 10/57] Move User.cleanup_username to Namespace.cleanup_path. --- app/models/namespace.rb | 43 +++++++++++++++++++++++++---------- app/models/user.rb | 16 ------------- lib/gitlab/oauth/user.rb | 2 +- spec/models/namespace_spec.rb | 10 ++++++++ spec/models/user_spec.rb | 10 -------- 5 files changed, 42 insertions(+), 39 deletions(-) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 35280889a8..0c0252ed8e 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -44,21 +44,40 @@ class Namespace < ActiveRecord::Base scope :root, -> { where('type IS NULL') } - def self.by_path(path) - where('lower(path) = :value', value: path.downcase).first - end + class << self + def by_path(path) + where('lower(path) = :value', value: path.downcase).first + end - # Case insensetive search for namespace by path or name - def self.find_by_path_or_name(path) - find_by("lower(path) = :path OR lower(name) = :path", path: path.downcase) - end + # Case insensetive search for namespace by path or name + def find_by_path_or_name(path) + find_by("lower(path) = :path OR lower(name) = :path", path: path.downcase) + end - def self.search(query) - where("name LIKE :query OR path LIKE :query", query: "%#{query}%") - end + def search(query) + where("name LIKE :query OR path LIKE :query", query: "%#{query}%") + end - def self.global_id - 'GLN' + def global_id + 'GLN' + end + + def clean_path(path) + path.gsub!(/@.*\z/, "") + path.gsub!(/\.git\z/, "") + path.gsub!(/\A-/, "") + path.gsub!(/\z./, "") + path.gsub!(/[^a-zA-Z0-9_\-\.]/, "") + + counter = 0 + base = path + while Namespace.by_path(path).present? + counter += 1 + path = "#{base}#{counter}" + end + + path + end end def to_param diff --git a/app/models/user.rb b/app/models/user.rb index 979150b4d6..3f3a8394d1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -229,22 +229,6 @@ class User < ActiveRecord::Base def build_user(attrs = {}) User.new(attrs) end - - def clean_username(username) - username.gsub!(/@.*\z/, "") - username.gsub!(/\.git\z/, "") - username.gsub!(/\A-/, "") - username.gsub!(/[^a-zA-Z0-9_\-\.]/, "") - - counter = 0 - base = username - while User.by_login(username).present? || Namespace.by_path(username).present? - counter += 1 - username = "#{base}#{counter}" - end - - username - end end # diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb index c023d27570..2f5c217d76 100644 --- a/lib/gitlab/oauth/user.rb +++ b/lib/gitlab/oauth/user.rb @@ -86,7 +86,7 @@ module Gitlab def user_attributes { name: auth_hash.name, - username: ::User.clean_username(auth_hash.username), + username: ::Namespace.clean_path(auth_hash.username), email: auth_hash.email, password: auth_hash.password, password_confirmation: auth_hash.password, diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index ed6845c82c..48a3ab9c5a 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -85,4 +85,14 @@ describe Namespace do it { expect(Namespace.find_by_path_or_name('WOW')).to eq(@namespace) } it { expect(Namespace.find_by_path_or_name('unknown')).to eq(nil) } end + + describe ".clean_path" do + + let!(:user) { create(:user, username: "johngitlab-etc") } + let!(:namespace) { create(:namespace, path: "JohnGitLab-etc1") } + + it "cleans the path and makes sure it's available" do + expect(Namespace.clean_path("-john+gitlab-ETC%.git@gmail.com")).to eq("johngitlab-ETC2") + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 10e90cae14..24384e8bf2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -307,16 +307,6 @@ describe User do end end - describe ".clean_username" do - - let!(:user) { create(:user, username: "johngitlab-etc") } - let!(:namespace) { create(:namespace, path: "JohnGitLab-etc1") } - - it "cleans a username and makes sure it's available" do - expect(User.clean_username("-john+gitlab-ETC%.git@gmail.com")).to eq("johngitlab-ETC2") - end - end - describe 'all_ssh_keys' do it { is_expected.to have_many(:keys).dependent(:destroy) } From dfe0f9eedf040c6c266161239b6e2ede77c2b7dc Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Mar 2015 14:55:14 +0100 Subject: [PATCH 11/57] Use more specific regexes. --- app/models/namespace.rb | 8 ++-- app/models/project.rb | 6 +-- app/models/snippet.rb | 4 +- app/models/user.rb | 4 +- app/services/files/create_service.rb | 4 +- lib/gitlab/regex.rb | 64 +++++++++++++++------------- spec/requests/api/projects_spec.rb | 2 +- 7 files changed, 48 insertions(+), 44 deletions(-) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 0c0252ed8e..07d85d05bb 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -24,8 +24,8 @@ class Namespace < ActiveRecord::Base validates :name, presence: true, uniqueness: true, length: { within: 0..255 }, - format: { with: Gitlab::Regex.name_regex, - message: Gitlab::Regex.name_regex_message } + format: { with: Gitlab::Regex.namespace_name_regex, + message: Gitlab::Regex.namespace_name_regex_message } validates :description, length: { within: 0..255 } validates :path, @@ -33,8 +33,8 @@ class Namespace < ActiveRecord::Base presence: true, length: { within: 1..255 }, exclusion: { in: Gitlab::Blacklist.path }, - format: { with: Gitlab::Regex.path_regex, - message: Gitlab::Regex.path_regex_message } + format: { with: Gitlab::Regex.namespace_regex, + message: Gitlab::Regex.namespace_regex_message } delegate :name, to: :owner, allow_nil: true, prefix: true diff --git a/app/models/project.rb b/app/models/project.rb index c50b8a1262..79572f255d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -124,12 +124,12 @@ class Project < ActiveRecord::Base presence: true, length: { within: 0..255 }, format: { with: Gitlab::Regex.project_name_regex, - message: Gitlab::Regex.project_regex_message } + message: Gitlab::Regex.project_name_regex_message } validates :path, presence: true, length: { within: 0..255 }, - format: { with: Gitlab::Regex.path_regex, - message: Gitlab::Regex.path_regex_message } + format: { with: Gitlab::Regex.project_path_regex, + message: Gitlab::Regex.project_path_regex_message } validates :issues_enabled, :merge_requests_enabled, :wiki_enabled, inclusion: { in: [true, false] } validates :issues_tracker_id, length: { maximum: 255 }, allow_blank: true diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 3fb2ec1d66..b35e72c4bd 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -33,8 +33,8 @@ class Snippet < ActiveRecord::Base validates :file_name, presence: true, length: { within: 0..255 }, - format: { with: Gitlab::Regex.path_regex, - message: Gitlab::Regex.path_regex_message } + format: { with: Gitlab::Regex.file_name_regex, + message: Gitlab::Regex.file_name_regex_message } validates :content, presence: true validates :visibility_level, inclusion: { in: Gitlab::VisibilityLevel.values } diff --git a/app/models/user.rb b/app/models/user.rb index 3f3a8394d1..515f29ea0b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -129,8 +129,8 @@ class User < ActiveRecord::Base presence: true, uniqueness: { case_sensitive: false }, exclusion: { in: Gitlab::Blacklist.path }, - format: { with: Gitlab::Regex.username_regex, - message: Gitlab::Regex.username_regex_message } + format: { with: Gitlab::Regex.namespace_regex, + message: Gitlab::Regex.namespace_regex_message } validates :notification_level, inclusion: { in: Notification.notification_levels }, presence: true validate :namespace_uniq, if: ->(user) { user.username_changed? } diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index eeafefc25a..23833aa78e 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -12,10 +12,10 @@ module Files file_name = File.basename(path) file_path = path - unless file_name =~ Gitlab::Regex.path_regex + unless file_name =~ Gitlab::Regex.file_name_regex return error( 'Your changes could not be committed, because the file name ' + - Gitlab::Regex.path_regex_message + Gitlab::Regex.file_name_regex_message ) end diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index cf6e260f25..08fcf9c6dc 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -2,49 +2,64 @@ module Gitlab module Regex extend self - def username_regex - default_regex + def namespace_regex + @namespace_regex ||= /\A[a-zA-Z0-9_.][a-zA-Z0-9_\-\.]*(? Date: Tue, 24 Mar 2015 15:21:37 +0100 Subject: [PATCH 12/57] Don't allow username to end in period. --- CHANGELOG | 2 + app/models/namespace.rb | 2 +- ...047_remove_periods_at_ends_of_usernames.rb | 39 +++++++++++++++++++ lib/gitlab/markdown.rb | 2 +- lib/gitlab/regex.rb | 6 ++- 5 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb diff --git a/CHANGELOG b/CHANGELOG index 06b7413e61..187de477a8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -46,6 +46,8 @@ v 7.10.0 (unreleased) - Refactor issue filtering - AJAX selectbox for issue assignee and author filters - Fix issue with missing options in issue filtering dropdown if selected one + - Fix "Hello @username." references not working by no longer allowing usernames to end in period. + v 7.9.0 - Send EmailsOnPush email when branch or tag is created or deleted. diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 07d85d05bb..e2bcd25be2 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -66,7 +66,7 @@ class Namespace < ActiveRecord::Base path.gsub!(/@.*\z/, "") path.gsub!(/\.git\z/, "") path.gsub!(/\A-/, "") - path.gsub!(/\z./, "") + path.gsub!(/.\z/, "") path.gsub!(/[^a-zA-Z0-9_\-\.]/, "") counter = 0 diff --git a/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb b/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb new file mode 100644 index 0000000000..36e3faa53b --- /dev/null +++ b/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb @@ -0,0 +1,39 @@ +class RemovePeriodsAtEndsOfUsernames < ActiveRecord::Migration + class Namespace < ActiveRecord::Base + class << self + def by_path(path) + where('lower(path) = :value', value: path.downcase).first + end + + def clean_path(path) + path.gsub!(/@.*\z/, "") + path.gsub!(/\.git\z/, "") + path.gsub!(/\A-/, "") + path.gsub!(/.\z/, "") + path.gsub!(/[^a-zA-Z0-9_\-\.]/, "") + + counter = 0 + base = path + while Namespace.by_path(path).present? + counter += 1 + path = "#{base}#{counter}" + end + + path + end + end + end + + def up + select_all("SELECT id, username FROM users WHERE username LIKE '%.'").each do |user| + username = quote_string(Namespace.clean_path(user["username"])) + execute "UPDATE users SET username = '#{username}' WHERE id = #{user["id"]}" + execute "UPDATE namespaces SET path = '#{username}', name = '#{username}' WHERE type = NULL AND owner_id = #{user["id"]}" + end + + select_all("SELECT id, path FROM namespaces WHERE type = 'Group' AND path LIKE '%.'").each do |group| + path = quote_string(Namespace.clean_path(group["path"])) + execute "UPDATE namespaces SET path = '#{path}' WHERE id = #{group["id"]}" + end + end +end diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 41bb8d0892..0b29138a72 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -152,7 +152,7 @@ module Gitlab text end - NAME_STR = '[a-zA-Z0-9_][a-zA-Z0-9_\-\.]*' + NAME_STR = Gitlab::Regex::NAMESPACE_REGEX_STR PROJ_STR = "(?#{NAME_STR}/#{NAME_STR})" REFERENCE_PATTERN = %r{ diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 08fcf9c6dc..0571574aa4 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -2,13 +2,15 @@ module Gitlab module Regex extend self + NAMESPACE_REGEX_STR = '(?:[a-zA-Z0-9_\.][a-zA-Z0-9_\-\.]*[a-zA-Z0-9_\-]|[a-zA-Z0-9_])'.freeze + def namespace_regex - @namespace_regex ||= /\A[a-zA-Z0-9_.][a-zA-Z0-9_\-\.]*(? Date: Tue, 24 Mar 2015 16:58:44 +0100 Subject: [PATCH 13/57] Fix tests. --- features/steps/project/source/browse_files.rb | 2 +- spec/lib/gitlab/regex_spec.rb | 16 ++++++++-------- spec/requests/api/projects_spec.rb | 2 +- spec/requests/api/users_spec.rb | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb index 557555aee5..caf6c73ee0 100644 --- a/features/steps/project/source/browse_files.rb +++ b/features/steps/project/source/browse_files.rb @@ -74,7 +74,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps end step 'I fill the new file name with an illegal name' do - fill_in :file_name, with: '.git' + fill_in :file_name, with: 'Spaces Not Allowed' end step 'I fill the commit message' do diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index 1db9f15b79..727884c41c 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -1,14 +1,14 @@ require 'spec_helper' describe Gitlab::Regex do - describe 'path regex' do - it { expect('gitlab-ce').to match(Gitlab::Regex.path_regex) } - it { expect('gitlab_git').to match(Gitlab::Regex.path_regex) } - it { expect('_underscore.js').to match(Gitlab::Regex.path_regex) } - it { expect('100px.com').to match(Gitlab::Regex.path_regex) } - it { expect('?gitlab').not_to match(Gitlab::Regex.path_regex) } - it { expect('git lab').not_to match(Gitlab::Regex.path_regex) } - it { expect('gitlab.git').not_to match(Gitlab::Regex.path_regex) } + describe 'project path regex' do + it { expect('gitlab-ce').to match(Gitlab::Regex.project_path_regex) } + it { expect('gitlab_git').to match(Gitlab::Regex.project_path_regex) } + it { expect('_underscore.js').to match(Gitlab::Regex.project_path_regex) } + it { expect('100px.com').to match(Gitlab::Regex.project_path_regex) } + it { expect('?gitlab').not_to match(Gitlab::Regex.project_path_regex) } + it { expect('git lab').not_to match(Gitlab::Regex.project_path_regex) } + it { expect('gitlab.git').not_to match(Gitlab::Regex.project_path_regex) } end describe 'project name regex' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 9868b8694c..b713f1fe89 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -252,7 +252,7 @@ describe API::API, api: true do expect(json_response['message']['path']).to eq([ 'can\'t be blank', 'is too short (minimum is 0 characters)', - Gitlab::Regex.send(:default_regex_message) + Gitlab::Regex.send(:project_path_regex_message) ]) end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 081400cded..e6d5545f81 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -140,7 +140,7 @@ describe API::API, api: true do expect(json_response['message']['projects_limit']). to eq(['must be greater than or equal to 0']) expect(json_response['message']['username']). - to eq([Gitlab::Regex.send(:default_regex_message)]) + to eq([Gitlab::Regex.send(:namespace_regex_message)]) end it "shouldn't available for non admin users" do @@ -266,7 +266,7 @@ describe API::API, api: true do expect(json_response['message']['projects_limit']). to eq(['must be greater than or equal to 0']) expect(json_response['message']['username']). - to eq([Gitlab::Regex.send(:default_regex_message)]) + to eq([Gitlab::Regex.send(:namespace_regex_message)]) end context "with existing user" do From 351e61f4b27f287778cf778a41f1a4e4cef977e2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 27 Mar 2015 12:30:21 +0100 Subject: [PATCH 14/57] Prevent note form from being cleared when submitting failed. --- CHANGELOG | 1 + app/assets/javascripts/notes.js.coffee | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 06b7413e61..3a5749b5e3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -46,6 +46,7 @@ v 7.10.0 (unreleased) - Refactor issue filtering - AJAX selectbox for issue assignee and author filters - Fix issue with missing options in issue filtering dropdown if selected one + - Prevent note form from being cleared when submitting failed. v 7.9.0 - Send EmailsOnPush email when branch or tag is created or deleted. diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index c366c98cf5..dc43a06dbe 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -37,7 +37,8 @@ class @Notes $(document).on "click", ".js-note-attachment-delete", @removeAttachment # reset main target form after submit - $(document).on "ajax:complete", ".js-main-target-form", @resetMainTargetForm + $(document).on "ajax:complete", ".js-main-target-form", @reenableTargetFormSubmitButton + $(document).on "ajax:success", ".js-main-target-form", @resetMainTargetForm # update the file name when an attachment is selected $(document).on "change", ".js-note-attachment-input", @updateFormAttachment @@ -70,6 +71,7 @@ class @Notes $(document).off "click", ".js-note-delete" $(document).off "click", ".js-note-attachment-delete" $(document).off "ajax:complete", ".js-main-target-form" + $(document).off "ajax:success", ".js-main-target-form" $(document).off "click", ".js-discussion-reply-button" $(document).off "click", ".js-add-diff-note-button" $(document).off "visibilitychange" @@ -169,6 +171,11 @@ class @Notes form.find(".js-note-text").data("autosave").reset() + reenableTargetFormSubmitButton: -> + form = $(".js-main-target-form") + + form.find(".js-note-text").trigger "input" + ### Shows the main form and does some setup on it. From bfc36fff1134c0a0435b54520021d07af3324430 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Fri, 27 Mar 2015 09:25:35 -0700 Subject: [PATCH 15/57] We've moved external trackers to service, remove from gitlab.yml. --- config/gitlab.yml.example | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 11278ef40d..3f1ca34a66 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -86,35 +86,6 @@ production: &base # The default is 'tmp/repositories' relative to the root of the Rails app. # repository_downloads_path: tmp/repositories - ## External issues trackers - issues_tracker: - # redmine: - # title: "Redmine" - # ## If not nil, link 'Issues' on project page will be replaced with this - # ## Use placeholders: - # ## :project_id - GitLab project identifier - # ## :issues_tracker_id - Project Name or Id in external issue tracker - # project_url: "http://redmine.sample/projects/:issues_tracker_id" - # - # ## If not nil, links from /#\d/ entities from commit messages will replaced with this - # ## Use placeholders: - # ## :project_id - GitLab project identifier - # ## :issues_tracker_id - Project Name or Id in external issue tracker - # ## :id - Issue id (from commit messages) - # issues_url: "http://redmine.sample/issues/:id" - # - # ## If not nil, links to creating new issues will be replaced with this - # ## Use placeholders: - # ## :project_id - GitLab project identifier - # ## :issues_tracker_id - Project Name or Id in external issue tracker - # new_issue_url: "http://redmine.sample/projects/:issues_tracker_id/issues/new" - # - # jira: - # title: "Atlassian Jira" - # project_url: "http://jira.sample/issues/?jql=project=:issues_tracker_id" - # issues_url: "http://jira.sample/browse/:id" - # new_issue_url: "http://jira.sample/secure/CreateIssue.jspa" - ## Gravatar ## For Libravatar see: http://doc.gitlab.com/ce/customization/libravatar.html gravatar: From 65c4dc19f77286374cbc93aa1e18dea8cd00dea5 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 3 Feb 2015 20:13:06 -0500 Subject: [PATCH 16/57] Simplify toggle-nav-collapse JS --- app/assets/javascripts/sidebar.js.coffee | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/sidebar.js.coffee b/app/assets/javascripts/sidebar.js.coffee index 7febcba0e9..2e3f560825 100644 --- a/app/assets/javascripts/sidebar.js.coffee +++ b/app/assets/javascripts/sidebar.js.coffee @@ -3,12 +3,7 @@ $(document).on("click", '.toggle-nav-collapse', (e) -> collapsed = 'page-sidebar-collapsed' expanded = 'page-sidebar-expanded' - if $('.page-with-sidebar').hasClass(collapsed) - $('.page-with-sidebar').removeClass(collapsed).addClass(expanded) - $('.toggle-nav-collapse i').removeClass('fa-angle-right').addClass('fa-angle-left') - $.cookie("collapsed_nav", "false", { path: '/' }) - else - $('.page-with-sidebar').removeClass(expanded).addClass(collapsed) - $('.toggle-nav-collapse i').removeClass('fa-angle-left').addClass('fa-angle-right') - $.cookie("collapsed_nav", "true", { path: '/' }) + $('.page-with-sidebar').toggleClass("#{collapsed} #{expanded}") + $('.toggle-nav-collapse i').toggleClass("fa-angle-right fa-angle-left") + $.cookie("collapsed_nav", $('.page-with-sidebar').hasClass(collapsed), { path: '/' }) ) From 190e08979c25177e47a4055bc9b59e58bfcf3134 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 27 Mar 2015 15:31:16 -0400 Subject: [PATCH 17/57] Add ActiveRecord::Migration.maintain_test_schema! to spec_helper New in Rails 4.1, this eliminates spec failures due to forgetting to run `db:test:prepare`. --- spec/spec_helper.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index eaec2198dc..53ccaa4fd6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -44,3 +44,5 @@ RSpec.configure do |config| TestEnv.init end end + +ActiveRecord::Migration.maintain_test_schema! From 0f1d8e771f3115c12af9ca6e24ea57b2f403b826 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 27 Mar 2015 16:39:41 -0400 Subject: [PATCH 18/57] Define GIT_TEMPLATE_DIR environment variable in TestEnv See http://schacon.github.io/git/git-init.html#_template_directory Without this variable, any global git hooks a developer might have in ~/.git_template would be linked in the `.git/hooks` folder for every test repository that gets checked out by TestEnv, and would cause certain specs to fail due to pre-existing hook files. --- spec/support/test_env.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index f869488d8d..44d70e741b 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -85,7 +85,7 @@ module TestEnv end # We must copy bare repositories because we will push to them. - system(*%W(git clone -q --bare #{factory_repo_path} #{factory_repo_path_bare})) + system(git_env, *%W(git clone -q --bare #{factory_repo_path} #{factory_repo_path_bare})) end def copy_repo(project) @@ -113,4 +113,10 @@ module TestEnv def factory_repo_name 'gitlab-test' end + + # Prevent developer git configurations from being persisted to test + # repositories + def git_env + {'GIT_TEMPLATE_DIR' => ''} + end end From 32d6a14098551b2b8d8210c8c388ec471dc9c5c4 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 27 Mar 2015 22:53:27 -0400 Subject: [PATCH 19/57] Move asana_service_spec to its correct location --- spec/models/{ => project_services}/asana_service_spec.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/models/{ => project_services}/asana_service_spec.rb (100%) diff --git a/spec/models/asana_service_spec.rb b/spec/models/project_services/asana_service_spec.rb similarity index 100% rename from spec/models/asana_service_spec.rb rename to spec/models/project_services/asana_service_spec.rb From 0f78d92e4cfaf5c6674701fdf54a7188686cb8eb Mon Sep 17 00:00:00 2001 From: Vinnie Okada Date: Sat, 28 Mar 2015 08:38:48 -0600 Subject: [PATCH 20/57] Don't use chmod_R for backup tars When creating backup tar files, only change permissions on the `db`, `uploads`, and `repositories` directories, not their contents. --- lib/backup/manager.rb | 2 +- spec/tasks/gitlab/backup_rake_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index afd0589750..d445150c55 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -17,7 +17,7 @@ module Backup file << s.to_yaml.gsub(/^---\n/,'') end - FileUtils.chmod_R(0700, %w{db uploads repositories}) + FileUtils.chmod(0700, %w{db uploads repositories}) # create archive $progress.print "Creating backup archive: #{tar_file} ... " diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index 8a411b7720..3d5d21c2a1 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -87,7 +87,7 @@ describe 'gitlab:app namespace rake task' do expect(tar_contents).to match('db/') expect(tar_contents).to match('uploads/') expect(tar_contents).to match('repositories/') - expect(tar_contents).not_to match(/^.{4,9}[rwx]/) + expect(tar_contents).not_to match(/^.{4,9}[rwx].*(db|uploads|repositories)\/$/) end it 'should delete temp directories' do From fabaeb096fad0307308c9af3b617a5c7a69734e6 Mon Sep 17 00:00:00 2001 From: Vinnie Okada Date: Sat, 28 Mar 2015 09:07:25 -0600 Subject: [PATCH 21/57] Upgrade gitlab_git gem to version 7.1.3 --- CHANGELOG | 1 + Gemfile | 2 +- Gemfile.lock | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index c7c1ae36b2..fe499e7aa9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -12,6 +12,7 @@ v 7.10.0 (unreleased) - Improve diff UI - Fix alignment of navbar toggle button (Cody Mize) - Identical look of selectboxes in UI + - Upgrade the gitlab_git gem to version 7.1.3 - Move "Import existing repository by URL" option to button. - Improve error message when save profile has error. - Passing the name of pushed ref to CI service (requires GitLab CI 7.9+) diff --git a/Gemfile b/Gemfile index 285ccf32b6..f43d5bad1a 100644 --- a/Gemfile +++ b/Gemfile @@ -39,7 +39,7 @@ gem "browser" # Extracting information from a git repository # Provide access to Gitlab::Git library -gem "gitlab_git", '~> 7.1.2' +gem "gitlab_git", '~> 7.1.3' # Ruby/Rack Git Smart-HTTP Server Handler gem 'gitlab-grack', '~> 2.0.0.rc2', require: 'grack' diff --git a/Gemfile.lock b/Gemfile.lock index 80eebc16e4..a608c70d48 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -212,7 +212,7 @@ GEM mime-types (~> 1.19) gitlab_emoji (0.1.0) gemojione (~> 2.0) - gitlab_git (7.1.2) + gitlab_git (7.1.3) activesupport (~> 4.0) charlock_holmes (~> 0.6) gitlab-linguist (~> 3.0) @@ -698,7 +698,7 @@ DEPENDENCIES gitlab-grack (~> 2.0.0.rc2) gitlab-linguist (~> 3.0.1) gitlab_emoji (~> 0.1) - gitlab_git (~> 7.1.2) + gitlab_git (~> 7.1.3) gitlab_meta (= 7.0) gitlab_omniauth-ldap (= 1.2.1) gollum-lib (~> 4.0.2) From be65373fc78ea887aa2d59ee49ae8bb14c64b084 Mon Sep 17 00:00:00 2001 From: Nihad Abbasov Date: Sun, 29 Mar 2015 03:39:32 +0400 Subject: [PATCH 22/57] update acts-as-taggable-on --- Gemfile | 2 +- Gemfile.lock | 6 +- .../initializers/acts_as_taggable_on_patch.rb | 131 ------------------ 3 files changed, 4 insertions(+), 135 deletions(-) delete mode 100644 config/initializers/acts_as_taggable_on_patch.rb diff --git a/Gemfile b/Gemfile index e7f75055f3..b5df7cfcd7 100644 --- a/Gemfile +++ b/Gemfile @@ -115,7 +115,7 @@ end gem "state_machine" # Issue tags -gem "acts-as-taggable-on" +gem 'acts-as-taggable-on', '~> 3.4' # Background jobs gem 'slim' diff --git a/Gemfile.lock b/Gemfile.lock index 7da4d3c358..5adaffb5dc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -33,8 +33,8 @@ GEM minitest (~> 5.1) thread_safe (~> 0.1) tzinfo (~> 1.1) - acts-as-taggable-on (2.4.1) - rails (>= 3, < 5) + acts-as-taggable-on (3.5.0) + activerecord (>= 3.2, < 5) addressable (2.3.5) annotate (2.6.0) activerecord (>= 2.3.0) @@ -662,7 +662,7 @@ PLATFORMS DEPENDENCIES RedCloth ace-rails-ap - acts-as-taggable-on + acts-as-taggable-on (~> 3.4) addressable annotate (~> 2.6.0.beta2) asana (~> 0.0.6) diff --git a/config/initializers/acts_as_taggable_on_patch.rb b/config/initializers/acts_as_taggable_on_patch.rb deleted file mode 100644 index 0d535cb5ca..0000000000 --- a/config/initializers/acts_as_taggable_on_patch.rb +++ /dev/null @@ -1,131 +0,0 @@ -# This is a patch to address the issue in https://github.com/mbleigh/acts-as-taggable-on/issues/427 caused by -# https://github.com/rails/rails/commit/31a43ebc107fbd50e7e62567e5208a05909ec76c -# gem 'acts-as-taggable-on' has the fix included https://github.com/mbleigh/acts-as-taggable-on/commit/89bbed3864a9252276fb8dd7d535fce280454b90 -# but not in the currently used version of gem ('2.4.1') -# With replacement of 'acts-as-taggable-on' gem this file will become obsolete - -module ActsAsTaggableOn::Taggable - module Core - module ClassMethods - def tagged_with(tags, options = {}) - tag_list = ActsAsTaggableOn::TagList.from(tags) - empty_result = where("1 = 0") - - return empty_result if tag_list.empty? - - joins = [] - conditions = [] - having = [] - select_clause = [] - - context = options.delete(:on) - owned_by = options.delete(:owned_by) - alias_base_name = undecorated_table_name.gsub('.','_') - quote = ActsAsTaggableOn::Tag.using_postgresql? ? '"' : '' - - if options.delete(:exclude) - if options.delete(:wild) - tags_conditions = tag_list.map { |t| sanitize_sql(["#{ActsAsTaggableOn::Tag.table_name}.name #{like_operator} ? ESCAPE '!'", "%#{escape_like(t)}%"]) }.join(" OR ") - else - tags_conditions = tag_list.map { |t| sanitize_sql(["#{ActsAsTaggableOn::Tag.table_name}.name #{like_operator} ?", t]) }.join(" OR ") - end - - conditions << "#{table_name}.#{primary_key} NOT IN (SELECT #{ActsAsTaggableOn::Tagging.table_name}.taggable_id FROM #{ActsAsTaggableOn::Tagging.table_name} JOIN #{ActsAsTaggableOn::Tag.table_name} ON #{ActsAsTaggableOn::Tagging.table_name}.tag_id = #{ActsAsTaggableOn::Tag.table_name}.#{ActsAsTaggableOn::Tag.primary_key} AND (#{tags_conditions}) WHERE #{ActsAsTaggableOn::Tagging.table_name}.taggable_type = #{quote_value(base_class.name, nil)})" - - if owned_by - joins << "JOIN #{ActsAsTaggableOn::Tagging.table_name}" + - " ON #{ActsAsTaggableOn::Tagging.table_name}.taggable_id = #{quote}#{table_name}#{quote}.#{primary_key}" + - " AND #{ActsAsTaggableOn::Tagging.table_name}.taggable_type = #{quote_value(base_class.name, nil)}" + - " AND #{ActsAsTaggableOn::Tagging.table_name}.tagger_id = #{owned_by.id}" + - " AND #{ActsAsTaggableOn::Tagging.table_name}.tagger_type = #{quote_value(owned_by.class.base_class.to_s, nil)}" - end - - elsif options.delete(:any) - # get tags, drop out if nothing returned (we need at least one) - tags = - if options.delete(:wild) - ActsAsTaggableOn::Tag.named_like_any(tag_list) - else - ActsAsTaggableOn::Tag.named_any(tag_list) - end - - return empty_result unless tags.length > 0 - - # setup taggings alias so we can chain, ex: items_locations_taggings_awesome_cool_123 - # avoid ambiguous column name - taggings_context = context ? "_#{context}" : '' - - taggings_alias = adjust_taggings_alias( - "#{alias_base_name[0..4]}#{taggings_context[0..6]}_taggings_#{sha_prefix(tags.map(&:name).join('_'))}" - ) - - tagging_join = "JOIN #{ActsAsTaggableOn::Tagging.table_name} #{taggings_alias}" + - " ON #{taggings_alias}.taggable_id = #{quote}#{table_name}#{quote}.#{primary_key}" + - " AND #{taggings_alias}.taggable_type = #{quote_value(base_class.name, nil)}" - tagging_join << " AND " + sanitize_sql(["#{taggings_alias}.context = ?", context.to_s]) if context - - # don't need to sanitize sql, map all ids and join with OR logic - conditions << tags.map { |t| "#{taggings_alias}.tag_id = #{t.id}" }.join(" OR ") - select_clause = "DISTINCT #{table_name}.*" unless context and tag_types.one? - - if owned_by - tagging_join << " AND " + - sanitize_sql([ - "#{taggings_alias}.tagger_id = ? AND #{taggings_alias}.tagger_type = ?", - owned_by.id, - owned_by.class.base_class.to_s - ]) - end - - joins << tagging_join - else - tags = ActsAsTaggableOn::Tag.named_any(tag_list) - - return empty_result unless tags.length == tag_list.length - - tags.each do |tag| - taggings_alias = adjust_taggings_alias("#{alias_base_name[0..11]}_taggings_#{sha_prefix(tag.name)}") - tagging_join = "JOIN #{ActsAsTaggableOn::Tagging.table_name} #{taggings_alias}" + - " ON #{taggings_alias}.taggable_id = #{quote}#{table_name}#{quote}.#{primary_key}" + - " AND #{taggings_alias}.taggable_type = #{quote_value(base_class.name, nil)}" + - " AND #{taggings_alias}.tag_id = #{tag.id}" - - tagging_join << " AND " + sanitize_sql(["#{taggings_alias}.context = ?", context.to_s]) if context - - if owned_by - tagging_join << " AND " + - sanitize_sql([ - "#{taggings_alias}.tagger_id = ? AND #{taggings_alias}.tagger_type = ?", - owned_by.id, - owned_by.class.base_class.to_s - ]) - end - - joins << tagging_join - end - end - - taggings_alias, tags_alias = adjust_taggings_alias("#{alias_base_name}_taggings_group"), "#{alias_base_name}_tags_group" - - if options.delete(:match_all) - joins << "LEFT OUTER JOIN #{ActsAsTaggableOn::Tagging.table_name} #{taggings_alias}" + - " ON #{taggings_alias}.taggable_id = #{quote}#{table_name}#{quote}.#{primary_key}" + - " AND #{taggings_alias}.taggable_type = #{quote_value(base_class.name, nil)}" - - - group_columns = ActsAsTaggableOn::Tag.using_postgresql? ? grouped_column_names_for(self) : "#{table_name}.#{primary_key}" - group = group_columns - having = "COUNT(#{taggings_alias}.taggable_id) = #{tags.size}" - end - - select(select_clause) \ - .joins(joins.join(" ")) \ - .where(conditions.join(" AND ")) \ - .group(group) \ - .having(having) \ - .order(options[:order]) \ - .readonly(false) - end - end - end -end From bba2b10eb5430195b3f5f874c0379fe77213246a Mon Sep 17 00:00:00 2001 From: Nihad Abbasov Date: Sun, 29 Mar 2015 05:36:53 +0500 Subject: [PATCH 23/57] properly paginate project events in API --- lib/api/projects.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 83f65eec6c..620922092d 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -95,10 +95,7 @@ module API # Example Request: # GET /projects/:id/events get ":id/events" do - limit = (params[:per_page] || 20).to_i - offset = (params[:page] || 0).to_i * limit - events = user_project.events.recent.limit(limit).offset(offset) - + events = paginate user_project.events.recent present events, with: Entities::Event end From 7652510c6ba5ed784506830667b94923d9eb8ca8 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Sun, 29 Mar 2015 12:23:56 +0100 Subject: [PATCH 24/57] Moved download button into sidebar Star and fork buttons moved up --- app/views/projects/_home_panel.html.haml | 14 +++++--------- app/views/projects/show.html.haml | 5 ++++- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index 599afe6e77..e0ceafc99e 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -14,11 +14,6 @@ – = link_to namespace_project_blob_path(@project.namespace, @project, tree_join(@repository.root_ref, readme.name)) do = readme.name - - .project-home-row.hidden-xs - - if current_user && !empty_repo - .project-home-dropdown - = render "dropdown" %span.star.pull-right.prepend-left-10.js-toggler-container{class: @show_star ? 'on' : ''} - if current_user = link_to_toggle_star('Star this project.', false, true) @@ -33,8 +28,9 @@ - else = link_to new_namespace_project_fork_path(@project.namespace, @project), title: "Fork project", class: 'btn btn-default' do = link_to_toggle_fork - - unless @project.empty_repo? - - if can? current_user, :download_code, @project - .pull-right.prepend-left-10 - = render 'projects/repositories/download_archive', split_button: true + + .project-home-row.hidden-xs + - if current_user && !empty_repo + .project-home-dropdown + = render "dropdown" = render "shared/clone_panel" diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index cfa6f558dd..b9d65a9bf5 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -87,6 +87,10 @@ - else %span.light CI provided by = link_to ci_service.title, ci_service.builds_path, :'data-no-turbolink' => 'data-no-turbolink' + - unless @project.empty_repo? + - if can? current_user, :download_code, @project + .pull-right.prepend-left-10 + = render 'projects/repositories/download_archive', split_button: true - if readme .tab-pane#tab-readme @@ -97,4 +101,3 @@ = readme.name .wiki = render_readme(readme) - From 6920fd3dddf2960204545f64ca37b2e56972b2d7 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Sun, 29 Mar 2015 12:56:45 +0100 Subject: [PATCH 25/57] Alignment fixes for repo buttons --- app/assets/stylesheets/pages/projects.scss | 8 ++++++++ app/helpers/projects_helper.rb | 2 +- app/views/projects/_home_panel.html.haml | 16 ++++++++-------- app/views/projects/show.html.haml | 3 +-- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 9ad1be4157..de39fc3e9f 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -48,6 +48,10 @@ .project-home-row { @extend .clearfix; margin-bottom: 15px; + + &.project-home-row-top { + margin-bottom: 11px; + } .project-home-desc { font-size: 16px; @@ -71,6 +75,10 @@ color: inherit; } } + + .project-repo-buttons { + margin-top: -5px; + } } .project-home-links { diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 3199ff3b99..5ca71b14d0 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -81,7 +81,7 @@ module ProjectsHelper end def link_to_toggle_star(title, starred, signed_in) - cls = 'star-btn btn btn-default' + cls = 'star-btn btn btn-sm btn-default' cls << ' disabled' unless signed_in toggle_html = content_tag('span', class: 'toggle') do diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index e0ceafc99e..750d305462 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -2,7 +2,7 @@ .project-home-panel{:class => ("empty-project" if empty_repo)} .project-identicon-holder = project_icon(@project, alt: '', class: 'avatar project-avatar') - .project-home-row + .project-home-row.project-home-row-top .project-home-desc - if @project.description.present? = escaped_autolink(@project.description) @@ -14,20 +14,20 @@ – = link_to namespace_project_blob_path(@project.namespace, @project, tree_join(@repository.root_ref, readme.name)) do = readme.name - %span.star.pull-right.prepend-left-10.js-toggler-container{class: @show_star ? 'on' : ''} - - if current_user - = link_to_toggle_star('Star this project.', false, true) - = link_to_toggle_star('Unstar this project.', true, true) - .pull-right.prepend-left-10 + .pull-right.prepend-left-10.project-repo-buttons - unless @project.empty_repo? .fork-buttons - if current_user && can?(current_user, :fork_project, @project) && @project.namespace != current_user.namespace - if current_user.already_forked?(@project) && current_user.manageable_namespaces.size < 2 - = link_to namespace_project_path(current_user, current_user.fork_of(@project)), title: 'Go to your fork', class: 'btn btn-default' do + = link_to namespace_project_path(current_user, current_user.fork_of(@project)), title: 'Go to your fork', class: 'btn btn-sm btn-default' do = link_to_toggle_fork - else - = link_to new_namespace_project_fork_path(@project.namespace, @project), title: "Fork project", class: 'btn btn-default' do + = link_to new_namespace_project_fork_path(@project.namespace, @project), title: "Fork project", class: 'btn btn-sm btn-default' do = link_to_toggle_fork + .star.pull-right.prepend-left-10.project-repo-buttons.js-toggler-container{class: @show_star ? 'on' : ''} + - if current_user + = link_to_toggle_star('Star this project.', false, true) + = link_to_toggle_star('Unstar this project.', true, true) .project-home-row.hidden-xs - if current_user && !empty_repo diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index b9d65a9bf5..9a2ddeb590 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -89,8 +89,7 @@ = link_to ci_service.title, ci_service.builds_path, :'data-no-turbolink' => 'data-no-turbolink' - unless @project.empty_repo? - if can? current_user, :download_code, @project - .pull-right.prepend-left-10 - = render 'projects/repositories/download_archive', split_button: true + = render 'projects/repositories/download_archive', split_button: true - if readme .tab-pane#tab-readme From 405f91d2b8eb7c8276341ac44fcabc758fc89343 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 29 Mar 2015 18:15:54 -0700 Subject: [PATCH 26/57] Refactor star/fork buttons --- app/assets/stylesheets/pages/projects.scss | 30 ++++++++++++++----- app/helpers/projects_helper.rb | 10 +++++-- app/views/projects/_home_panel.html.haml | 14 ++++----- .../repositories/_download_archive.html.haml | 4 +-- app/views/projects/show.html.haml | 13 +++++--- 5 files changed, 47 insertions(+), 24 deletions(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index de39fc3e9f..5bd725d122 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -48,24 +48,21 @@ .project-home-row { @extend .clearfix; margin-bottom: 15px; - + &.project-home-row-top { - margin-bottom: 11px; + margin-bottom: 15px; } .project-home-desc { font-size: 16px; line-height: 1.3; + margin-right: 215px; } .project-home-desc { float: left; color: #666; } - - .btn-action-count { - margin-left: 5px; - } } .visibility-level-label { @@ -75,9 +72,26 @@ color: inherit; } } - + .project-repo-buttons { - margin-top: -5px; + margin-top: -3px; + position: absolute; + right: 0; + width: 260px; + text-align: right; + + .btn { + font-weight: bold; + font-size: 14px; + line-height: 16px; + + .count { + padding-left: 10px; + border-left: 1px solid #ccc; + display: inline-block; + margin-left: 10px; + } + } } } diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 5ca71b14d0..4629de2eca 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -118,11 +118,15 @@ module ProjectsHelper end def link_to_toggle_fork - out = icon('code-fork') - out << ' Fork' - out << content_tag(:span, class: 'count btn-action-count') do + html = content_tag('span') do + icon('code-fork') + ' Fork' + end + + count_html = content_tag(:span, class: 'count') do @project.forks_count.to_s end + + html + count_html end private diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index 750d305462..774be6cd13 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -14,20 +14,20 @@ – = link_to namespace_project_blob_path(@project.namespace, @project, tree_join(@repository.root_ref, readme.name)) do = readme.name - .pull-right.prepend-left-10.project-repo-buttons + .project-repo-buttons + .inline.star.js-toggler-container{class: @show_star ? 'on' : ''} + - if current_user + = link_to_toggle_star('Star this project.', false, true) + = link_to_toggle_star('Unstar this project.', true, true) - unless @project.empty_repo? - .fork-buttons - - if current_user && can?(current_user, :fork_project, @project) && @project.namespace != current_user.namespace + - if current_user && can?(current_user, :fork_project, @project) && @project.namespace != current_user.namespace + .inline.fork-buttons.prepend-left-10 - if current_user.already_forked?(@project) && current_user.manageable_namespaces.size < 2 = link_to namespace_project_path(current_user, current_user.fork_of(@project)), title: 'Go to your fork', class: 'btn btn-sm btn-default' do = link_to_toggle_fork - else = link_to new_namespace_project_fork_path(@project.namespace, @project), title: "Fork project", class: 'btn btn-sm btn-default' do = link_to_toggle_fork - .star.pull-right.prepend-left-10.project-repo-buttons.js-toggler-container{class: @show_star ? 'on' : ''} - - if current_user - = link_to_toggle_star('Star this project.', false, true) - = link_to_toggle_star('Unstar this project.', true, true) .project-home-row.hidden-xs - if current_user && !empty_repo diff --git a/app/views/projects/repositories/_download_archive.html.haml b/app/views/projects/repositories/_download_archive.html.haml index 26669fb00a..1ba7a1f206 100644 --- a/app/views/projects/repositories/_download_archive.html.haml +++ b/app/views/projects/repositories/_download_archive.html.haml @@ -3,10 +3,10 @@ - split_button = split_button || false - if split_button == true %span.btn-group{class: btn_class} - = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: ref, format: 'zip'), class: 'btn', rel: 'nofollow' do + = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: ref, format: 'zip'), class: 'btn btn-sm', rel: 'nofollow' do %i.fa.fa-download %span Download zip - %a.btn.dropdown-toggle{ 'data-toggle' => 'dropdown' } + %a.btn-sm.btn.dropdown-toggle{ 'data-toggle' => 'dropdown' } %span.caret %span.sr-only Select Archive Format diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index 9a2ddeb590..85113ffa7e 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -68,7 +68,7 @@ = link_to license_url(@project), class: 'btn btn-block' do View license - .prepend-top-10 + .prepend-top-10.append-bottom-10 %p %span.light Created on #{@project.created_at.stamp('Aug 22, 2013')} @@ -79,17 +79,22 @@ - else #{link_to @project.owner_name, @project.owner} + - unless @project.empty_repo? + - if can? current_user, :download_code, @project + %hr + .prepend-top-10.append-bottom-10 + = render 'projects/repositories/download_archive', split_button: true + + .prepend-top-10 - @project.ci_services.each do |ci_service| - if ci_service.active? && ci_service.respond_to?(:builds_path) + %hr - if ci_service.respond_to?(:status_img_path) = link_to ci_service.builds_path, :'data-no-turbolink' => 'data-no-turbolink' do = image_tag ci_service.status_img_path, alt: "build status" - else %span.light CI provided by = link_to ci_service.title, ci_service.builds_path, :'data-no-turbolink' => 'data-no-turbolink' - - unless @project.empty_repo? - - if can? current_user, :download_code, @project - = render 'projects/repositories/download_archive', split_button: true - if readme .tab-pane#tab-readme From 9d6ffcfa5f09608d81031c7e366470b9c8a46db8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 29 Mar 2015 18:50:45 -0700 Subject: [PATCH 27/57] Refactor star btn logic for non-logged in user and fix tests --- app/helpers/projects_helper.rb | 22 ++++++++++------------ app/views/projects/_home_panel.html.haml | 11 +++++++++-- features/project/star.feature | 2 +- features/steps/project/star.rb | 8 ++++++-- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 4629de2eca..e3734023be 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -80,17 +80,17 @@ module ProjectsHelper @project.milestones.active.order("due_date, title ASC") end - def link_to_toggle_star(title, starred, signed_in) + def link_to_toggle_star(title, starred) cls = 'star-btn btn btn-sm btn-default' - cls << ' disabled' unless signed_in + + toggle_text = + if starred + ' Unstar' + else + ' Star' + end toggle_html = content_tag('span', class: 'toggle') do - toggle_text = if starred - ' Unstar' - else - ' Star' - end - icon('star') + toggle_text end @@ -106,12 +106,10 @@ module ProjectsHelper data: { type: 'json' } } + path = toggle_star_namespace_project_path(@project.namespace, @project) content_tag 'span', class: starred ? 'turn-on' : 'turn-off' do - link_to( - toggle_star_namespace_project_path(@project.namespace, @project), - link_opts - ) do + link_to(path, link_opts) do toggle_html + ' ' + count_html end end diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index 774be6cd13..5689bdee1c 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -17,8 +17,15 @@ .project-repo-buttons .inline.star.js-toggler-container{class: @show_star ? 'on' : ''} - if current_user - = link_to_toggle_star('Star this project.', false, true) - = link_to_toggle_star('Unstar this project.', true, true) + = link_to_toggle_star('Star this project.', false) + = link_to_toggle_star('Unstar this project.', true) + - else + = link_to new_user_session_path, class: 'btn star-btn has_tooltip', title: 'You must sign in to star a project' do + %span + = icon('star') + Star + %span.count + = @project.star_count - unless @project.empty_repo? - if current_user && can?(current_user, :fork_project, @project) && @project.namespace != current_user.namespace .inline.fork-buttons.prepend-left-10 diff --git a/features/project/star.feature b/features/project/star.feature index 3322f89180..a45f9c470e 100644 --- a/features/project/star.feature +++ b/features/project/star.feature @@ -13,7 +13,7 @@ Feature: Project Star Given public project "Community" And I visit project "Community" page When I click on the star toggle button - Then The project has 0 stars + Then I redirected to sign in page @javascript Scenario: Signed in users can toggle star diff --git a/features/steps/project/star.rb b/features/steps/project/star.rb index ae2e4c7a20..50cdfd73c3 100644 --- a/features/steps/project/star.rb +++ b/features/steps/project/star.rb @@ -22,12 +22,16 @@ class Spinach::Features::ProjectStar < Spinach::FeatureSteps # Requires @javascript step "I click on the star toggle button" do - find(".star .toggle", visible: true).click + find(".star-btn", visible: true).click + end + + step 'I redirected to sign in page' do + current_path.should == new_user_session_path end protected def has_n_stars(n) - expect(page).to have_css(".star .count", text: /^#{n}$/, visible: true) + expect(page).to have_css(".star-btn .count", text: n, visible: true) end end From 3371e40bd40ad91f476af52c8d89f022c9895784 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 29 Mar 2015 18:55:18 -0700 Subject: [PATCH 28/57] Include brakeman in rake test --- lib/tasks/gitlab/test.rake | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/tasks/gitlab/test.rake b/lib/tasks/gitlab/test.rake index b4076f8238..b4c0ae3ff7 100644 --- a/lib/tasks/gitlab/test.rake +++ b/lib/tasks/gitlab/test.rake @@ -2,6 +2,7 @@ namespace :gitlab do desc "GITLAB | Run all tests" task :test do cmds = [ + %W(rake brakeman), %W(rake rubocop), %W(rake spinach), %W(rake spec), From 227535761d48355cd4523446750e129893016cea Mon Sep 17 00:00:00 2001 From: Andrew Date: Mon, 30 Mar 2015 11:03:42 +0300 Subject: [PATCH 29/57] LICENSE year update --- LICENSE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index d11b8730bf..d8cb29f363 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2011-2014 GitLab B.V. +Copyright (c) 2011-2015 GitLab B.V. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal From b06a4ff5a73b81362f24a3893860a936ed84a5c9 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 30 Mar 2015 15:18:43 +0000 Subject: [PATCH 30/57] Bump Docker build to GitLab v7.9.1 --- docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index b228a66832..f0a8b9f53d 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -11,7 +11,7 @@ RUN apt-get update -q \ # If the Omnibus package version below is outdated please contribute a merge request to update it. # If you run GitLab Enterprise Edition point it to a location where you have downloaded it. RUN TMP_FILE=$(mktemp); \ - wget -q -O $TMP_FILE https://downloads-packages.s3.amazonaws.com/ubuntu-14.04/gitlab_7.9.0-omnibus.2-1_amd64.deb \ + wget -q -O $TMP_FILE https://downloads-packages.s3.amazonaws.com/ubuntu-14.04/gitlab_7.9.1-omnibus.1-1_amd64.deb \ && dpkg -i $TMP_FILE \ && rm -f $TMP_FILE From 718661f9aa4bfe9e767f2e49880bd3774499e3d2 Mon Sep 17 00:00:00 2001 From: Marco Vito Moscaritolo Date: Mon, 30 Mar 2015 18:17:41 +0200 Subject: [PATCH 31/57] Fix merge errors on CHANGELOG During merges (d554070a and 497fd75d) changelog was "damaged", I restored the tagged 7.9.1 and added the required changes about 7.10.0 (unreleased). I think we need to ensure all PR are rebased on master before merge. --- CHANGELOG | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 021110b0e2..90bd9db8c3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,10 +5,7 @@ v 7.10.0 (unreleased) - Set Application controller default URL options to ensure all url_for calls are consistent (Stan Hu) - Allow HTML tags in Markdown input - Fix code unfold not working on Compare commits page (Stan Hu) - - Include missing events and fix save functionality in admin service template settings form (Stan Hu) - - Fix "Import projects from" button to show the correct instructions (Stan Hu) - Fix dots in Wiki slugs causing errors (Stan Hu) - - Fix OAuth2 issue importing a new project from GitHub and GitLab (Stan Hu) - Update poltergeist to version 1.6.0 to support PhantomJS 2.0 (Zeger-Jan van de Weg) - Fix cross references when usernames, milestones, or project names contain underscores (Stan Hu) - Disable reference creation for comments surrounded by code/preformatted blocks (Stan Hu) @@ -39,7 +36,6 @@ v 7.10.0 (unreleased) - Add ability to unlink connected accounts - Replace commits calendar with faster contribution calendar that includes issues and merge requests - Add inifinite scroll to user page activity - - Don't show commit comment button when user is not signed in. - Don't include system notes in issue/MR comment count. - Don't mark merge request as updated when merge status relative to target branch changes. - Link note avatar to user. @@ -51,13 +47,20 @@ v 7.10.0 (unreleased) - Prevent holding Control-Enter or Command-Enter from posting comment multiple times. - Prevent note form from being cleared when submitting failed. - Improve file icons rendering on tree (Sullivan Sénéchal) - -v 7.9.0 - Send EmailsOnPush email when branch or tag is created or deleted. - Faster merge request processing for large repository - Prevent doubling AJAX request with each commit visit via Turbolink - Prevent unnecessary doubling of js events on import pages and user calendar +v 7.9.1 + - Include missing events and fix save functionality in admin service template settings form (Stan Hu) + - Fix "Import projects from" button to show the correct instructions (Stan Hu) + - Fix OAuth2 issue importing a new project from GitHub and GitLab (Stan Hu) + - Fix for LDAP with commas in DN + - Fix missing events and in admin Slack service template settings form (Stan Hu) + - Don't show commit comment button when user is not signed in. + - Downgrade gemnasium-gitlab-service gem + v 7.9.0 - Add HipChat integration documentation (Stan Hu) - Update documentation for object_kind field in Webhook push and tag push Webhooks (Stan Hu) From 82db3dc8b06a7af97c85bd2ac54652a52739f831 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 30 Mar 2015 19:31:13 +0100 Subject: [PATCH 32/57] Fixed issue where only 25 commits would load in file listings --- app/views/projects/refs/logs_tree.js.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/refs/logs_tree.js.haml b/app/views/projects/refs/logs_tree.js.haml index 49ce6c0888..35c15cf3a9 100644 --- a/app/views/projects/refs/logs_tree.js.haml +++ b/app/views/projects/refs/logs_tree.js.haml @@ -15,5 +15,5 @@ if(current_url == log_url) { // Load 10 more commit log for each file in tree // if we still on the same page - ajaxGet('#{logs_file_namespace_project_ref_path(@project.namespace, @project, @ref, @path || '/', offset: (@offset + @limit))}'); + ajaxGet('#{logs_file_namespace_project_ref_path(@project.namespace, @project, @ref, @path || '', offset: (@offset + @limit))}'); } From 64cd7ebf39ea7ca0ebded0547ed6a3b1996d3a21 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Sun, 29 Mar 2015 23:52:05 +0300 Subject: [PATCH 33/57] Update documentation on issue closing pattern. --- config/gitlab.yml.example | 2 +- doc/customization/issue_closing.md | 35 ++++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 3f1ca34a66..1a700e1d72 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -67,7 +67,7 @@ production: &base # This happens when the commit is pushed or merged into the default branch of a project. # When not specified the default issue_closing_pattern as specified below will be used. # Tip: you can test your closing pattern at http://rubular.com - # issue_closing_pattern: '((?:[Cc]los(?:e[sd]|ing)|[Ff]ix(?:e[sd]|ing)?) +(?:(?:issues? +)?#\d+(?:(?:, *| +and +)?))+)' + # issue_closing_pattern: '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?) +(?:(?:issues? +)?#\d+(?:(?:, *| +and +)?))+)' ## Default project features settings default_projects_features: diff --git a/doc/customization/issue_closing.md b/doc/customization/issue_closing.md index ddc0c8eac2..aa65a082a5 100644 --- a/doc/customization/issue_closing.md +++ b/doc/customization/issue_closing.md @@ -1,5 +1,36 @@ # Issue closing pattern -By default you can close issues from commit messages by saying 'Closes #12' or 'Fixed #101'. +If a commit message matches the regular expression below, all issues referenced from +the matched text will be closed. This happens when the commit is pushed or merged +into the default branch of a project. -If you want to customize the message please do so in [gitlab.yml](https://gitlab.com/gitlab-org/gitlab-ce/blob/73b92f85bcd6c213b845cc997843a969cf0906cf/config/gitlab.yml.example#L73) +When not specified, the default issue_closing_pattern as shown below will be used: + +```bash +((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?) +(?:(?:issues? +)?#\d+(?:(?:, *| +and +)?))+) +``` + +For example: + +``` +git commit -m "Awesome commit message (Fix #20, Fixes #21 and Closes #22). This commit is also related to #17 and fixes #18, #19 and #23." +``` + +will close `#20`, `#21`, `#22`, `#18`, `#19` and `#23`, but `#17` won't be closed +as it does not match the pattern. It also works with multiline commit messages. + +Tip: you can test this closing pattern at [http://rubular.com][1]. Use this site +to test your own patterns. + +## Change the pattern + +For Omnibus installs you can change the default pattern in `/etc/gitlab/gitlab.rb`: + +``` +issue_closing_pattern: '((?:[Cc]los(?:e[sd]|ing)|[Ff]ix(?:e[sd]|ing)?) +(?:(?:issues? +)?#\d+(?:(?:, *| +and +)?))+)' +``` + +For manual installs you can customize the pattern in [gitlab.yml][0]. + +[0]: https://gitlab.com/gitlab-org/gitlab-ce/blob/40c3675372320febf5264061c9bcd63db2dfd13c/config/gitlab.yml.example#L65 +[1]: http://rubular.com/r/Xmbexed1OJ From 7a02b5c20cdada26ceb256fd83c9df8c9bc32298 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Mon, 30 Mar 2015 23:55:12 +0300 Subject: [PATCH 34/57] Missing dot. --- config/gitlab.yml.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 1a700e1d72..760a589d6e 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -66,7 +66,7 @@ production: &base # If a commit message matches this regular expression, all issues referenced from the matched text will be closed. # This happens when the commit is pushed or merged into the default branch of a project. # When not specified the default issue_closing_pattern as specified below will be used. - # Tip: you can test your closing pattern at http://rubular.com + # Tip: you can test your closing pattern at http://rubular.com. # issue_closing_pattern: '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?) +(?:(?:issues? +)?#\d+(?:(?:, *| +and +)?))+)' ## Default project features settings From b7e2be247f3414ec5635db382358a6c3a3ddf089 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 30 Mar 2015 14:17:49 -0700 Subject: [PATCH 35/57] Fix adding new members to group --- .../group_members/_new_group_member.html.haml | 2 +- features/groups.feature | 8 ++++++ features/steps/groups.rb | 26 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/app/views/groups/group_members/_new_group_member.html.haml b/app/views/groups/group_members/_new_group_member.html.haml index c4c29bb2e8..a52b819738 100644 --- a/app/views/groups/group_members/_new_group_member.html.haml +++ b/app/views/groups/group_members/_new_group_member.html.haml @@ -1,7 +1,7 @@ = form_for @group_member, url: group_group_members_path(@group), html: { class: 'form-horizontal users-group-form' } do |f| .form-group = f.label :user_ids, "People", class: 'control-label' - .col-sm-10= users_select_tag(:user_ids, multiple: true, class: 'input-large') + .col-sm-10= users_select_tag(:user_ids, multiple: true, class: 'input-large', scope: :all) .form-group = f.label :access_level, "Group Access", class: 'control-label' diff --git a/features/groups.feature b/features/groups.feature index 05546e0d6e..65d06a0daf 100644 --- a/features/groups.feature +++ b/features/groups.feature @@ -47,6 +47,14 @@ Feature: Groups Then I should not see group "Owned" avatar And I should not see the "Remove avatar" button + @javascript + Scenario: Add user to group + Given gitlab user "Mike" + When I visit group "Owned" members page + And I click link "Add members" + When I select "Mike" as "Reporter" + Then I should see "Mike" in team list as "Reporter" + # Leave @javascript diff --git a/features/steps/groups.rb b/features/steps/groups.rb index 91921f5e21..ec5213e4b9 100644 --- a/features/steps/groups.rb +++ b/features/steps/groups.rb @@ -5,6 +5,32 @@ class Spinach::Features::Groups < Spinach::FeatureSteps include SharedUser include Select2Helper + step 'gitlab user "Mike"' do + create(:user, name: "Mike") + end + + step 'I click link "Add members"' do + find(:css, 'button.btn-new').click + end + + step 'I select "Mike" as "Reporter"' do + user = User.find_by(name: "Mike") + + within ".users-group-form" do + select2(user.id, from: "#user_ids", multiple: true) + select "Reporter", from: "access_level" + end + + click_button "Add users to group" + end + + step 'I should see "Mike" in team list as "Reporter"' do + within '.well-list' do + page.should have_content('Mike') + page.should have_content('Reporter') + end + end + step 'I should see group "Owned" projects list' do Group.find_by(name: "Owned").projects.each do |project| page.should have_link project.name From 4d7cd4f329a3ee88864ac7594227c50357c2ccfc Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 26 Mar 2015 21:35:10 +0000 Subject: [PATCH 36/57] Added badge to commits tab --- app/views/projects/commits/_head.html.haml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/projects/commits/_head.html.haml b/app/views/projects/commits/_head.html.haml index 83e4d24cf5..a714f5f79e 100644 --- a/app/views/projects/commits/_head.html.haml +++ b/app/views/projects/commits/_head.html.haml @@ -1,6 +1,8 @@ %ul.nav.nav-tabs = nav_link(controller: [:commit, :commits]) do - = link_to 'Commits', namespace_project_commits_path(@project.namespace, @project, @repository.root_ref) + = link_to namespace_project_commits_path(@project.namespace, @project, @repository.root_ref) do + Commits + %span.badge= number_with_precision(@repository.commit_count, precision: 0, delimiter: ',') = nav_link(controller: :compare) do = link_to 'Compare', namespace_project_compare_index_path(@project.namespace, @project, from: @repository.root_ref, to: @ref || @repository.root_ref) From 549e6c100b8b07cc31591cde33bae055a60a6ec9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 31 Mar 2015 05:50:10 +0300 Subject: [PATCH 37/57] Better legend for contribution calendar --- app/assets/javascripts/calendar.js.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/calendar.js.coffee b/app/assets/javascripts/calendar.js.coffee index 37b7ba2cc1..ff8cab6fab 100644 --- a/app/assets/javascripts/calendar.js.coffee +++ b/app/assets/javascripts/calendar.js.coffee @@ -20,9 +20,9 @@ class @Calendar position: "top" legend: [ 0 - 1 - 4 - 7 + 5 + 10 + 20 ] legendCellPadding: 3 onClick: (date, count) -> From 0191857fac465fbfb4acad1b923c29f3b05529aa Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 31 Mar 2015 05:57:36 +0300 Subject: [PATCH 38/57] Better legend for contribution calendar pt2 :) --- app/assets/javascripts/calendar.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/calendar.js.coffee b/app/assets/javascripts/calendar.js.coffee index ff8cab6fab..44d75bd694 100644 --- a/app/assets/javascripts/calendar.js.coffee +++ b/app/assets/javascripts/calendar.js.coffee @@ -20,9 +20,9 @@ class @Calendar position: "top" legend: [ 0 - 5 10 20 + 30 ] legendCellPadding: 3 onClick: (date, count) -> From 2d5f4458a004dd5e8ef646a2e166bbfe8e78cf77 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Mar 2015 16:04:30 +0100 Subject: [PATCH 39/57] Fix admin user projects lists. --- CHANGELOG | 1 + app/assets/javascripts/dispatcher.js.coffee | 2 ++ app/views/users/_projects.html.haml | 8 ++++---- app/views/users/show.html.haml | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 25936eb1e1..ede883ca98 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -31,6 +31,7 @@ v 7.10.0 (unreleased) - Replace commits calendar with faster contribution calendar that includes issues and merge requests - Add inifinite scroll to user page activity - Don't show commit comment button when user is not signed in. + - Fix admin user projects lists. v 7.9.0 - Send EmailsOnPush email when branch or tag is created or deleted. diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index deabaf8a78..b4e9eb2bae 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -98,6 +98,8 @@ class Dispatcher when 'users:show' new User() new Activities() + when 'admin:users:show' + new ProjectsList() switch path.first() when 'admin' diff --git a/app/views/users/_projects.html.haml b/app/views/users/_projects.html.haml index b7383d5594..297fa53739 100644 --- a/app/views/users/_projects.html.haml +++ b/app/views/users/_projects.html.haml @@ -1,13 +1,13 @@ -- if @contributed_projects.present? +- if local_assigns.has_key?(:contributed_projects) && contributed_projects.present? .panel.panel-default.contributed-projects .panel-heading Projects contributed to = render 'shared/projects_list', - projects: @contributed_projects.sort_by(&:star_count).reverse, + projects: contributed_projects.sort_by(&:star_count).reverse, projects_limit: 5, stars: true, avatar: false -- if @projects.present? +- if local_assigns.has_key?(:projects) && projects.present? .panel.panel-default .panel-heading Personal projects = render 'shared/projects_list', - projects: @projects.sort_by(&:star_count).reverse, + projects: projects.sort_by(&:star_count).reverse, projects_limit: 10, stars: true, avatar: false diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 0653fb871a..512acb47d8 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -44,7 +44,7 @@ = spinner %aside.col-md-4 = render 'profile', user: @user - = render 'projects' + = render 'projects', projects: @projects, contributed_projects: @contributed_projects :coffeescript $ -> From 175f7f68a6c7857523f1d18b184caa8b84fe0ea3 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 31 Mar 2015 09:30:34 +0200 Subject: [PATCH 40/57] Move files for moved namespaces. --- app/models/namespace.rb | 4 -- ...047_remove_periods_at_ends_of_usernames.rb | 41 ++++++++++++++++++- spec/models/namespace_spec.rb | 2 - 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index e2bcd25be2..dd74165f88 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -58,10 +58,6 @@ class Namespace < ActiveRecord::Base where("name LIKE :query OR path LIKE :query", query: "%#{query}%") end - def global_id - 'GLN' - end - def clean_path(path) path.gsub!(/@.*\z/, "") path.gsub!(/\.git\z/, "") diff --git a/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb b/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb index 36e3faa53b..bd49caef68 100644 --- a/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb +++ b/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb @@ -1,4 +1,6 @@ class RemovePeriodsAtEndsOfUsernames < ActiveRecord::Migration + include Gitlab::ShellAdapter + class Namespace < ActiveRecord::Base class << self def by_path(path) @@ -6,6 +8,7 @@ class RemovePeriodsAtEndsOfUsernames < ActiveRecord::Migration end def clean_path(path) + path = path.dup path.gsub!(/@.*\z/, "") path.gsub!(/\.git\z/, "") path.gsub!(/\A-/, "") @@ -25,15 +28,49 @@ class RemovePeriodsAtEndsOfUsernames < ActiveRecord::Migration end def up + changed_paths = {} + select_all("SELECT id, username FROM users WHERE username LIKE '%.'").each do |user| - username = quote_string(Namespace.clean_path(user["username"])) + username_was = user["username"] + username = Namespace.clean_path(username_was) + changed_paths[username_was] = username + + username = quote_string(username) execute "UPDATE users SET username = '#{username}' WHERE id = #{user["id"]}" execute "UPDATE namespaces SET path = '#{username}', name = '#{username}' WHERE type = NULL AND owner_id = #{user["id"]}" end select_all("SELECT id, path FROM namespaces WHERE type = 'Group' AND path LIKE '%.'").each do |group| - path = quote_string(Namespace.clean_path(group["path"])) + path_was = group["path"] + path = Namespace.clean_path(path_was) + changed_paths[path_was] = path + + path = quote_string(path) execute "UPDATE namespaces SET path = '#{path}' WHERE id = #{group["id"]}" end + + changed_paths.each do |path_was, path| + if gitlab_shell.mv_namespace(path_was, path) + # If repositories moved successfully we need to remove old satellites + # and send update instructions to users. + # However we cannot allow rollback since we moved namespace dir + # So we basically we mute exceptions in next actions + begin + gitlab_shell.rm_satellites(path_was) + # We cannot send update instructions since models and mailers + # can't safely be used from migrations as they may be written for + # later versions of the database. + # send_update_instructions + rescue + # Returning false does not rollback after_* transaction but gives + # us information about failing some of tasks + false + end + else + # if we cannot move namespace directory we should rollback + # db changes in order to prevent out of sync between db and fs + raise Exception.new('namespace directory cannot be moved') + end + end end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 48a3ab9c5a..e87432fdf6 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -33,8 +33,6 @@ describe Namespace do it { is_expected.to respond_to(:to_param) } end - it { expect(Namespace.global_id).to eq('GLN') } - describe :to_param do it { expect(namespace.to_param).to eq(namespace.path) } end From 2cfd0b59aeb410c1541530ca3eb5f5c472b978e0 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Mar 2015 13:15:59 +0100 Subject: [PATCH 41/57] Archive repositories in background worker. --- Procfile | 2 +- .../projects/repositories_controller.rb | 8 ++- app/models/repository.rb | 3 + app/services/archive_repository_service.rb | 67 ++++++++++++++++--- app/workers/repository_archive_worker.rb | 46 +++++++++++++ lib/api/repositories.rb | 11 +-- 6 files changed, 121 insertions(+), 16 deletions(-) create mode 100644 app/workers/repository_archive_worker.rb diff --git a/Procfile b/Procfile index a0ab4a734a..799b92729f 100644 --- a/Procfile +++ b/Procfile @@ -1,2 +1,2 @@ web: bundle exec unicorn_rails -p ${PORT:="3000"} -E ${RAILS_ENV:="development"} -c ${UNICORN_CONFIG:="config/unicorn.rb"} -worker: bundle exec sidekiq -q post_receive -q mailer -q system_hook -q project_web_hook -q gitlab_shell -q common -q default +worker: bundle exec sidekiq -q post_receive -q mailer -q archive_repo -q system_hook -q project_web_hook -q gitlab_shell -q common -q default diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb index cbb888b25e..112365ba91 100644 --- a/app/controllers/projects/repositories_controller.rb +++ b/app/controllers/projects/repositories_controller.rb @@ -15,14 +15,18 @@ class Projects::RepositoriesController < Projects::ApplicationController render_404 and return end - file_path = ArchiveRepositoryService.new.execute(@project, params[:ref], params[:format]) + begin + file_path = ArchiveRepositoryService.new(@project, params[:ref], params[:format]).execute + rescue + return render_404 + end if file_path # Send file to user response.headers["Content-Length"] = File.open(file_path).size.to_s send_file file_path else - render_404 + redirect_to request.fullpath end end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 77765cae1a..7276949887 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -267,6 +267,9 @@ class Repository # Remove archives older than 2 hours def clean_old_archives repository_downloads_path = Gitlab.config.gitlab.repository_downloads_path + + return unless File.directory?(repository_downloads_path) + Gitlab::Popen.popen(%W(find #{repository_downloads_path} -not -path #{repository_downloads_path} -mmin +120 -delete)) end diff --git a/app/services/archive_repository_service.rb b/app/services/archive_repository_service.rb index 8823f6fdc6..cb2026fedb 100644 --- a/app/services/archive_repository_service.rb +++ b/app/services/archive_repository_service.rb @@ -1,14 +1,65 @@ class ArchiveRepositoryService - def execute(project, ref, format) - storage_path = Gitlab.config.gitlab.repository_downloads_path + attr_reader :project, :ref, :format - unless File.directory?(storage_path) - FileUtils.mkdir_p(storage_path) + def initialize(project, ref, format) + format ||= 'tar.gz' + @project, @ref, @format = project, ref, format + end + + def execute + project.repository.clean_old_archives + + raise "No archive file path" unless file_path + + return file_path if archived? + + unless archiving? + RepositoryArchiveWorker.perform_async(project.id, ref, format) end - format ||= 'tar.gz' - repository = project.repository - repository.clean_old_archives - repository.archive_repo(ref, storage_path, format.downcase) + archived = wait_until_archived + + file_path if archived + end + + private + + def storage_path + Gitlab.config.gitlab.repository_downloads_path + end + + def archive_args + @archive_args ||= [ref, storage_path, format.downcase] + end + + def file_path + @file_path ||= project.repository.archive_file_path(*archive_args) + end + + def pid_file_path + @pid_file_path ||= project.repository.archive_pid_file_path(*archive_args) + end + + def archived? + File.exist?(file_path) + end + + def archiving? + File.exist?(pid_file_path) + end + + def wait_until_archived + timeout = 5.0 + t1 = Time.now + + begin + sleep 0.1 + + success = archived? + + t2 = Time.now + end until success || t2 - t1 >= timeout + + success end end diff --git a/app/workers/repository_archive_worker.rb b/app/workers/repository_archive_worker.rb new file mode 100644 index 0000000000..3f4681a80f --- /dev/null +++ b/app/workers/repository_archive_worker.rb @@ -0,0 +1,46 @@ +class RepositoryArchiveWorker + include Sidekiq::Worker + + sidekiq_options queue: :archive_repo + + attr_accessor :project, :ref, :format + + def perform(project_id, ref, format) + @project = Project.find(project_id) + @ref, @format = ref, format + + repository = project.repository + + repository.clean_old_archives + + return if archived? || archiving? + + repository.archive_repo(*archive_args) + end + + private + + def storage_path + Gitlab.config.gitlab.repository_downloads_path + end + + def archive_args + @archive_args ||= [ref, storage_path, format.downcase] + end + + def file_path + @file_path ||= project.repository.archive_file_path(*archive_args) + end + + def pid_file_path + @pid_file_path ||= project.repository.archive_pid_file_path(*archive_args) + end + + def archived? + File.exist?(file_path) + end + + def archiving? + File.exist?(pid_file_path) + end +end diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index b259914a01..1fbf3dca3c 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -133,10 +133,11 @@ module API authorize! :download_code, user_project begin - file_path = ArchiveRepositoryService.new.execute( - user_project, - params[:sha], - params[:format]) + file_path = ArchiveRepositoryService.new( + user_project, + params[:sha], + params[:format] + ).execute rescue not_found!('File') end @@ -149,7 +150,7 @@ module API env['api.format'] = :binary present data else - not_found!('File') + redirect request.fullpath end end From e7bbd85b3424ec64cb7f56452fd7f5ee48663a60 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Mar 2015 13:16:11 +0100 Subject: [PATCH 42/57] Limit number of concurrent archive_repo jobs to 2. --- Gemfile | 1 + Gemfile.lock | 3 +++ config/initializers/4_sidekiq.rb | 2 ++ 3 files changed, 6 insertions(+) diff --git a/Gemfile b/Gemfile index e767aec505..0dd285d0fd 100644 --- a/Gemfile +++ b/Gemfile @@ -122,6 +122,7 @@ gem 'slim' gem 'sinatra', require: nil gem 'sidekiq', '~> 3.3' gem 'sidetiq', '0.6.3' +gem 'sidekiq-limit_fetch' # HTTP requests gem "httparty" diff --git a/Gemfile.lock b/Gemfile.lock index ed8663b358..c6ecdb4db0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -547,6 +547,8 @@ GEM json redis (>= 3.0.6) redis-namespace (>= 1.3.1) + sidekiq-limit_fetch (2.4.1) + sidekiq (>= 2.6.5, < 4.0) sidetiq (0.6.3) celluloid (>= 0.14.1) ice_cube (= 0.11.1) @@ -769,6 +771,7 @@ DEPENDENCIES settingslogic shoulda-matchers (~> 2.7.0) sidekiq (~> 3.3) + sidekiq-limit_fetch sidetiq (= 0.6.3) simplecov sinatra diff --git a/config/initializers/4_sidekiq.rb b/config/initializers/4_sidekiq.rb index e856499732..da2e8a08b0 100644 --- a/config/initializers/4_sidekiq.rb +++ b/config/initializers/4_sidekiq.rb @@ -25,3 +25,5 @@ Sidekiq.configure_client do |config| namespace: 'resque:gitlab' } end + +Sidekiq::Queue["archive_repo"].limit = 2 From ccc01b36ba3cd0e1f9c3a6e0201207114e3d1d41 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Mar 2015 13:24:35 +0100 Subject: [PATCH 43/57] Small code cleanup. --- app/services/archive_repository_service.rb | 13 ++++--------- app/workers/repository_archive_worker.rb | 12 ++++-------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/app/services/archive_repository_service.rb b/app/services/archive_repository_service.rb index cb2026fedb..40b0a64fb7 100644 --- a/app/services/archive_repository_service.rb +++ b/app/services/archive_repository_service.rb @@ -3,7 +3,7 @@ class ArchiveRepositoryService def initialize(project, ref, format) format ||= 'tar.gz' - @project, @ref, @format = project, ref, format + @project, @ref, @format = project, ref, format.downcase end def execute @@ -28,16 +28,12 @@ class ArchiveRepositoryService Gitlab.config.gitlab.repository_downloads_path end - def archive_args - @archive_args ||= [ref, storage_path, format.downcase] - end - def file_path - @file_path ||= project.repository.archive_file_path(*archive_args) + @file_path ||= project.repository.archive_file_path(ref, storage_path, format) end def pid_file_path - @pid_file_path ||= project.repository.archive_pid_file_path(*archive_args) + @pid_file_path ||= project.repository.archive_pid_file_path(ref, storage_path, format) end def archived? @@ -48,8 +44,7 @@ class ArchiveRepositoryService File.exist?(pid_file_path) end - def wait_until_archived - timeout = 5.0 + def wait_until_archived(timeout = 5.0) t1 = Time.now begin diff --git a/app/workers/repository_archive_worker.rb b/app/workers/repository_archive_worker.rb index 3f4681a80f..42ac77c588 100644 --- a/app/workers/repository_archive_worker.rb +++ b/app/workers/repository_archive_worker.rb @@ -7,7 +7,7 @@ class RepositoryArchiveWorker def perform(project_id, ref, format) @project = Project.find(project_id) - @ref, @format = ref, format + @ref, @format = ref, format.downcase repository = project.repository @@ -15,7 +15,7 @@ class RepositoryArchiveWorker return if archived? || archiving? - repository.archive_repo(*archive_args) + repository.archive_repo(ref, storage_path, format) end private @@ -24,16 +24,12 @@ class RepositoryArchiveWorker Gitlab.config.gitlab.repository_downloads_path end - def archive_args - @archive_args ||= [ref, storage_path, format.downcase] - end - def file_path - @file_path ||= project.repository.archive_file_path(*archive_args) + @file_path ||= project.repository.archive_file_path(ref, storage_path, format) end def pid_file_path - @pid_file_path ||= project.repository.archive_pid_file_path(*archive_args) + @pid_file_path ||= project.repository.archive_pid_file_path(ref, storage_path, format) end def archived? From 2fdae52fd1048779a84770b39fe2961f31b1bd79 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Mar 2015 13:24:38 +0100 Subject: [PATCH 44/57] Add item to changelog. --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 37054da46b..03c20e8f3e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -48,6 +48,7 @@ v 7.10.0 (unreleased) - Prevent note form from being cleared when submitting failed. - Improve file icons rendering on tree (Sullivan Sénéchal) - API: Add pagination to project events + - Archive repositories in background worker. v 7.9.0 - Send EmailsOnPush email when branch or tag is created or deleted. From 91761b06851832be643258f390ea7593beacae16 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 31 Mar 2015 13:37:21 +0200 Subject: [PATCH 45/57] Add tests. --- .../projects/repositories_controller.rb | 6 +- app/services/archive_repository_service.rb | 6 +- app/workers/repository_archive_worker.rb | 1 + .../projects/repositories_controller_spec.rb | 65 +++++++++++++ .../archive_repository_service_spec.rb | 93 +++++++++++++++++++ .../workers/repository_archive_worker_spec.rb | 80 ++++++++++++++++ 6 files changed, 244 insertions(+), 7 deletions(-) create mode 100644 spec/controllers/projects/repositories_controller_spec.rb create mode 100644 spec/services/archive_repository_service_spec.rb create mode 100644 spec/workers/repository_archive_worker_spec.rb diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb index 112365ba91..96defb0c72 100644 --- a/app/controllers/projects/repositories_controller.rb +++ b/app/controllers/projects/repositories_controller.rb @@ -11,14 +11,10 @@ class Projects::RepositoriesController < Projects::ApplicationController end def archive - unless can?(current_user, :download_code, @project) - render_404 and return - end - begin file_path = ArchiveRepositoryService.new(@project, params[:ref], params[:format]).execute rescue - return render_404 + return head :not_found end if file_path diff --git a/app/services/archive_repository_service.rb b/app/services/archive_repository_service.rb index 40b0a64fb7..e1b41527d8 100644 --- a/app/services/archive_repository_service.rb +++ b/app/services/archive_repository_service.rb @@ -6,7 +6,7 @@ class ArchiveRepositoryService @project, @ref, @format = project, ref, format.downcase end - def execute + def execute(options = {}) project.repository.clean_old_archives raise "No archive file path" unless file_path @@ -17,7 +17,7 @@ class ArchiveRepositoryService RepositoryArchiveWorker.perform_async(project.id, ref, format) end - archived = wait_until_archived + archived = wait_until_archived(options[:timeout] || 5.0) file_path if archived end @@ -45,6 +45,8 @@ class ArchiveRepositoryService end def wait_until_archived(timeout = 5.0) + return archived? if timeout == 0.0 + t1 = Time.now begin diff --git a/app/workers/repository_archive_worker.rb b/app/workers/repository_archive_worker.rb index 42ac77c588..021c113956 100644 --- a/app/workers/repository_archive_worker.rb +++ b/app/workers/repository_archive_worker.rb @@ -13,6 +13,7 @@ class RepositoryArchiveWorker repository.clean_old_archives + return unless file_path return if archived? || archiving? repository.archive_repo(ref, storage_path, format) diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb new file mode 100644 index 0000000000..91856ed0cc --- /dev/null +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -0,0 +1,65 @@ +require "spec_helper" + +describe Projects::RepositoriesController do + let(:project) { create(:project) } + let(:user) { create(:user) } + + describe "GET archive" do + before do + sign_in(user) + project.team << [user, :developer] + + allow(ArchiveRepositoryService).to receive(:new).and_return(service) + end + + let(:service) { ArchiveRepositoryService.new(project, "master", "zip") } + + it "executes ArchiveRepositoryService" do + expect(ArchiveRepositoryService).to receive(:new).with(project, "master", "zip") + expect(service).to receive(:execute) + + get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + end + + context "when the service raises an error" do + + before do + allow(service).to receive(:execute).and_raise("Archive failed") + end + + it "renders Not Found" do + get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + + expect(response.status).to eq(404) + end + end + + context "when the service doesn't return a path" do + + before do + allow(service).to receive(:execute).and_return(nil) + end + + it "reloads the page" do + get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + + expect(response).to redirect_to(archive_namespace_project_repository_path(project.namespace, project, ref: "master", format: "zip")) + end + end + + context "when the service returns a path" do + + let(:path) { Rails.root.join("spec/fixtures/dk.png").to_s } + + before do + allow(service).to receive(:execute).and_return(path) + end + + it "sends the file" do + get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + + expect(response.body).to eq(File.binread(path)) + end + end + end +end diff --git a/spec/services/archive_repository_service_spec.rb b/spec/services/archive_repository_service_spec.rb new file mode 100644 index 0000000000..f168a91397 --- /dev/null +++ b/spec/services/archive_repository_service_spec.rb @@ -0,0 +1,93 @@ +require 'spec_helper' + +describe ArchiveRepositoryService do + let(:project) { create(:project) } + subject { ArchiveRepositoryService.new(project, "master", "zip") } + + describe "#execute" do + it "cleans old archives" do + expect(project.repository).to receive(:clean_old_archives) + + subject.execute(timeout: 0.0) + end + + context "when the repository doesn't have an archive file path" do + before do + allow(project.repository).to receive(:archive_file_path).and_return(nil) + end + + it "raises an error" do + expect { + subject.execute(timeout: 0.0) + }.to raise_error + end + end + + context "when the repository has an archive file path" do + let(:file_path) { "/archive.zip" } + let(:pid_file_path) { "/archive.zip.pid" } + + before do + allow(project.repository).to receive(:archive_file_path).and_return(file_path) + allow(project.repository).to receive(:archive_pid_file_path).and_return(pid_file_path) + end + + context "when the archive file already exists" do + before do + allow(File).to receive(:exist?).with(file_path).and_return(true) + end + + it "returns the file path" do + expect(subject.execute(timeout: 0.0)).to eq(file_path) + end + end + + context "when the archive file doesn't exist yet" do + before do + allow(File).to receive(:exist?).with(file_path).and_return(false) + allow(File).to receive(:exist?).with(pid_file_path).and_return(true) + end + + context "when the archive pid file doesn't exist yet" do + before do + allow(File).to receive(:exist?).with(pid_file_path).and_return(false) + end + + it "queues the RepositoryArchiveWorker" do + expect(RepositoryArchiveWorker).to receive(:perform_async) + + subject.execute(timeout: 0.0) + end + end + + context "when the archive pid file already exists" do + it "doesn't queue the RepositoryArchiveWorker" do + expect(RepositoryArchiveWorker).not_to receive(:perform_async) + + subject.execute(timeout: 0.0) + end + end + + context "when the archive file exists after a little while" do + before do + Thread.new do + sleep 0.1 + allow(File).to receive(:exist?).with(file_path).and_return(true) + end + end + + it "returns the file path" do + expect(subject.execute(timeout: 0.2)).to eq(file_path) + end + end + + context "when the archive file doesn't exist after the timeout" do + it "returns nil" do + expect(subject.execute(timeout: 0.0)).to eq(nil) + end + end + end + end + end +end + diff --git a/spec/workers/repository_archive_worker_spec.rb b/spec/workers/repository_archive_worker_spec.rb new file mode 100644 index 0000000000..c2362058cf --- /dev/null +++ b/spec/workers/repository_archive_worker_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +describe RepositoryArchiveWorker do + let(:project) { create(:project) } + subject { RepositoryArchiveWorker.new } + + before do + allow(Project).to receive(:find).and_return(project) + end + + describe "#perform" do + it "cleans old archives" do + expect(project.repository).to receive(:clean_old_archives) + + subject.perform(project.id, "master", "zip") + end + + context "when the repository doesn't have an archive file path" do + before do + allow(project.repository).to receive(:archive_file_path).and_return(nil) + end + + it "doesn't archive the repo" do + expect(project.repository).not_to receive(:archive_repo) + + subject.perform(project.id, "master", "zip") + end + end + + context "when the repository has an archive file path" do + let(:file_path) { "/archive.zip" } + let(:pid_file_path) { "/archive.zip.pid" } + + before do + allow(project.repository).to receive(:archive_file_path).and_return(file_path) + allow(project.repository).to receive(:archive_pid_file_path).and_return(pid_file_path) + end + + context "when the archive file already exists" do + before do + allow(File).to receive(:exist?).with(file_path).and_return(true) + end + + it "doesn't archive the repo" do + expect(project.repository).not_to receive(:archive_repo) + + subject.perform(project.id, "master", "zip") + end + end + + context "when the archive file doesn't exist yet" do + before do + allow(File).to receive(:exist?).with(file_path).and_return(false) + allow(File).to receive(:exist?).with(pid_file_path).and_return(true) + end + + context "when the archive pid file doesn't exist yet" do + before do + allow(File).to receive(:exist?).with(pid_file_path).and_return(false) + end + + it "archives the repo" do + expect(project.repository).to receive(:archive_repo) + + subject.perform(project.id, "master", "zip") + end + end + + context "when the archive pid file already exists" do + it "doesn't archive the repo" do + expect(project.repository).not_to receive(:archive_repo) + + subject.perform(project.id, "master", "zip") + end + end + end + end + end +end + From 737f322e41a640194b3df1a4c6cd2b1019268221 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 31 Mar 2015 16:34:13 +0200 Subject: [PATCH 46/57] Import GitHub, Bitbucket or GitLab.com projects owned by authenticated user into current namespace. --- CHANGELOG | 1 + app/controllers/import/bitbucket_controller.rb | 5 ++++- app/controllers/import/github_controller.rb | 5 ++++- app/controllers/import/gitlab_controller.rb | 5 ++++- lib/gitlab/gitlab_import/client.rb | 4 ++++ 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 37054da46b..23ceb35050 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -48,6 +48,7 @@ v 7.10.0 (unreleased) - Prevent note form from being cleared when submitting failed. - Improve file icons rendering on tree (Sullivan Sénéchal) - API: Add pagination to project events + - Import GitHub, Bitbucket or GitLab.com projects owned by authenticated user into current namespace. v 7.9.0 - Send EmailsOnPush email when branch or tag is created or deleted. diff --git a/app/controllers/import/bitbucket_controller.rb b/app/controllers/import/bitbucket_controller.rb index 83ebc5fddc..bb8d7e0235 100644 --- a/app/controllers/import/bitbucket_controller.rb +++ b/app/controllers/import/bitbucket_controller.rb @@ -36,8 +36,11 @@ class Import::BitbucketController < Import::BaseController def create @repo_id = params[:repo_id] || "" repo = client.project(@repo_id.gsub("___", "/")) - @target_namespace = params[:new_namespace].presence || repo["owner"] @project_name = repo["slug"] + + repo_owner = repo["owner"] + repo_owner = current_user.username if repo_owner == client.user["user"]["username"] + @target_namespace = params[:new_namespace].presence || repo_owner namespace = get_or_create_namespace || (render and return) diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index 8650b6464d..87b41454c7 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -31,8 +31,11 @@ class Import::GithubController < Import::BaseController def create @repo_id = params[:repo_id].to_i repo = client.repo(@repo_id) - @target_namespace = params[:new_namespace].presence || repo.owner.login @project_name = repo.name + + repo_owner = repo.owner.login + repo_owner = current_user.username if repo_owner == client.user.login + @target_namespace = params[:new_namespace].presence || repo_owner namespace = get_or_create_namespace || (render and return) diff --git a/app/controllers/import/gitlab_controller.rb b/app/controllers/import/gitlab_controller.rb index e979dad4b1..bddbfded81 100644 --- a/app/controllers/import/gitlab_controller.rb +++ b/app/controllers/import/gitlab_controller.rb @@ -28,8 +28,11 @@ class Import::GitlabController < Import::BaseController def create @repo_id = params[:repo_id].to_i repo = client.project(@repo_id) - @target_namespace = params[:new_namespace].presence || repo["namespace"]["path"] @project_name = repo["name"] + + repo_owner = repo["namespace"]["path"] + repo_owner = current_user.username if repo_owner == client.user["username"] + @target_namespace = params[:new_namespace].presence || repo_owner namespace = get_or_create_namespace || (render and return) diff --git a/lib/gitlab/gitlab_import/client.rb b/lib/gitlab/gitlab_import/client.rb index f48ede9d06..9c00896c91 100644 --- a/lib/gitlab/gitlab_import/client.rb +++ b/lib/gitlab/gitlab_import/client.rb @@ -28,6 +28,10 @@ module Gitlab client.auth_code.get_token(code, redirect_uri: redirect_uri).token end + def user + api.get("/api/v3/user").parsed + end + def issues(project_identifier) lazy_page_iterator(PER_PAGE) do |page| api.get("/api/v3/projects/#{project_identifier}/issues?per_page=#{PER_PAGE}&page=#{page}").parsed From a6c633567173e4675f37ec19d31094bf6c50ed3c Mon Sep 17 00:00:00 2001 From: Dan Tudor Date: Tue, 31 Mar 2015 17:08:33 +0100 Subject: [PATCH 47/57] Added the missing comma --- lib/api/branches.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/branches.rb b/lib/api/branches.rb index 431d52a98e..592100a704 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -100,7 +100,7 @@ module API # branch (required) - The name of the branch # Example Request: # DELETE /projects/:id/repository/branches/:branch - delete ":id/repository/branches/:branch" + delete ":id/repository/branches/:branch", requirements: { branch: /.*/ } do authorize_push_project result = DeleteBranchService.new(user_project, current_user). From 45ee9009f54da2e538bfe4f27d3f9a084fbb39c4 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 31 Mar 2015 23:42:49 +0200 Subject: [PATCH 48/57] Fix migration SQL. --- .../20150324133047_remove_periods_at_ends_of_usernames.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb b/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb index bd49caef68..7ce53c2a0d 100644 --- a/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb +++ b/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb @@ -37,7 +37,7 @@ class RemovePeriodsAtEndsOfUsernames < ActiveRecord::Migration username = quote_string(username) execute "UPDATE users SET username = '#{username}' WHERE id = #{user["id"]}" - execute "UPDATE namespaces SET path = '#{username}', name = '#{username}' WHERE type = NULL AND owner_id = #{user["id"]}" + execute "UPDATE namespaces SET path = '#{username}', name = '#{username}' WHERE type IS NULL AND owner_id = #{user["id"]}" end select_all("SELECT id, path FROM namespaces WHERE type = 'Group' AND path LIKE '%.'").each do |group| From 085f12288d92f915f74f2aa85ea21bfb905f27b2 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 30 Mar 2015 17:59:52 -0400 Subject: [PATCH 49/57] Unbind task item checkbox events and then rebind them Fixes #1340 --- app/assets/javascripts/issue.js.coffee | 8 ++------ app/assets/javascripts/merge_request.js.coffee | 8 ++------ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/issue.js.coffee b/app/assets/javascripts/issue.js.coffee index bf71c144ea..0aaa62d2b8 100644 --- a/app/assets/javascripts/issue.js.coffee +++ b/app/assets/javascripts/issue.js.coffee @@ -9,12 +9,8 @@ class @Issue if $("a.btn-close").length $("li.task-list-item input:checkbox").prop("disabled", false) - $(".task-list-item input:checkbox").on( - "click" - null - "issue" - updateTaskState - ) + $('.task-list-item input:checkbox').off('change') + $('.task-list-item input:checkbox').change('issue', updateTaskState) $('.issue-details').waitForImages -> $('.issuable-affix').affix offset: diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee index 6127d2bb48..2cbe03b986 100644 --- a/app/assets/javascripts/merge_request.js.coffee +++ b/app/assets/javascripts/merge_request.js.coffee @@ -81,12 +81,8 @@ class @MergeRequest this.$('.remove_source_branch_in_progress').hide() this.$('.remove_source_branch_widget.failed').show() - $(".task-list-item input:checkbox").on( - "click" - null - "merge_request" - updateTaskState - ) + $('.task-list-item input:checkbox').off('change') + $('.task-list-item input:checkbox').change('merge_request', updateTaskState) activateTab: (action) -> this.$('.merge-request-tabs li').removeClass 'active' From 6a790b79638c1f5cb2f4519655101f8f7eaf887e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 1 Apr 2015 02:45:47 +0300 Subject: [PATCH 50/57] Better looking milestone lists --- app/assets/stylesheets/generic/common.scss | 2 +- app/helpers/gitlab_routing_helper.rb | 4 +++ .../dashboard/milestones/_milestone.html.haml | 20 ++++++++++++++ .../dashboard/milestones/index.html.haml | 20 +------------- .../groups/milestones/_milestone.html.haml | 25 ++++++++++++++++++ app/views/groups/milestones/index.html.haml | 26 +------------------ .../projects/milestones/_milestone.html.haml | 22 +++++++--------- 7 files changed, 62 insertions(+), 57 deletions(-) create mode 100644 app/views/dashboard/milestones/_milestone.html.haml create mode 100644 app/views/groups/milestones/_milestone.html.haml diff --git a/app/assets/stylesheets/generic/common.scss b/app/assets/stylesheets/generic/common.scss index db393e0881..7c3021989a 100644 --- a/app/assets/stylesheets/generic/common.scss +++ b/app/assets/stylesheets/generic/common.scss @@ -246,7 +246,7 @@ li.note { .milestone { &.milestone-closed { - background: #eee; + background: #f9f9f9; } .progress { margin-bottom: 0; diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index 3386fac865..9703c8d9e9 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -29,6 +29,10 @@ module GitlabRoutingHelper namespace_project_merge_request_path(entity.project.namespace, entity.project, entity, *args) end + def milestone_path(entity, *args) + namespace_project_milestone_path(entity.project.namespace, entity.project, entity, *args) + end + def project_url(project, *args) namespace_project_url(project.namespace, project, *args) end diff --git a/app/views/dashboard/milestones/_milestone.html.haml b/app/views/dashboard/milestones/_milestone.html.haml new file mode 100644 index 0000000000..21e730bb7f --- /dev/null +++ b/app/views/dashboard/milestones/_milestone.html.haml @@ -0,0 +1,20 @@ +%li{class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: dom_id(milestone.milestones.first) } + %h4 + = link_to_gfm truncate(milestone.title, length: 100), dashboard_milestone_path(milestone.safe_title, title: milestone.title) + .row + .col-sm-6 + = link_to dashboard_milestone_path(milestone.safe_title, title: milestone.title) do + = pluralize milestone.issue_count, 'Issue' +   + = link_to dashboard_milestone_path(milestone.safe_title, title: milestone.title) do + = pluralize milestone.merge_requests_count, 'Merge Request' +   + %span.light #{milestone.percent_complete}% complete + + .col-sm-6 + = milestone_progress_bar(milestone) + %div + - milestone.milestones.each do |milestone| + = link_to milestone_path(milestone) do + %span.label.label-gray + = milestone.project.name_with_namespace diff --git a/app/views/dashboard/milestones/index.html.haml b/app/views/dashboard/milestones/index.html.haml index caf3b68586..9944c0df81 100644 --- a/app/views/dashboard/milestones/index.html.haml +++ b/app/views/dashboard/milestones/index.html.haml @@ -16,23 +16,5 @@ .nothing-here-block No milestones to show - else - @dashboard_milestones.each do |milestone| - %li{class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: dom_id(milestone.milestones.first) } - %h4 - = link_to_gfm truncate(milestone.title, length: 100), dashboard_milestone_path(milestone.safe_title, title: milestone.title) - %div - %div - = link_to dashboard_milestone_path(milestone.safe_title, title: milestone.title) do - = pluralize milestone.issue_count, 'Issue' -   - = link_to dashboard_milestone_path(milestone.safe_title, title: milestone.title) do - = pluralize milestone.merge_requests_count, 'Merge Request' -   - %span.light #{milestone.percent_complete}% complete - = milestone_progress_bar(milestone) - %div - %br - - milestone.milestones.each do |milestone| - = link_to namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone) do - %span.label.label-default - = milestone.project.name_with_namespace + = render 'milestone', milestone: milestone = paginate @dashboard_milestones, theme: "gitlab" diff --git a/app/views/groups/milestones/_milestone.html.haml b/app/views/groups/milestones/_milestone.html.haml new file mode 100644 index 0000000000..94fc43a581 --- /dev/null +++ b/app/views/groups/milestones/_milestone.html.haml @@ -0,0 +1,25 @@ +%li{class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: dom_id(milestone.milestones.first) } + .pull-right + - if can?(current_user, :manage_group, @group) + - if milestone.closed? + = link_to 'Reopen Milestone', group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-sm btn-grouped btn-reopen" + - else + = link_to 'Close Milestone', group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: {state_event: :close }), method: :put, class: "btn btn-sm btn-close" + %h4 + = link_to_gfm truncate(milestone.title, length: 100), group_milestone_path(@group, milestone.safe_title, title: milestone.title) + .row + .col-sm-6 + = link_to group_milestone_path(@group, milestone.safe_title, title: milestone.title) do + = pluralize milestone.issue_count, 'Issue' +   + = link_to group_milestone_path(@group, milestone.safe_title, title: milestone.title) do + = pluralize milestone.merge_requests_count, 'Merge Request' +   + %span.light #{milestone.percent_complete}% complete + .col-sm-6 + = milestone_progress_bar(milestone) + %div + - milestone.milestones.each do |milestone| + = link_to milestone_path(milestone) do + %span.label.label-gray + = milestone.project.name diff --git a/app/views/groups/milestones/index.html.haml b/app/views/groups/milestones/index.html.haml index 57dc235f5b..008d5a6bd2 100644 --- a/app/views/groups/milestones/index.html.haml +++ b/app/views/groups/milestones/index.html.haml @@ -18,29 +18,5 @@ .nothing-here-block No milestones to show - else - @group_milestones.each do |milestone| - %li{class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: dom_id(milestone.milestones.first) } - .pull-right - - if can?(current_user, :manage_group, @group) - - if milestone.closed? - = link_to 'Reopen Milestone', group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-sm btn-grouped btn-reopen" - - else - = link_to 'Close Milestone', group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: {state_event: :close }), method: :put, class: "btn btn-sm btn-close" - %h4 - = link_to_gfm truncate(milestone.title, length: 100), group_milestone_path(@group, milestone.safe_title, title: milestone.title) - %div - %div - = link_to group_milestone_path(@group, milestone.safe_title, title: milestone.title) do - = pluralize milestone.issue_count, 'Issue' -   - = link_to group_milestone_path(@group, milestone.safe_title, title: milestone.title) do - = pluralize milestone.merge_requests_count, 'Merge Request' -   - %span.light #{milestone.percent_complete}% complete - = milestone_progress_bar(milestone) - %div - %br - - milestone.milestones.each do |milestone| - = link_to namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone) do - %span.label.label-default - = milestone.project.name + = render 'milestone', milestone: milestone = paginate @group_milestones, theme: "gitlab" diff --git a/app/views/projects/milestones/_milestone.html.haml b/app/views/projects/milestones/_milestone.html.haml index 7039c85bb2..62360158ff 100644 --- a/app/views/projects/milestones/_milestone.html.haml +++ b/app/views/projects/milestones/_milestone.html.haml @@ -11,16 +11,14 @@ %span.cred (Expired) %small = milestone.expires_at - - if milestone.is_empty? - %span.muted Empty - - else - %div - %div - = link_to namespace_project_issues_path(milestone.project.namespace, milestone.project, milestone_id: milestone.id) do - = pluralize milestone.issues.count, 'Issue' -   - = link_to namespace_project_merge_requests_path(milestone.project.namespace, milestone.project, milestone_id: milestone.id) do - = pluralize milestone.merge_requests.count, 'Merge Request' -   - %span.light #{milestone.percent_complete}% complete + .row + .col-sm-6 + = link_to namespace_project_issues_path(milestone.project.namespace, milestone.project, milestone_id: milestone.id) do + = pluralize milestone.issues.count, 'Issue' +   + = link_to namespace_project_merge_requests_path(milestone.project.namespace, milestone.project, milestone_id: milestone.id) do + = pluralize milestone.merge_requests.count, 'Merge Request' +   + %span.light #{milestone.percent_complete}% complete + .col-sm-6 = milestone_progress_bar(milestone) From 85b29f99f0c0bc8cad71d323b744c740d8be7ce0 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 1 Apr 2015 14:36:17 +0200 Subject: [PATCH 51/57] Make sure Sidekiq picks up archive_repo queue in production. --- bin/background_jobs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/background_jobs b/bin/background_jobs index 59a51c5c86..a041a4b043 100755 --- a/bin/background_jobs +++ b/bin/background_jobs @@ -37,7 +37,7 @@ start_no_deamonize() start_sidekiq() { - bundle exec sidekiq -q post_receive -q mailer -q system_hook -q project_web_hook -q gitlab_shell -q common -q default -e $RAILS_ENV -P $sidekiq_pidfile $@ >> $sidekiq_logfile 2>&1 + bundle exec sidekiq -q post_receive -q mailer -q archive_repo -q system_hook -q project_web_hook -q gitlab_shell -q common -q default -e $RAILS_ENV -P $sidekiq_pidfile $@ >> $sidekiq_logfile 2>&1 } load_ok() From d92e4ccc6eeff1c28ed55c4b27a4f97a76d78127 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 1 Apr 2015 17:17:18 +0200 Subject: [PATCH 52/57] Add tests. --- .../import/bitbucket_controller_spec.rb | 113 +++++++++++++++--- .../import/github_controller_spec.rb | 96 +++++++++++++-- .../import/gitlab_controller_spec.rb | 108 +++++++++++++++-- 3 files changed, 282 insertions(+), 35 deletions(-) diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb index 5dd4124061..c31563e6d7 100644 --- a/spec/controllers/import/bitbucket_controller_spec.rb +++ b/spec/controllers/import/bitbucket_controller_spec.rb @@ -55,24 +55,109 @@ describe Import::BitbucketController do end describe "POST create" do - before do - @repo = { - slug: 'vim', - owner: "john" + let(:bitbucket_username) { user.username } + + let(:bitbucket_user) { + { + user: { + username: bitbucket_username + } }.with_indifferent_access + } + + let(:bitbucket_repo) { + { + slug: "vim", + owner: bitbucket_username + }.with_indifferent_access + } + + before do + allow(Gitlab::BitbucketImport::KeyAdder). + to receive(:new).with(bitbucket_repo, user). + and_return(double(execute: true)) + + controller.stub_chain(:client, :user).and_return(bitbucket_user) + controller.stub_chain(:client, :project).and_return(bitbucket_repo) end - it "takes already existing namespace" do - namespace = create(:namespace, name: "john", owner: user) - expect(Gitlab::BitbucketImport::KeyAdder). - to receive(:new).with(@repo, user). - and_return(double(execute: true)) - expect(Gitlab::BitbucketImport::ProjectCreator). - to receive(:new).with(@repo, namespace, user). - and_return(double(execute: true)) - controller.stub_chain(:client, :project).and_return(@repo) + context "when the repository owner is the Bitbucket user" do + context "when the Bitbucket user and GitLab user's usernames match" do + it "takes the current user's namespace" do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, user.namespace, user). + and_return(double(execute: true)) - post :create, format: :js + post :create, format: :js + end + end + + context "when the Bitbucket user and GitLab user's usernames don't match" do + let(:bitbucket_username) { "someone_else" } + + it "takes the current user's namespace" do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, user.namespace, user). + and_return(double(execute: true)) + + post :create, format: :js + end + end + end + + context "when the repository owner is not the Bitbucket user" do + let(:other_username) { "someone_else" } + + before do + bitbucket_repo["owner"] = other_username + end + + context "when a namespace with the Bitbucket user's username already exists" do + let!(:existing_namespace) { create(:namespace, name: other_username, owner: user) } + + context "when the namespace is owned by the GitLab user" do + it "takes the existing namespace" do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, existing_namespace, user). + and_return(double(execute: true)) + + post :create, format: :js + end + end + + context "when the namespace is not owned by the GitLab user" do + before do + existing_namespace.owner = create(:user) + existing_namespace.save + end + + it "doesn't create a project" do + expect(Gitlab::BitbucketImport::ProjectCreator). + not_to receive(:new) + + post :create, format: :js + end + end + end + + context "when a namespace with the Bitbucket user's username doesn't exist" do + it "creates the namespace" do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).and_return(double(execute: true)) + + post :create, format: :js + + expect(Namespace.where(name: other_username).first).not_to be_nil + end + + it "takes the new namespace" do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, an_instance_of(Group), user). + and_return(double(execute: true)) + + post :create, format: :js + end + end end end end diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 5b967bfcc0..3d3846b2e3 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -56,18 +56,98 @@ describe Import::GithubController do end describe "POST create" do + let(:github_username) { user.username } + + let(:github_user) { + OpenStruct.new(login: github_username) + } + + let(:github_repo) { + OpenStruct.new(name: 'vim', full_name: "#{github_username}/vim", owner: OpenStruct.new(login: github_username)) + } + before do - @repo = OpenStruct.new(login: 'vim', full_name: 'asd/vim', owner: OpenStruct.new(login: "john")) + controller.stub_chain(:client, :user).and_return(github_user) + controller.stub_chain(:client, :repo).and_return(github_repo) end - it "takes already existing namespace" do - namespace = create(:namespace, name: "john", owner: user) - expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).with(@repo, namespace, user). - and_return(double(execute: true)) - controller.stub_chain(:client, :repo).and_return(@repo) + context "when the repository owner is the GitHub user" do + context "when the GitHub user and GitLab user's usernames match" do + it "takes the current user's namespace" do + expect(Gitlab::GithubImport::ProjectCreator). + to receive(:new).with(github_repo, user.namespace, user). + and_return(double(execute: true)) - post :create, format: :js + post :create, format: :js + end + end + + context "when the GitHub user and GitLab user's usernames don't match" do + let(:github_username) { "someone_else" } + + it "takes the current user's namespace" do + expect(Gitlab::GithubImport::ProjectCreator). + to receive(:new).with(github_repo, user.namespace, user). + and_return(double(execute: true)) + + post :create, format: :js + end + end + end + + context "when the repository owner is not the GitHub user" do + let(:other_username) { "someone_else" } + + before do + github_repo.owner = OpenStruct.new(login: other_username) + end + + context "when a namespace with the GitHub user's username already exists" do + let!(:existing_namespace) { create(:namespace, name: other_username, owner: user) } + + context "when the namespace is owned by the GitLab user" do + it "takes the existing namespace" do + expect(Gitlab::GithubImport::ProjectCreator). + to receive(:new).with(github_repo, existing_namespace, user). + and_return(double(execute: true)) + + post :create, format: :js + end + end + + context "when the namespace is not owned by the GitLab user" do + before do + existing_namespace.owner = create(:user) + existing_namespace.save + end + + it "doesn't create a project" do + expect(Gitlab::GithubImport::ProjectCreator). + not_to receive(:new) + + post :create, format: :js + end + end + end + + context "when a namespace with the GitHub user's username doesn't exist" do + it "creates the namespace" do + expect(Gitlab::GithubImport::ProjectCreator). + to receive(:new).and_return(double(execute: true)) + + post :create, format: :js + + expect(Namespace.where(name: other_username).first).not_to be_nil + end + + it "takes the new namespace" do + expect(Gitlab::GithubImport::ProjectCreator). + to receive(:new).with(github_repo, an_instance_of(Group), user). + and_return(double(execute: true)) + + post :create, format: :js + end + end end end end diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb index b6b86b1bce..112e51d431 100644 --- a/spec/controllers/import/gitlab_controller_spec.rb +++ b/spec/controllers/import/gitlab_controller_spec.rb @@ -48,23 +48,105 @@ describe Import::GitlabController do end describe "POST create" do - before do - @repo = { - path: 'vim', - path_with_namespace: 'asd/vim', - owner: {name: "john"}, - namespace: {path: "john"} + let(:gitlab_username) { user.username } + + let(:gitlab_user) { + { + username: gitlab_username }.with_indifferent_access + } + + let(:gitlab_repo) { + { + path: 'vim', + path_with_namespace: "#{gitlab_username}/vim", + owner: { name: gitlab_username }, + namespace: { path: gitlab_username } + }.with_indifferent_access + } + + before do + controller.stub_chain(:client, :user).and_return(gitlab_user) + controller.stub_chain(:client, :project).and_return(gitlab_repo) end - it "takes already existing namespace" do - namespace = create(:namespace, name: "john", owner: user) - expect(Gitlab::GitlabImport::ProjectCreator). - to receive(:new).with(@repo, namespace, user). - and_return(double(execute: true)) - controller.stub_chain(:client, :project).and_return(@repo) + context "when the repository owner is the GitLab.com user" do + context "when the GitLab.com user and GitLab server user's usernames match" do + it "takes the current user's namespace" do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, user.namespace, user). + and_return(double(execute: true)) - post :create, format: :js + post :create, format: :js + end + end + + context "when the GitLab.com user and GitLab server user's usernames don't match" do + let(:gitlab_username) { "someone_else" } + + it "takes the current user's namespace" do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, user.namespace, user). + and_return(double(execute: true)) + + post :create, format: :js + end + end + end + + context "when the repository owner is not the GitLab.com user" do + let(:other_username) { "someone_else" } + + before do + gitlab_repo["namespace"]["path"] = other_username + end + + context "when a namespace with the GitLab.com user's username already exists" do + let!(:existing_namespace) { create(:namespace, name: other_username, owner: user) } + + context "when the namespace is owned by the GitLab server user" do + it "takes the existing namespace" do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, existing_namespace, user). + and_return(double(execute: true)) + + post :create, format: :js + end + end + + context "when the namespace is not owned by the GitLab server user" do + before do + existing_namespace.owner = create(:user) + existing_namespace.save + end + + it "doesn't create a project" do + expect(Gitlab::GitlabImport::ProjectCreator). + not_to receive(:new) + + post :create, format: :js + end + end + end + + context "when a namespace with the GitLab.com user's username doesn't exist" do + it "creates the namespace" do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).and_return(double(execute: true)) + + post :create, format: :js + + expect(Namespace.where(name: other_username).first).not_to be_nil + end + + it "takes the new namespace" do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, an_instance_of(Group), user). + and_return(double(execute: true)) + + post :create, format: :js + end + end end end end From 897fb55c784363090c93a4aee319f531168926cf Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Thu, 2 Apr 2015 05:01:49 +0200 Subject: [PATCH 53/57] Update changelog for 7.9.2. --- CHANGELOG | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 7cca0835af..79a5b22f50 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -58,6 +58,8 @@ v 7.9.0 - Faster merge request processing for large repository - Prevent doubling AJAX request with each commit visit via Turbolink - Prevent unnecessary doubling of js events on import pages and user calendar +v 7.9.2 + - Contains no changes v 7.9.1 - Include missing events and fix save functionality in admin service template settings form (Stan Hu) From 254698d6b8628b5ce57597526d6b34f332c22903 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Thu, 2 Apr 2015 05:06:07 +0200 Subject: [PATCH 54/57] Update CHANGELOG. --- CHANGELOG | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 79a5b22f50..259e1a3007 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -53,11 +53,6 @@ v 7.10.0 (unreleased) - Fix admin user projects lists. - Don't leak private group existence by redirecting from namespace controller to group controller. -v 7.9.0 - - Send EmailsOnPush email when branch or tag is created or deleted. - - Faster merge request processing for large repository - - Prevent doubling AJAX request with each commit visit via Turbolink - - Prevent unnecessary doubling of js events on import pages and user calendar v 7.9.2 - Contains no changes @@ -149,6 +144,10 @@ v 7.9.0 - Fix invalid Atom feeds when using emoji, horizontal rules, or images (Christian Walther) - Backup of repositories with tar instead of git bundle (only now are git-annex files included in the backup) - Add canceled status for CI + - Send EmailsOnPush email when branch or tag is created or deleted. + - Faster merge request processing for large repository + - Prevent doubling AJAX request with each commit visit via Turbolink + - Prevent unnecessary doubling of js events on import pages and user calendar v 7.8.4 - Fix issue_tracker_id substitution in custom issue trackers From 6ab8275cd1554e1509e2e9bc996c73c1635446ed Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 1 Apr 2015 23:32:16 -0400 Subject: [PATCH 55/57] Check symlink mode as a String for file_type_icon_class See https://gitlab.com/gitlab-org/gitlab_git/commit/8ae14bb84b94a2ec15f8a639fb82f0f55c77ad69 --- app/helpers/icons_helper.rb | 2 +- spec/helpers/icons_helper_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/icons_helper.rb b/app/helpers/icons_helper.rb index 187e21832f..a9030729b4 100644 --- a/app/helpers/icons_helper.rb +++ b/app/helpers/icons_helper.rb @@ -40,7 +40,7 @@ module IconsHelper def file_type_icon_class(type, mode, name) if type == 'folder' icon_class = 'folder' - elsif mode == 0120000 + elsif mode == '120000' icon_class = 'share' else # Guess which icon to choose based on file extension. diff --git a/spec/helpers/icons_helper_spec.rb b/spec/helpers/icons_helper_spec.rb index 0b1cf07b7b..c052981fe7 100644 --- a/spec/helpers/icons_helper_spec.rb +++ b/spec/helpers/icons_helper_spec.rb @@ -7,7 +7,7 @@ describe IconsHelper do end it 'returns share class' do - expect(file_type_icon_class('file', 0120000, 'link')).to eq 'share' + expect(file_type_icon_class('file', '120000', 'link')).to eq 'share' end it 'returns file-pdf-o class with .pdf' do From 67c83489cac6029ac0eb072b12fd6a9955343cd0 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Fri, 27 Mar 2015 15:18:59 +0200 Subject: [PATCH 56/57] ability to skip some items in backup --- CHANGELOG | 1 + doc/raketasks/backup_restore.md | 7 ++++ lib/backup/manager.rb | 42 ++++++++++++++++++---- lib/tasks/gitlab/backup.rake | 33 ++++++++++++----- spec/tasks/gitlab/backup_rake_spec.rb | 51 +++++++++++++++++++++++++++ 5 files changed, 119 insertions(+), 15 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 259e1a3007..6d6b114a8b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -52,6 +52,7 @@ v 7.10.0 (unreleased) - Don't show commit comment button when user is not signed in. - Fix admin user projects lists. - Don't leak private group existence by redirecting from namespace controller to group controller. + - Ability to skip some items from backup (database, respositories or uploads) v 7.9.2 - Contains no changes diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index 99cdfff0ac..2e41fad89e 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -17,6 +17,13 @@ sudo gitlab-rake gitlab:backup:create sudo -u git -H bundle exec rake gitlab:backup:create RAILS_ENV=production ``` +Also you can choose what should be backed up by adding environment variable SKIP. Available options: db, +uploads (attachments), repositories. Use a comma to specify several options at the same time. + +``` +sudo gitlab-rake gitlab:backup:create SKIP=db,uploads +``` + Example output: ``` diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index d445150c55..b69aebf9fe 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -1,7 +1,5 @@ module Backup class Manager - BACKUP_CONTENTS = %w{repositories/ db/ uploads/ backup_information.yml} - def pack # saving additional informations s = {} @@ -9,6 +7,7 @@ module Backup s[:backup_created_at] = Time.now s[:gitlab_version] = Gitlab::VERSION s[:tar_version] = tar_version + s[:skipped] = ENV["SKIP"] tar_file = "#{s[:backup_created_at].to_i}_gitlab_backup.tar" Dir.chdir(Gitlab.config.backup.path) do @@ -17,12 +16,12 @@ module Backup file << s.to_yaml.gsub(/^---\n/,'') end - FileUtils.chmod(0700, %w{db uploads repositories}) + FileUtils.chmod(0700, folders_to_backup) # create archive $progress.print "Creating backup archive: #{tar_file} ... " orig_umask = File.umask(0077) - if Kernel.system('tar', '-cf', tar_file, *BACKUP_CONTENTS) + if Kernel.system('tar', '-cf', tar_file, *backup_contents) $progress.puts "done".green else puts "creating archive #{tar_file} failed".red @@ -46,6 +45,7 @@ module Backup connection = ::Fog::Storage.new(connection_settings) directory = connection.directories.get(remote_directory) + if directory.files.create(key: tar_file, body: File.open(tar_file), public: false) $progress.puts "done".green else @@ -56,7 +56,10 @@ module Backup def cleanup $progress.print "Deleting tmp directories ... " - BACKUP_CONTENTS.each do |dir| + + backup_contents.each do |dir| + next unless File.exist?(File.join(Gitlab.config.backup.path, dir)) + if FileUtils.rm_rf(File.join(Gitlab.config.backup.path, dir)) $progress.puts "done".green else @@ -73,6 +76,7 @@ module Backup if keep_time > 0 removed = 0 + Dir.chdir(Gitlab.config.backup.path) do file_list = Dir.glob('*_gitlab_backup.tar') file_list.map! { |f| $1.to_i if f =~ /(\d+)_gitlab_backup.tar/ } @@ -84,6 +88,7 @@ module Backup end end end + $progress.puts "done. (#{removed} removed)".green else $progress.puts "skipping".yellow @@ -96,6 +101,7 @@ module Backup # check for existing backups in the backup dir file_list = Dir.glob("*_gitlab_backup.tar").each.map { |f| f.split(/_/).first.to_i } puts "no backups found" if file_list.count == 0 + if file_list.count > 1 && ENV["BACKUP"].nil? puts "Found more than one backup, please specify which one you want to restore:" puts "rake gitlab:backup:restore BACKUP=timestamp_of_backup" @@ -110,6 +116,7 @@ module Backup end $progress.print "Unpacking backup ... " + unless Kernel.system(*%W(tar -xf #{tar_file})) puts "unpacking backup failed".red exit 1 @@ -117,7 +124,6 @@ module Backup $progress.puts "done".green end - settings = YAML.load_file("backup_information.yml") ENV["VERSION"] = "#{settings[:db_version]}" if settings[:db_version].to_i > 0 # restoring mismatching backups can lead to unexpected problems @@ -136,5 +142,29 @@ module Backup tar_version, _ = Gitlab::Popen.popen(%W(tar --version)) tar_version.force_encoding('locale').split("\n").first end + + def skipped?(item) + settings[:skipped] && settings[:skipped].include?(item) + end + + private + + def backup_contents + folders_to_backup + ["backup_information.yml"] + end + + def folders_to_backup + folders = %w{repositories db uploads} + + if ENV["SKIP"] + return folders.reject{ |folder| ENV["SKIP"].include?(folder) } + end + + folders + end + + def settings + @settings ||= YAML.load_file("backup_information.yml") + end end end diff --git a/lib/tasks/gitlab/backup.rake b/lib/tasks/gitlab/backup.rake index 0230fbb010..84445b3bf2 100644 --- a/lib/tasks/gitlab/backup.rake +++ b/lib/tasks/gitlab/backup.rake @@ -27,9 +27,9 @@ namespace :gitlab do backup = Backup::Manager.new backup.unpack - Rake::Task["gitlab:backup:db:restore"].invoke - Rake::Task["gitlab:backup:repo:restore"].invoke - Rake::Task["gitlab:backup:uploads:restore"].invoke + Rake::Task["gitlab:backup:db:restore"].invoke unless backup.skipped?("db") + Rake::Task["gitlab:backup:repo:restore"].invoke unless backup.skipped?("repositories") + Rake::Task["gitlab:backup:uploads:restore"].invoke unless backup.skipped?("uploads") Rake::Task["gitlab:shell:setup"].invoke backup.cleanup @@ -38,8 +38,13 @@ namespace :gitlab do namespace :repo do task create: :environment do $progress.puts "Dumping repositories ...".blue - Backup::Repository.new.dump - $progress.puts "done".green + + if ENV["SKIP"] && ENV["SKIP"].include?("repositories") + $progress.puts "[SKIPPED]".cyan + else + Backup::Repository.new.dump + $progress.puts "done".green + end end task restore: :environment do @@ -52,8 +57,13 @@ namespace :gitlab do namespace :db do task create: :environment do $progress.puts "Dumping database ... ".blue - Backup::Database.new.dump - $progress.puts "done".green + + if ENV["SKIP"] && ENV["SKIP"].include?("db") + $progress.puts "[SKIPPED]".cyan + else + Backup::Database.new.dump + $progress.puts "done".green + end end task restore: :environment do @@ -66,8 +76,13 @@ namespace :gitlab do namespace :uploads do task create: :environment do $progress.puts "Dumping uploads ... ".blue - Backup::Uploads.new.dump - $progress.puts "done".green + + if ENV["SKIP"] && ENV["SKIP"].include?("uploads") + $progress.puts "[SKIPPED]".cyan + else + Backup::Uploads.new.dump + $progress.puts "done".green + end end task restore: :environment do diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index 3d5d21c2a1..60144be551 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -98,4 +98,55 @@ describe 'gitlab:app namespace rake task' do expect(temp_dirs).to be_empty end end # backup_create task + + describe "Skipping items" do + def tars_glob + Dir.glob(File.join(Gitlab.config.backup.path, '*_gitlab_backup.tar')) + end + + before :all do + @origin_cd = Dir.pwd + + Rake::Task["gitlab:backup:db:create"].reenable + Rake::Task["gitlab:backup:repo:create"].reenable + Rake::Task["gitlab:backup:uploads:create"].reenable + + # Record the existing backup tars so we don't touch them + existing_tars = tars_glob + + # Redirect STDOUT and run the rake task + orig_stdout = $stdout + $stdout = StringIO.new + ENV["SKIP"] = "repositories" + run_rake_task('gitlab:backup:create') + $stdout = orig_stdout + + @backup_tar = (tars_glob - existing_tars).first + end + + after :all do + FileUtils.rm(@backup_tar) + Dir.chdir @origin_cd + end + + it "does not contain skipped item" do + tar_contents, exit_status = Gitlab::Popen.popen( + %W{tar -tvf #{@backup_tar} db uploads repositories} + ) + + expect(tar_contents).to match('db/') + expect(tar_contents).to match('uploads/') + expect(tar_contents).not_to match('repositories/') + end + + it 'does not invoke repositories restore' do + Rake::Task["gitlab:shell:setup"].stub invoke: true + allow($stdout).to receive :write + + expect(Rake::Task["gitlab:backup:db:restore"]).to receive :invoke + expect(Rake::Task["gitlab:backup:repo:restore"]).not_to receive :invoke + expect(Rake::Task["gitlab:shell:setup"]).to receive :invoke + expect { run_rake_task('gitlab:backup:restore') }.to_not raise_error + end + end end # gitlab:app namespace From d365004e684e98459061fcd5fbaf9bea880934a8 Mon Sep 17 00:00:00 2001 From: Sullivan SENECHAL Date: Tue, 7 Oct 2014 14:06:05 +0200 Subject: [PATCH 57/57] Fix and improve help rendering --- CHANGELOG | 1 + app/controllers/help_controller.rb | 30 ++++++++++++++++++----- app/views/help/show.html.haml | 2 +- config/initializers/mime_types.rb | 1 + config/routes.rb | 2 +- features/steps/dashboard/help.rb | 2 +- spec/features/help_pages_spec.rb | 2 +- spec/routing/routing_spec.rb | 38 +++++++++++++++--------------- 8 files changed, 49 insertions(+), 29 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index c67ce17dc3..06da6694fe 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -56,6 +56,7 @@ v 7.10.0 (unreleased) - Fix "Hello @username." references not working by no longer allowing usernames to end in period. - Archive repositories in background worker. - Import GitHub, Bitbucket or GitLab.com projects owned by authenticated user into current namespace. + - Fix and improve help rendering (Sullivan Sénéchal) v 7.9.2 diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb index c4d620d87b..fbd9e67e6d 100644 --- a/app/controllers/help_controller.rb +++ b/app/controllers/help_controller.rb @@ -3,19 +3,37 @@ class HelpController < ApplicationController end def show - @category = params[:category] - @file = params[:file] + @filepath = params[:filepath] + @format = params[:format] - if File.exists?(Rails.root.join('doc', @category, @file + '.md')) - render 'show' - else - not_found! + respond_to do |format| + format.md { render_doc } + format.all { send_file_data } end end def shortcuts end + private + + def render_doc + if File.exists?(Rails.root.join('doc', @filepath + '.md')) + render 'show.html.haml' + else + not_found! + end + end + + def send_file_data + path = Rails.root.join('doc', "#{@filepath}.#{@format}") + if File.exists?(path) + send_file(path, disposition: 'inline') + else + head :not_found + end + end + def ui end end diff --git a/app/views/help/show.html.haml b/app/views/help/show.html.haml index eca34dbff0..f22aa92caf 100644 --- a/app/views/help/show.html.haml +++ b/app/views/help/show.html.haml @@ -1,2 +1,2 @@ .documentation.wiki - = markdown File.read(Rails.root.join('doc', @category, @file + '.md')).gsub("$your_email", current_user.email) + = markdown File.read(Rails.root.join('doc', @filepath + '.md')).gsub("$your_email", current_user.email) diff --git a/config/initializers/mime_types.rb b/config/initializers/mime_types.rb index 8f8bef42be..6978ad9302 100644 --- a/config/initializers/mime_types.rb +++ b/config/initializers/mime_types.rb @@ -6,3 +6,4 @@ Mime::Type.register_alias "text/plain", :diff Mime::Type.register_alias "text/plain", :patch +Mime::Type.register_alias 'text/html', :md diff --git a/config/routes.rb b/config/routes.rb index 388858d267..c1b85b025b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -39,9 +39,9 @@ Gitlab::Application.routes.draw do # Help get 'help' => 'help#index' - get 'help/:category/:file' => 'help#show', as: :help_page get 'help/shortcuts' get 'help/ui' => 'help#ui' + get 'help/:filepath' => 'help#show', as: :help_page, constraints: { filepath: /[^\.]+/ } # # Global snippets diff --git a/features/steps/dashboard/help.rb b/features/steps/dashboard/help.rb index ef433c57c6..fa52e391f0 100644 --- a/features/steps/dashboard/help.rb +++ b/features/steps/dashboard/help.rb @@ -8,7 +8,7 @@ class Spinach::Features::DashboardHelp < Spinach::FeatureSteps end step 'I visit the "Rake Tasks" help page' do - visit help_page_path("raketasks", "maintenance") + visit help_page_path('raketasks/maintenance', format: 'md') end step 'I should see "Rake Tasks" page markdown rendered' do diff --git a/spec/features/help_pages_spec.rb b/spec/features/help_pages_spec.rb index 41088ce827..28423eb8ca 100644 --- a/spec/features/help_pages_spec.rb +++ b/spec/features/help_pages_spec.rb @@ -6,7 +6,7 @@ describe 'Help Pages', feature: true do login_as :user end it 'replace the variable $your_email with the email of the user' do - visit help_page_path(category: 'ssh', file: 'README.md') + visit help_page_path(filepath: 'ssh/README', format: 'md') expect(page).to have_content("ssh-keygen -t rsa -C \"#{@user.email}\"") end end diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index d4915b5195..f5db548f97 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -73,41 +73,41 @@ end # help_markdown GET /help/markdown(.:format) help#markdown # help_ssh GET /help/ssh(.:format) help#ssh # help_raketasks GET /help/raketasks(.:format) help#raketasks -describe HelpController, "routing" do - it "to #index" do - expect(get("/help")).to route_to('help#index') +describe HelpController, 'routing' do + it 'to #index' do + expect(get('/help')).to route_to('help#index') end - it "to #permissions" do - expect(get("/help/permissions/permissions")).to route_to('help#show', category: "permissions", file: "permissions") + it 'to #permissions' do + expect(get('/help/permissions/permissions')).to route_to('help#show', filepath: 'permissions/permissions') end - it "to #workflow" do - expect(get("/help/workflow/README")).to route_to('help#show', category: "workflow", file: "README") + it 'to #workflow' do + expect(get('/help/workflow/README')).to route_to('help#show', filepath: 'workflow/README') end - it "to #api" do - expect(get("/help/api/README")).to route_to('help#show', category: "api", file: "README") + it 'to #api' do + expect(get('/help/api/README')).to route_to('help#show', filepath: 'api/README') end - it "to #web_hooks" do - expect(get("/help/web_hooks/web_hooks")).to route_to('help#show', category: "web_hooks", file: "web_hooks") + it 'to #web_hooks' do + expect(get('/help/web_hooks/web_hooks')).to route_to('help#show', filepath: 'web_hooks/web_hooks') end - it "to #system_hooks" do - expect(get("/help/system_hooks/system_hooks")).to route_to('help#show', category: "system_hooks", file: "system_hooks") + it 'to #system_hooks' do + expect(get('/help/system_hooks/system_hooks')).to route_to('help#show', filepath: 'system_hooks/system_hooks') end - it "to #markdown" do - expect(get("/help/markdown/markdown")).to route_to('help#show',category: "markdown", file: "markdown") + it 'to #markdown' do + expect(get('/help/markdown/markdown')).to route_to('help#show',filepath: 'markdown/markdown') end - it "to #ssh" do - expect(get("/help/ssh/README")).to route_to('help#show', category: "ssh", file: "README") + it 'to #ssh' do + expect(get('/help/ssh/README')).to route_to('help#show', filepath: 'ssh/README') end - it "to #raketasks" do - expect(get("/help/raketasks/README")).to route_to('help#show', category: "raketasks", file: "README") + it 'to #raketasks' do + expect(get('/help/raketasks/README')).to route_to('help#show', filepath: 'raketasks/README') end end