From a27b59f620ff26c9b358c5a34e175a6a513c9d15 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Sat, 22 Apr 2017 00:54:03 -0500 Subject: [PATCH 01/19] Fix diffs with edit-forking needs Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/31276 --- app/assets/javascripts/merge_request_tabs.js | 11 +++++ app/helpers/blob_helper.rb | 2 +- app/views/projects/_fork_suggestion.html.haml | 11 +++++ app/views/projects/blob/_header.html.haml | 12 +---- app/views/projects/diffs/_file.html.haml | 2 + spec/features/merge_requests/diffs_spec.rb | 47 +++++++++++++++---- 6 files changed, 64 insertions(+), 21 deletions(-) create mode 100644 app/views/projects/_fork_suggestion.html.haml diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index f7f6a773036..6075157ec31 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -5,6 +5,7 @@ import Cookies from 'js-cookie'; import './breakpoints'; import './flash'; +import BlobForkSuggestion from './blob/blob_fork_suggestion'; /* eslint-disable max-len */ // MergeRequestTabs @@ -266,6 +267,16 @@ import './flash'; new gl.Diff(); this.scrollToElement('#diffs'); + + $('.diff-file').each((i, el) => { + new BlobForkSuggestion({ + openButtons: $(el).find('.js-edit-blob-link-fork-toggler'), + forkButtons: $(el).find('.js-fork-suggestion-button'), + cancelButtons: $(el).find('.js-cancel-fork-suggestion-button'), + suggestionSections: $(el).find('.js-file-fork-suggestion-section'), + actionTextPieces: $(el).find('.js-file-fork-suggestion-section-action'), + }); + }); }, }); } diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 3736e1ffcbb..36b16421e8f 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -29,7 +29,7 @@ module BlobHelper link_to 'Edit', edit_path(project, ref, path, options), class: "#{common_classes} btn-sm" elsif current_user && can?(current_user, :fork_project, project) continue_params = { - to: edit_path, + to: edit_path(project, ref, path, options), notice: edit_in_new_fork_notice, notice_now: edit_in_new_fork_notice_now } diff --git a/app/views/projects/_fork_suggestion.html.haml b/app/views/projects/_fork_suggestion.html.haml new file mode 100644 index 00000000000..c855bfaf067 --- /dev/null +++ b/app/views/projects/_fork_suggestion.html.haml @@ -0,0 +1,11 @@ +- if current_user + .js-file-fork-suggestion-section.file-fork-suggestion.hidden + %span.file-fork-suggestion-note + You're not allowed to + %span.js-file-fork-suggestion-section-action + edit + files in this project directly. Please fork this project, + make your changes there, and submit a merge request. + = link_to 'Fork', nil, method: :post, class: 'js-fork-suggestion-button btn btn-grouped btn-inverted btn-new' + %button.js-cancel-fork-suggestion-button.btn.btn-grouped{ type: 'button' } + Cancel diff --git a/app/views/projects/blob/_header.html.haml b/app/views/projects/blob/_header.html.haml index d46e4534497..c553db84ee0 100644 --- a/app/views/projects/blob/_header.html.haml +++ b/app/views/projects/blob/_header.html.haml @@ -39,14 +39,4 @@ = replace_blob_link = delete_blob_link -- if current_user - .js-file-fork-suggestion-section.file-fork-suggestion.hidden - %span.file-fork-suggestion-note - You're not allowed to - %span.js-file-fork-suggestion-section-action - edit - files in this project directly. Please fork this project, - make your changes there, and submit a merge request. - = link_to 'Fork', nil, method: :post, class: 'js-fork-suggestion-button btn btn-grouped btn-inverted btn-new' - %button.js-cancel-fork-suggestion-button.btn.btn-grouped{ type: 'button' } - Cancel += render 'projects/fork_suggestion' diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index 0232a09b4a8..4622b980754 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -18,4 +18,6 @@ = view_file_button(diff_commit.id, diff_file.new_path, project) = view_on_environment_button(diff_commit.id, diff_file.new_path, environment) if environment + = render 'projects/fork_suggestion' + = render 'projects/diffs/content', diff_file: diff_file, diff_commit: diff_commit, blob: blob, project: project diff --git a/spec/features/merge_requests/diffs_spec.rb b/spec/features/merge_requests/diffs_spec.rb index 4a6c76a5caf..32a6a4b2682 100644 --- a/spec/features/merge_requests/diffs_spec.rb +++ b/spec/features/merge_requests/diffs_spec.rb @@ -1,11 +1,13 @@ require 'spec_helper' feature 'Diffs URL', js: true, feature: true do - before do - login_as :admin - @merge_request = create(:merge_request) - @project = @merge_request.source_project - end + include ApplicationHelper + + let(:author_user) { create(:user) } + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + let(:forked_project) { Projects::ForkService.new(project, author_user).execute } + let(:merge_request) { create(:merge_request_with_diffs, source_project: forked_project, target_project: project, author: author_user) } context 'when visit with */* as accept header' do before(:each) do @@ -13,9 +15,9 @@ feature 'Diffs URL', js: true, feature: true do end it 'renders the notes' do - create :note_on_merge_request, project: @project, noteable: @merge_request, note: 'Rebasing with master' + create :note_on_merge_request, project: project, noteable: merge_request, note: 'Rebasing with master' - visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request) + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) # Load notes and diff through AJAX expect(page).to have_css('.note-text', visible: false, text: 'Rebasing with master') @@ -28,11 +30,38 @@ feature 'Diffs URL', js: true, feature: true do allow_any_instance_of(MergeRequestDiff).to receive(:overflow?).and_return(true) allow(Commit).to receive(:max_diff_options).and_return(max_files: 20, max_lines: 20) - visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request) + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) page.within('.alert') do expect(page).to have_text("Too many changes to show. Plain diff Email patch To preserve - performance only 3 of 3+ files are displayed.") + performance only 3 of 3 files are displayed.") + end + end + end + + describe 'when editing file' do + let(:changelog_id) { hexdigest("CHANGELOG") } + + context 'as author' do + it 'shows direct edit link' do + login_as(author_user) + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) + + # Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax + expect(page).to have_selector("[id=\"#{changelog_id}\"] a.js-edit-blob") + end + end + + context 'as user who needs to fork' do + it 'shows fork/cancel confirmation' do + login_as(user) + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) + + # Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax + find("[id=\"#{changelog_id}\"] .js-edit-blob").click + + expect(page).to have_selector('.js-fork-suggestion-button', count: 1) + expect(page).to have_selector('.js-cancel-fork-suggestion-button', count: 1) end end end From 485b63d12efee9ebe73b12e2fc176a3867c456a8 Mon Sep 17 00:00:00 2001 From: Marland Sitt Date: Mon, 24 Apr 2017 16:39:47 +0000 Subject: [PATCH 02/19] Fix typo in discussions doc. --- doc/user/discussions/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index c5123c06ce0..59e343ebe51 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -12,7 +12,7 @@ You can leave a comment in the following places: The comment area supports [Markdown] and [slash commands]. One can edit their own comment at any time, and anyone with [Master access level][permissions] or -higher can also a comment made by someone else. +higher can also edit a comment made by someone else. Apart from the standard comments, you also have the option to create a comment in the form of a resolvable or threaded discussion. From d7e2ac729317ace2ccf0203663637ba32f328d1a Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 24 Apr 2017 16:12:14 -0500 Subject: [PATCH 03/19] Fix OAuth, LDAP and SAML SSO when regular sign-ups are disabled --- app/services/users/build_service.rb | 4 ++-- app/services/users/create_service.rb | 4 ++-- .../unreleased/dm-fix-oauth-user-creation.yml | 4 ++++ lib/gitlab/o_auth/user.rb | 2 +- spec/lib/gitlab/ldap/user_spec.rb | 12 ++++++++++++ spec/lib/gitlab/o_auth/user_spec.rb | 14 ++++++++++++++ spec/lib/gitlab/saml/user_spec.rb | 12 ++++++++++++ 7 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/dm-fix-oauth-user-creation.yml diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index 9a0a5a12f91..d2a1c161026 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -6,8 +6,8 @@ module Users @params = params.dup end - def execute - raise Gitlab::Access::AccessDeniedError unless can_create_user? + def execute(skip_authorization: false) + raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_create_user? user = User.new(build_user_params) diff --git a/app/services/users/create_service.rb b/app/services/users/create_service.rb index a2105d31f71..e22f7225ae2 100644 --- a/app/services/users/create_service.rb +++ b/app/services/users/create_service.rb @@ -6,8 +6,8 @@ module Users @params = params.dup end - def execute - user = Users::BuildService.new(current_user, params).execute + def execute(skip_authorization: false) + user = Users::BuildService.new(current_user, params).execute(skip_authorization: skip_authorization) @reset_token = user.generate_reset_token if user.recently_sent_password_reset? diff --git a/changelogs/unreleased/dm-fix-oauth-user-creation.yml b/changelogs/unreleased/dm-fix-oauth-user-creation.yml new file mode 100644 index 00000000000..161b114394a --- /dev/null +++ b/changelogs/unreleased/dm-fix-oauth-user-creation.yml @@ -0,0 +1,4 @@ +--- +title: Fix OAuth, LDAP and SAML SSO when regular sign-ups are disabled +merge_request: +author: diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 6e42d8941fb..afd24b4dcc5 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -148,7 +148,7 @@ module Gitlab def build_new_user user_params = user_attributes.merge(extern_uid: auth_hash.uid, provider: auth_hash.provider, skip_confirmation: true) - Users::BuildService.new(nil, user_params).execute + Users::BuildService.new(nil, user_params).execute(skip_authorization: true) end def user_attributes diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb index 346cf0d117c..65a304d1468 100644 --- a/spec/lib/gitlab/ldap/user_spec.rb +++ b/spec/lib/gitlab/ldap/user_spec.rb @@ -108,6 +108,18 @@ describe Gitlab::LDAP::User, lib: true do it "creates a new user if not found" do expect{ ldap_user.save }.to change{ User.count }.by(1) end + + context 'when signup is disabled' do + before do + stub_application_setting signup_enabled: false + end + + it 'creates the user' do + ldap_user.save + + expect(gl_user).to be_persisted + end + end end describe 'updating email' do diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 8f09266c3b3..6d3ac62d9e9 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -40,6 +40,20 @@ describe Gitlab::OAuth::User, lib: true do let(:provider) { 'twitter' } describe 'signup' do + context 'when signup is disabled' do + before do + stub_application_setting signup_enabled: false + end + + it 'creates the user' do + stub_omniauth_config(allow_single_sign_on: ['twitter']) + + oauth_user.save + + expect(gl_user).to be_persisted + end + end + it 'marks user as having password_automatically_set' do stub_omniauth_config(allow_single_sign_on: ['twitter'], external_providers: ['twitter']) diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb index 4f6ef3c10fc..b3b76a6d629 100644 --- a/spec/lib/gitlab/saml/user_spec.rb +++ b/spec/lib/gitlab/saml/user_spec.rb @@ -211,6 +211,18 @@ describe Gitlab::Saml::User, lib: true do end end end + + context 'when signup is disabled' do + before do + stub_application_setting signup_enabled: false + end + + it 'creates the user' do + saml_user.save + + expect(gl_user).to be_persisted + end + end end describe 'blocking' do From 5a8dd22766a72772f12c0008d4eb98e2b3cc264f Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 24 Apr 2017 21:45:53 -0700 Subject: [PATCH 04/19] Upgrade Sidekiq to 4.2.10 See https://github.com/mperham/sidekiq/blob/master/Changes.md#4210 --- Gemfile.lock | 2 +- changelogs/unreleased/sh-bump-sidekiq-version.yml | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/sh-bump-sidekiq-version.yml diff --git a/Gemfile.lock b/Gemfile.lock index bb91db1e805..69a5106ed52 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -716,7 +716,7 @@ GEM rack shoulda-matchers (2.8.0) activesupport (>= 3.0.0) - sidekiq (4.2.7) + sidekiq (4.2.10) concurrent-ruby (~> 1.0) connection_pool (~> 2.2, >= 2.2.0) rack-protection (>= 1.5.0) diff --git a/changelogs/unreleased/sh-bump-sidekiq-version.yml b/changelogs/unreleased/sh-bump-sidekiq-version.yml new file mode 100644 index 00000000000..5369b78b76a --- /dev/null +++ b/changelogs/unreleased/sh-bump-sidekiq-version.yml @@ -0,0 +1,4 @@ +--- +title: Upgrade Sidekiq to 4.2.10 +merge_request: +author: From 34b71e734b0b01dd28e18be4728f93fbd4d1a561 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Fri, 21 Apr 2017 09:47:58 +0000 Subject: [PATCH 05/19] Don't display the `is_admin?` flag for user API responses. - To prevent an attacker from enumerating the `/users` API to get a list of all the admins. - Display the `is_admin?` flag wherever we display the `private_token` - at the moment, there are two instances: - When an admin uses `sudo` to view the `/user` endpoint - When logging in using the `/session` endpoint --- lib/api/entities.rb | 4 ++-- lib/api/session.rb | 4 ++-- lib/api/users.rb | 2 +- spec/fixtures/api/schemas/public_api/v4/user/public.json | 2 -- spec/requests/api/keys_spec.rb | 6 ++++++ spec/requests/api/users_spec.rb | 8 ++++++-- spec/requests/api/v3/users_spec.rb | 6 ++++++ 7 files changed, 23 insertions(+), 9 deletions(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 64ab6f01eb5..6d6ccefe877 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -14,7 +14,6 @@ module API class User < UserBasic expose :created_at - expose :admin?, as: :is_admin expose :bio, :location, :skype, :linkedin, :twitter, :website_url, :organization end @@ -41,8 +40,9 @@ module API expose :external end - class UserWithPrivateToken < UserPublic + class UserWithPrivateDetails < UserPublic expose :private_token + expose :admin?, as: :is_admin end class Email < Grape::Entity diff --git a/lib/api/session.rb b/lib/api/session.rb index 002ffd1d154..016415c3023 100644 --- a/lib/api/session.rb +++ b/lib/api/session.rb @@ -1,7 +1,7 @@ module API class Session < Grape::API desc 'Login to get token' do - success Entities::UserWithPrivateToken + success Entities::UserWithPrivateDetails end params do optional :login, type: String, desc: 'The username' @@ -14,7 +14,7 @@ module API return unauthorized! unless user return render_api_error!('401 Unauthorized. You have 2FA enabled. Please use a personal access token to access the API', 401) if user.two_factor_enabled? - present user, with: Entities::UserWithPrivateToken + present user, with: Entities::UserWithPrivateDetails end end end diff --git a/lib/api/users.rb b/lib/api/users.rb index 46f221f68fe..40acaebf670 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -433,7 +433,7 @@ module API success Entities::UserPublic end get do - present current_user, with: sudo? ? Entities::UserWithPrivateToken : Entities::UserPublic + present current_user, with: sudo? ? Entities::UserWithPrivateDetails : Entities::UserPublic end desc "Get the currently authenticated user's SSH keys" do diff --git a/spec/fixtures/api/schemas/public_api/v4/user/public.json b/spec/fixtures/api/schemas/public_api/v4/user/public.json index 5587cfec61a..faa126b65f2 100644 --- a/spec/fixtures/api/schemas/public_api/v4/user/public.json +++ b/spec/fixtures/api/schemas/public_api/v4/user/public.json @@ -9,7 +9,6 @@ "avatar_url", "web_url", "created_at", - "is_admin", "bio", "location", "skype", @@ -43,7 +42,6 @@ "avatar_url": { "type": "string" }, "web_url": { "type": "string" }, "created_at": { "type": "date" }, - "is_admin": { "type": "boolean" }, "bio": { "type": ["string", "null"] }, "location": { "type": ["string", "null"] }, "skype": { "type": "string" }, diff --git a/spec/requests/api/keys_spec.rb b/spec/requests/api/keys_spec.rb index 4c80987d680..adb33166332 100644 --- a/spec/requests/api/keys_spec.rb +++ b/spec/requests/api/keys_spec.rb @@ -34,6 +34,12 @@ describe API::Keys, api: true do expect(json_response['user']['id']).to eq(user.id) expect(json_response['user']['username']).to eq(user.username) end + + it "does not include the user's `is_admin` flag" do + get api("/keys/#{key.id}", admin) + + expect(json_response['user']['is_admin']).to be_nil + end end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 165ab389917..1db85da5c2c 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -137,6 +137,12 @@ describe API::Users, api: true do expect(json_response['username']).to eq(user.username) end + it "does not return the user's `is_admin` flag" do + get api("/users/#{user.id}", user) + + expect(json_response['is_admin']).to be_nil + end + it "returns a 401 if unauthenticated" do get api("/users/9998") expect(response).to have_http_status(401) @@ -399,7 +405,6 @@ describe API::Users, api: true do it "updates admin status" do put api("/users/#{user.id}", admin), { admin: true } expect(response).to have_http_status(200) - expect(json_response['is_admin']).to eq(true) expect(user.reload.admin).to eq(true) end @@ -413,7 +418,6 @@ describe API::Users, api: true do it "does not update admin status" do put api("/users/#{admin_user.id}", admin), { can_create_group: false } expect(response).to have_http_status(200) - expect(json_response['is_admin']).to eq(true) expect(admin_user.reload.admin).to eq(true) expect(admin_user.can_create_group).to eq(false) end diff --git a/spec/requests/api/v3/users_spec.rb b/spec/requests/api/v3/users_spec.rb index b38cbe74b85..19465a9a4ea 100644 --- a/spec/requests/api/v3/users_spec.rb +++ b/spec/requests/api/v3/users_spec.rb @@ -276,5 +276,11 @@ describe API::V3::Users, api: true do expect(new_user).to be_confirmed end + + it 'does not reveal the `is_admin` flag of the user' do + post v3_api('/users', admin), attributes_for(:user) + + expect(json_response['is_admin']).to be_nil + end end end From 45601dcbe1ef0782a9e40257aeb990be19d9057f Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Fri, 21 Apr 2017 09:58:30 +0000 Subject: [PATCH 06/19] Add changelog entry for !10846 --- .../unreleased/29903-remove-user-is-admin-flag-from-api.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/29903-remove-user-is-admin-flag-from-api.yml diff --git a/changelogs/unreleased/29903-remove-user-is-admin-flag-from-api.yml b/changelogs/unreleased/29903-remove-user-is-admin-flag-from-api.yml new file mode 100644 index 00000000000..a0d497ac1e9 --- /dev/null +++ b/changelogs/unreleased/29903-remove-user-is-admin-flag-from-api.yml @@ -0,0 +1,4 @@ +--- +title: Don't display the is_admin flag in most API responses +merge_request: 10846 +author: From 0befa887b52613831809380d2cd5d3d2bff88220 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Fri, 21 Apr 2017 10:02:49 +0000 Subject: [PATCH 07/19] Update documentation to reflect the removal of `is_admin` from most API responses. --- doc/api/deployments.md | 3 --- doc/api/jobs.md | 5 ----- doc/api/keys.md | 1 - doc/api/users.md | 5 ----- 4 files changed, 14 deletions(-) diff --git a/doc/api/deployments.md b/doc/api/deployments.md index 0273c819614..ab9e63e01d3 100644 --- a/doc/api/deployments.md +++ b/doc/api/deployments.md @@ -48,7 +48,6 @@ Example of response "bio": null, "created_at": "2016-08-11T07:09:20.351Z", "id": 1, - "is_admin": true, "linkedin": "", "location": null, "name": "Administrator", @@ -106,7 +105,6 @@ Example of response "bio": null, "created_at": "2016-08-11T07:09:20.351Z", "id": 1, - "is_admin": true, "linkedin": "", "location": null, "name": "Administrator", @@ -195,7 +193,6 @@ Example of response "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", "web_url": "http://localhost:3000/root", "created_at": "2016-08-11T07:09:20.351Z", - "is_admin": true, "bio": null, "location": null, "skype": "", diff --git a/doc/api/jobs.md b/doc/api/jobs.md index bea2b96c97a..7ce67ca607d 100644 --- a/doc/api/jobs.md +++ b/doc/api/jobs.md @@ -57,7 +57,6 @@ Example of response "bio": null, "created_at": "2015-12-21T13:14:24.077Z", "id": 1, - "is_admin": true, "linkedin": "", "name": "Administrator", "skype": "", @@ -101,7 +100,6 @@ Example of response "bio": null, "created_at": "2015-12-21T13:14:24.077Z", "id": 1, - "is_admin": true, "linkedin": "", "name": "Administrator", "skype": "", @@ -173,7 +171,6 @@ Example of response "bio": null, "created_at": "2015-12-21T13:14:24.077Z", "id": 1, - "is_admin": true, "linkedin": "", "name": "Administrator", "skype": "", @@ -217,7 +214,6 @@ Example of response "bio": null, "created_at": "2015-12-21T13:14:24.077Z", "id": 1, - "is_admin": true, "linkedin": "", "name": "Administrator", "skype": "", @@ -284,7 +280,6 @@ Example of response "bio": null, "created_at": "2015-12-21T13:14:24.077Z", "id": 1, - "is_admin": true, "linkedin": "", "name": "Administrator", "skype": "", diff --git a/doc/api/keys.md b/doc/api/keys.md index 3b55c2baf56..3ace1040f38 100644 --- a/doc/api/keys.md +++ b/doc/api/keys.md @@ -26,7 +26,6 @@ Parameters: "avatar_url": "http://www.gravatar.com/avatar/cfa35b8cd2ec278026357769582fa563?s=40\u0026d=identicon", "web_url": "http://localhost:3000/john_smith", "created_at": "2015-09-03T07:24:01.670Z", - "is_admin": false, "bio": null, "skype": "", "linkedin": "", diff --git a/doc/api/users.md b/doc/api/users.md index ff5bffa178f..e940ba5bbe1 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -62,7 +62,6 @@ GET /users "avatar_url": "http://localhost:3000/uploads/user/avatar/1/index.jpg", "web_url": "http://localhost:3000/john_smith", "created_at": "2012-05-23T08:00:58Z", - "is_admin": false, "bio": null, "location": null, "skype": "", @@ -95,7 +94,6 @@ GET /users "avatar_url": "http://localhost:3000/uploads/user/avatar/2/index.jpg", "web_url": "http://localhost:3000/jack_smith", "created_at": "2012-05-23T08:01:01Z", - "is_admin": false, "bio": null, "location": null, "skype": "", @@ -169,7 +167,6 @@ Parameters: "avatar_url": "http://localhost:3000/uploads/user/avatar/1/cd8.jpeg", "web_url": "http://localhost:3000/john_smith", "created_at": "2012-05-23T08:00:58Z", - "is_admin": false, "bio": null, "location": null, "skype": "", @@ -200,7 +197,6 @@ Parameters: "avatar_url": "http://localhost:3000/uploads/user/avatar/1/index.jpg", "web_url": "http://localhost:3000/john_smith", "created_at": "2012-05-23T08:00:58Z", - "is_admin": false, "bio": null, "location": null, "skype": "", @@ -325,7 +321,6 @@ GET /user "avatar_url": "http://localhost:3000/uploads/user/avatar/1/index.jpg", "web_url": "http://localhost:3000/john_smith", "created_at": "2012-05-23T08:00:58Z", - "is_admin": false, "bio": null, "location": null, "skype": "", From 119707d37a113d0554bf86fee622aa800d666882 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Tue, 25 Apr 2017 12:09:13 +0200 Subject: [PATCH 08/19] Ignore all builds* directories from version control The restore process copies the old builds/ dir into builds./ which results in Git complaining about untracked files --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 51b4d06b01b..dfc99a4ee48 100644 --- a/.gitignore +++ b/.gitignore @@ -49,7 +49,7 @@ eslint-report.html /tags /tmp/* /vendor/bundle/* -/builds/* +/builds* /shared/* /.gitlab_workhorse_secret /webpack-report/ From 08a864e2d9246d655599a26e9e1f7ff8642b7849 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 25 Apr 2017 11:58:43 +0100 Subject: [PATCH 09/19] Fix usage ping docs link from empty cohorts page --- app/views/admin/cohorts/index.html.haml | 2 +- changelogs/unreleased/fix-usage-ping-doc-link.yml | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/fix-usage-ping-doc-link.yml diff --git a/app/views/admin/cohorts/index.html.haml b/app/views/admin/cohorts/index.html.haml index 46fe12a5a99..be8644c0ca6 100644 --- a/app/views/admin/cohorts/index.html.haml +++ b/app/views/admin/cohorts/index.html.haml @@ -9,7 +9,7 @@ .bs-callout.bs-callout-warning.clearfix %p User cohorts are only shown when the - = link_to 'usage ping', help_page_path('user/admin_area/usage_statistics'), target: '_blank' + = link_to 'usage ping', help_page_path('user/admin_area/settings/usage_statistics', anchor: 'usage-ping'), target: '_blank' is enabled. To enable it and see user cohorts, visit = succeed '.' do diff --git a/changelogs/unreleased/fix-usage-ping-doc-link.yml b/changelogs/unreleased/fix-usage-ping-doc-link.yml new file mode 100644 index 00000000000..5217a4e4e4b --- /dev/null +++ b/changelogs/unreleased/fix-usage-ping-doc-link.yml @@ -0,0 +1,4 @@ +--- +title: Fix usage ping docs link from empty cohorts page +merge_request: +author: From 848204ba7434386d15be895f293aa1e8d83340b7 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 25 Apr 2017 12:43:21 +0000 Subject: [PATCH 10/19] Show group name on flash container when group is created from Admin area --- app/controllers/admin/groups_controller.rb | 2 +- changelogs/unreleased/21683-show-created-group-name-flash.yml | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/21683-show-created-group-name-flash.yml diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index fc8d4d02ddf..5885b3543bb 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -28,7 +28,7 @@ class Admin::GroupsController < Admin::ApplicationController if @group.save @group.add_owner(current_user) - redirect_to [:admin, @group], notice: 'Group was successfully created.' + redirect_to [:admin, @group], notice: "Group '#{@group.name}' was successfully created." else render "new" end diff --git a/changelogs/unreleased/21683-show-created-group-name-flash.yml b/changelogs/unreleased/21683-show-created-group-name-flash.yml new file mode 100644 index 00000000000..06ef5e972fc --- /dev/null +++ b/changelogs/unreleased/21683-show-created-group-name-flash.yml @@ -0,0 +1,4 @@ +--- +title: Show group name on flash container when group is created from Admin area. +merge_request: 10905 +author: From a3f67eb9d82fe31bd216102bd428a44ce1b2d7b0 Mon Sep 17 00:00:00 2001 From: Simon Hardt Date: Tue, 25 Apr 2017 13:19:03 +0000 Subject: [PATCH 11/19] Update markdown.md --- doc/user/markdown.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/doc/user/markdown.md b/doc/user/markdown.md index 97de428d11d..0d29b471d52 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -431,7 +431,7 @@ Emphasis, aka italics, with *asterisks* or _underscores_. Strong emphasis, aka bold, with **asterisks** or __underscores__. -Combined emphasis with **_asterisks and underscores_**. +Combined emphasis with **asterisks and _underscores_**. Strikethrough uses two tildes. ~~Scratch this.~~ ``` @@ -640,10 +640,11 @@ Here's a line for us to start with. This line is separated from the one above by two newlines, so it will be a *separate paragraph*. This line is also a separate paragraph, but... -This line is only separated by a single newline, so it's a separate line in the *same paragraph*. +This line is only separated by a single newline, so it *does not break* and just follows the previous line in the *same paragraph*. + +This line is also a separate paragraph, and... +This line is *on its own line*, because the previous line ends with two spaces. (but still in the *same paragraph*) -This line is also a separate paragraph, and... -This line is on its own line, because the previous line ends with two spaces. ``` @@ -651,11 +652,12 @@ Here's a line for us to start with. This line is separated from the one above by two newlines, so it will be a *separate paragraph*. -This line is also begins a separate paragraph, but... -This line is only separated by a single newline, so it's a separate line in the *same paragraph*. +This line is also a separate paragraph, but... +This line is only separated by a single newline, so it *does not break* and just follows the previous line in the *same paragraph*. + +This line is also a separate paragraph, and... +This line is *on its own line*, because the previous line ends with two spaces. (but still in the *same paragraph*) -This line is also a separate paragraph, and... -This line is on its own line, because the previous line ends with two spaces. ### Tables From ad4b6302d4dc13eb8a742b9e1c742d40d8cf6ec8 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 25 Apr 2017 15:21:18 +0000 Subject: [PATCH 12/19] Add index to webhooks type column --- changelogs/unreleased/fix-web_hooks-index.yml | 4 ++++ .../20170424142900_add_index_to_web_hooks_type.rb | 15 +++++++++++++++ db/schema.rb | 3 ++- 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/fix-web_hooks-index.yml create mode 100644 db/migrate/20170424142900_add_index_to_web_hooks_type.rb diff --git a/changelogs/unreleased/fix-web_hooks-index.yml b/changelogs/unreleased/fix-web_hooks-index.yml new file mode 100644 index 00000000000..16f233e2e7c --- /dev/null +++ b/changelogs/unreleased/fix-web_hooks-index.yml @@ -0,0 +1,4 @@ +--- +title: Add index to webhooks type column +merge_request: +author: diff --git a/db/migrate/20170424142900_add_index_to_web_hooks_type.rb b/db/migrate/20170424142900_add_index_to_web_hooks_type.rb new file mode 100644 index 00000000000..9af158e3844 --- /dev/null +++ b/db/migrate/20170424142900_add_index_to_web_hooks_type.rb @@ -0,0 +1,15 @@ +class AddIndexToWebHooksType < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :web_hooks, :type + end + + def down + remove_concurrent_index :web_hooks, :type + end +end diff --git a/db/schema.rb b/db/schema.rb index 7b13f2a98a1..bd6fef22d06 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170423064036) do +ActiveRecord::Schema.define(version: 20170424142900) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1371,6 +1371,7 @@ ActiveRecord::Schema.define(version: 20170423064036) do end add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree + add_index "web_hooks", ["type"], name: "index_web_hooks_on_type", using: :btree add_foreign_key "boards", "projects" add_foreign_key "chat_teams", "namespaces", on_delete: :cascade From a0979c05fd5976cabe3c0633c168848d66320bfa Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 20 Apr 2017 15:47:32 +0100 Subject: [PATCH 13/19] Show correct size when MR diff overflows The problem is that we often go via a diff object constructed from the diffs stored in the DB. Those diffs, by definition, don't overflow, so we don't have access to the 'correct' `real_size` - that is stored on the MR diff object iself. --- app/models/merge_request.rb | 17 +++++----- app/models/merge_request_diff.rb | 2 +- .../unreleased/mr-diff-size-overflow.yml | 4 +++ .../file_collection/merge_request_diff.rb | 4 +++ spec/features/merge_requests/diffs_spec.rb | 20 ++++++------ spec/models/merge_request_spec.rb | 32 +++++++++++++++---- 6 files changed, 52 insertions(+), 27 deletions(-) create mode 100644 changelogs/unreleased/mr-diff-size-overflow.yml diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 1d4827375d7..9d2288c311e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -191,22 +191,23 @@ class MergeRequest < ActiveRecord::Base merge_request_diff ? merge_request_diff.raw_diffs(*args) : compare.raw_diffs(*args) end - def diffs(diff_options = nil) + def diffs(diff_options = {}) if compare - compare.diffs(diff_options) + # When saving MR diffs, `no_collapse` is implicitly added (because we need + # to save the entire contents to the DB), so add that here for + # consistency. + compare.diffs(diff_options.merge(no_collapse: true)) else merge_request_diff.diffs(diff_options) end end def diff_size - # The `#diffs` method ends up at an instance of a class inheriting from - # `Gitlab::Diff::FileCollection::Base`, so use those options as defaults - # here too, to get the same diff size without performing highlighting. - # - opts = Gitlab::Diff::FileCollection::Base.default_options.merge(diff_options || {}) + # Calling `merge_request_diff.diffs.real_size` will also perform + # highlighting, which we don't need here. + return real_size if merge_request_diff - raw_diffs(opts).size + diffs.real_size end def diff_base_commit diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 6604af2b47e..f0a3c30ea74 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -260,7 +260,7 @@ class MergeRequestDiff < ActiveRecord::Base new_attributes[:state] = :empty else diff_collection = compare.diffs(Commit.max_diff_options) - new_attributes[:real_size] = compare.diffs.real_size + new_attributes[:real_size] = diff_collection.real_size if diff_collection.any? new_diffs = dump_diffs(diff_collection) diff --git a/changelogs/unreleased/mr-diff-size-overflow.yml b/changelogs/unreleased/mr-diff-size-overflow.yml new file mode 100644 index 00000000000..87449930cf2 --- /dev/null +++ b/changelogs/unreleased/mr-diff-size-overflow.yml @@ -0,0 +1,4 @@ +--- +title: Show sizes correctly in merge requests when diffs overflow +merge_request: +author: diff --git a/lib/gitlab/diff/file_collection/merge_request_diff.rb b/lib/gitlab/diff/file_collection/merge_request_diff.rb index 329d12f13d1..0bd226ef050 100644 --- a/lib/gitlab/diff/file_collection/merge_request_diff.rb +++ b/lib/gitlab/diff/file_collection/merge_request_diff.rb @@ -15,6 +15,10 @@ module Gitlab super.tap { |_| store_highlight_cache } end + def real_size + @merge_request_diff.real_size + end + private # Extracted method to highlight in the same iteration to the diff_collection. diff --git a/spec/features/merge_requests/diffs_spec.rb b/spec/features/merge_requests/diffs_spec.rb index 32a6a4b2682..7dee3b852ca 100644 --- a/spec/features/merge_requests/diffs_spec.rb +++ b/spec/features/merge_requests/diffs_spec.rb @@ -1,13 +1,8 @@ require 'spec_helper' feature 'Diffs URL', js: true, feature: true do - include ApplicationHelper - - let(:author_user) { create(:user) } - let(:user) { create(:user) } let(:project) { create(:project, :public) } - let(:forked_project) { Projects::ForkService.new(project, author_user).execute } - let(:merge_request) { create(:merge_request_with_diffs, source_project: forked_project, target_project: project, author: author_user) } + let(:merge_request) { create(:merge_request, source_project: project) } context 'when visit with */* as accept header' do before(:each) do @@ -27,20 +22,23 @@ feature 'Diffs URL', js: true, feature: true do context 'when merge request has overflow' do it 'displays warning' do - allow_any_instance_of(MergeRequestDiff).to receive(:overflow?).and_return(true) - allow(Commit).to receive(:max_diff_options).and_return(max_files: 20, max_lines: 20) + allow(Commit).to receive(:max_diff_options).and_return(max_files: 3) visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) page.within('.alert') do expect(page).to have_text("Too many changes to show. Plain diff Email patch To preserve - performance only 3 of 3 files are displayed.") + performance only 3 of 3+ files are displayed.") end end end - describe 'when editing file' do - let(:changelog_id) { hexdigest("CHANGELOG") } + context 'when editing file' do + let(:author_user) { create(:user) } + let(:user) { create(:user) } + let(:forked_project) { Projects::ForkService.new(project, author_user).execute } + let(:merge_request) { create(:merge_request_with_diffs, source_project: forked_project, target_project: project, author: author_user) } + let(:changelog_id) { Digest::SHA1.hexdigest("CHANGELOG") } context 'as author' do it 'shows direct edit link' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 415d3e7b200..be08b96641a 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -199,10 +199,10 @@ describe MergeRequest, models: true do end context 'when there are no MR diffs' do - it 'delegates to the compare object' do + it 'delegates to the compare object, setting no_collapse: true' do merge_request.compare = double(:compare) - expect(merge_request.compare).to receive(:diffs).with(options) + expect(merge_request.compare).to receive(:diffs).with(options.merge(no_collapse: true)) merge_request.diffs(options) end @@ -215,15 +215,22 @@ describe MergeRequest, models: true do end context 'when there are MR diffs' do - before do + it 'returns the correct count' do merge_request.save + + expect(merge_request.diff_size).to eq('105') end - it 'returns the correct count' do - expect(merge_request.diff_size).to eq(105) + it 'returns the correct overflow count' do + allow(Commit).to receive(:max_diff_options).and_return(max_files: 2) + merge_request.save + + expect(merge_request.diff_size).to eq('2+') end it 'does not perform highlighting' do + merge_request.save + expect(Gitlab::Diff::Highlight).not_to receive(:new) merge_request.diff_size @@ -231,7 +238,7 @@ describe MergeRequest, models: true do end context 'when there are no MR diffs' do - before do + def set_compare(merge_request) merge_request.compare = CompareService.new( merge_request.source_project, merge_request.source_branch @@ -242,10 +249,21 @@ describe MergeRequest, models: true do end it 'returns the correct count' do - expect(merge_request.diff_size).to eq(105) + set_compare(merge_request) + + expect(merge_request.diff_size).to eq('105') + end + + it 'returns the correct overflow count' do + allow(Commit).to receive(:max_diff_options).and_return(max_files: 2) + set_compare(merge_request) + + expect(merge_request.diff_size).to eq('2+') end it 'does not perform highlighting' do + set_compare(merge_request) + expect(Gitlab::Diff::Highlight).not_to receive(:new) merge_request.diff_size From 03534bb2b85c9e525da8564229378dfd23af695f Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 25 Apr 2017 16:02:39 +0000 Subject: [PATCH 14/19] Add sub-nav for Project Integration Services edit page --- app/views/layouts/nav/_project.html.haml | 2 +- app/views/projects/services/edit.html.haml | 1 + app/views/projects/settings/_head.html.haml | 2 +- .../unreleased/31174-project-integration-service-sub-nav.yml | 4 ++++ 4 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/31174-project-integration-service-sub-nav.yml diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 37429c7cfc0..8ab9747efc5 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -56,7 +56,7 @@ Snippets - if project_nav_tab? :settings - = nav_link(path: %w[projects#edit members#show integrations#show repository#show ci_cd#show pages#show]) do + = nav_link(path: %w[projects#edit members#show integrations#show services#edit repository#show ci_cd#show pages#show]) do = link_to edit_project_path(@project), title: 'Settings', class: 'shortcuts-tree' do %span Settings diff --git a/app/views/projects/services/edit.html.haml b/app/views/projects/services/edit.html.haml index 50ed78286d2..0f1a76a104a 100644 --- a/app/views/projects/services/edit.html.haml +++ b/app/views/projects/services/edit.html.haml @@ -1,2 +1,3 @@ - page_title @service.title, "Services" += render "projects/settings/head" = render 'form' diff --git a/app/views/projects/settings/_head.html.haml b/app/views/projects/settings/_head.html.haml index 88bcb541dac..e50a543ffa8 100644 --- a/app/views/projects/settings/_head.html.haml +++ b/app/views/projects/settings/_head.html.haml @@ -14,7 +14,7 @@ %span Members - if can_edit - = nav_link(controller: :integrations) do + = nav_link(controller: [:integrations, :services]) do = link_to project_settings_integrations_path(@project), title: 'Integrations' do %span Integrations diff --git a/changelogs/unreleased/31174-project-integration-service-sub-nav.yml b/changelogs/unreleased/31174-project-integration-service-sub-nav.yml new file mode 100644 index 00000000000..f3f91f92428 --- /dev/null +++ b/changelogs/unreleased/31174-project-integration-service-sub-nav.yml @@ -0,0 +1,4 @@ +--- +title: Add sub-nav for Project Integration Services edit page +merge_request: 10813 +author: From 4a972aa2293af24f5b1a343b073acd9c1b18ca97 Mon Sep 17 00:00:00 2001 From: Taurie Davis Date: Tue, 25 Apr 2017 17:13:59 +0000 Subject: [PATCH 15/19] Resolve "Clean up padding with Markdown headers" --- app/assets/stylesheets/framework/awards.scss | 3 +- app/assets/stylesheets/framework/common.scss | 2 +- app/assets/stylesheets/framework/files.scss | 8 ---- .../stylesheets/framework/timeline.scss | 2 +- .../stylesheets/framework/typography.scss | 40 +++++++++++++------ app/assets/stylesheets/pages/detail_page.scss | 6 --- app/assets/stylesheets/pages/issuable.scss | 6 ++- app/assets/stylesheets/pages/note_form.scss | 2 +- app/assets/stylesheets/pages/notes.scss | 9 +++-- app/views/projects/issues/show.html.haml | 2 +- app/views/projects/milestones/show.html.haml | 2 +- .../unreleased/27827-cleanup-markdown.yml | 4 ++ 12 files changed, 48 insertions(+), 38 deletions(-) create mode 100644 changelogs/unreleased/27827-cleanup-markdown.yml diff --git a/app/assets/stylesheets/framework/awards.scss b/app/assets/stylesheets/framework/awards.scss index f614f262316..b2102d2fbc5 100644 --- a/app/assets/stylesheets/framework/awards.scss +++ b/app/assets/stylesheets/framework/awards.scss @@ -108,8 +108,7 @@ } .award-control { - margin: 3px 5px 3px 0; - padding: .35em .4em; + margin-right: 5px; outline: 0; &.disabled { diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 0fd7203e72b..638c1403b25 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -70,7 +70,7 @@ pre { } hr { - margin: $gl-padding 0; + margin: 24px 0; border-top: 1px solid darken($gray-normal, 8%); } diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index a5a8522739e..df819ffe4bc 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -73,14 +73,6 @@ &.wiki { padding: 30px $gl-padding; - - .highlight { - margin-bottom: 9px; - - > pre { - margin: 0; - } - } } &.blob-no-preview { diff --git a/app/assets/stylesheets/framework/timeline.scss b/app/assets/stylesheets/framework/timeline.scss index cd23deb6d75..d2164a1d333 100644 --- a/app/assets/stylesheets/framework/timeline.scss +++ b/app/assets/stylesheets/framework/timeline.scss @@ -4,7 +4,7 @@ padding: 0; .timeline-entry { - padding: $gl-padding $gl-btn-padding 14px; + padding: $gl-padding $gl-btn-padding 0; border-color: $white-normal; color: $gl-text-color; border-bottom: 1px solid $border-white-light; diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 1839cadcc10..96d8a812723 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -8,6 +8,13 @@ img { max-width: 100%; + margin: 0 0 8px; + } + + p a:not(.no-attachment-icon) img { + // Remove bottom padding because + //

