Improvements #2

app/models/custom_actions/conditions/base.rb

@@ -50,8 +50,8 @@ class CustomActions::Conditions::Base
   end
 
   def fulfilled_by?(work_package, _user)
-    (work_package.respond_to?(:"#{key}_id") && values.include?(work_package.send(:"#{key}_id"))) ||
-      values.empty?
+    values.empty? ||
+    (work_package.respond_to?(:"#{key}_id") && values.include?(work_package.send(:"#{key}_id")))
   end
 
   def key
@@ -110,7 +110,7 @@ class CustomActions::Conditions::Base
   private_class_method :habtm_table
 
   def self.key_id
-    "#{key}_id".to_sym
+    @key_id ||= "#{key}_id".to_sym
   end
   private_class_method :key_id
 

app/services/service_result.rb

@@ -27,20 +27,24 @@
 #++
 
 class ServiceResult
+  SUCCESS = true
+  FAILURE = false
+
   attr_accessor :success,
                 :result,
                 :errors,
-                :state,
                 :dependent_results
 
+  attr_writer :state
+
   # Creates a successful ServiceResult.
   def self.success(errors: nil,
                    message: nil,
                    message_type: nil,
-                   state: ::Shared::ServiceState.new,
+                   state: nil,
                    dependent_results: [],
                    result: nil)
-    new(success: true,
+    new(success: SUCCESS,
         errors:,
         message:,
         message_type:,
@@ -53,10 +57,10 @@ class ServiceResult
   def self.failure(errors: nil,
                    message: nil,
                    message_type: nil,
-                   state: ::Shared::ServiceState.new,
+                   state: nil,
                    dependent_results: [],
                    result: nil)
-    new(success: false,
+    new(success: FAILURE,
         errors:,
         message:,
         message_type:,
@@ -69,7 +73,7 @@ class ServiceResult
                  errors: nil,
                  message: nil,
                  message_type: nil,
-                 state: ::Shared::ServiceState.new,
+                 state: nil,
                  dependent_results: [],
                  result: nil)
     self.success = success
@@ -198,6 +202,10 @@ class ServiceResult
     end
   end
 
+  def state
+    @state ||= ::Shared::ServiceState.build
+  end
+
   private
 
   def initialize_errors(errors)

lib/api/v3/work_packages/work_package_collection_representer.rb

@@ -153,7 +153,8 @@ module API
                  render_nil: false
 
         def current_user_allowed_to_add_work_packages?
-          current_user.allowed_to?(:add_work_packages, project, global: project.nil?)
+          @current_user_allowed_to_add_work_packages ||=
+            current_user.allowed_to?(:add_work_packages, project, global: project.nil?)
         end
 
         def current_user_allowed_to_edit_work_packages?

lib/api/v3/work_packages/work_package_representer.rb

@@ -104,7 +104,7 @@ module API
         end
 
         link :copy,
-             cache_if: -> { current_user_allowed_to(:add_work_packages, context: represented.project) } do
+             cache_if: -> { add_work_packages_allowed? } do
           next if represented.new_record?
 
           {
@@ -114,7 +114,7 @@ module API
         end
 
         link :pdf,
-             cache_if: -> { current_user_allowed_to(:export_work_packages, context: represented.project) } do
+             cache_if: -> { export_work_packages_allowed? } do
           next if represented.new_record?
 
           {
@@ -125,7 +125,7 @@ module API
         end
 
         link :atom,
-             cache_if: -> { current_user_allowed_to(:export_work_packages, context: represented.project) } do
+             cache_if: -> { export_work_packages_allowed? } do
           next if represented.new_record? || !Setting.feeds_enabled?
 
           {
@@ -248,7 +248,7 @@ module API
         end
 
         link :addChild,
-             cache_if: -> { current_user_allowed_to(:add_work_packages, context: represented.project) } do
+             cache_if: -> { add_work_packages_allowed? } do
           next if represented.milestone? || represented.new_record?
 
           {
@@ -555,13 +555,35 @@ module API
           super
         end
 
+        # Permissions
         def current_user_watcher?
-          represented.watchers.any? { |w| w.user_id == current_user.id }
+          @current_user_watcher ||= represented.watchers.any? { |w| w.user_id == current_user.id }
         end
 
         def current_user_update_allowed?
-          current_user_allowed_to(:edit_work_packages, context: represented.project) ||
-            current_user_allowed_to(:assign_versions, context: represented.project)
+          @current_user_update_allowed ||=
+            current_user_allowed_to(:edit_work_packages, context: represented.project) ||
+              current_user_allowed_to(:assign_versions, context: represented.project)
+        end
+
+        def view_time_entries_allowed?
+          @view_time_entries_allowed ||=
+            current_user_allowed_to(:view_time_entries, context: represented.project) ||
+              current_user_allowed_to(:view_own_time_entries, context: represented.project)
+        end
+
+        def view_budgets_allowed?
+          @view_budgets_allowed ||= current_user_allowed_to(:view_budgets, context: represented.project)
+        end
+
+        def export_work_packages_allowed?
+          @export_work_packages_allowed ||=
+            current_user_allowed_to(:export_work_packages, context: represented.project)
+        end
+
+        def add_work_packages_allowed?
+          @add_work_packages_allowed ||=
+            current_user_allowed_to(:add_work_packages, context: represented.project)
         end
 
         def relations
@@ -604,7 +626,7 @@ module API
 
         def ordered_custom_actions
           # As the custom actions are sometimes set as an array
-          represented.custom_actions(current_user).to_a.sort_by(&:position)
+          @ordered_custom_actions ||= represented.custom_actions(current_user).to_a.sort_by(&:position)
         end
 
         # Attachments need to be eager loaded for the description
@@ -629,15 +651,6 @@ module API
            Setting.feeds_enabled?]
         end
 
-        def view_time_entries_allowed?
-          current_user_allowed_to(:view_time_entries, context: represented.project) ||
-            current_user_allowed_to(:view_own_time_entries, context: represented.project)
-        end
-
-        def view_budgets_allowed?
-          current_user_allowed_to(:view_budgets, context: represented.project)
-        end
-
         def load_complete_model(model)
           ::API::V3::WorkPackages::WorkPackageEagerLoadingWrapper.wrap_one(model, current_user)
         end