Resolve discussions

app/controllers/users_controller.rb

@@ -3,7 +3,6 @@ class UsersController < ApplicationController
 
   skip_before_action :authenticate_user!
   before_action :user, except: [:exists]
-  before_action :authorize_read_user!, except: [:exists]
 
   def show
     respond_to do |format|
@@ -93,14 +92,17 @@ class UsersController < ApplicationController
 
   private
 
-  def authorize_read_user!
-    render_404 unless can?(current_user, :read_user, user)
+  def user
+    return @user if @user
 
-    ensure_canonical_path(user.namespace, params[:username])
-  end
+    @user = User.find_by_full_path(params[:username], follow_redirects: true)
 
-  def user
-    @user ||= User.find_by_full_path(params[:username], follow_redirects: true)
+    return render_404 unless @user
+    return render_404 unless can?(current_user, :read_user, @user)
+
+    ensure_canonical_path(@user.namespace, params[:username])
+
+    @user
   end
 
   def contributed_projects

app/models/route.rb

@@ -16,7 +16,8 @@ class Route < ActiveRecord::Base
   scope :direct_descendant_routes, -> (path) { where('routes.path LIKE ? AND routes.path NOT LIKE ?', "#{sanitize_sql_like(path)}/%", "#{sanitize_sql_like(path)}/%/%") }
 
   def rename_direct_descendant_routes
-    if path_changed? || name_changed?
+    return if !path_changed? && !name_changed?
+
     direct_descendant_routes = self.class.direct_descendant_routes(path_was)
 
     direct_descendant_routes.each do |route|
@@ -33,7 +34,6 @@ class Route < ActiveRecord::Base
       route.update(attributes) unless attributes.empty?
     end
   end
-  end
 
   def delete_conflicting_redirects
     conflicting_redirects.delete_all

app/models/user.rb

@@ -335,7 +335,7 @@ class User < ActiveRecord::Base
 
     def find_by_full_path(path, follow_redirects: false)
       namespace = Namespace.find_by_full_path(path, follow_redirects: follow_redirects)
-      namespace.owner if namespace && namespace.owner
+      namespace&.owner
     end
 
     def reference_prefix

db/migrate/20170503184421_add_index_to_redirect_routes.rb

 # See http://doc.gitlab.com/ce/development/migration_style_guide.html
 # for more information on how to write migrations for GitLab.
 
-# rubocop:disable RemoveIndex
 class AddIndexToRedirectRoutes < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
@@ -16,7 +15,7 @@ class AddIndexToRedirectRoutes < ActiveRecord::Migration
   end
 
   def down
-    remove_index(:redirect_routes, :path) if index_exists?(:redirect_routes, :path)
-    remove_index(:redirect_routes, [:source_type, :source_id]) if index_exists?(:redirect_routes, [:source_type, :source_id])
+    remove_concurrent_index(:redirect_routes, :path) if index_exists?(:redirect_routes, :path)
+    remove_concurrent_index(:redirect_routes, [:source_type, :source_id]) if index_exists?(:redirect_routes, [:source_type, :source_id])
   end
 end

spec/controllers/users_controller_spec.rb

@@ -84,6 +84,24 @@ describe UsersController do
         expect(response).to redirect_to(user)
       end
     end
+
+    context 'when a user by that username does not exist' do
+      context 'when logged out' do
+        it 'renders 404 (does not redirect to login)' do
+          get :show, username: 'nonexistent'
+          expect(response).to have_http_status(404)
+        end
+      end
+
+      context 'when logged in' do
+        before { sign_in(user) }
+
+        it 'renders 404' do
+          get :show, username: 'nonexistent'
+          expect(response).to have_http_status(404)
+        end
+      end
+    end
   end
 
   describe 'GET #calendar' do