From 90a67a76d5b852c62b59dd52b9dafd58722f2237 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 14 Apr 2016 17:32:46 -0400 Subject: Always read diff_view setting from the cookie Prior, when the user had their view set to "parallel" and then visited a merge request's changes tab _without_ passing the `view` parameter via query string, the view would be parallel but the `Notes` class was always instantiated with the default value from `diff_view` ("inline"), resulting in broken markup when the form to add a line note was dynamically inserted. The cookie is set whenever the view is changed, so this value should always be up-to-date. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/14557 --- app/helpers/diff_helper.rb | 8 +++++++- app/views/projects/merge_requests/_show.html.haml | 2 +- spec/helpers/diff_helper_spec.rb | 20 ++++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 6a3ec83b8c0..97466d532f4 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -9,7 +9,13 @@ module DiffHelper end def diff_view - params[:view] == 'parallel' ? 'parallel' : 'inline' + diff_views = %w(inline parallel) + + if diff_views.include?(cookies[:diff_view]) + cookies[:diff_view] + else + diff_views.first + end end def diff_hard_limit_enabled? diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 1dd8f721f7e..1733622e9b7 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -4,7 +4,7 @@ = render "header_title" -- if params[:view] == 'parallel' +- if diff_view == 'parallel' - fluid_layout true .merge-request{'data-url' => merge_request_path(@merge_request)} diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 982c113e84b..b7810185d16 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -11,6 +11,26 @@ describe DiffHelper do let(:diff_refs) { [commit.parent, commit] } let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs) } + describe 'diff_view' do + it 'returns a valid value when cookie is set' do + helper.request.cookies[:diff_view] = 'parallel' + + expect(helper.diff_view).to eq 'parallel' + end + + it 'returns a default value when cookie is invalid' do + helper.request.cookies[:diff_view] = 'invalid' + + expect(helper.diff_view).to eq 'inline' + end + + it 'returns a default value when cookie is nil' do + expect(helper.request.cookies).to be_empty + + expect(helper.diff_view).to eq 'inline' + end + end + describe 'diff_hard_limit_enabled?' do it 'should return true if param is provided' do allow(controller).to receive(:params) { { force_show_diff: true } } -- cgit v1.2.1 From 136887da4b1c1184630ca2242055fe35bf04a69b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 19 Apr 2016 10:54:27 +0100 Subject: Removed JS update templates --- app/controllers/projects/issues_controller.rb | 1 - app/controllers/projects/merge_requests_controller.rb | 1 - app/views/projects/issues/update.js.haml | 0 app/views/projects/merge_requests/update.js.haml | 0 app/views/shared/issuable/_sidebar.html.haml | 2 +- 5 files changed, 1 insertion(+), 3 deletions(-) delete mode 100644 app/views/projects/issues/update.js.haml delete mode 100644 app/views/projects/merge_requests/update.js.haml diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 38214f04793..05f12dad6ac 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -100,7 +100,6 @@ class Projects::IssuesController < Projects::ApplicationController end respond_to do |format| - format.js format.html do if @issue.valid? redirect_to issue_path(@issue) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 3e0cfc6aa65..debe0475a20 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -148,7 +148,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController if @merge_request.valid? respond_to do |format| - format.js format.html do redirect_to([@merge_request.target_project.namespace.becomes(Namespace), @merge_request.target_project, @merge_request]) diff --git a/app/views/projects/issues/update.js.haml b/app/views/projects/issues/update.js.haml deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/app/views/projects/merge_requests/update.js.haml b/app/views/projects/merge_requests/update.js.haml deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 08bfd93f4e6..4560667d917 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -20,7 +20,7 @@ %a.btn.btn-default.disabled{href: '#'} Next - = form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, html: {class: 'issuable-context-form inline-update js-issuable-update'} do |f| + = form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, format: :json, html: {class: 'issuable-context-form inline-update js-issuable-update'} do |f| .block.assignee .sidebar-collapsed-icon.sidebar-collapsed-user{data: {toggle: "tooltip", placement: "left", container: "body"}, title: (issuable.assignee.to_reference if issuable.assignee)} - if issuable.assignee -- cgit v1.2.1 From 8530ce4c6fbc411e00cf426f3b3baca74a4370f7 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 18 Apr 2016 14:13:38 -0400 Subject: Clarify that the diff view setting always comes from the cookie This invalidates one test, which we've removed. --- app/controllers/projects/application_controller.rb | 3 +-- spec/controllers/projects/merge_requests_controller_spec.rb | 8 -------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 74150ad606b..be872a93fee 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -83,8 +83,7 @@ class Projects::ApplicationController < ApplicationController end def apply_diff_view_cookie! - view = params[:view] || cookies[:diff_view] - cookies.permanent[:diff_view] = params[:view] = view if view + cookies.permanent[:diff_view] = params.delete(:view) if params[:view].present? end def builds_enabled diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index c54e83339a1..c0a1f45195f 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -300,14 +300,6 @@ describe Projects::MergeRequestsController do expect(response.cookies['diff_view']).to eq('parallel') end - - it 'assigns :view param based on cookie' do - request.cookies['diff_view'] = 'parallel' - - go - - expect(controller.params[:view]).to eq 'parallel' - end end describe 'GET commits' do -- cgit v1.2.1 From dbe06ac790befe2d49dbb6edecb79d82abfe46e7 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 20 Apr 2016 23:55:06 -0500 Subject: Add noreferrer value to rel attribute for external links --- lib/banzai/filter/external_link_filter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/banzai/filter/external_link_filter.rb b/lib/banzai/filter/external_link_filter.rb index d179bea181e..699de56fd4c 100644 --- a/lib/banzai/filter/external_link_filter.rb +++ b/lib/banzai/filter/external_link_filter.rb @@ -15,7 +15,7 @@ module Banzai # Skip internal links next if link.start_with?(internal_url) - node.set_attribute('rel', 'nofollow') + node.set_attribute('rel', 'nofollow noreferrer') end doc -- cgit v1.2.1 From 7f23e0b8c051da34c8587e4096476939a57fee24 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 21 Apr 2016 08:50:15 +0100 Subject: Fixes text color on labels in sidebar --- app/assets/javascripts/labels_select.js.coffee | 5 +++-- app/controllers/projects/issues_controller.rb | 2 +- app/controllers/projects/merge_requests_controller.rb | 2 +- app/helpers/labels_helper.rb | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 021ade73d44..5cb225c9baa 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -34,8 +34,8 @@ class @LabelsSelect if issueUpdateURL labelHTMLTemplate = _.template( '<% _.each(labels, function(label){ %> - issues?label_name=<%= _.escape(label.title) %>"> - + issues?label_name[]=<%= _.escape(label.title) %>"> + <%= _.escape(label.title) %> @@ -157,6 +157,7 @@ class @LabelsSelect data.issueURLSplit = issueURLSplit labelCount = 0 if data.labels.length + console.log data.labels template = labelHTMLTemplate(data) labelCount = data.labels.length else diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index e86428147ef..346b0556159 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -110,7 +110,7 @@ class Projects::IssuesController < Projects::ApplicationController end end format.json do - render json: @issue.to_json(include: [:milestone, :labels, assignee: { methods: :avatar_url }]) + render json: @issue.to_json(include: [:milestone, labels: { methods: :text_color }, assignee: { methods: :avatar_url }]) end end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 1388ea9d66c..d82a4290ed9 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -155,7 +155,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_request.target_project, @merge_request]) end format.json do - render json: @merge_request.to_json(include: [:milestone, :labels, assignee: { methods: :avatar_url }]) + render json: @merge_request.to_json(include: [:milestone, labels: { methods: :text_color }, assignee: { methods: :avatar_url }]) end end else diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index 3dded7c2f23..c99b137cdaa 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -37,7 +37,7 @@ module LabelsHelper link = send("namespace_project_#{type.to_s.pluralize}_path", project.namespace, project, - label_name: label.name) + label_name: [label.name]) if block_given? link_to link, &block -- cgit v1.2.1 From 15d1804d9f6a2c37255150f43c164fec0af8371f Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 21 Apr 2016 10:54:03 +0100 Subject: Updated tests --- spec/helpers/labels_helper_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 39042ff7e91..501f150cfda 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -11,13 +11,13 @@ describe LabelsHelper do end it 'uses the instance variable' do - expect(link_to_label(label)).to match %r{} + expect(link_to_label(label)).to match %r{} end end context 'without @project set' do it "uses the label's project" do - expect(link_to_label(label)).to match %r{.*} + expect(link_to_label(label)).to match %r{.*} end end @@ -25,7 +25,7 @@ describe LabelsHelper do let(:another_project) { double('project', namespace: 'foo3', to_param: 'bar3') } it 'links to merge requests page' do - expect(link_to_label(label, project: another_project)).to match %r{.*} + expect(link_to_label(label, project: another_project)).to match %r{.*} end end @@ -33,7 +33,7 @@ describe LabelsHelper do ['issue', :issue, 'merge_request', :merge_request].each do |type| context "set to #{type}" do it 'links to correct page' do - expect(link_to_label(label, type: type)).to match %r{.*} + expect(link_to_label(label, type: type)).to match %r{.*} end end end -- cgit v1.2.1 From b80bf57d378aec82cc933df233a30e0390564419 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 21 Apr 2016 16:50:26 +0200 Subject: Disable 'repository check' feature in 8.7.0 It still causes too many false alarms. --- CHANGELOG | 2 +- db/migrate/20160421130527_disable_repository_checks.rb | 11 +++++++++++ db/schema.rb | 4 ++-- doc/administration/repository_checks.md | 3 ++- 4 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20160421130527_disable_repository_checks.rb diff --git a/CHANGELOG b/CHANGELOG index 89572b9e156..c9f78599322 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -53,7 +53,7 @@ v 8.7.0 (unreleased) - Add links to CI setup documentation from project settings and builds pages - Display project members page to all members - Handle nil descriptions in Slack issue messages (Stan Hu) - - Add automated repository integrity checks + - Add automated repository integrity checks (OFF by default) - API: Expose open_issues_count, closed_issues_count, open_merge_requests_count for labels (Robert Schilling) - API: Ability to star and unstar a project (Robert Schilling) - Add default scope to projects to exclude projects pending deletion diff --git a/db/migrate/20160421130527_disable_repository_checks.rb b/db/migrate/20160421130527_disable_repository_checks.rb new file mode 100644 index 00000000000..808a4b93c7c --- /dev/null +++ b/db/migrate/20160421130527_disable_repository_checks.rb @@ -0,0 +1,11 @@ +class DisableRepositoryChecks < ActiveRecord::Migration + def up + change_column_default :application_settings, :repository_checks_enabled, false + execute 'UPDATE application_settings SET repository_checks_enabled = false' + end + + def down + change_column_default :application_settings, :repository_checks_enabled, true + execute 'UPDATE application_settings SET repository_checks_enabled = true' + end +end diff --git a/db/schema.rb b/db/schema.rb index a743e682423..42457d92353 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: 20160419120017) do +ActiveRecord::Schema.define(version: 20160421130527) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -77,7 +77,7 @@ ActiveRecord::Schema.define(version: 20160419120017) do t.string "akismet_api_key" t.boolean "email_author_in_body", default: false t.integer "default_group_visibility" - t.boolean "repository_checks_enabled", default: true + t.boolean "repository_checks_enabled", default: false t.integer "metrics_packet_size", default: 1 t.text "shared_runners_text" end diff --git a/doc/administration/repository_checks.md b/doc/administration/repository_checks.md index 61bf8ce6161..3411e4af6a7 100644 --- a/doc/administration/repository_checks.md +++ b/doc/administration/repository_checks.md @@ -1,7 +1,8 @@ # Repository checks >**Note:** -This feature was [introduced][ce-3232] in GitLab 8.7. +This feature was [introduced][ce-3232] in GitLab 8.7. It is OFF by +default because it still causes too many false alarms. Git has a built-in mechanism, [git fsck][git-fsck], to verify the integrity of all data commited to a repository. GitLab administrators -- cgit v1.2.1 From 639ba1b09b421a8c4c115d7e9f4e66923faf531f Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 21 Apr 2016 17:22:16 +0100 Subject: Removed console.log --- app/assets/javascripts/labels_select.js.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 5cb225c9baa..0417ba823f4 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -157,7 +157,6 @@ class @LabelsSelect data.issueURLSplit = issueURLSplit labelCount = 0 if data.labels.length - console.log data.labels template = labelHTMLTemplate(data) labelCount = data.labels.length else -- cgit v1.2.1 From 5e37d02d0ebd64249186577e306b7c502953a2e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 21 Apr 2016 18:49:08 +0200 Subject: Remove the `.distinct` when finding issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is not needed anymore after !3815. Signed-off-by: Rémy Coutable --- app/finders/issuable_finder.rb | 4 +--- spec/features/issues_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 93aa30b3255..f00f3f709e9 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -278,9 +278,7 @@ class IssuableFinder end end - # When filtering by multiple labels we may end up duplicating issues (if one - # has multiple labels). This ensures we only return unique issues. - items.distinct + items end def by_due_date(items) diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 90476ab369b..b1d49875ac4 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -178,6 +178,19 @@ describe 'Issues', feature: true do expect(first_issue).to include('foo') end + + context 'with a filter on labels' do + let(:label) { create(:label, project: project) } + before { create(:label_link, label: label, target: foo) } + + it 'sorts by least recently due date by excluding nil due dates' do + bar.update(due_date: nil) + + visit namespace_project_issues_path(project.namespace, project, label_names: [label.name], sort: sort_value_due_date_later) + + expect(first_issue).to include('foo') + end + end end describe 'filtering by due date' do -- cgit v1.2.1 From 97baf41adc6338c45cca2a274539d155b767a538 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 21 Apr 2016 18:46:12 +0100 Subject: Reverted link changes --- app/assets/javascripts/labels_select.js.coffee | 2 +- spec/helpers/labels_helper_spec.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 0417ba823f4..11e9227ba95 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -34,7 +34,7 @@ class @LabelsSelect if issueUpdateURL labelHTMLTemplate = _.template( '<% _.each(labels, function(label){ %> - issues?label_name[]=<%= _.escape(label.title) %>"> + issues?label_name=<%= _.escape(label.title) %>"> <%= _.escape(label.title) %> diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 501f150cfda..39042ff7e91 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -11,13 +11,13 @@ describe LabelsHelper do end it 'uses the instance variable' do - expect(link_to_label(label)).to match %r{} + expect(link_to_label(label)).to match %r{} end end context 'without @project set' do it "uses the label's project" do - expect(link_to_label(label)).to match %r{.*} + expect(link_to_label(label)).to match %r{.*} end end @@ -25,7 +25,7 @@ describe LabelsHelper do let(:another_project) { double('project', namespace: 'foo3', to_param: 'bar3') } it 'links to merge requests page' do - expect(link_to_label(label, project: another_project)).to match %r{.*} + expect(link_to_label(label, project: another_project)).to match %r{.*} end end @@ -33,7 +33,7 @@ describe LabelsHelper do ['issue', :issue, 'merge_request', :merge_request].each do |type| context "set to #{type}" do it 'links to correct page' do - expect(link_to_label(label, type: type)).to match %r{.*} + expect(link_to_label(label, type: type)).to match %r{.*} end end end -- cgit v1.2.1 From c079ae273c3fcab6f73b000a0e89d4faf233a9d9 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 21 Apr 2016 18:47:16 +0100 Subject: Reverted label link helper --- app/helpers/labels_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index c99b137cdaa..3dded7c2f23 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -37,7 +37,7 @@ module LabelsHelper link = send("namespace_project_#{type.to_s.pluralize}_path", project.namespace, project, - label_name: [label.name]) + label_name: label.name) if block_given? link_to link, &block -- cgit v1.2.1 From 72debd840cb76a5fc0929c0caeca179c39378680 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 21 Apr 2016 13:03:37 -0500 Subject: Fix failing spec --- lib/banzai/filter/external_link_filter.rb | 3 +-- spec/features/markdown_spec.rb | 7 ++++++- spec/lib/banzai/filter/external_link_filter_spec.rb | 10 +++++++++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/banzai/filter/external_link_filter.rb b/lib/banzai/filter/external_link_filter.rb index 699de56fd4c..38c4219518e 100644 --- a/lib/banzai/filter/external_link_filter.rb +++ b/lib/banzai/filter/external_link_filter.rb @@ -1,7 +1,6 @@ module Banzai module Filter - # HTML Filter to add a `rel="nofollow"` attribute to external links - # + # HTML Filter to modify the attributes of external links class ExternalLinkFilter < HTML::Pipeline::Filter def call doc.search('a').each do |node| diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb index 3d0d0e59fd7..0148c87084a 100644 --- a/spec/features/markdown_spec.rb +++ b/spec/features/markdown_spec.rb @@ -165,7 +165,12 @@ describe 'GitLab Markdown', feature: true do describe 'ExternalLinkFilter' do it 'adds nofollow to external link' do link = doc.at_css('a:contains("Google")') - expect(link.attr('rel')).to match 'nofollow' + expect(link.attr('rel')).to include('nofollow') + end + + it 'adds noreferrer to external link' do + link = doc.at_css('a:contains("Google")') + expect(link.attr('rel')).to include('noreferrer') end it 'ignores internal link' do diff --git a/spec/lib/banzai/filter/external_link_filter_spec.rb b/spec/lib/banzai/filter/external_link_filter_spec.rb index e3a8e15330e..f4c5c621bd0 100644 --- a/spec/lib/banzai/filter/external_link_filter_spec.rb +++ b/spec/lib/banzai/filter/external_link_filter_spec.rb @@ -24,6 +24,14 @@ describe Banzai::Filter::ExternalLinkFilter, lib: true do doc = filter(act) expect(doc.at_css('a')).to have_attribute('rel') - expect(doc.at_css('a')['rel']).to eq 'nofollow' + expect(doc.at_css('a')['rel']).to include 'nofollow' + end + + it 'adds rel="noreferrer" to external links' do + act = %q(Google) + doc = filter(act) + + expect(doc.at_css('a')).to have_attribute('rel') + expect(doc.at_css('a')['rel']).to include 'noreferrer' end end -- cgit v1.2.1 From 8277bd79681e18d121d54c6552e20bd4744781b3 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Thu, 21 Apr 2016 13:41:47 -0500 Subject: Remove float from icon --- app/assets/stylesheets/framework/files.scss | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 148a7cab6e2..32cc15113f2 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -131,6 +131,11 @@ td.line-numbers { float: none; border-left: 1px solid #ddd; + + i { + float: none; + margin-right: 0; + } } td.lines { padding: 0; -- cgit v1.2.1 From 50ed43e49099648fa5c242d0cee15f05e50349c3 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 11 Mar 2016 16:14:29 +0000 Subject: Improved confirmation UX Closes #4228 --- CHANGELOG | 1 + app/assets/stylesheets/framework/buttons.scss | 4 ++++ app/assets/stylesheets/pages/confirmation.scss | 18 ++++++++++++++++++ app/controllers/confirmations_controller.rb | 9 +++++++++ app/controllers/registrations_controller.rb | 4 ++-- app/views/devise/confirmations/almost_there.haml | 10 ++++++++++ app/views/layouts/devise_empty.html.haml | 17 +++++++++++++++++ config/routes.rb | 1 + spec/features/signup_spec.rb | 4 ++-- 9 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 app/assets/stylesheets/pages/confirmation.scss create mode 100644 app/views/devise/confirmations/almost_there.haml create mode 100644 app/views/layouts/devise_empty.html.haml diff --git a/CHANGELOG b/CHANGELOG index ab8b86ba928..d78833d2204 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -81,6 +81,7 @@ v 8.7.0 (unreleased) - Remove "Congratulations!" tweet button on newly-created project. (Connor Shea) - Fix admin/projects when using visibility levels on search (PotHix) - Build status notifications + - Update email confirmation interface - API: Expose user location (Robert Schilling) - API: Do not leak group existence via return code (Robert Schilling) - ClosingIssueExtractor regex now also works with colons. e.g. "Fixes: #1234" !3591 diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index e8c0172680d..18a74fe21a0 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -144,6 +144,10 @@ } } +.btn-lg { + padding: 12px 20px; +} + .btn-transparent { color: $btn-transparent-color; background-color: transparent; diff --git a/app/assets/stylesheets/pages/confirmation.scss b/app/assets/stylesheets/pages/confirmation.scss new file mode 100644 index 00000000000..125f495d6d4 --- /dev/null +++ b/app/assets/stylesheets/pages/confirmation.scss @@ -0,0 +1,18 @@ +.well-confirmation { + margin-bottom: 20px; + border-bottom: 1px solid #eee; + + > h1 { + font-weight: 400; + } + + .lead { + margin-bottom: 20px; + } +} + +.confirmation-content { + a { + color: $md-link-color; + } +} diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index af1faca93f6..7b66ad3f92c 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -1,7 +1,16 @@ class ConfirmationsController < Devise::ConfirmationsController + def almost_there + flash[:notice] = nil + render layout: "devise_empty" + end + protected + def after_resending_confirmation_instructions_path_for(resource) + users_almost_there_path + end + def after_confirmation_path_for(resource_name, resource) if signed_in?(resource_name) after_sign_in_path_for(resource) diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index c48175a4c5a..059b88e2253 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -31,11 +31,11 @@ class RegistrationsController < Devise::RegistrationsController end def after_sign_up_path_for(_resource) - new_user_session_path + users_almost_there_path end def after_inactive_sign_up_path_for(_resource) - new_user_session_path + users_almost_there_path end private diff --git a/app/views/devise/confirmations/almost_there.haml b/app/views/devise/confirmations/almost_there.haml new file mode 100644 index 00000000000..3c3830a3f10 --- /dev/null +++ b/app/views/devise/confirmations/almost_there.haml @@ -0,0 +1,10 @@ +.well-confirmation.text-center + %h1.prepend-top-0 + Almost there... + %p.lead + Please check your email to confirm your account +%p.confirmation-content.text-center + No confirmation email received? Please check your spam folder or +.append-bottom-20.prepend-top-20.text-center + %a.btn.btn-lg.btn-success{ href: new_user_confirmation_path } + Request new confirmation email diff --git a/app/views/layouts/devise_empty.html.haml b/app/views/layouts/devise_empty.html.haml new file mode 100644 index 00000000000..7c061dd531f --- /dev/null +++ b/app/views/layouts/devise_empty.html.haml @@ -0,0 +1,17 @@ +!!! 5 +%html{ lang: "en"} + = render "layouts/head" + %body.ui_charcoal.login-page.application.navless + = render "layouts/header/empty" + = render "layouts/broadcast" + .container.navless-container + .content + = render "layouts/flash" + = yield + + %hr + .container + .footer-links + = link_to "Explore", explore_root_path + = link_to "Help", help_path + = link_to "About GitLab", "https://about.gitlab.com/" diff --git a/config/routes.rb b/config/routes.rb index 79b62a0b1bb..2f820aafed1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -418,6 +418,7 @@ Rails.application.routes.draw do devise_scope :user do get '/users/auth/:provider/omniauth_error' => 'omniauth_callbacks#omniauth_error', as: :omniauth_error + get '/users/almost_there' => 'confirmations#almost_there' end root to: "root#index" diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb index 01472743b2a..51b754ff85c 100644 --- a/spec/features/signup_spec.rb +++ b/spec/features/signup_spec.rb @@ -13,8 +13,8 @@ feature 'Signup', feature: true do fill_in 'user_password_sign_up', with: user.password click_button "Sign up" - expect(current_path).to eq user_session_path - expect(page).to have_content("A message with a confirmation link has been sent to your email address.") + expect(current_path).to eq users_almost_there_path + expect(page).to have_content("Please check your email to confirm your account") end end -- cgit v1.2.1 From 4adfd501a5d31abd16bccf08586bf8a125b03450 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 21 Apr 2016 12:20:05 +0200 Subject: Verify label affiliation before assigning to issue This also verify if milestone belongs to correct project before creating a new issue. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/15439 --- app/services/issuable_base_service.rb | 28 ++++++++++++++++++++++++++-- spec/services/issues/create_service_spec.rb | 28 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 18f76d3f650..ab110001f91 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -37,8 +37,9 @@ class IssuableBaseService < BaseService end def filter_params(issuable_ability_name = :issue) - params[:assignee_id] = "" if params[:assignee_id] == IssuableFinder::NONE - params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE + filter_assignee + filter_milestone + filter_labels ability = :"admin_#{issuable_ability_name}" @@ -49,6 +50,29 @@ class IssuableBaseService < BaseService end end + def filter_assignee + if params[:assignee_id] == IssuableFinder::NONE + params[:assignee_id] = '' + end + end + + def filter_milestone + return unless params[:milestone_id] + + if params[:milestone_id] == IssuableFinder::NONE || + Milestone.find(params[:milestone_id]).try(:project) != project + params[:milestone_id] = '' + end + end + + def filter_labels + return if params[:label_ids].to_a.empty? + + params[:label_ids].select! do |label_id| + Label.find(label_id).try(:project) == project + end + end + def update(issuable) change_state(issuable) filter_params diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 5e7915db7e1..d11c45df8ff 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -37,6 +37,34 @@ describe Issues::CreateService, services: true do expect(Todo.where(attributes).count).to eq 1 end + + context 'label that belongs to different project' do + let(:issue) { Issues::CreateService.new(project, user, opts).execute } + let(:label) { create(:label) } + let(:opts) do + { title: 'Title', + description: 'Description', + label_ids: [label.id] } + end + + it 'does not assign label'do + expect(issue.labels).to_not include label + end + end + + context 'milestone that belongs to different project' do + let(:issue) { Issues::CreateService.new(project, user, opts).execute } + let(:milestone) { create(:milestone) } + let(:opts) do + { title: 'Title', + description: 'Description', + milestone_id: milestone.id } + end + + it 'does not assign label' do + expect(issue.milestone).to_not eq milestone + end + end end end end -- cgit v1.2.1 From 8df63d1513118f0b3643365e2950a68c80e46176 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 21 Apr 2016 12:25:21 +0200 Subject: Update specs to accomodate issuable assign changes --- spec/services/issues/bulk_update_service_spec.rb | 2 +- spec/services/issues/create_service_spec.rb | 28 +++++++++++----------- spec/services/issues/update_service_spec.rb | 11 ++++++--- .../services/merge_requests/update_service_spec.rb | 11 ++++++--- 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/spec/services/issues/bulk_update_service_spec.rb b/spec/services/issues/bulk_update_service_spec.rb index 6a7ea4b2f44..e91906d0d49 100644 --- a/spec/services/issues/bulk_update_service_spec.rb +++ b/spec/services/issues/bulk_update_service_spec.rb @@ -100,7 +100,7 @@ describe Issues::BulkUpdateService, services: true do describe :update_milestone do before do - @milestone = create :milestone + @milestone = create(:milestone, project: @project) @params = { issues_ids: [issue.id], milestone_id: @milestone.id diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index d11c45df8ff..64921c5f473 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -6,31 +6,31 @@ describe Issues::CreateService, services: true do let(:assignee) { create(:user) } describe :execute do + let(:issue) { Issues::CreateService.new(project, user, opts).execute } + context 'valid params' do before do project.team << [user, :master] project.team << [assignee, :master] + end - opts = { - title: 'Awesome issue', + let(:opts) do + { title: 'Awesome issue', description: 'please fix', - assignee: assignee - } - - @issue = Issues::CreateService.new(project, user, opts).execute + assignee: assignee } end - it { expect(@issue).to be_valid } - it { expect(@issue.title).to eq('Awesome issue') } - it { expect(@issue.assignee).to eq assignee } + it { expect(issue).to be_valid } + it { expect(issue.title).to eq('Awesome issue') } + it { expect(issue.assignee).to eq assignee } it 'creates a pending todo for new assignee' do attributes = { project: project, author: user, user: assignee, - target_id: @issue.id, - target_type: @issue.class.name, + target_id: issue.id, + target_type: issue.class.name, action: Todo::ASSIGNED, state: :pending } @@ -39,8 +39,8 @@ describe Issues::CreateService, services: true do end context 'label that belongs to different project' do - let(:issue) { Issues::CreateService.new(project, user, opts).execute } let(:label) { create(:label) } + let(:opts) do { title: 'Title', description: 'Description', @@ -53,15 +53,15 @@ describe Issues::CreateService, services: true do end context 'milestone that belongs to different project' do - let(:issue) { Issues::CreateService.new(project, user, opts).execute } let(:milestone) { create(:milestone) } + let(:opts) do { title: 'Title', description: 'Description', milestone_id: milestone.id } end - it 'does not assign label' do + it 'does not assign milestone' do expect(issue.milestone).to_not eq milestone end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 6b214a0d96b..52f69306994 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -4,10 +4,15 @@ describe Issues::UpdateService, services: true do let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } - let(:issue) { create(:issue, title: 'Old title', assignee_id: user3.id) } - let(:label) { create(:label) } + let(:project) { create(:empty_project) } + let(:label) { create(:label, project: project) } let(:label2) { create(:label) } - let(:project) { issue.project } + + let(:issue) do + create(:issue, title: 'Old title', + assignee_id: user3.id, + project: project) + end before do project.team << [user, :master] diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index cb8cff2fa8c..213e8c2eb3a 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -1,14 +1,19 @@ require 'spec_helper' describe MergeRequests::UpdateService, services: true do + let(:project) { create(:project) } let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } - let(:merge_request) { create(:merge_request, :simple, title: 'Old title', assignee_id: user3.id) } - let(:project) { merge_request.project } - let(:label) { create(:label) } + let(:label) { create(:label, project: project) } let(:label2) { create(:label) } + let(:merge_request) do + create(:merge_request, :simple, title: 'Old title', + assignee_id: user3.id, + source_project: project) + end + before do project.team << [user, :master] project.team << [user2, :developer] -- cgit v1.2.1 From 006d8b268ca2964ca3363fae53fdff7d26a9ae08 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 21 Apr 2016 12:46:48 +0200 Subject: Add affinity checks in issue create service --- spec/services/issues/create_service_spec.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 64921c5f473..2972c387836 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -3,12 +3,15 @@ require 'spec_helper' describe Issues::CreateService, services: true do let(:project) { create(:empty_project) } let(:user) { create(:user) } - let(:assignee) { create(:user) } describe :execute do let(:issue) { Issues::CreateService.new(project, user, opts).execute } context 'valid params' do + let(:assignee) { create(:user) } + let(:milestone) { create(:milestone, project: project) } + let(:labels) { create_list(:label, 4, project: project) } + before do project.team << [user, :master] project.team << [assignee, :master] @@ -17,12 +20,16 @@ describe Issues::CreateService, services: true do let(:opts) do { title: 'Awesome issue', description: 'please fix', - assignee: assignee } + assignee: assignee, + label_ids: labels.map(&:id), + milestone_id: milestone.id } end it { expect(issue).to be_valid } it { expect(issue.title).to eq('Awesome issue') } it { expect(issue.assignee).to eq assignee } + it { expect(issue.labels).to match_array labels } + it { expect(issue.milestone).to eq milestone } it 'creates a pending todo for new assignee' do attributes = { -- cgit v1.2.1 From 01ab6d704c36b211b5a06e290e6912346c8bf496 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 21 Apr 2016 13:40:52 +0200 Subject: Use association search in issuable create service --- app/services/issuable_base_service.rb | 12 ++++++------ spec/services/issues/create_service_spec.rb | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index ab110001f91..2b16089df1b 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -57,10 +57,11 @@ class IssuableBaseService < BaseService end def filter_milestone - return unless params[:milestone_id] + milestone_id = params[:milestone_id] + return unless milestone_id - if params[:milestone_id] == IssuableFinder::NONE || - Milestone.find(params[:milestone_id]).try(:project) != project + if milestone_id == IssuableFinder::NONE || + project.milestones.find_by(id: milestone_id).nil? params[:milestone_id] = '' end end @@ -68,9 +69,8 @@ class IssuableBaseService < BaseService def filter_labels return if params[:label_ids].to_a.empty? - params[:label_ids].select! do |label_id| - Label.find(label_id).try(:project) == project - end + params[:label_ids] = + project.labels.where(id: params[:label_ids]).pluck(:id) end def update(issuable) diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 2972c387836..ac28b6f71f9 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -4,13 +4,13 @@ describe Issues::CreateService, services: true do let(:project) { create(:empty_project) } let(:user) { create(:user) } - describe :execute do - let(:issue) { Issues::CreateService.new(project, user, opts).execute } + describe '#execute' do + let(:issue) { described_class.new(project, user, opts).execute } - context 'valid params' do + context 'when params are valid' do let(:assignee) { create(:user) } let(:milestone) { create(:milestone, project: project) } - let(:labels) { create_list(:label, 4, project: project) } + let(:labels) { create_pair(:label, project: project) } before do project.team << [user, :master] @@ -45,7 +45,7 @@ describe Issues::CreateService, services: true do expect(Todo.where(attributes).count).to eq 1 end - context 'label that belongs to different project' do + context 'when label belongs to different project' do let(:label) { create(:label) } let(:opts) do @@ -59,7 +59,7 @@ describe Issues::CreateService, services: true do end end - context 'milestone that belongs to different project' do + context 'when milestone belongs to different project' do let(:milestone) { create(:milestone) } let(:opts) do -- cgit v1.2.1 From 55df95c3886b42e92b0079b4d9d5eef0011f44d5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 21 Apr 2016 19:44:19 +0200 Subject: Add Changelog entry for private labels security fix --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index ab8b86ba928..e7428834c1b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.8.0 (unreleased) v 8.7.0 (unreleased) - Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented + - Fix vulnerability that made it possible to gain access to private labels and milestones - The number of InfluxDB points stored per UDP packet can now be configured - Fix error when cross-project label reference used with non-existent project - Transactions for /internal/allowed now have an "action" tag set -- cgit v1.2.1 From de39959f559d8f1be73a11714abcd2782f476fea Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Fri, 22 Apr 2016 10:42:30 +0300 Subject: Use new Note styleguide [ci skip] --- doc/workflow/cherry_pick_changes.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/workflow/cherry_pick_changes.md b/doc/workflow/cherry_pick_changes.md index b0ca0879643..4a499009842 100644 --- a/doc/workflow/cherry_pick_changes.md +++ b/doc/workflow/cherry_pick_changes.md @@ -1,6 +1,7 @@ # Cherry-pick changes -_**Note:** This feature was [introduced][ce-3514] in GitLab 8.7._ +>**Note:** +This feature was [introduced][ce-3514] in GitLab 8.7. --- -- cgit v1.2.1 From 846a9ef5ad5d946fefa135ce31efefab384f760c Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Fri, 22 Apr 2016 11:20:37 +0300 Subject: Add newest enhancements to GH importer docs [ci skip] --- doc/workflow/importing/import_projects_from_github.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/doc/workflow/importing/import_projects_from_github.md b/doc/workflow/importing/import_projects_from_github.md index f693f430a42..e670e415c71 100644 --- a/doc/workflow/importing/import_projects_from_github.md +++ b/doc/workflow/importing/import_projects_from_github.md @@ -1,7 +1,8 @@ # Import your project from GitHub to GitLab -_**Note:** In order to enable the GitHub import setting, you should first -enable the [GitHub integration][gh-import] in your GitLab instance._ +>**Note:** +In order to enable the GitHub import setting, you should first +enable the [GitHub integration][gh-import] in your GitLab instance. At its current state, GitHub importer can import: @@ -10,10 +11,13 @@ At its current state, GitHub importer can import: - the issues (introduced in GitLab 7.7) - the pull requests (introduced in GitLab 8.4) - the wiki pages (introduced in GitLab 8.4) +- the milestones (introduced in GitLab 8.7) +- the labels (introduced in GitLab 8.7) -It is not yet possible to import your labels, milestones and cross-repository -pull requests (those from forks). We are working on improving this in the near -future. +With GitLab 8.7+, references to pull requests and issues are preserved. + +It is not yet possible to import your cross-repository pull requests (those from +forks). We are working on improving this in the near future. The importer page is visible when you [create a new project][new-project]. Click on the **GitHub** link and you will be redirected to GitHub for -- cgit v1.2.1