From: Andy Allan <git@gravitystorm.co.uk>
Date: Wed, 30 Aug 2023 14:11:57 +0000 (+0100)
Subject: Merge pull request #4197 from AntonKhorev/user-resources
X-Git-Tag: live~607
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/e68d4efc3cdf02f35144e3967be998aaa633a3de?hp=a1657f03a836fa5ade55e52083eff359b486e8f3

Merge pull request #4197 from AntonKhorev/user-resources

Move user lookup/error methods from app controller to concerns
---

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 5dcfee07c..a30816a8e 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -215,24 +215,6 @@ class ApplicationController < ActionController::Base
     render :action => "timeout"
   end
 
-  ##
-  # ensure that there is a "user" instance variable
-  def lookup_user
-    render_unknown_user params[:display_name] unless @user = User.active.find_by(:display_name => params[:display_name])
-  end
-
-  ##
-  # render a "no such user" page
-  def render_unknown_user(name)
-    @title = t "users.no_such_user.title"
-    @not_found_user = name
-
-    respond_to do |format|
-      format.html { render :template => "users/no_such_user", :status => :not_found }
-      format.all { head :not_found }
-    end
-  end
-
   ##
   # Unfortunately if a PUT or POST request that has a body fails to
   # read it then Apache will sometimes fail to return the response it
diff --git a/app/controllers/changesets_controller.rb b/app/controllers/changesets_controller.rb
index 7796dfeb2..22d3356b7 100644
--- a/app/controllers/changesets_controller.rb
+++ b/app/controllers/changesets_controller.rb
@@ -1,6 +1,8 @@
 # The ChangesetController is the RESTful interface to Changeset objects
 
 class ChangesetsController < ApplicationController
+  include UserMethods
+
   layout "site"
   require "xml/libxml"
 
diff --git a/app/controllers/concerns/user_methods.rb b/app/controllers/concerns/user_methods.rb
index 81e9f0064..eb7d38988 100644
--- a/app/controllers/concerns/user_methods.rb
+++ b/app/controllers/concerns/user_methods.rb
@@ -3,6 +3,26 @@ module UserMethods
 
   private
 
+  ##
+  # ensure that there is a "user" instance variable
+  def lookup_user
+    @user = User.active.find_by!(:display_name => params[:display_name])
+  rescue ActiveRecord::RecordNotFound
+    render_unknown_user params[:display_name]
+  end
+
+  ##
+  # render a "no such user" page
+  def render_unknown_user(name)
+    @title = t "users.no_such_user.title"
+    @not_found_user = name
+
+    respond_to do |format|
+      format.html { render :template => "users/no_such_user", :status => :not_found, :layout => "site" }
+      format.all { head :not_found }
+    end
+  end
+
   ##
   # update a user's details
   def update_user(user, params)
diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb
index a482bc96d..65f560571 100644
--- a/app/controllers/confirmations_controller.rb
+++ b/app/controllers/confirmations_controller.rb
@@ -1,5 +1,6 @@
 class ConfirmationsController < ApplicationController
   include SessionMethods
+  include UserMethods
 
   layout "site"
 
diff --git a/app/controllers/diary_entries_controller.rb b/app/controllers/diary_entries_controller.rb
index 6981ed797..ea6d1d276 100644
--- a/app/controllers/diary_entries_controller.rb
+++ b/app/controllers/diary_entries_controller.rb
@@ -1,4 +1,6 @@
 class DiaryEntriesController < ApplicationController
+  include UserMethods
+
   layout "site", :except => :rss
 
   before_action :authorize_web
diff --git a/app/controllers/friendships_controller.rb b/app/controllers/friendships_controller.rb
index 4d1161147..731dda453 100644
--- a/app/controllers/friendships_controller.rb
+++ b/app/controllers/friendships_controller.rb
@@ -1,4 +1,6 @@
 class FriendshipsController < ApplicationController
+  include UserMethods
+
   layout "site"
 
   before_action :authorize_web
diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index 22d0c88ba..e0b5b05d3 100644
--- a/app/controllers/messages_controller.rb
+++ b/app/controllers/messages_controller.rb
@@ -1,4 +1,6 @@
 class MessagesController < ApplicationController
+  include UserMethods
+
   layout "site"
 
   before_action :authorize_web
diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index 440a620e8..97efc3eda 100644
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -1,4 +1,6 @@
 class NotesController < ApplicationController
+  include UserMethods
+
   layout :map_layout
 
   before_action :check_api_readable
@@ -7,30 +9,22 @@ class NotesController < ApplicationController
 
   authorize_resource
 
+  before_action :lookup_user, :only => [:index]
   before_action :set_locale
   around_action :web_timeout
 
   ##
   # Display a list of notes by a specified user
   def index
-    if params[:display_name]
-      if @user = User.active.find_by(:display_name => params[:display_name])
-        @params = params.permit(:display_name)
-        @title = t ".title", :user => @user.display_name
-        @page = (params[:page] || 1).to_i
-        @page_size = 10
-        @notes = @user.notes
-        @notes = @notes.visible unless current_user&.moderator?
-        @notes = @notes.order("updated_at DESC, id").distinct.offset((@page - 1) * @page_size).limit(@page_size).preload(:comments => :author)
-
-        render :layout => "site"
-      else
-        @title = t "users.no_such_user.title"
-        @not_found_user = params[:display_name]
-
-        render :template => "users/no_such_user", :status => :not_found, :layout => "site"
-      end
-    end
+    @params = params.permit(:display_name)
+    @title = t ".title", :user => @user.display_name
+    @page = (params[:page] || 1).to_i
+    @page_size = 10
+    @notes = @user.notes
+    @notes = @notes.visible unless current_user&.moderator?
+    @notes = @notes.order("updated_at DESC, id").distinct.offset((@page - 1) * @page_size).limit(@page_size).preload(:comments => :author)
+
+    render :layout => "site"
   end
 
   def show
diff --git a/app/controllers/traces_controller.rb b/app/controllers/traces_controller.rb
index 0b7dbc94a..90ab34a48 100644
--- a/app/controllers/traces_controller.rb
+++ b/app/controllers/traces_controller.rb
@@ -1,4 +1,6 @@
 class TracesController < ApplicationController
+  include UserMethods
+
   layout "site", :except => :georss
 
   before_action :authorize_web
diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb
index 63ebdad71..546c8233c 100644
--- a/app/controllers/user_blocks_controller.rb
+++ b/app/controllers/user_blocks_controller.rb
@@ -1,4 +1,6 @@
 class UserBlocksController < ApplicationController
+  include UserMethods
+
   layout "site"
 
   before_action :authorize_web
diff --git a/app/controllers/user_roles_controller.rb b/app/controllers/user_roles_controller.rb
index cf5b4de9e..b54cd0bd7 100644
--- a/app/controllers/user_roles_controller.rb
+++ b/app/controllers/user_roles_controller.rb
@@ -1,4 +1,6 @@
 class UserRolesController < ApplicationController
+  include UserMethods
+
   layout "site"
 
   before_action :authorize_web