17 files changed, 276 insertions, 45 deletions

diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js
index 30902767705..0344ce9ffb4 100644
--- a/app/assets/javascripts/users_select.js
+++ b/app/assets/javascripts/users_select.js
@@ -33,6 +33,7 @@
           var $block, $collapsedSidebar, $dropdown, $loading, $selectbox, $value, abilityName, assignTo, assigneeTemplate, collapsedAssigneeTemplate, defaultLabel, firstUser, issueURL, selectedId, showAnyUser, showNullUser, showMenuAbove;
           $dropdown = $(dropdown);
           options.projectId = $dropdown.data('project-id');
+          options.groupId = $dropdown.data('group-id');
           options.showCurrentUser = $dropdown.data('current-user');
           options.todoFilter = $dropdown.data('todo-filter');
           options.todoStateFilter = $dropdown.data('todo-state-filter');
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index 38be073c8dc..e347f61fb8d 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -56,11 +56,12 @@ module MergeRequestsHelper
   end
 
   def issues_sentence(issues)
-    # Sorting based on the `#123` or `group/project#123` reference will sort
-    # local issues first.
-    issues.map do |issue|
+    # Issuable sorter will sort local issues, then issues from the same
+    # namespace, then all other issues.
+    issues = Gitlab::IssuableSorter.sort(@project, issues).map do |issue|
       issue.to_reference(@project)
-    end.sort.to_sentence
+    end
+    issues.to_sentence
   end
 
   def mr_closes_issues
diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb
index d2a1c161026..363135ef09b 100644
--- a/app/services/users/build_service.rb
+++ b/app/services/users/build_service.rb
@@ -9,15 +9,13 @@ module Users
     def execute(skip_authorization: false)
       raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_create_user?
 
-      user = User.new(build_user_params)
+      user_params = build_user_params(skip_authorization: skip_authorization)
+      user = User.new(user_params)
 
       if current_user&.admin?
-        if params[:reset_password]
-          user.generate_reset_token
-          params[:force_random_password] = true
-        end
+        @reset_token = user.generate_reset_token if params[:reset_password]
 
-        if params[:force_random_password]
+        if user_params[:force_random_password]
           random_password = Devise.friendly_token.first(Devise.password_length.min)
           user.password = user.password_confirmation = random_password
         end
@@ -81,7 +79,7 @@ module Users
       ]
     end
 
-    def build_user_params
+    def build_user_params(skip_authorization:)
       if current_user&.admin?
         user_params = params.slice(*admin_create_params)
         user_params[:created_by_id] = current_user&.id
@@ -90,11 +88,20 @@ module Users
           user_params.merge!(force_random_password: true, password_expires_at: nil)
         end
       else
-        user_params = params.slice(*signup_params)
-        user_params[:skip_confirmation] = !current_application_settings.send_user_confirmation_email
+        allowed_signup_params = signup_params
+        allowed_signup_params << :skip_confirmation if skip_authorization
+
+        user_params = params.slice(*allowed_signup_params)
+        if user_params[:skip_confirmation].nil?
+          user_params[:skip_confirmation] = skip_user_confirmation_email_from_setting
+        end
       end
 
       user_params
     end
+
+    def skip_user_confirmation_email_from_setting
+      !current_application_settings.send_user_confirmation_email
+    end
   end
 end
diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml
index c72268473ca..1a12f110945 100644
--- a/app/views/shared/issuable/_filter.html.haml
+++ b/app/views/shared/issuable/_filter.html.haml
@@ -21,7 +21,7 @@
           - if params[:assignee_id].present?
             = hidden_field_tag(:assignee_id, params[:assignee_id])
           = dropdown_tag(user_dropdown_label(params[:assignee_id], "Assignee"), options: { toggle_class: "js-user-search js-filter-submit js-assignee-search", title: "Filter by assignee", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-assignee js-filter-submit",
-            placeholder: "Search assignee", data: { any_user: "Any Assignee", first_user: current_user.try(:username), null_user: true, current_user: true, project_id: @project.try(:id), selected: params[:assignee_id], field_name: "assignee_id", default_label: "Assignee" } })
+            placeholder: "Search assignee", data: { any_user: "Any Assignee", first_user: current_user.try(:username), null_user: true, current_user: true, project_id: @project.try(:id), group_id: @group&.id, selected: params[:assignee_id], field_name: "assignee_id", default_label: "Assignee" } })
 
         .filter-item.inline.milestone-filter
           = render "shared/issuable/milestone_dropdown", selected: finder.milestones.try(:first), name: :milestone_title, show_any: true, show_upcoming: true, show_started: true
diff --git a/changelogs/unreleased/20378-natural-sort-issue-numbers.yml b/changelogs/unreleased/20378-natural-sort-issue-numbers.yml
new file mode 100644
index 00000000000..2ebc8485ddf
--- /dev/null
+++ b/changelogs/unreleased/20378-natural-sort-issue-numbers.yml
@@ -0,0 +1,4 @@
+---
+title: Change issues list in MR to natural sorting
+merge_request: 7110
+author: Jeff Stubler
diff --git a/changelogs/unreleased/31280-fix-showing-issues-from-pending-delete-projects.yml b/changelogs/unreleased/31280-fix-showing-issues-from-pending-delete-projects.yml
new file mode 100644
index 00000000000..410480de573
--- /dev/null
+++ b/changelogs/unreleased/31280-fix-showing-issues-from-pending-delete-projects.yml
@@ -0,0 +1,4 @@
+---
+title: Fix 500 error due to trying to show issues from pending deleting projects
+merge_request: 10906
+author:
diff --git a/changelogs/unreleased/group-assignee-dropdown-send-group-id.yml b/changelogs/unreleased/group-assignee-dropdown-send-group-id.yml
new file mode 100644
index 00000000000..4f153f9817d
--- /dev/null
+++ b/changelogs/unreleased/group-assignee-dropdown-send-group-id.yml
@@ -0,0 +1,4 @@
+---
+title: Fixed group issues assignee dropdown loading all users
+merge_request:
+author:
diff --git a/lib/banzai/issuable_extractor.rb b/lib/banzai/issuable_extractor.rb
index c5ce360e172..cbabf9156de 100644
--- a/lib/banzai/issuable_extractor.rb
+++ b/lib/banzai/issuable_extractor.rb
@@ -28,9 +28,13 @@ module Banzai
       issue_parser = Banzai::ReferenceParser::IssueParser.new(project, user)
       merge_request_parser = Banzai::ReferenceParser::MergeRequestParser.new(project, user)
 
