From 7d9f0cd3ef3944f7ec6d827ed42f0e30c3ec710d Mon Sep 17 00:00:00 2001
From: Klaus Zanders <k.zanders@openproject.com>
Date: Thu, 16 Nov 2023 16:30:38 +0100
Subject: [PATCH] Remove caches from policies and use correct perm checks

---
 app/policies/query_policy.rb        | 46 ++++++++++++-----------------
 app/policies/work_package_policy.rb | 42 +++++---------------------
 2 files changed, 26 insertions(+), 62 deletions(-)

diff --git a/app/policies/query_policy.rb b/app/policies/query_policy.rb
index 2003b3ea4af..ee64c1bb736 100644
--- a/app/policies/query_policy.rb
+++ b/app/policies/query_policy.rb
@@ -90,50 +90,42 @@ class QueryPolicy < BasePolicy
   end
 
   def view_work_packages_allowed?(query)
-    @view_work_packages_cache ||= Hash.new do |hash, project|
-      hash[project] = allowed_in_project_or_any_project?(:view_work_packages, project)
+    if query.project
+      user.allowed_in_any_work_package?(:view_work_packages, in_project: query.project)
+    else
+      user.allowed_in_any_work_package?(:view_work_packages)
     end
-
-    @view_work_packages_cache[query.project]
   end
 
   def edit_work_packages_allowed?(query)
-    @edit_work_packages_cache ||= Hash.new do |hash, project|
-      hash[project] = allowed_in_project_or_any_project?(:edit_work_packages, project)
+    if query.project
+      user.allowed_in_any_work_package?(:edit_work_packages, in_project: query.project)
+    else
+      user.allowed_in_any_work_package?(:edit_work_packages)
     end
-
-    @edit_work_packages_cache[query.project]
   end
 
   def save_queries_allowed?(query)
-    @save_queries_cache ||= Hash.new do |hash, project|
-      hash[project] = allowed_in_project_or_any_project?(:save_queries, project)
+    if query.project
+      user.allowed_in_project?(:save_queries, query.project)
+    else
+      user.allowed_in_any_project?(:save_queries)
     end
-
-    @save_queries_cache[query.project]
   end
 
   def manage_public_queries_allowed?(query)
-    @manage_public_queries_cache ||= Hash.new do |hash, project|
-      hash[project] = allowed_in_project_or_any_project?(:manage_public_queries, project)
+    if query.project
+      user.allowed_in_project?(:manage_public_queries, query.project)
+    else
+      user.allowed_in_any_project?(:manage_public_queries)
     end
-
-    @manage_public_queries_cache[query.project]
   end
 
   def share_via_ical_allowed?(query)
-    @share_via_ical_cache ||= Hash.new do |hash, project|
-      hash[project] = allowed_in_project_or_any_project?(:share_calendars, project)
-    end
-
-    @share_via_ical_cache[query.project]
-  end
-
-  def allowed_in_project_or_any_project?(permission, project)
-    if project
-      user.allowed_in_project?(permission, project)
+    if query.project
+      user.allowed_in_project?(:share_calendars, query.project)
     else
-      user.allowed_in_any_project?(permission)
+      user.allowed_in_any_project?(:share_calendars)
     end
   end
 end
diff --git a/app/policies/work_package_policy.rb b/app/policies/work_package_policy.rb
index a70788dab45..3e5bcaf919f 100644
--- a/app/policies/work_package_policy.rb
+++ b/app/policies/work_package_policy.rb
@@ -56,19 +56,11 @@ class WorkPackagePolicy < BasePolicy
   end
 
   def edit_allowed?(work_package)
-    @edit_cache ||= Hash.new do |hash, wp|
-      hash[wp] = work_package.persisted? && user.allowed_in_work_package?(:edit_work_packages, wp)
-    end
-
-    @edit_cache[work_package]
+    work_package.persisted? && user.allowed_in_work_package?(:edit_work_packages, work_package)
   end
 
   def move_allowed?(work_package)
-    @move_cache ||= Hash.new do |hash, project|
-      hash[project] = user.allowed_in_project?(:move_work_packages, project)
-    end
-
-    @move_cache[work_package.project]
+    user.allowed_in_project?(:move_work_packages, work_package.project)
   end
 
   def copy_allowed?(work_package)
@@ -76,19 +68,11 @@ class WorkPackagePolicy < BasePolicy
   end
 
   def delete_allowed?(work_package)
-    @delete_cache ||= Hash.new do |hash, project|
-      hash[project] = user.allowed_in_project?(:delete_work_packages, project)
-    end
-
-    @delete_cache[work_package.project]
+    user.allowed_in_project?(:delete_work_packages, work_package.project)
   end
 
   def add_allowed?(work_package)
-    @add_cache ||= Hash.new do |hash, project|
-      hash[project] = user.allowed_in_project?(:add_work_packages, project)
-    end
-
-    @add_cache[work_package.project]
+    user.allowed_in_project?(:add_work_packages, work_package.project)
   end
 
   def type_active_in_project?(work_package)
@@ -102,26 +86,14 @@ class WorkPackagePolicy < BasePolicy
   end
 
   def manage_subtasks_allowed?(work_package)
-    @manage_subtasks_cache ||= Hash.new do |hash, project|
-      hash[project] = user.allowed_in_project?(:manage_subtasks, project)
-    end
-
-    @manage_subtasks_cache[work_package.project]
+    user.allowed_in_project?(:manage_subtasks, work_package.project)
   end
 
   def comment_allowed?(work_package)
-    @comment_cache ||= Hash.new do |hash, wp|
-      hash[wp] = user.allowed_in_work_package?(:add_work_package_notes, wp) || edit_allowed?(wp)
-    end
-
-    @comment_cache[work_package]
+    user.allowed_in_work_package?(:add_work_package_notes, work_package) || edit_allowed?(work_package)
   end
 
   def assign_version_allowed?(work_package)
-    @assign_version_cache ||= Hash.new do |hash, project|
-      hash[project] = user.allowed_in_project?(:assign_versions, project)
-    end
-
-    @assign_version_cache[work_package.project]
+    user.allowed_in_project?(:assign_versions, work_package.project)
   end
 end