Merge branch 'release/17.4' into release/17.5

app/workers/call_service_job.rb

@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+##
+# Calls a service in a background job.
+#
+# Example:
+#
+#   CallServiceJob.perform_later(
+#     "Members::AddRoleService",
+#     { user_id: 42, role_id: 7, project_id: nil, send_notifications: false },
+#     check_pending_migrations: true
+#   )
+class CallServiceJob < ApplicationJob
+  class ServiceCallFailed < StandardError; end
+
+  # Transient errors are retried
+  retry_on StandardError, wait: :polynomially_longer, attempts: 3
+
+  # If there are pending migrations wait for up to an hour (well 78 minutes altogether)
+  # for them to finish before giving up
+  retry_on ActiveRecord::PendingMigrationError, wait: :polynomially_longer, attempts: 7
+
+  # Service result failures are permanent — fail the job immediately without retry.
+  # Declared after retry_on StandardError so it takes precedence for ServiceCallFailed.
+  retry_on ServiceCallFailed, attempts: 1
+
+  queue_with_priority :low
+
+  # @param service_class_name [String] Fully-qualified service class name
+  # @param kwargs [Hash] Keyword arguments forwarded to service.call(...)
+  # @param current_user_id [Integer, nil] Current user ID; nil uses the system user
+  # @param check_pending_migrations [Boolean] Ensure this job is not executed while there are migrations pending.
+  def perform(service_class_name, kwargs, current_user_id: User.system.id, check_pending_migrations: false)
+    ActiveRecord::Migration.check_pending_migrations if check_pending_migrations
+
+    service = service_class_name.constantize.new current_user: User.find(current_user_id)
+    result = service.call **kwargs.symbolize_keys
+
+    result.on_failure do |r|
+      raise ServiceCallFailed, "#{service_class_name}#call failed: #{r.message}"
+    end
+  end
+end

db/migrate/20250929070310_add_view_all_principals_permission_to_existing_roles.rb

@@ -63,12 +63,14 @@ class AddViewAllPrincipalsPermissionToExistingRoles < ActiveRecord::Migration[8.
   private
 
   def add_global_role_for_manage_members
-    service = Members::AddRoleService.new(current_user: User.system)
+    role_id = global_role_view_all_users.id
 
     find_user_ids_with_manage_members.each do |user_id|
-      service
-        .call(user_id:, role_id: global_role_view_all_users.id, project_id: nil, send_notifications: false)
-        .on_failure { |result| Rails.logger.error("Failed to assign global role to user #{user_id}: #{result.message}") }
+      CallServiceJob.perform_later(
+        "Members::AddRoleService",
+        { user_id:, role_id:, project_id: nil, send_notifications: false },
+        check_pending_migrations: true
+      )
     end
   end
 

spec/migrations/add_view_all_principals_permission_to_existing_roles_spec.rb

@@ -36,6 +36,12 @@ RSpec.describe AddViewAllPrincipalsPermissionToExistingRoles, type: :model do
   let(:regular_user) { create(:user) }
   let(:project) { create(:project) }
 
+  def migrate
+    perform_enqueued_jobs do
+      ActiveRecord::Migration.suppress_messages { described_class.migrate(:up) }
+    end
+  end
+
   describe "up migration" do
     context "when global roles have manage_user permission" do
       let(:global_role) { create(:global_role, name: "Staff Manager") }
@@ -67,7 +73,7 @@ RSpec.describe AddViewAllPrincipalsPermissionToExistingRoles, type: :model do
         expect(GlobalRole.find_by(name: "View all users (migration)")).to be_nil
         expect(user_with_manage_members.members.where(project: nil)).to be_empty
 
-        ActiveRecord::Migration.suppress_messages { described_class.migrate(:up) }
+        migrate
 
         migration_role = GlobalRole.find_by(name: "View all users (migration)")
         expect(migration_role).to be_present
@@ -79,10 +85,10 @@ RSpec.describe AddViewAllPrincipalsPermissionToExistingRoles, type: :model do
       end
 
       it "does not duplicate assignments for users already having the global role" do
-        ActiveRecord::Migration.suppress_messages { described_class.migrate(:up) }
+        migrate
 
         # Run migration again
-        ActiveRecord::Migration.suppress_messages { described_class.migrate(:up) }
+        migrate
 
         migration_role = GlobalRole.find_by(name: "View all users (migration)")
         user_with_manage_members.reload
@@ -106,7 +112,7 @@ RSpec.describe AddViewAllPrincipalsPermissionToExistingRoles, type: :model do
       end
 
       it "assigns the global role only once per user" do
-        ActiveRecord::Migration.suppress_messages { described_class.migrate(:up) }
+        migrate
 
         migration_role = GlobalRole.find_by(name: "View all users (migration)")
         user_with_multiple_projects.reload
@@ -196,7 +202,7 @@ RSpec.describe AddViewAllPrincipalsPermissionToExistingRoles, type: :model do
       end
 
       it "assigns the migration global role even if user already has view_all_users via global role" do
-        ActiveRecord::Migration.suppress_messages { described_class.migrate(:up) }
+        migrate
 
         migration_role = GlobalRole.find_by(name: "View all users (migration)")
         user_with_both.reload