already has $gl-padding bottom + margin-bottom: 0; } *:first-child:not(.katex-display) { @@ -47,44 +54,50 @@ h1 { font-size: 1.75em; font-weight: 600; - margin: 16px 0 10px; - padding: 0 0 0.3em; + margin: 24px 0 16px; + padding-bottom: 0.3em; border-bottom: 1px solid $white-dark; color: $gl-text-color; + + &:first-child { + margin-top: 0; + } } h2 { font-size: 1.5em; font-weight: 600; - margin: 16px 0 10px; + margin: 24px 0 16px; + padding-bottom: 0.3em; + border-bottom: 1px solid $white-dark; color: $gl-text-color; } h3 { - margin: 16px 0 10px; + margin: 24px 0 16px; font-size: 1.3em; } h4 { - margin: 16px 0 10px; + margin: 24px 0 16px; font-size: 1.2em; } h5 { - margin: 16px 0 10px; + margin: 24px 0 16px; font-size: 1em; } h6 { - margin: 16px 0 10px; + margin: 24px 0 16px; font-size: 0.95em; } blockquote { color: $gl-grayish-blue; font-size: inherit; - padding: 8px 21px; - margin: 12px 0; + padding: 8px 24px; + margin: 16px 0; border-left: 3px solid $white-dark; } @@ -95,19 +108,20 @@ blockquote p { color: $gl-grayish-blue !important; + margin: 0; font-size: inherit; line-height: 1.5; } p { color: $gl-text-color; - margin: 6px 0 0; + margin: 0 0 16px; } table { @extend .table; @extend .table-bordered; - margin: 12px 0; + margin: 16px 0; color: $gl-text-color; th { @@ -120,7 +134,7 @@ } pre { - margin: 12px 0; + margin-bottom: 16px; font-size: 13px; line-height: 1.6em; overflow-x: auto; @@ -134,7 +148,7 @@ ul, ol { padding: 0; - margin: 3px 0 !important; + margin: 0 0 16px !important; } ul:dir(rtl), diff --git a/app/assets/stylesheets/pages/detail_page.scss b/app/assets/stylesheets/pages/detail_page.scss index 46fd19c93f9..f3de05aa5f6 100644 --- a/app/assets/stylesheets/pages/detail_page.scss +++ b/app/assets/stylesheets/pages/detail_page.scss @@ -29,11 +29,5 @@ .description { margin-top: 6px; - - p { - &:last-child { - margin-bottom: 0; - } - } } } diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 8d3d1a72b9b..97fab513b01 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -52,7 +52,7 @@ .title { padding: 0; - margin: 0; + margin-bottom: 16px; border-bottom: none; } @@ -357,6 +357,8 @@ } .detail-page-description { + padding: 16px 0 0; + small { color: $gray-darkest; } @@ -364,6 +366,8 @@ .edited-text { color: $gray-darkest; + display: block; + margin: 0 0 16px; .author_link { color: $gray-darkest; diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 03d1c3067c7..62f654ed343 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -28,7 +28,7 @@ .note-edit-form { .note-form-actions { position: relative; - margin-top: $gl-padding; + margin: $gl-padding 0; } .note-preview-holder { diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 98b93cca6e3..69a95db6920 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -102,13 +102,12 @@ ul.notes { .note-awards { .js-awards-block { - padding: 2px; - margin-top: 10px; + margin-bottom: 16px; } } .note-header { - padding-bottom: 3px; + padding-bottom: 8px; padding-right: 20px; @media (min-width: $screen-sm-min) { @@ -151,6 +150,10 @@ ul.notes { margin-left: 65px; } + .note-header { + padding-bottom: 0; + } + &.timeline-entry::after { clear: none; } diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index fcbd8829595..4d56aa214e2 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -50,7 +50,7 @@ = link_to 'Edit', edit_namespace_project_issue_path(@project.namespace, @project, @issue), class: 'hidden-xs hidden-sm btn btn-grouped issuable-edit' .issue-details.issuable-details - .detail-page-description.content-block{ class: ('hide-bottom-border' unless @issue.description.present? ) } + .detail-page-description.content-block .issue-title-data.hidden{ "data" => { "initial-title" => markdown_field(@issue, :title), "endpoint" => rendered_title_namespace_project_issue_path(@project.namespace, @project, @issue), } } diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index e8c9d7f8429..33bbbd9a3f8 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/app/views/projects/milestones/show.html.haml @@ -36,7 +36,7 @@ %a.btn.btn-default.btn-grouped.pull-right.visible-xs-block.js-sidebar-toggle{ href: "#" } = icon('angle-double-left') - .detail-page-description.milestone-detail{ class: ('hide-bottom-border' unless @milestone.description.present? ) } + .detail-page-description.milestone-detail %h2.title = markdown_field(@milestone, :title) %div diff --git a/changelogs/unreleased/27827-cleanup-markdown.yml b/changelogs/unreleased/27827-cleanup-markdown.yml new file mode 100644 index 00000000000..a8890b78763 --- /dev/null +++ b/changelogs/unreleased/27827-cleanup-markdown.yml @@ -0,0 +1,4 @@ +--- +title: Cleanup markdown spacing +merge_request: +author: From 61abdb1544c487cfe62f2ab6ac746757615ce518 Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Lopez Date: Tue, 25 Apr 2017 18:03:08 +0000 Subject: [PATCH 16/19] Metrics graph error fix --- .../monitoring/prometheus_graph.js | 63 ++++++++++--------- .../unreleased/metrics-graph-error-fix.yml | 4 ++ 2 files changed, 38 insertions(+), 29 deletions(-) create mode 100644 changelogs/unreleased/metrics-graph-error-fix.yml diff --git a/app/assets/javascripts/monitoring/prometheus_graph.js b/app/assets/javascripts/monitoring/prometheus_graph.js index aff507abb91..78bb0e6fb47 100644 --- a/app/assets/javascripts/monitoring/prometheus_graph.js +++ b/app/assets/javascripts/monitoring/prometheus_graph.js @@ -22,6 +22,7 @@ class PrometheusGraph { const hasMetrics = $prometheusContainer.data('has-metrics'); this.docLink = $prometheusContainer.data('doc-link'); this.integrationLink = $prometheusContainer.data('prometheus-integration'); + this.state = ''; $(document).ajaxError(() => {}); @@ -38,8 +39,9 @@ class PrometheusGraph { this.configureGraph(); this.init(); } else { + const prevState = this.state; this.state = '.js-getting-started'; - this.updateState(); + this.updateState(prevState); } } @@ -53,26 +55,26 @@ class PrometheusGraph { } init() { - this.getData().then((metricsResponse) => { + return this.getData().then((metricsResponse) => { let enoughData = true; - Object.keys(metricsResponse.metrics).forEach((key) => { - let currentKey; - if (key === 'cpu_values' || key === 'memory_values') { - currentKey = metricsResponse.metrics[key]; - if (Object.keys(currentKey).length === 0) { - enoughData = false; - } - } - }); - if (!enoughData) { - this.state = '.js-loading'; - this.updateState(); + if (typeof metricsResponse === 'undefined') { + enoughData = false; } else { + Object.keys(metricsResponse.metrics).forEach((key) => { + if (key === 'cpu_values' || key === 'memory_values') { + const currentData = (metricsResponse.metrics[key])[0]; + if (currentData.values.length <= 2) { + enoughData = false; + } + } + }); + } + if (enoughData) { + $(prometheusStatesContainer).hide(); + $(prometheusParentGraphContainer).show(); this.transformData(metricsResponse); this.createGraph(); } - }).catch(() => { - new Flash('An error occurred when trying to load metrics. Please try again.'); }); } @@ -342,6 +344,8 @@ class PrometheusGraph { getData() { const maxNumberOfRequests = 3; + this.state = '.js-loading'; + this.updateState(); return gl.utils.backOff((next, stop) => { $.ajax({ url: metricsEndpoint, @@ -352,12 +356,11 @@ class PrometheusGraph { this.backOffRequestCounter = this.backOffRequestCounter += 1; if (this.backOffRequestCounter < maxNumberOfRequests) { next(); - } else { - stop({ - status: resp.status, - metrics: data, - }); + } else if (this.backOffRequestCounter >= maxNumberOfRequests) { + stop(new Error('loading')); } + } else if (!data.success) { + stop(new Error('loading')); } else { stop({ status: resp.status, @@ -373,8 +376,9 @@ class PrometheusGraph { return resp.metrics; }) .catch(() => { + const prevState = this.state; this.state = '.js-unable-to-connect'; - this.updateState(); + this.updateState(prevState); }); } @@ -382,19 +386,20 @@ class PrometheusGraph { Object.keys(metricsResponse.metrics).forEach((key) => { if (key === 'cpu_values' || key === 'memory_values') { const metricValues = (metricsResponse.metrics[key])[0]; - if (metricValues !== undefined) { - this.graphSpecificProperties[key].data = metricValues.values.map(metric => ({ - time: new Date(metric[0] * 1000), - value: metric[1], - })); - } + this.graphSpecificProperties[key].data = metricValues.values.map(metric => ({ + time: new Date(metric[0] * 1000), + value: metric[1], + })); } }); } - updateState() { + updateState(prevState) { const $statesContainer = $(prometheusStatesContainer); $(prometheusParentGraphContainer).hide(); + if (prevState) { + $(`${prevState}`, $statesContainer).addClass('hidden'); + } $(`${this.state}`, $statesContainer).removeClass('hidden'); $(prometheusStatesContainer).show(); } diff --git a/changelogs/unreleased/metrics-graph-error-fix.yml b/changelogs/unreleased/metrics-graph-error-fix.yml new file mode 100644 index 00000000000..2698b92e1f1 --- /dev/null +++ b/changelogs/unreleased/metrics-graph-error-fix.yml @@ -0,0 +1,4 @@ +--- +title: Fixed Prometheus monitoring graphs not showing empty states in certain scenarios +merge_request: +author: From c6b5ec0fd87a9a7ddea67f6b628fa1a65cdef3c2 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 25 Apr 2017 13:24:27 -0500 Subject: [PATCH 17/19] Update MR diff blob_fork_suggestion after jQuery update `.init()` was added in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10858 We need to add it the MR diff instance that was added in a separate MR and of course didn't add a conflict because it is a completely different piece --- app/assets/javascripts/merge_request_tabs.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 6075157ec31..93c30c54a8e 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -275,7 +275,8 @@ import BlobForkSuggestion from './blob/blob_fork_suggestion'; cancelButtons: $(el).find('.js-cancel-fork-suggestion-button'), suggestionSections: $(el).find('.js-file-fork-suggestion-section'), actionTextPieces: $(el).find('.js-file-fork-suggestion-section-action'), - }); + }) + .init(); }); }, }); From 3b560dd04bb49bcefa3abfd251224969cfca427f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 25 Apr 2017 15:42:11 +0200 Subject: [PATCH 18/19] Remove outdated ci_setup.md doc page and document MySQL and RSpec profiling for specific branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .gitlab-ci.yml | 2 +- doc/development/README.md | 1 - doc/development/ci_setup.md | 47 ------------------------------------- doc/development/testing.md | 15 +++++++++--- 4 files changed, 13 insertions(+), 52 deletions(-) delete mode 100644 doc/development/ci_setup.md diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 12385b293c0..4a16a0aaba0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -59,7 +59,7 @@ stages: .only-master-and-ee-or-mysql: &only-master-and-ee-or-mysql only: - - /\-(?i)mysql$/ + - /mysql/ - master@gitlab-org/gitlab-ce - master@gitlab/gitlabhq - tags@gitlab-org/gitlab-ce diff --git a/doc/development/README.md b/doc/development/README.md index 3c797505aa9..77bb0263374 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -33,7 +33,6 @@ ## Backend howtos - [Architecture](architecture.md) of GitLab -- [CI setup](ci_setup.md) for testing GitLab - [Gotchas](gotchas.md) to avoid - [How to dump production data to staging](db_dump.md) - [Instrumentation](instrumentation.md) diff --git a/doc/development/ci_setup.md b/doc/development/ci_setup.md deleted file mode 100644 index 0810b32efd7..00000000000 --- a/doc/development/ci_setup.md +++ /dev/null @@ -1,47 +0,0 @@ -# CI setup - -This document describes what services we use for testing GitLab and GitLab CI. - -We currently use four CI services to test GitLab: - -1. GitLab CI on [GitHost.io](https://gitlab-ce.githost.io/projects/4/) for the [GitLab.com repo](https://gitlab.com/gitlab-org/gitlab-ce) -2. GitLab CI at ci.gitlab.org to test the private GitLab B.V. repo at dev.gitlab.org -3. [Semephore](https://semaphoreapp.com/gitlabhq/gitlabhq/) for [GitHub.com repo](https://github.com/gitlabhq/gitlabhq) -4. [Mock CI Service](../user/project/integrations/mock_ci.md) for local development - -| Software @ configuration being tested | GitLab CI (ci.gitlab.org) | GitLab CI (GitHost.io) | Semaphore | -|---------------------------------------|---------------------------|---------------------------------------------------------------------------|-----------| -| GitLab CE @ MySQL | ✓ | ✓ [Core team can trigger builds](https://gitlab-ce.githost.io/projects/4) | | -| GitLab CE @ PostgreSQL | | | ✓ [Core team can trigger builds](https://semaphoreapp.com/gitlabhq/gitlabhq/branches/master) | -| GitLab EE @ MySQL | ✓ | | | -| GitLab CI @ MySQL | ✓ | | | -| GitLab CI @ PostgreSQL | | | ✓ | -| GitLab CI Runner | ✓ | | ✓ | -| GitLab Shell | ✓ | | ✓ | -| GitLab Shell | ✓ | | ✓ | - -Core team has access to trigger builds if needed for GitLab CE. - -We use [these build scripts](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.gitlab-ci.yml) for testing with GitLab CI. - -# Build configuration on [Semaphore](https://semaphoreapp.com/gitlabhq/gitlabhq/) for testing the [GitHub.com repo](https://github.com/gitlabhq/gitlabhq) - -- Language: Ruby -- Ruby version: 2.1.8 -- database.yml: pg - -Build commands - -```bash -sudo apt-get install cmake libicu-dev -y (Setup) -bundle install --deployment --path vendor/bundle (Setup) -cp config/gitlab.yml.example config/gitlab.yml (Setup) -bundle exec rake db:create (Setup) -bundle exec rake spinach (Thread #1) -bundle exec rake spec (thread #2) -bundle exec rake rubocop (thread #3) -bundle exec rake brakeman (thread #4) -bundle exec rake jasmine:ci (thread #5) -``` - -Use rubygems mirror. diff --git a/doc/development/testing.md b/doc/development/testing.md index ad540ec13db..2c7154f1dea 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -448,13 +448,22 @@ is used for Spinach tests as well. ### Monitoring -The GitLab test suite is [monitored] and a [public dashboard] is available for -everyone to see. Feel free to look at the slowest test files and try to improve -them. +The GitLab test suite is [monitored] for the `master` branch, and any branch +that includes `rspec-profile` in their name. + +A [public dashboard] is available for everyone to see. Feel free to look at the +slowest test files and try to improve them. [monitored]: ./performance.md#rspec-profiling [public dashboard]: https://redash.gitlab.com/public/dashboards/l1WhHXaxrCWM5Ai9D7YDqHKehq6OU3bx5gssaiWe?org_slug=default +## CI setup + +- On CE, the test suite only runs against PostgreSQL by default. We additionally + run the suite against MySQL for tags, `master`, and any branch that includes + `mysql` in the name. +- On EE, the test suite always runs both PostgreSQL and MySQL. + ## Spinach (feature) tests GitLab [moved from Cucumber to Spinach](https://github.com/gitlabhq/gitlabhq/pull/1426) From 46973f3d4602c7ea6366d6401116b89d72b83b9e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 25 Apr 2017 21:48:02 +0300 Subject: [PATCH 19/19] Bump gitlab-shell version to 5.0.3 Signed-off-by: Dmitriy Zaporozhets --- GITLAB_SHELL_VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index a1ef0cae183..50e2274e6d3 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -5.0.2 +5.0.3