Commit 46ac3107 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'issue_15557' into 'master'

Fix error 500 when sorting issues by milestone due date and filtering by labels

fixes #15557 

See merge request !4327
parents cb261abe 56f3b243
...@@ -318,7 +318,7 @@ Style/LambdaCall: ...@@ -318,7 +318,7 @@ Style/LambdaCall:
# Comments should start with a space. # Comments should start with a space.
Style/LeadingCommentSpace: Style/LeadingCommentSpace:
Enabled: false Enabled: true
# Use \ instead of + or << to concatenate two string literals at line end. # Use \ instead of + or << to concatenate two string literals at line end.
Style/LineEndConcatenation: Style/LineEndConcatenation:
......
...@@ -15,6 +15,7 @@ v 8.9.0 (unreleased) ...@@ -15,6 +15,7 @@ v 8.9.0 (unreleased)
- Changed the Slack build message to use the singular duration if necessary (Aran Koning) - Changed the Slack build message to use the singular duration if necessary (Aran Koning)
- Fix issues filter when ordering by milestone - Fix issues filter when ordering by milestone
- Todos will display target state if issuable target is 'Closed' or 'Merged' - Todos will display target state if issuable target is 'Closed' or 'Merged'
- Fix bug when sorting issues by milestone due date and filtering by two or more labels
- Remove 'main language' feature - Remove 'main language' feature
- Projects pending deletion will render a 404 page - Projects pending deletion will render a 404 page
- Measure queue duration between gitlab-workhorse and Rails - Measure queue duration between gitlab-workhorse and Rails
......
...@@ -263,7 +263,7 @@ class ApplicationController < ActionController::Base ...@@ -263,7 +263,7 @@ class ApplicationController < ActionController::Base
# internal repos where you are not a member. Enable this filter # internal repos where you are not a member. Enable this filter
# or improve current implementation to filter only issues you # or improve current implementation to filter only issues you
# created or assigned or mentioned # created or assigned or mentioned
#@filter_params[:authorized_only] = true # @filter_params[:authorized_only] = true
end end
@filter_params @filter_params
......
...@@ -205,7 +205,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -205,7 +205,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end end
def branch_from def branch_from
#This is always source # This is always source
@source_project = @merge_request.nil? ? @project : @merge_request.source_project @source_project = @merge_request.nil? ? @project : @merge_request.source_project
@commit = @repository.commit(params[:ref]) if params[:ref].present? @commit = @repository.commit(params[:ref]) if params[:ref].present?
render layout: false render layout: false
......
...@@ -271,7 +271,7 @@ class IssuableFinder ...@@ -271,7 +271,7 @@ class IssuableFinder
if filter_by_no_label? if filter_by_no_label?
items = items.without_label items = items.without_label
else else
items = items.with_label(label_names) items = items.with_label(label_names, params[:sort])
if projects if projects
items = items.where(labels: { project_id: projects }) items = items.where(labels: { project_id: projects })
end end
......
...@@ -126,13 +126,29 @@ module Issuable ...@@ -126,13 +126,29 @@ module Issuable
joins(join_clause).group(issuable_table[:id]).reorder("COUNT(notes.id) DESC") joins(join_clause).group(issuable_table[:id]).reorder("COUNT(notes.id) DESC")
end end
def with_label(title) def with_label(title, sort = nil)
if title.is_a?(Array) && title.size > 1 if title.is_a?(Array) && title.size > 1
joins(:labels).where(labels: { title: title }).group(arel_table[:id]).having("COUNT(DISTINCT labels.title) = #{title.size}") joins(:labels).where(labels: { title: title }).group(*grouping_columns(sort)).having("COUNT(DISTINCT labels.title) = #{title.size}")
else else
joins(:labels).where(labels: { title: title }) joins(:labels).where(labels: { title: title })
end end
end end
# Includes table keys in group by clause when sorting
# preventing errors in postgres
#
# Returns an array of arel columns
def grouping_columns(sort)
grouping_columns = [arel_table[:id]]
if ["milestone_due_desc", "milestone_due_asc"].include?(sort)
milestone_table = Milestone.arel_table
grouping_columns << milestone_table[:id]
grouping_columns << milestone_table[:due_date]
end
grouping_columns
end
end end
def today? def today?
......
...@@ -26,7 +26,7 @@ class Key < ActiveRecord::Base ...@@ -26,7 +26,7 @@ class Key < ActiveRecord::Base
end end
def publishable_key def publishable_key
#Removes anything beyond the keytype and key itself # Removes anything beyond the keytype and key itself
self.key.split[0..1].join(' ') self.key.split[0..1].join(' ')
end end
......
...@@ -52,7 +52,7 @@ Doorkeeper.configure do ...@@ -52,7 +52,7 @@ Doorkeeper.configure do
# For more information go to # For more information go to
# https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes
default_scopes :api default_scopes :api
#optional_scopes :write, :update # optional_scopes :write, :update
# Change the way client credentials are retrieved from the request object. # Change the way client credentials are retrieved from the request object.
# By default it retrieves first from the `HTTP_AUTHORIZATION` header, then # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then
......
...@@ -13,7 +13,7 @@ end ...@@ -13,7 +13,7 @@ end
OmniAuth.config.full_host = Settings.gitlab['base_url'] OmniAuth.config.full_host = Settings.gitlab['base_url']
OmniAuth.config.allowed_request_methods = [:post] OmniAuth.config.allowed_request_methods = [:post]
#In case of auto sign-in, the GET method is used (users don't get to click on a button) # In case of auto sign-in, the GET method is used (users don't get to click on a button)
OmniAuth.config.allowed_request_methods << :get if Gitlab.config.omniauth.auto_sign_in_with_provider.present? OmniAuth.config.allowed_request_methods << :get if Gitlab.config.omniauth.auto_sign_in_with_provider.present?
OmniAuth.config.before_request_phase do |env| OmniAuth.config.before_request_phase do |env|
OmniAuth::RequestForgeryProtection.call(env) OmniAuth::RequestForgeryProtection.call(env)
......
...@@ -17,16 +17,16 @@ module Gitlab ...@@ -17,16 +17,16 @@ module Gitlab
Enumerator.new do |yielder| Enumerator.new do |yielder|
@lines.each do |line| @lines.each do |line|
next if filename?(line) next if filename?(line)
full_line = line.delete("\n") full_line = line.delete("\n")
if line.match(/^@@ -/) if line.match(/^@@ -/)
type = "match" type = "match"
line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0 line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0
line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0 line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0
next if line_old <= 1 && line_new <= 1 #top of file next if line_old <= 1 && line_new <= 1 # top of file
yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new)
line_obj_index += 1 line_obj_index += 1
next next
...@@ -39,8 +39,8 @@ module Gitlab ...@@ -39,8 +39,8 @@ module Gitlab
yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new)
line_obj_index += 1 line_obj_index += 1
end end
case line[0] case line[0]
when "+" when "+"
line_new += 1 line_new += 1
......
...@@ -17,7 +17,7 @@ module Gitlab ...@@ -17,7 +17,7 @@ module Gitlab
def execute def execute
project_identifier = CGI.escape(project.import_source) project_identifier = CGI.escape(project.import_source)
#Issues && Comments # Issues && Comments
issues = client.issues(project_identifier) issues = client.issues(project_identifier)
issues.each do |issue| issues.each do |issue|
......
...@@ -131,6 +131,15 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true ...@@ -131,6 +131,15 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true
expect(first_merge_request).to include('fix') expect(first_merge_request).to include('fix')
expect(count_merge_requests).to eq(1) expect(count_merge_requests).to eq(1)
end end
it 'sorts by recently due milestone' do
visit namespace_project_merge_requests_path(project.namespace, project,
label_name: [label.name, label2.name],
assignee_id: user.id,
sort: sort_value_milestone_soon)
expect(first_merge_request).to include('fix')
end
end end
end end
......
...@@ -118,10 +118,10 @@ describe Issue, "Issuable" do ...@@ -118,10 +118,10 @@ describe Issue, "Issuable" do
let(:project) { build_stubbed(:empty_project) } let(:project) { build_stubbed(:empty_project) }
context "by milestone due date" do context "by milestone due date" do
#Correct order is: # Correct order is:
#Issues/MRs with milestones ordered by date # Issues/MRs with milestones ordered by date
#Issues/MRs with milestones without dates # Issues/MRs with milestones without dates
#Issues/MRs without milestones # Issues/MRs without milestones
let!(:issue) { create(:issue, project: project) } let!(:issue) { create(:issue, project: project) }
let!(:early_milestone) { create(:milestone, project: project, due_date: 10.days.from_now) } let!(:early_milestone) { create(:milestone, project: project, due_date: 10.days.from_now) }
......
...@@ -20,7 +20,7 @@ describe API::API, api: true do ...@@ -20,7 +20,7 @@ describe API::API, api: true do
end end
context "when authenticated" do context "when authenticated" do
#These specs are written just in case API authentication is not required anymore # These specs are written just in case API authentication is not required anymore
context "when public level is restricted" do context "when public level is restricted" do
before do before do
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
......
...@@ -41,11 +41,11 @@ Teaspoon.configure do |config| ...@@ -41,11 +41,11 @@ Teaspoon.configure do |config|
suite.matcher = "{spec/javascripts,app/assets}/**/*_spec.{js,js.coffee,coffee}" suite.matcher = "{spec/javascripts,app/assets}/**/*_spec.{js,js.coffee,coffee}"
# Load additional JS files, but requiring them in your spec helper is the preferred way to do this. # Load additional JS files, but requiring them in your spec helper is the preferred way to do this.
#suite.javascripts = [] # suite.javascripts = []
# You can include your own stylesheets if you want to change how Teaspoon looks. # You can include your own stylesheets if you want to change how Teaspoon looks.
# Note: Spec related CSS can and should be loaded using fixtures. # Note: Spec related CSS can and should be loaded using fixtures.
#suite.stylesheets = ["teaspoon"] # suite.stylesheets = ["teaspoon"]
# This suites spec helper, which can require additional support files. This file is loaded before any of your test # This suites spec helper, which can require additional support files. This file is loaded before any of your test
# files are loaded. # files are loaded.
...@@ -62,19 +62,19 @@ Teaspoon.configure do |config| ...@@ -62,19 +62,19 @@ Teaspoon.configure do |config|
# Hooks allow you to use `Teaspoon.hook("fixtures")` before, after, or during your spec run. This will make a # Hooks allow you to use `Teaspoon.hook("fixtures")` before, after, or during your spec run. This will make a
# synchronous Ajax request to the server that will call all of the blocks you've defined for that hook name. # synchronous Ajax request to the server that will call all of the blocks you've defined for that hook name.
#suite.hook :fixtures, &proc{} # suite.hook :fixtures, &proc{}
# Determine whether specs loaded into the test harness should be embedded as individual script tags or concatenated # Determine whether specs loaded into the test harness should be embedded as individual script tags or concatenated
# into a single file. Similar to Rails' asset `debug: true` and `config.assets.debug = true` options. By default, # into a single file. Similar to Rails' asset `debug: true` and `config.assets.debug = true` options. By default,
# Teaspoon expands all assets to provide more valuable stack traces that reference individual source files. # Teaspoon expands all assets to provide more valuable stack traces that reference individual source files.
#suite.expand_assets = true # suite.expand_assets = true
end end
# Example suite. Since we're just filtering to files already within the root test/javascripts, these files will also # Example suite. Since we're just filtering to files already within the root test/javascripts, these files will also
# be run in the default suite -- but can be focused into a more specific suite. # be run in the default suite -- but can be focused into a more specific suite.
#config.suite :targeted do |suite| # config.suite :targeted do |suite|
# suite.matcher = "spec/javascripts/targeted/*_spec.{js,js.coffee,coffee}" # suite.matcher = "spec/javascripts/targeted/*_spec.{js,js.coffee,coffee}"
#end # end
# CONSOLE RUNNER SPECIFIC # CONSOLE RUNNER SPECIFIC
# #
...@@ -94,45 +94,45 @@ Teaspoon.configure do |config| ...@@ -94,45 +94,45 @@ Teaspoon.configure do |config|
# PhantomJS: https://github.com/modeset/teaspoon/wiki/Using-PhantomJS # PhantomJS: https://github.com/modeset/teaspoon/wiki/Using-PhantomJS
# Selenium Webdriver: https://github.com/modeset/teaspoon/wiki/Using-Selenium-WebDriver # Selenium Webdriver: https://github.com/modeset/teaspoon/wiki/Using-Selenium-WebDriver
# Capybara Webkit: https://github.com/modeset/teaspoon/wiki/Using-Capybara-Webkit # Capybara Webkit: https://github.com/modeset/teaspoon/wiki/Using-Capybara-Webkit
#config.driver = :phantomjs # config.driver = :phantomjs
# Specify additional options for the driver. # Specify additional options for the driver.
# #
# PhantomJS: https://github.com/modeset/teaspoon/wiki/Using-PhantomJS # PhantomJS: https://github.com/modeset/teaspoon/wiki/Using-PhantomJS
# Selenium Webdriver: https://github.com/modeset/teaspoon/wiki/Using-Selenium-WebDriver # Selenium Webdriver: https://github.com/modeset/teaspoon/wiki/Using-Selenium-WebDriver
# Capybara Webkit: https://github.com/modeset/teaspoon/wiki/Using-Capybara-Webkit # Capybara Webkit: https://github.com/modeset/teaspoon/wiki/Using-Capybara-Webkit
#config.driver_options = nil # config.driver_options = nil
# Specify the timeout for the driver. Specs are expected to complete within this time frame or the run will be # Specify the timeout for the driver. Specs are expected to complete within this time frame or the run will be
# considered a failure. This is to avoid issues that can arise where tests stall. # considered a failure. This is to avoid issues that can arise where tests stall.
#config.driver_timeout = 180 # config.driver_timeout = 180
# Specify a server to use with Rack (e.g. thin, mongrel). If nil is provided Rack::Server is used. # Specify a server to use with Rack (e.g. thin, mongrel). If nil is provided Rack::Server is used.
#config.server = nil # config.server = nil
# Specify a port to run on a specific port, otherwise Teaspoon will use a random available port. # Specify a port to run on a specific port, otherwise Teaspoon will use a random available port.
#config.server_port = nil # config.server_port = nil
# Timeout for starting the server in seconds. If your server is slow to start you may have to bump this, or you may # Timeout for starting the server in seconds. If your server is slow to start you may have to bump this, or you may
# want to lower this if you know it shouldn't take long to start. # want to lower this if you know it shouldn't take long to start.
#config.server_timeout = 20 # config.server_timeout = 20
# Force Teaspoon to fail immediately after a failing suite. Can be useful to make Teaspoon fail early if you have # Force Teaspoon to fail immediately after a failing suite. Can be useful to make Teaspoon fail early if you have
# several suites, but in environments like CI this may not be desirable. # several suites, but in environments like CI this may not be desirable.
#config.fail_fast = true # config.fail_fast = true
# Specify the formatters to use when outputting the results. # Specify the formatters to use when outputting the results.
# Note: Output files can be specified by using `"junit>/path/to/output.xml"`. # Note: Output files can be specified by using `"junit>/path/to/output.xml"`.
# #
# Available: :dot, :clean, :documentation, :json, :junit, :pride, :rspec_html, :snowday, :swayze_or_oprah, :tap, :tap_y, :teamcity # Available: :dot, :clean, :documentation, :json, :junit, :pride, :rspec_html, :snowday, :swayze_or_oprah, :tap, :tap_y, :teamcity
#config.formatters = [:dot] # config.formatters = [:dot]
# Specify if you want color output from the formatters. # Specify if you want color output from the formatters.
#config.color = true # config.color = true
# Teaspoon pipes all console[log/debug/error] to $stdout. This is useful to catch places where you've forgotten to # Teaspoon pipes all console[log/debug/error] to $stdout. This is useful to catch places where you've forgotten to
# remove them, but in verbose applications this may not be desirable. # remove them, but in verbose applications this may not be desirable.
#config.suppress_log = false # config.suppress_log = false
# COVERAGE REPORTS / THRESHOLD ASSERTIONS # COVERAGE REPORTS / THRESHOLD ASSERTIONS
# #
...@@ -149,7 +149,7 @@ Teaspoon.configure do |config| ...@@ -149,7 +149,7 @@ Teaspoon.configure do |config|
# Specify that you always want a coverage configuration to be used. Otherwise, specify that you want coverage # Specify that you always want a coverage configuration to be used. Otherwise, specify that you want coverage
# on the CLI. # on the CLI.
# Set this to "true" or the name of your coverage config. # Set this to "true" or the name of your coverage config.
#config.use_coverage = nil # config.use_coverage = nil
# You can have multiple coverage configs by passing a name to config.coverage. # You can have multiple coverage configs by passing a name to config.coverage.
# e.g. config.coverage :ci do |coverage| # e.g. config.coverage :ci do |coverage|
...@@ -158,21 +158,21 @@ Teaspoon.configure do |config| ...@@ -158,21 +158,21 @@ Teaspoon.configure do |config|
# Which coverage reports Istanbul should generate. Correlates directly to what Istanbul supports. # Which coverage reports Istanbul should generate. Correlates directly to what Istanbul supports.
# #
# Available: text-summary, text, html, lcov, lcovonly, cobertura, teamcity # Available: text-summary, text, html, lcov, lcovonly, cobertura, teamcity
#coverage.reports = ["text-summary", "html"] # coverage.reports = ["text-summary", "html"]
# The path that the coverage should be written to - when there's an artifact to write to disk. # The path that the coverage should be written to - when there's an artifact to write to disk.
# Note: Relative to `config.root`. # Note: Relative to `config.root`.
#coverage.output_path = "coverage" # coverage.output_path = "coverage"
# Assets to be ignored when generating coverage reports. Accepts an array of filenames or regular expressions. The # Assets to be ignored when generating coverage reports. Accepts an array of filenames or regular expressions. The
# default excludes assets from vendor, gems and support libraries. # default excludes assets from vendor, gems and support libraries.
#coverage.ignore = [%r{/lib/ruby/gems/}, %r{/vendor/assets/}, %r{/support/}, %r{/(.+)_helper.}] # coverage.ignore = [%r{/lib/ruby/gems/}, %r{/vendor/assets/}, %r{/support/}, %r{/(.+)_helper.}]
# Various thresholds requirements can be defined, and those thresholds will be checked at the end of a run. If any # Various thresholds requirements can be defined, and those thresholds will be checked at the end of a run. If any
# aren't met the run will fail with a message. Thresholds can be defined as a percentage (0-100), or nil. # aren't met the run will fail with a message. Thresholds can be defined as a percentage (0-100), or nil.
#coverage.statements = nil # coverage.statements = nil
#coverage.functions = nil # coverage.functions = nil
#coverage.branches = nil # coverage.branches = nil
#coverage.lines = nil # coverage.lines = nil
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment