From de15253cc2c8d136cde529c07fd7920cb30715e8 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Mon, 27 Apr 2026 09:20:11 +0200 Subject: [PATCH] Fix DynamicFindBy issues in our Codebase --- .rubocop.yml | 13 ++++++++----- app/models/work_packages/costs.rb | 2 +- .../strategies/warden/user_api_token.rb | 2 +- modules/bim/app/models/bim/bcf/viewpoint.rb | 2 +- modules/bim/app/models/bim/ifc_models/ifc_model.rb | 2 +- modules/budgets/spec/models/budget_spec.rb | 4 ++-- .../notification_handler/issue_hook.rb | 4 ++-- .../notification_handler/merge_request_hook.rb | 4 ++-- .../notification_handler/push_hook.rb | 4 ++-- .../notification_handler/system_hook.rb | 4 ++-- 10 files changed, 22 insertions(+), 19 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index ed6d645fa8d..5b55c2a7ae1 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -60,7 +60,7 @@ Layout/MultilineOperationIndentation: Enabled: false Lint/AmbiguousBlockAssociation: - AllowedMethods: [change] + AllowedMethods: [ change ] Lint/AmbiguousOperator: Enabled: false @@ -147,9 +147,9 @@ Naming/PredicatePrefix: Naming/VariableNumber: AllowedPatterns: - - '\w_20\d\d' # allow dates like christmas_2022 or date_2034_04_12 - - '\w\d++(_\d++)+' # allow hierarchical data like child1_2_5 (second + in regex is possessive qualifier) - - 'custom_field_\d+' # allow custom field method names to be called with send :custom_field_1001 + - "\\w_20\\d\\d" # allow dates like christmas_2022 or date_2034_04_12 + - "\\w\\d++(_\\d++)+" # allow hierarchical data like child1_2_5 (second + in regex is possessive qualifier) + - "custom_field_\\d+" # allow custom field method names to be called with send :custom_field_1001 OpenProject/AddPreviewForViewComponent: Include: @@ -181,6 +181,9 @@ Rails/DynamicFindBy: - find_by_login - find_by_mail - find_by_plaintext_value + - find_by_rss_key + - find_by_unique + - find_by_api_key # Allow reorder to prevent find each cop triggering Rails/FindEach: @@ -402,7 +405,7 @@ Style/FormatString: Enabled: false Style/FormatStringToken: - AllowedMethods: [redirect] + AllowedMethods: [ redirect ] Style/FrozenStringLiteralComment: Enabled: true diff --git a/app/models/work_packages/costs.rb b/app/models/work_packages/costs.rb index 4a9ed1cd631..52a1f942113 100644 --- a/app/models/work_packages/costs.rb +++ b/app/models/work_packages/costs.rb @@ -118,7 +118,7 @@ module WorkPackages::Costs reassign_to = ::WorkPackage .joins(:project) .merge(Project.allowed_to(user, :edit_cost_entries)) - .find_by_id(ids) + .find_by(id: ids) if reassign_to.nil? work_packages.each do |wp| diff --git a/lib_static/open_project/authentication/strategies/warden/user_api_token.rb b/lib_static/open_project/authentication/strategies/warden/user_api_token.rb index 475982bdcc2..62879936f77 100644 --- a/lib_static/open_project/authentication/strategies/warden/user_api_token.rb +++ b/lib_static/open_project/authentication/strategies/warden/user_api_token.rb @@ -51,7 +51,7 @@ module OpenProject end def authenticate! - token = ::Token::API.find_by_plaintext_value(@access_token) # rubocop:disable Rails/DynamicFindBy + token = ::Token::API.find_by_plaintext_value(@access_token) return fail_with_header!(error: "invalid_token") if token.nil? authentication_result(token.user) diff --git a/modules/bim/app/models/bim/bcf/viewpoint.rb b/modules/bim/app/models/bim/bcf/viewpoint.rb index 921a6ae134c..1d983a23f41 100644 --- a/modules/bim/app/models/bim/bcf/viewpoint.rb +++ b/modules/bim/app/models/bim/bcf/viewpoint.rb @@ -58,7 +58,7 @@ module Bim::Bcf if attachments.loaded? attachments.detect { |a| a.description == "snapshot" } else - attachments.find_by_description("snapshot") + attachments.find_by(description: "snapshot") end end diff --git a/modules/bim/app/models/bim/ifc_models/ifc_model.rb b/modules/bim/app/models/bim/ifc_models/ifc_model.rb index 307d0b2b139..8d91f16641b 100644 --- a/modules/bim/app/models/bim/ifc_models/ifc_model.rb +++ b/modules/bim/app/models/bim/ifc_models/ifc_model.rb @@ -60,7 +60,7 @@ module Bim if attachments.loaded? attachments.detect { |a| a.description == key.to_s && !a.marked_for_destruction? } else - attachments.find_by_description(key.to_s) + attachments.find_by(description: key.to_s) end end diff --git a/modules/budgets/spec/models/budget_spec.rb b/modules/budgets/spec/models/budget_spec.rb index c10774f2c7c..f1aeb133558 100644 --- a/modules/budgets/spec/models/budget_spec.rb +++ b/modules/budgets/spec/models/budget_spec.rb @@ -47,8 +47,8 @@ RSpec.describe Budget do budget.destroy end - it { expect(Budget.find_by_id(budget.id)).to be_nil } - it { expect(WorkPackage.find_by_id(work_package.id)).to eq(work_package) } + it { expect(described_class.find_by(id: budget.id)).to be_nil } + it { expect(WorkPackage.find_by(id: work_package.id)).to eq(work_package) } it { expect(work_package.reload.budget).to be_nil } end diff --git a/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/issue_hook.rb b/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/issue_hook.rb index 685dc83da7c..a02f72e34ea 100644 --- a/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/issue_hook.rb +++ b/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/issue_hook.rb @@ -36,9 +36,9 @@ module OpenProject::GitlabIntegration def process(payload_params) # rubocop:disable Metrics/AbcSize @payload = wrap_payload(payload_params) - user = User.find_by_id(payload.open_project_user_id) + user = User.find_by(id: payload.open_project_user_id) text = [payload.object_attributes.title, payload.object_attributes.description] - .select(&:present?) + .compact_blank .join(" - ") work_packages = find_mentioned_work_packages(text, user) notes = generate_notes(payload) diff --git a/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/merge_request_hook.rb b/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/merge_request_hook.rb index 4268cd7fd5a..3223b558f54 100644 --- a/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/merge_request_hook.rb +++ b/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/merge_request_hook.rb @@ -47,9 +47,9 @@ module OpenProject::GitlabIntegration @payload = wrap_payload(payload_params) return unless (accepted_actions.include? payload.object_attributes.action) || (accepted_states.include? payload.object_attributes.state) - user = User.find_by_id(payload.open_project_user_id) + user = User.find_by(id: payload.open_project_user_id) text = [payload.object_attributes.title, payload.object_attributes.description] - .select(&:present?) + .compact_blank .join(" - ") work_packages = find_mentioned_work_packages(text, user) notes = generate_notes(payload) diff --git a/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/push_hook.rb b/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/push_hook.rb index 81489a81ea2..36c1ae66b2e 100644 --- a/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/push_hook.rb +++ b/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/push_hook.rb @@ -39,9 +39,9 @@ module OpenProject::GitlabIntegration return nil unless payload.object_kind == "push" payload.commits.each do |commit| - user = User.find_by_id(payload.open_project_user_id) + user = User.find_by(id: payload.open_project_user_id) text = [commit["title"], commit["message"]] - .select(&:present?) + .compact_blank .join(" - ") work_packages = find_mentioned_work_packages(text, user) notes = generate_notes(commit, payload) diff --git a/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/system_hook.rb b/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/system_hook.rb index 47e4c83a711..569be2e6428 100644 --- a/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/system_hook.rb +++ b/modules/gitlab_integration/lib/open_project/gitlab_integration/notification_handler/system_hook.rb @@ -39,9 +39,9 @@ module OpenProject::GitlabIntegration return nil unless payload.object_kind == "push" payload.commits.each do |commit| - user = User.find_by_id(payload.open_project_user_id) + user = User.find_by(id: payload.open_project_user_id) text = [commit["title"], commit["message"]] - .select(&:present?) + .compact_blank .join(" - ") work_packages = find_mentioned_work_packages(text, user) notes = generate_notes(commit, payload)