From 871d7bfd1ff5e6fd75db0cefde1368d4e8efc86c Mon Sep 17 00:00:00 2001
From: Jan Sandbrink <j.sandbrink@openproject.com>
Date: Tue, 7 Jan 2025 13:33:04 +0100
Subject: [PATCH] Change replacement_patterns_defined to require an attribute
 name

This ensures that the method is only used in contexts where we know
for which attribute we expect a replacement pattern to be defined.

This change makes errors less likely where one would expect that
all replacement patterns are defined for subjects (which is the status
quo as implemented, but not as designed).

Also all code that I've seen so far that calls this method, needed to
validate the subject attribute separately afterwards. This change should
make such code shorter.
---
 app/models/type.rb                                   | 6 ++----
 app/services/work_packages/create_service.rb         | 3 +--
 app/services/work_packages/set_attributes_service.rb | 4 +---
 app/services/work_packages/update_service.rb         | 4 +---
 4 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/app/models/type.rb b/app/models/type.rb
index 82d3a9b6886..f41382aa2be 100644
--- a/app/models/type.rb
+++ b/app/models/type.rb
@@ -104,10 +104,8 @@ class Type < ApplicationRecord
     object.types.include?(self)
   end
 
-  def replacement_patterns_defined?
-    return false if patterns.blank?
-
-    enabled_patterns.any?
+  def replacement_pattern_defined_for?(attribute)
+    enabled_patterns.key?(attribute)
   end
 
   def enabled_patterns
diff --git a/app/services/work_packages/create_service.rb b/app/services/work_packages/create_service.rb
index bda9e360e5d..69961b847cd 100644
--- a/app/services/work_packages/create_service.rb
+++ b/app/services/work_packages/create_service.rb
@@ -73,8 +73,7 @@ class WorkPackages::CreateService < BaseServices::BaseCallable
   end
 
   def set_templated_subject(work_package)
-    return true unless work_package.type&.replacement_patterns_defined?
-    return true unless work_package.type.enabled_patterns[:subject]
+    return true unless work_package.type&.replacement_pattern_defined_for?(:subject)
 
     work_package.subject = work_package.type.enabled_patterns[:subject].resolve(work_package)
     work_package.save
diff --git a/app/services/work_packages/set_attributes_service.rb b/app/services/work_packages/set_attributes_service.rb
index 264cdeb57b1..5a312bc2f72 100644
--- a/app/services/work_packages/set_attributes_service.rb
+++ b/app/services/work_packages/set_attributes_service.rb
@@ -49,9 +49,7 @@ class WorkPackages::SetAttributesService < BaseServices::SetAttributes
   end
 
   def mark_templated_subject
-    return true unless work_package.type&.replacement_patterns_defined?
-
-    if work_package.type.patterns.all_enabled[:subject]
+    if work_package.type&.replacement_pattern_defined_for?(:subject)
       work_package.subject = "Templated by #{work_package.type.name}"
     end
   end
diff --git a/app/services/work_packages/update_service.rb b/app/services/work_packages/update_service.rb
index ee45cd51626..bed33bd2214 100644
--- a/app/services/work_packages/update_service.rb
+++ b/app/services/work_packages/update_service.rb
@@ -42,9 +42,7 @@ class WorkPackages::UpdateService < BaseServices::Update
   private
 
   def set_templated_attributes
-    return unless model.type.replacement_patterns_defined?
-
-    model.type.patterns.all_enabled.each do |key, pattern|
+    model.type.enabled_patterns.each do |key, pattern|
       model.public_send(:"#{key}=", pattern.resolve(model))
     end
   end