Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/manual_ordering.js

@@ -21,7 +21,7 @@ const updateIssue = (url, issueList, { move_before_id, move_after_id }) =>
 const initManualOrdering = () => {
   const issueList = document.querySelector('.manual-ordering');
 
-  if (!issueList || !(gon.features && gon.features.manualSorting) || !(gon.current_user_id > 0)) {
+  if (!issueList || !(gon.current_user_id > 0)) {
     return;
   }
 

app/controllers/groups_controller.rb

@@ -7,10 +7,6 @@ class GroupsController < Groups::ApplicationController
   include PreviewMarkdown
   include RecordUserLastActivity
 
-  before_action do
-    push_frontend_feature_flag(:manual_sorting, default_enabled: true)
-  end
-
   respond_to :html
 
   prepend_before_action(only: [:show, :issues]) { authenticate_sessionless_user!(:rss) }

app/controllers/projects/issues_controller.rb

@@ -10,10 +10,6 @@ class Projects::IssuesController < Projects::ApplicationController
   include SpammableActions
   include RecordUserLastActivity
 
-  before_action do
-    push_frontend_feature_flag(:manual_sorting, default_enabled: true)
-  end
-
   def issue_except_actions
     %i[index calendar new create bulk_update import_csv]
   end

app/models/environment.rb

@@ -204,7 +204,7 @@ class Environment < ApplicationRecord
     public_path = project.public_path_for_source_path(path, commit_sha)
     return unless public_path
 
-    [external_url, public_path].join('/')
+    [external_url.delete_suffix('/'), public_path.delete_prefix('/')].join('/')
   end
 
   def expire_etag_cache

app/views/shared/issuable/_sort_dropdown.html.haml

 - sort_value = @sort
 - sort_title = issuable_sort_option_title(sort_value)
 - viewing_issues = controller.controller_name == 'issues' || controller.action_name == 'issues'
-- manual_sorting = viewing_issues && controller.controller_name != 'dashboard' && Feature.enabled?(:manual_sorting, default_enabled: true)
+- manual_sorting = viewing_issues && controller.controller_name != 'dashboard'
 
 .dropdown.inline.prepend-left-10.issue-sort-dropdown
   .btn-group{ role: 'group' }

changelogs/unreleased/double-slash-64592.yml

+---
+title: Prevent double slash in review apps path
+merge_request: 31212
+author:
+type: fixed

spec/models/environment_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe Environment, :use_clean_rails_memory_store_caching do
   include ReactiveCachingHelpers
+  using RSpec::Parameterized::TableSyntax
 
   let(:project) { create(:project, :stubbed_repository) }
   subject(:environment) { create(:environment, project: project) }
@@ -782,12 +783,9 @@ describe Environment, :use_clean_rails_memory_store_caching do
     let(:source_path) { 'source/file.html' }
     let(:sha) { RepoHelpers.sample_commit.id }
 
-    before do
-      environment.external_url = 'http://example.com'
-    end
-
     context 'when the public path is not known' do
       before do
+        environment.external_url = 'http://example.com'
         allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return(nil)
       end
 
@@ -797,12 +795,23 @@ describe Environment, :use_clean_rails_memory_store_caching do
     end
 
     context 'when the public path is known' do
-      before do
-        allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return('file.html')
-      end
-
-      it 'returns the full external URL' do
-        expect(environment.external_url_for(source_path, sha)).to eq('http://example.com/file.html')
+      where(:external_url, :public_path, :full_url) do
+        'http://example.com'          | 'file.html'         | 'http://example.com/file.html'
+        'http://example.com/'         | 'file.html'         | 'http://example.com/file.html'
+        'http://example.com'          | '/file.html'        | 'http://example.com/file.html'
+        'http://example.com/'         | '/file.html'        | 'http://example.com/file.html'
+        'http://example.com/subpath'  | 'public/file.html'  | 'http://example.com/subpath/public/file.html'
+        'http://example.com/subpath/' | 'public/file.html'  | 'http://example.com/subpath/public/file.html'
+        'http://example.com/subpath'  | '/public/file.html' | 'http://example.com/subpath/public/file.html'
+        'http://example.com/subpath/' | '/public/file.html' | 'http://example.com/subpath/public/file.html'
+      end
+      with_them do
+        it 'returns the full external URL' do
+          environment.external_url = external_url
+          allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return(public_path)
+
+          expect(environment.external_url_for(source_path, sha)).to eq(full_url)
+        end
       end
     end
   end

spec/models/project_spec.rb

@@ -2950,6 +2950,16 @@ describe Project do
           expect(project.public_path_for_source_path('file.html', sha)).to be_nil
         end
       end
+
+      it 'returns a public path with a leading slash unmodified' do
+        route_map = Gitlab::RouteMap.new(<<-MAP.strip_heredoc)
+          - source: 'source/file.html'
+            public: '/public/file'
+        MAP
+        allow(project).to receive(:route_map_for).with(sha).and_return(route_map)
+
+        expect(project.public_path_for_source_path('source/file.html', sha)).to eq('/public/file')
+      end
     end
 
     context 'when there is no route map' do