Remove generic find methods in controllers

This commit is contained in:
Klaus Zanders
2026-02-02 13:36:51 +01:00
parent 2b7a3c819c
commit b4dcdf467e
22 changed files with 126 additions and 193 deletions
@@ -34,10 +34,8 @@ class Admin::CustomFields::CustomFieldProjectsController < ApplicationController
layout "admin"
model_object CustomField
before_action :require_admin
before_action :find_model_object
before_action :find_custom_field, except: %i[index new create]
before_action :available_custom_fields_projects_query, only: %i[index destroy]
before_action :initialize_custom_field_project, only: :new
@@ -99,14 +97,13 @@ class Admin::CustomFields::CustomFieldProjectsController < ApplicationController
)
end
def find_model_object(object_id = :custom_field_id)
super
@custom_field = @object
def find_custom_field
@custom_field = CustomField.find(params[:custom_field_id])
end
def find_projects_to_activate_for_custom_field
if (project_ids = params.to_unsafe_h[:custom_fields_project][:project_ids]).present?
@projects = Project.find(project_ids)
@projects = Project.visible.find(project_ids)
else
initialize_custom_field_project
@custom_field_project.errors.add(:project_ids, :blank)
@@ -36,9 +36,8 @@ module Admin
include Dry::Monads[:result]
layout :admin_or_frame_layout
model_object CustomField
before_action :require_admin, :find_model_object, :find_active_item
before_action :require_admin, :find_custom_field, :find_active_item
# See https://github.com/hotwired/turbo-rails?tab=readme-ov-file#a-note-on-custom-layouts
def admin_or_frame_layout
@@ -225,11 +224,6 @@ module Admin
end
end
def find_model_object
@object = find_custom_field
@custom_field = @object
end
def find_custom_field
raise NotImplementedError, "SubclassResponsibility"
end
+3 -65
View File
@@ -34,7 +34,6 @@ require "cgi"
require "doorkeeper/dashboard_helper"
class ApplicationController < ActionController::Base
class_attribute :_model_object
class_attribute :_model_scope
class_attribute :accept_key_auth_actions
@@ -246,18 +245,18 @@ class ApplicationController < ActionController::Base
# Find project of id params[:id]
# Note: find() is Project.friendly.find()
def find_project
@project = Project.find(params[:id])
@project = Project.visible.find(params[:id])
end
# Find project of id params[:project_id]
# Note: find() is Project.friendly.find()
def find_project_by_project_id
@project = Project.find(params[:project_id])
@project = Project.visible.find(params[:project_id])
end
# Find project by project_id if given
def find_optional_project
@project = Project.find(params[:project_id]) if params[:project_id].present?
@project = Project.visible.find(params[:project_id]) if params[:project_id].present?
rescue ActiveRecord::RecordNotFound
render_404
end
@@ -269,67 +268,6 @@ class ApplicationController < ActionController::Base
@project = @object.project
end
def find_model_object(object_id = :id)
model = self.class._model_object
if model
@object = model.find(params[object_id])
instance_variable_set(:"@#{controller_name.singularize}", @object) if @object
end
end
def find_model_object_and_project(object_id = :id)
if params[object_id]
model_object = self.class._model_object
instance = model_object.find(params[object_id])
@project = instance.project
instance_variable_set(:"@#{model_object.to_s.underscore}", instance)
else
@project = Project.find(params[:project_id])
end
end
# TODO: this method is right now only suited for controllers of objects that somehow have an association to Project
def find_object_and_scope
model_object = self.class._model_object.find(params[:id]) if params[:id].present?
associations = self.class._model_scope + [Project]
associated = find_belongs_to_chained_objects(associations, model_object)
associated.each do |a|
instance_variable_set("@" + a.class.to_s.downcase, a)
end
end
# this method finds all records that are specified in the associations param
# after the first object is found it traverses the belongs_to chain of that first object
# if a start_object is provided it is taken as the starting point of the traversal
# e.g associations [Message, Board, Project] finds Message by find(:message_id)
# then message.forum and board.project
def find_belongs_to_chained_objects(associations, start_object = nil)
associations.inject([start_object].compact) do |instances, association|
scope_name, scope_association = if association.is_a?(Hash)
[association.keys.first.to_s.downcase, association.values.first]
else
[association.to_s.downcase, association.to_s.downcase]
end
# TODO: Remove this hidden dependency on params
instances << (
if instances.last.nil?
scope_name.camelize.constantize.find(params[:"#{scope_name}_id"])
else
instances.last.send(scope_association.to_sym)
end)
instances
end
end
def self.model_object(model, options = {})
self._model_object = model
self._model_scope = Array(options[:scope]) if options[:scope]
end
# Filter for bulk work package operations
def find_work_packages
@work_packages = WorkPackage.includes(:project)
+6 -9
View File
@@ -30,9 +30,8 @@
class CategoriesController < ApplicationController
menu_item :settings_categories
model_object Category
before_action :find_model_object, except: %i[new create]
before_action :find_project_from_association, except: %i[new create]
before_action :find_category_and_project, except: %i[new create]
before_action :find_project, only: %i[new create]
before_action :authorize
@@ -81,14 +80,12 @@ class CategoriesController < ApplicationController
private
# Wrap ApplicationController's find_model_object method to set
# @category instead of just @category
def find_model_object
super
@category = @object
def find_category_and_project
@category = Category.find(params[:id])
@project = @category.project
end
def find_project
@project = Project.find(params[:project_id])
@project = Project.visible.find(params[:project_id])
end
end
+5 -2
View File
@@ -35,8 +35,7 @@ class CustomActionsController < ApplicationController
redirect_to action: :index
end
self._model_object = CustomAction
before_action :find_model_object, only: %i(edit update destroy)
before_action :find_custom_action, only: %i(edit update destroy)
before_action :pad_params, only: %i(create update)
layout "admin"
@@ -73,6 +72,10 @@ class CustomActionsController < ApplicationController
private
def find_custom_action
@custom_action = CustomAction.find(params[:id])
end
def index_or_render(render_action)
->(call) {
call.on_success do
@@ -31,13 +31,13 @@
class LdapAuthSourcesController < ApplicationController
menu_item :ldap_authentication
include PaginationHelper
layout "admin"
before_action :require_admin
before_action :block_if_password_login_disabled
self._model_object = LdapAuthSource
before_action :find_model_object, only: %i(edit update destroy)
before_action :find_ldap_auth_source, only: %i(edit update destroy)
before_action :prevent_editing_when_seeded, only: %i(update)
def index
@@ -101,6 +101,10 @@ class LdapAuthSourcesController < ApplicationController
protected
def find_ldap_auth_source
@ldap_auth_source = LdapAuthSource.find(params[:id])
end
def prevent_editing_when_seeded
if @ldap_auth_source.seeded_from_env?
flash[:warning] = I18n.t(:label_seeded_from_env_warning)
+6 -2
View File
@@ -31,8 +31,7 @@
class MembersController < ApplicationController
include MemberHelper
model_object Member
before_action :find_model_object_and_project, except: %i[autocomplete_for_member destroy_by_principal]
before_action :find_member, except: %i[autocomplete_for_member destroy_by_principal]
before_action :find_project_by_project_id, only: %i[autocomplete_for_member destroy_by_principal]
before_action :authorize
@@ -118,6 +117,10 @@ class MembersController < ApplicationController
private
def find_member
@member = @project.members.visible.find(params[:id])
end
def authorize_for(controller, action)
current_user.allowed_in_project?({ controller:, action: }, @project)
end
@@ -205,6 +208,7 @@ class MembersController < ApplicationController
def possible_members(criteria, limit, type: nil)
Principal
.visible
.possible_member(@project, type:)
.like(criteria, email: user_allowed_to_view_emails?)
.limit(limit)
+7 -2
View File
@@ -31,8 +31,7 @@
class MessagesController < ApplicationController
menu_item :forums
default_search_scope :messages
model_object Message, scope: Forum
before_action :find_object_and_scope
before_action :find_project_forum_and_message
before_action :authorize, except: %i[edit update destroy]
# Checked inside the method.
no_authorization_required! :edit, :update, :destroy
@@ -157,6 +156,12 @@ class MessagesController < ApplicationController
private
def find_project_forum_and_message
@message = Message.visible(find_params[:id])
@forum = @message.forum
@project = @forum.project
end
def update_message(message)
Messages::UpdateService
.new(user: current_user,
+15 -2
View File
@@ -30,8 +30,8 @@
class News::CommentsController < ApplicationController
default_search_scope :news
model_object Comment, scope: [News => :commented]
before_action :find_object_and_scope
before_action :find_project_news_and_comment, only: %i[destroy]
before_action :find_news, only: %i[create]
before_action :authorize
def create
@@ -48,4 +48,17 @@ class News::CommentsController < ApplicationController
@comment.destroy
redirect_to news_path(@news), status: :see_other
end
private
def find_project_news_and_comment
@comment = Comment.find(params[:id])
@news = News.visible.find(@comment.commented_id)
@project = @news.project
end
def find_news
@news = News.visible.find(params[:news_id])
@project = @news.project
end
end
+7 -4
View File
@@ -32,9 +32,7 @@ class VersionsController < ApplicationController
menu_item :roadmap, only: %i(index show)
menu_item :settings_versions
model_object Version
before_action :find_model_object, except: %i[index new create close_completed]
before_action :find_project_from_association, except: %i[index new create close_completed]
before_action :find_version, except: %i[index new create close_completed]
before_action :find_project, only: %i[index new create close_completed]
before_action :authorize
@@ -114,6 +112,11 @@ class VersionsController < ApplicationController
private
def find_version
@version = Version.visible.find(params[:id])
@project = @version.project
end
def archived_project_mesage
if current_user.admin?
ApplicationController.helpers.sanitize(
@@ -135,7 +138,7 @@ class VersionsController < ApplicationController
end
def find_project
@project = Project.find(params[:project_id])
@project = Project.visible.find(params[:project_id])
end
def retrieve_selected_type_ids(selectable_types, default_types = nil)
+1 -1
View File
@@ -34,7 +34,7 @@ module Members::Scopes
class_methods do
# Find all members visible to the inquiring user
def visible(user)
def visible(user = User.current)
if user.admin?
visible_for_admins
else
@@ -30,12 +30,8 @@ module OpenProject::Backlogs::Patches::VersionsControllerPatch
def self.included(base)
base.class_eval do
include VersionSettingsHelper
helper :version_settings
# Find project explicitly on update and edit
skip_before_action :find_project_from_association, only: %i[edit update]
skip_before_action :find_model_object, only: %i[edit update]
prepend_before_action :find_project_and_version, only: %i[edit update]
helper :version_settings
before_action :add_project_to_version_settings_attributes, only: %i[update create]
@@ -56,15 +52,6 @@ module OpenProject::Backlogs::Patches::VersionsControllerPatch
end
end
def find_project_and_version
find_model_object
if params[:project_id]
find_project
else
find_project_from_association
end
end
# This forces the current project for the nested version settings in order
# to prevent it from being set through firebug etc. #mass_assignment
def add_project_to_version_settings_attributes
@@ -35,11 +35,8 @@ class DocumentsController < ApplicationController
include OpTurbo::ComponentStream
default_search_scope :documents
model_object Document
before_action :find_project_by_project_id, only: %i[index search new create]
before_action :find_model_object, except: %i[index search new create]
before_action :find_project_from_association, except: %i[index search new create]
before_action :authorize
def index
@@ -189,6 +186,11 @@ class DocumentsController < ApplicationController
private
def find_document
@document = Document.visible.find(params[:id])
@project = @document.project
end
def document_params
params.fetch(:document, {}).permit("type_id", "title", "description", "content_binary", "kind")
end
@@ -27,9 +27,8 @@
#++
class WorkPackageCostlogController < ApplicationController
model_object WorkPackage
menu_item :work_packages
before_action :find_objects
before_action :authorize
before_action :redirect_when_outside_project
@@ -74,7 +73,7 @@ class WorkPackageCostlogController < ApplicationController
# 1. Work package from :work_package_id and its #project
# 2. Cost Type from param
def find_objects
find_model_object_and_project :work_package_id
@work_package = WorkPackage.visible.find_by(id: params[:work_package_id])
if params[:cost_type_id].present?
@cost_type = CostType.find(params[:cost_type_id])
@@ -37,8 +37,7 @@ class Storages::Admin::AccessManagementController < ApplicationController
before_action :require_admin
model_object Storages::Storage
before_action :find_model_object, only: %i[new create edit update]
before_action :find_storage, only: %i[new create edit update]
# menu_item is defined in the Redmine::MenuManager::MenuController
# module, included from ApplicationController.
@@ -92,9 +91,8 @@ class Storages::Admin::AccessManagementController < ApplicationController
private
def find_model_object(object_id = :storage_id)
super
@storage = @object
def find_storage
@storage = Storages::Storage.find(params[:storage_id])
end
def call_update_service
@@ -41,9 +41,7 @@ class Storages::Admin::AutomaticallyManagedProjectFoldersController < Applicatio
# and set the @<controller_name> variable to the object referenced in the URL.
before_action :require_admin
# specify which model #find_model_object should look up
model_object Storages::NextcloudStorage
before_action :find_model_object, only: %i[new create edit update]
before_action :find_nextcloud_storage, only: %i[new create edit update]
# menu_item is defined in the Redmine::MenuManager::MenuController
# module, included from ApplicationController.
@@ -69,6 +67,13 @@ class Storages::Admin::AutomaticallyManagedProjectFoldersController < Applicatio
respond_with_ampf_form_turbo_stream_or_edit_html
end
# Renders an edit page (allowing the user to change automatically_managed bool and password).
# Used by: The StoragesController#edit, when user wants to update application credentials.
# Called by: Global app/config/routes.rb to serve Web page
def edit
respond_with_ampf_form_turbo_stream_or_edit_html
end
def create
service_result = call_update_service
@@ -83,13 +88,6 @@ class Storages::Admin::AutomaticallyManagedProjectFoldersController < Applicatio
end
end
# Renders an edit page (allowing the user to change automatically_managed bool and password).
# Used by: The StoragesController#edit, when user wants to update application credentials.
# Called by: Global app/config/routes.rb to serve Web page
def edit
respond_with_ampf_form_turbo_stream_or_edit_html
end
# Update is similar to create above
# See also: create above
# Called by: Global app/config/routes.rb to serve Web page
@@ -119,12 +117,8 @@ class Storages::Admin::AutomaticallyManagedProjectFoldersController < Applicatio
end
end
# Override default url param `:id` to `:storage` controller is a nested storage resource
# GET /admin/settings/storages/:storage_id/automatically_managed_project_folders/new
# POST /admin/settings/storages/:storage_id/automatically_managed_project_folders
def find_model_object(object_id = :storage_id)
super
@storage = @object
def find_nextcloud_storage
@storage = Storages::Storage.find(params[:storage_id])
end
def call_update_service
@@ -35,10 +35,8 @@ module Storages
layout :admin_or_frame_layout
model_object Storage
before_action :require_admin
before_action :find_model_object
before_action :find_storage
def admin_or_frame_layout
return "turbo_rails/frame" if turbo_frame_request?
@@ -83,9 +81,8 @@ module Storages
}.merge(@report.to_h).to_yaml(stringify_names: true)
end
def find_model_object(object_id = :storage_id)
super
@storage = @object
def find_storage
@storage = Storages::Storage.find(params[:storage_id])
end
def create_and_cache_report
@@ -32,9 +32,7 @@ class Storages::Admin::ProjectStoragesController < Projects::SettingsController
include Storages::OAuthAccessGrantable
include OpTurbo::ComponentStream
model_object Storages::ProjectStorage
before_action :find_model_object, only: %i[oauth_access_grant edit update destroy destroy_info]
before_action :find_project_storage, only: %i[oauth_access_grant edit update destroy destroy_info]
menu_item :settings_project_storages
def external_file_storages
@@ -61,7 +59,6 @@ class Storages::Admin::ProjectStoragesController < Projects::SettingsController
end
def edit
@project_storage = @object
@project_storage.project_folder_mode = project_folder_mode_from_params if project_folder_mode_from_params.present?
@last_project_folders = Storages::LastProjectFolder
@@ -88,7 +85,6 @@ class Storages::Admin::ProjectStoragesController < Projects::SettingsController
end
def oauth_access_grant
@project_storage = @object
storage = @project_storage.storage
auth_state = ::Storages::Adapters::Authentication.authorization_state(storage:, user: current_user)
@@ -105,7 +101,7 @@ class Storages::Admin::ProjectStoragesController < Projects::SettingsController
def update
service_result = ::Storages::ProjectStorages::UpdateService
.new(user: current_user, model: @object)
.new(user: current_user, model: @project_storage)
.call(permitted_storage_settings_params)
if service_result.success?
@@ -113,14 +109,13 @@ class Storages::Admin::ProjectStoragesController < Projects::SettingsController
flash[:notice] = I18n.t(:notice_successful_update)
redirect_to_project_storages_path_with_oauth_access_grant_confirmation(@project_storage.storage)
else
@project_storage = @object
render "/storages/project_settings/edit"
end
end
def destroy
Storages::ProjectStorages::DeleteService
.new(user: current_user, model: @object)
.new(user: current_user, model: @project_storage)
.call
.on_failure { |service_result| flash[:error] = service_result.errors.full_messages }
@@ -129,13 +124,17 @@ class Storages::Admin::ProjectStoragesController < Projects::SettingsController
def destroy_info
respond_with_dialog Storages::ProjectStorages::DestroyConfirmationDialogComponent.new(
project_storage: @object,
project_storage: @project_storage,
target: :project
)
end
private
def find_project_storage
@project_storage = Storages::ProjectStorage.find(params[:id])
end
def permitted_storage_settings_params
params
.require(:storages_project_storage)
@@ -35,10 +35,8 @@ class Storages::Admin::Storages::ProjectStoragesController < ApplicationControll
layout "admin"
model_object Storages::Storage
before_action :require_admin
before_action :find_model_object
before_action :find_storage
before_action :load_project_storage, only: %i(edit update destroy destroy_confirmation_dialog)
before_action :storage_projects_query, only: :index
@@ -69,6 +67,17 @@ class Storages::Admin::Storages::ProjectStoragesController < ApplicationControll
)
end
def edit
last_project_folders = Storages::LastProjectFolder
.where(project_storage: @project_storage)
.pluck(:mode, :origin_folder_id)
.to_h
respond_with_dialog Storages::Admin::Storages::ProjectsStorageModalComponent.new(
project_storage: @project_storage, last_project_folders:
)
end
def create # rubocop:disable Metrics/AbcSize
create_service = ::Storages::ProjectStorages::BulkCreateService
.new(user: current_user, projects: @projects, storage: @storage,
@@ -87,17 +96,6 @@ class Storages::Admin::Storages::ProjectStoragesController < ApplicationControll
respond_with_turbo_streams(status: create_service.success? ? :ok : :unprocessable_entity)
end
def edit
last_project_folders = Storages::LastProjectFolder
.where(project_storage: @project_storage)
.pluck(:mode, :origin_folder_id)
.to_h
respond_with_dialog Storages::Admin::Storages::ProjectsStorageModalComponent.new(
project_storage: @project_storage, last_project_folders:
)
end
def update
update_service = ::Storages::ProjectStorages::UpdateService
.new(user: current_user, model: @project_storage)
@@ -143,6 +141,10 @@ class Storages::Admin::Storages::ProjectStoragesController < ApplicationControll
private
def load_storage
@storage = Storages::Storage.visible.find(params[:storage_id])
end
def load_project_storage
@project_storage = Storages::ProjectStorage.find(params[:id])
rescue ActiveRecord::RecordNotFound
@@ -152,14 +154,9 @@ class Storages::Admin::Storages::ProjectStoragesController < ApplicationControll
respond_with_turbo_streams
end
def find_model_object(object_id = :storage_id)
super
@storage = @object
end
def find_projects_to_activate_for_storage
if (project_ids = params.to_unsafe_h[:storages_project_storage][:project_ids]).present?
@projects = Project.find(project_ids)
@projects = Project.visible.find(project_ids)
else
initialize_project_storage
@project_storage.errors.add(:project_ids, :blank)
@@ -41,13 +41,10 @@ module Storages
# See https://guides.rubyonrails.org/layouts_and_rendering.html for reference on layout
layout "admin"
# specify which model #find_model_object should look up
model_object Storage
# Before executing any action below: Make sure the current user is an admin
# and set the @<controller_name> variable to the object referenced in the URL.
before_action :require_admin
before_action :find_model_object,
before_action :find_storage,
only: %i[show_oauth_application destroy edit edit_host edit_storage_audience confirm_destroy update
change_health_notifications_enabled replace_oauth_application]
before_action :ensure_valid_wizard_parameters, only: [:new]
@@ -216,6 +213,10 @@ module Storages
private
def find_storage
@storage = Storages::Storage.visible.find(params[:id])
end
def prepare_storage_for_access_management_form
return unless @storage.automatic_management_unspecified?
@@ -36,12 +36,11 @@ class Storages::ProjectSettings::ProjectStorageMembersController < Projects::Set
menu_item :settings_project_storages
before_action :find_model_object, only: %i[index]
model_object Storages::ProjectStorage
before_action :find_project_storage, only: %i[index]
def index
@project_users = Member
.visible
.of_project(@project)
.joins(:principal)
.preload(roles: :role_permissions, principal: :remote_identities)
@@ -53,9 +52,8 @@ class Storages::ProjectSettings::ProjectStorageMembersController < Projects::Set
private
def find_model_object(object_id = :project_storage_id)
super
@project_storage = @object
def find_project_storage
@project_storage = Storages::ProjectStorage.find(params[:project_storage_id])
@storage = @project_storage.storage
end
end
@@ -32,10 +32,9 @@ class Storages::ProjectStoragesController < ApplicationController
using Storages::Peripherals::ServiceResultRefinements
menu_item :overview
model_object Storages::ProjectStorage
before_action :require_login
before_action :find_model_object
before_action :find_project_stroage
before_action :find_project_by_project_id
before_action :render_403, unless: -> { User.current.allowed_in_project?(:view_file_links, @project) }
no_authorization_required! :open
@@ -53,6 +52,10 @@ class Storages::ProjectStoragesController < ApplicationController
private
def find_project_stroage
@project_storage = Storages::ProjectStorage.find(params[:id])
end
def ensure_remote_identity
case Storages::Adapters::Authentication.authorization_state(storage:, user: current_user)
when :not_connected
@@ -114,7 +117,7 @@ class Storages::ProjectStoragesController < ApplicationController
end
def storage
@object.storage
@project_storage.storage
end
def project_storage_scope
@@ -129,6 +132,6 @@ class Storages::ProjectStoragesController < ApplicationController
def show_error(message)
flash[:error] = Array(message) + [I18n.t("project_storages.open.contact_admin")]
redirect_back(fallback_location: project_path(id: @project_storage.project_id))
redirect_back_or_to(project_path(id: @project_storage.project_id))
end
end