-      issue_parser.issues_for_nodes(nodes).merge(
+      issuables_for_nodes = issue_parser.issues_for_nodes(nodes).merge(
         merge_request_parser.merge_requests_for_nodes(nodes)
       )
+
+      # The project for the issue/MR might be pending for deletion!
+      # Filter them out because we don't care about them.
+      issuables_for_nodes.select { |node, issuable| issuable.project }
     end
   end
 end
diff --git a/lib/gitlab/issuable_sorter.rb b/lib/gitlab/issuable_sorter.rb
new file mode 100644
index 00000000000..d392214867a
--- /dev/null
+++ b/lib/gitlab/issuable_sorter.rb
@@ -0,0 +1,29 @@
+module Gitlab
+  module IssuableSorter
+    class << self
+      def sort(project, issuables, &sort_key)
+        grouped_items = issuables.group_by do |issuable|
+          if issuable.project.id == project.id
+            :project_ref
+          elsif issuable.project.namespace.id == project.namespace.id
+            :namespace_ref
+          else
+            :full_ref
+          end
+        end
+
+        natural_sort_issuables(grouped_items[:project_ref], project) +
+          natural_sort_issuables(grouped_items[:namespace_ref], project) +
+          natural_sort_issuables(grouped_items[:full_ref], project)
+      end
+
+      private
+
+      def natural_sort_issuables(issuables, project)
+        VersionSorter.sort(issuables || []) do |issuable|
+          issuable.to_reference(project)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb
index 0b6977e3b17..f1fd1fd7f73 100644
--- a/spec/factories/issues.rb
+++ b/spec/factories/issues.rb
@@ -8,6 +8,10 @@ FactoryGirl.define do
       confidential true
     end
 
+    trait :opened do
+      state :opened
+    end
+
     trait :closed do
       state :closed
     end
diff --git a/spec/features/groups/issues_spec.rb b/spec/features/groups/issues_spec.rb
index 1b3747c390b..45f57845c74 100644
--- a/spec/features/groups/issues_spec.rb
+++ b/spec/features/groups/issues_spec.rb
@@ -23,4 +23,20 @@ feature 'Group issues page', feature: true do
       it_behaves_like "an autodiscoverable RSS feed without a private token"
     end
   end
+
+  context 'assignee', :js do
+    let(:access_level) { ProjectFeature::ENABLED }
+    let(:user) { user_in_group }
+    let(:user2) { user_outside_group }
+    let(:path) { issues_group_path(group) }
+
+    it 'filters by only group users' do
+      click_button('Assignee')
+
+      wait_for_ajax
+
+      expect(find('.dropdown-menu-assignee')).to have_link(user.name)
+      expect(find('.dropdown-menu-assignee')).not_to have_link(user2.name)
+    end
+  end
 end
diff --git a/spec/helpers/merge_requests_helper_spec.rb b/spec/helpers/merge_requests_helper_spec.rb
index 25f23826648..e9037749ef2 100644
--- a/spec/helpers/merge_requests_helper_spec.rb
+++ b/spec/helpers/merge_requests_helper_spec.rb
@@ -22,24 +22,51 @@ describe MergeRequestsHelper do
   end
 
   describe '#issues_sentence' do
+    let(:project) { create :project }
+
     subject { issues_sentence(issues) }
     let(:issues) do
-      [build(:issue, iid: 1), build(:issue, iid: 2), build(:issue, iid: 3)]
+      [build(:issue, iid: 2, project: project),
+       build(:issue, iid: 3, project: project),
+       build(:issue, iid: 1, project: project)]
     end
 
-    it { is_expected.to eq('#1, #2, and #3') }
+    it do
+      @project = project
+
+      is_expected.to eq('#1, #2, and #3')
+    end
 
     context 'for JIRA issues' do
       let(:project) { create(:empty_project) }
       let(:issues) do
         [
-          ExternalIssue.new('JIRA-123', project),
           ExternalIssue.new('JIRA-456', project),
-          ExternalIssue.new('FOOBAR-7890', project)
+          ExternalIssue.new('FOOBAR-7890', project),
+          ExternalIssue.new('JIRA-123', project)
         ]
       end
 
-      it { is_expected.to eq('FOOBAR-7890, JIRA-123, and JIRA-456') }
+      it do
+        @project = project
+        is_expected.to eq('FOOBAR-7890, JIRA-123, and JIRA-456')
+      end
+    end
+
+    context 'for issues from multiple namespaces' do
+      let(:project) { create(:project) }
+      let(:other_project) { create(:project) }
+      let(:issues) do
+        [build(:issue, iid: 2, project: project),
+         build(:issue, iid: 3, project: other_project),
+         build(:issue, iid: 1, project: project)]
+      end
+
+      it do
+        @project = project
+        
+        is_expected.to eq("#1, #2, and #{other_project.namespace.path}/#{other_project.path}#3")
+      end
     end
   end
 
diff --git a/spec/lib/banzai/filter/issuable_state_filter_spec.rb b/spec/lib/banzai/filter/issuable_state_filter_spec.rb
index 600f3c123ed..9c2399815b9 100644
--- a/spec/lib/banzai/filter/issuable_state_filter_spec.rb
+++ b/spec/lib/banzai/filter/issuable_state_filter_spec.rb
@@ -6,11 +6,23 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do
 
   let(:user) { create(:user) }
   let(:context) { { current_user: user, issuable_state_filter_enabled: true } }
+  let(:closed_issue) { create_issue(:closed) }
+  let(:project) { create(:empty_project, :public) }
+  let(:other_project) { create(:empty_project, :public) }
 
   def create_link(text, data)
     link_to(text, '', class: 'gfm has-tooltip', data: data)
   end
 
+  def create_issue(state)
+    create(:issue, state, project: project)
+  end
+
+  def create_merge_request(state)
+    create(:merge_request, state,
+      source_project: project, target_project: project)
+  end
+
   it 'ignores non-GFM links' do
     html = %(See <a href="https://google.com/">Google</a>)
     doc = filter(html, current_user: user)
@@ -19,7 +31,6 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do
   end
 
   it 'ignores non-issuable links' do
-    project = create(:empty_project, :public)
     link = create_link('text', project: project, reference_type: 'issue')
     doc = filter(link, context)
 
@@ -27,27 +38,24 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do
   end
 
   it 'ignores issuable links with empty content' do
-    issue = create(:issue, :closed)
-    link = create_link('', issue: issue.id, reference_type: 'issue')
+    link = create_link('', issue: closed_issue.id, reference_type: 'issue')
     doc = filter(link, context)
 
     expect(doc.css('a').last.text).to eq('')
   end
 
   it 'ignores issuable links with custom anchor' do
-    issue = create(:issue, :closed)
-    link = create_link('something', issue: issue.id, reference_type: 'issue')
+    link = create_link('something', issue: closed_issue.id, reference_type: 'issue')
     doc = filter(link, context)
 
     expect(doc.css('a').last.text).to eq('something')
   end
 
   it 'ignores issuable links to specific comments' do
-    issue = create(:issue, :closed)
-    link = create_link("#{issue.to_reference} (comment 1)", issue: issue.id, reference_type: 'issue')
+    link = create_link("#{closed_issue.to_reference} (comment 1)", issue: closed_issue.id, reference_type: 'issue')
     doc = filter(link, context)
 
-    expect(doc.css('a').last.text).to eq("#{issue.to_reference} (comment 1)")
+    expect(doc.css('a').last.text).to eq("#{closed_issue.to_reference} (comment 1)")
   end
 
   it 'ignores merge request links to diffs tab' do
@@ -63,26 +71,36 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do
   end
 
   it 'handles cross project references' do
-    issue = create(:issue, :closed)
-    project = create(:empty_project)
-    link = create_link(issue.to_reference(project), issue: issue.id, reference_type: 'issue')
-    doc = filter(link, context.merge(project: project))
+    link = create_link(closed_issue.to_reference(other_project), issue: closed_issue.id, reference_type: 'issue')
+    doc = filter(link, context.merge(project: other_project))
 
-    expect(doc.css('a').last.text).to eq("#{issue.to_reference(project)} (closed)")
+    expect(doc.css('a').last.text).to eq("#{closed_issue.to_reference(other_project)} (closed)")
   end
 
   it 'does not append state when filter is not enabled' do
-    issue = create(:issue, :closed)
-    link = create_link('text', issue: issue.id, reference_type: 'issue')
+    link = create_link('text', issue: closed_issue.id, reference_type: 'issue')
     context = { current_user: user }
     doc = filter(link, context)
 
     expect(doc.css('a').last.text).to eq('text')
   end
 
+  context 'when project is in pending delete' do
+    before do
+      project.update!(pending_delete: true)
+    end
+
+    it 'does not append issue state' do
+      link = create_link('text', issue: closed_issue.id, reference_type: 'issue')
+      doc = filter(link, context)
+
+      expect(doc.css('a').last.text).to eq('text')
+    end
+  end
+
   context 'for issue references' do
     it 'ignores open issue references' do
-      issue = create(:issue)
+      issue = create_issue(:opened)
       link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue')
       doc = filter(link, context)
 
@@ -90,7 +108,7 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do
     end
 
     it 'ignores reopened issue references' do
-      issue = create(:issue, :reopened)
+      issue = create_issue(:reopened)
       link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue')
       doc = filter(link, context)
 
@@ -98,70 +116,79 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do
     end
 
     it 'appends state to closed issue references' do
-      issue = create(:issue, :closed)
-      link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue')
+      link = create_link(closed_issue.to_reference, issue: closed_issue.id, reference_type: 'issue')
       doc = filter(link, context)
 
-      expect(doc.css('a').last.text).to eq("#{issue.to_reference} (closed)")
+      expect(doc.css('a').last.text).to eq("#{closed_issue.to_reference} (closed)")
     end
   end
 
   context 'for merge request references' do
     it 'ignores open merge request references' do
-      merge_request = create(:merge_request)
+      merge_request = create_merge_request(:opened)
+
       link = create_link(
         merge_request.to_reference,
         merge_request: merge_request.id,
         reference_type: 'merge_request'
       )
+
       doc = filter(link, context)
 
       expect(doc.css('a').last.text).to eq(merge_request.to_reference)
     end
 
     it 'ignores reopened merge request references' do
-      merge_request = create(:merge_request, :reopened)
+      merge_request = create_merge_request(:reopened)
+
       link = create_link(
         merge_request.to_reference,
         merge_request: merge_request.id,
         reference_type: 'merge_request'
       )
+
       doc = filter(link, context)
 
       expect(doc.css('a').last.text).to eq(merge_request.to_reference)
     end
 
     it 'ignores locked merge request references' do
-      merge_request = create(:merge_request, :locked)
+      merge_request = create_merge_request(:locked)
+
       link = create_link(
         merge_request.to_reference,
         merge_request: merge_request.id,
         reference_type: 'merge_request'
       )
+
       doc = filter(link, context)
 
       expect(doc.css('a').last.text).to eq(merge_request.to_reference)
     end
 
     it 'appends state to closed merge request references' do
-      merge_request = create(:merge_request, :closed)
+      merge_request = create_merge_request(:closed)
+
       link = create_link(
         merge_request.to_reference,
         merge_request: merge_request.id,
         reference_type: 'merge_request'
       )
+
       doc = filter(link, context)
 
       expect(doc.css('a').last.text).to eq("#{merge_request.to_reference} (closed)")
     end
 
     it 'appends state to merged merge request references' do
-      merge_request = create(:merge_request, :merged)
+      merge_request = create_merge_request(:merged)
+
       link = create_link(
         merge_request.to_reference,
         merge_request: merge_request.id,
         reference_type: 'merge_request'
       )
+
       doc = filter(link, context)
 
       expect(doc.css('a').last.text).to eq("#{merge_request.to_reference} (merged)")
diff --git a/spec/lib/gitlab/issuable_sorter_spec.rb b/spec/lib/gitlab/issuable_sorter_spec.rb
new file mode 100644
index 00000000000..c9a434b2bcf
--- /dev/null
+++ b/spec/lib/gitlab/issuable_sorter_spec.rb
@@ -0,0 +1,62 @@
+require 'spec_helper'
+
+describe Gitlab::IssuableSorter, lib: true do
+  let(:namespace1) { build(:namespace, id: 1) }
+  let(:project1) { build(:project, id: 1, namespace: namespace1) }
+
+  let(:project2) { build(:project, id: 2, path: "a", namespace: project1.namespace) }
+  let(:project3) { build(:project, id: 3, path: "b", namespace: project1.namespace) }
+
+  let(:namespace2) { build(:namespace, id: 2, path: "a") }
+  let(:namespace3) { build(:namespace, id: 3, path: "b") }
+  let(:project4) { build(:project, id: 4, path: "a", namespace: namespace2) }
+  let(:project5) { build(:project, id: 5, path: "b", namespace: namespace2) }
+  let(:project6) { build(:project, id: 6, path: "a", namespace: namespace3) }
+
+  let(:unsorted) { [sorted[2], sorted[3], sorted[0], sorted[1]] }
+
+  let(:sorted) do
+    [build(:issue, iid: 1, project: project1),
+     build(:issue, iid: 2, project: project1),
+     build(:issue, iid: 10, project: project1),
+     build(:issue, iid: 20, project: project1)]
+  end
+
+  it 'sorts references by a given key' do
+    expect(described_class.sort(project1, unsorted)).to eq(sorted)
+  end
+
+  context 'for JIRA issues' do
+    let(:sorted) do
+      [ExternalIssue.new('JIRA-1', project1),
+       ExternalIssue.new('JIRA-2', project1),
+       ExternalIssue.new('JIRA-10', project1),
+       ExternalIssue.new('JIRA-20', project1)]
+    end
+
+    it 'sorts references by a given key' do
+      expect(described_class.sort(project1, unsorted)).to eq(sorted)
+    end
+  end
+
+  context 'for references from multiple projects and namespaces' do
+    let(:sorted) do
+      [build(:issue, iid: 1, project: project1),
+       build(:issue, iid: 2, project: project1),
+       build(:issue, iid: 10, project: project1),
+       build(:issue, iid: 1, project: project2),
+       build(:issue, iid: 1, project: project3),
+       build(:issue, iid: 1, project: project4),
+       build(:issue, iid: 1, project: project5),
+       build(:issue, iid: 1, project: project6)]
+    end
+    let(:unsorted) do
+      [sorted[3], sorted[1], sorted[4], sorted[2],
+       sorted[6], sorted[5], sorted[0], sorted[7]]
+    end
+
+    it 'sorts references by project and then by a given key' do
+      expect(subject.sort(project1, unsorted)).to eq(sorted)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb
index 65a304d1468..f4aab429931 100644
--- a/spec/lib/gitlab/ldap/user_spec.rb
+++ b/spec/lib/gitlab/ldap/user_spec.rb
@@ -120,6 +120,19 @@ describe Gitlab::LDAP::User, lib: true do
         expect(gl_user).to be_persisted
       end
     end
+
+    context 'when user confirmation email is enabled' do
+      before do
+        stub_application_setting send_user_confirmation_email: true
+      end
+
+      it 'creates and confirms the user anyway' do
+        ldap_user.save
+
+        expect(gl_user).to be_persisted
+        expect(gl_user).to be_confirmed
+      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 6d3ac62d9e9..828c953197d 100644
--- a/spec/lib/gitlab/o_auth/user_spec.rb
+++ b/spec/lib/gitlab/o_auth/user_spec.rb
@@ -54,6 +54,21 @@ describe Gitlab::OAuth::User, lib: true do
         end
       end
 
+      context 'when user confirmation email is enabled' do
+        before do
+          stub_application_setting send_user_confirmation_email: true
+        end
+
+        it 'creates and confirms the user anyway' do
+          stub_omniauth_config(allow_single_sign_on: ['twitter'])
+
+          oauth_user.save
+
+          expect(gl_user).to be_persisted
+          expect(gl_user).to be_confirmed
+        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 b3b76a6d629..b106d156b75 100644
--- a/spec/lib/gitlab/saml/user_spec.rb
+++ b/spec/lib/gitlab/saml/user_spec.rb
@@ -223,6 +223,19 @@ describe Gitlab::Saml::User, lib: true do
           expect(gl_user).to be_persisted
         end
       end
+
+      context 'when user confirmation email is enabled' do
+        before do
+          stub_application_setting send_user_confirmation_email: true
+        end
+
+        it 'creates and confirms the user anyway' do
+          saml_user.save
+
+          expect(gl_user).to be_persisted
+          expect(gl_user).to be_confirmed
+        end
+      end
     end
 
     describe 'blocking' do