Bug/53772 the label for spent time is still visible after deactiving the module time and costs active modules

app/controllers/groups_controller.rb

@@ -155,7 +155,8 @@ def group_members
   end
 
   def visible_group_members?
-    current_user.allowed_in_any_project?(:manage_members) ||
+    current_user.admin? ||
+      current_user.allowed_in_any_project?(:manage_members) ||
       Group.in_project(Project.allowed_to(current_user, :view_members)).exists?
   end
 

app/models/project.rb

@@ -252,6 +252,13 @@ def enabled_module_names
     enabled_modules.map(&:name)
   end
 
+  def reload(*)
+    @allowed_permissions = nil
+    @allowed_actions = nil
+
+    super
+  end
+
   def allowed_permissions
     @allowed_permissions ||=
       begin

app/models/users/permission_checks.rb

@@ -106,14 +106,24 @@ def member_of?(project)
     roles_for_project(project).any?(&:member?)
   end
 
+  # Returns all permissions the user may have for a given context.
+  # "May" because this method does not check e.g. whether the module
+  # the permission belongs to is active.
   def all_permissions_for(context)
-    Authorization
-      .roles(self, context)
-      .includes(:role_permissions)
-      .pluck(:permission)
-      .compact
-      .map(&:to_sym)
-      .uniq
+    if admin?
+      OpenProject::AccessControl
+        .permissions
+        .select { |p| p.permissible_on?(context) && p.grant_to_admin? }
+        .map(&:name)
+    else
+      Authorization
+        .roles(self, context)
+        .includes(:role_permissions)
+        .pluck(:permission)
+        .compact
+        .map(&:to_sym)
+        .uniq
+    end
   end
 
   # Helper method to be used in places where we just throw anything into the permission check and don't know what

app/services/authorization/user_permissible_service.rb

@@ -9,7 +9,6 @@ def initialize(user)
     def allowed_globally?(permission)
       perms = contextual_permissions(permission, :global)
       return false unless authorizable_user?
-      return true if admin_and_all_granted_to_admin?(perms)
 
       cached_permissions(nil).intersect?(perms.map(&:name))
     end
@@ -19,17 +18,14 @@ def allowed_in_project?(permission, projects_to_check)
       return false if projects_to_check.blank?
       return false unless authorizable_user?
 
-      projects = Array(projects_to_check)
-
-      projects.all? do |project|
+      Array(projects_to_check).all? do |project|
         allowed_in_single_project?(perms, project)
       end
     end
 
     def allowed_in_any_project?(permission)
       perms = contextual_permissions(permission, :project)
       return false unless authorizable_user?
-      return true if admin_and_all_granted_to_admin?(perms)
 
       cached_in_any_project?(perms)
     end
@@ -51,7 +47,6 @@ def allowed_in_any_entity?(permission, entity_class, in_project: nil)
       perms = contextual_permissions(permission, context_name(entity_class))
       return false unless authorizable_user?
       return false if in_project && !(in_project.active? || in_project.being_archived?)
-      return true if admin_and_all_granted_to_admin?(perms)
 
       if entity_is_project_scoped?(entity_class)
         allowed_in_any_project_scoped_entity?(perms, entity_class, in_project:)
@@ -85,7 +80,6 @@ def allowed_in_single_project?(permissions, project)
       permissions_filtered_for_project = permissions_by_enabled_project_modules(project, permissions)
 
       return false if permissions_filtered_for_project.empty?
-      return true if admin_and_all_granted_to_admin?(permissions)
 
       cached_permissions(project).intersect?(permissions_filtered_for_project)
     end
@@ -106,7 +100,6 @@ def allowed_in_single_project_scoped_entity?(permissions, entity)
       permissions_filtered_for_project = permissions_by_enabled_project_modules(entity.project, permissions)
 
       return false if permissions_filtered_for_project.empty?
-      return true if admin_and_all_granted_to_admin?(permissions)
 
       # The combination of this is better then doing
       # EntityClass.allowed_to(user, permission).exists?.

lib/open_project/access_control/permission.rb

@@ -89,7 +89,28 @@ def project_query?
       end
 
       def permissible_on?(context_type)
-        @permissible_on.include?(context_type)
+        # Sometimes the context_type passed in is a decorated object.
+        # Most of the times, this would then be an 'EagerLoadingWrapper' instance.
+        # We need to unwrap the object to get the actual object.
+        # Checking for `context_type.is_a?(SimpleDelegator)` fails for unknown reasons.
+        context_type = context_type.__getobj__ if context_type.class.ancestors.include?(SimpleDelegator)
+
+        context_symbol = case context_type
+                         when WorkPackage
+                           :work_package
+                         when Project
+                           :project
+                         when ::Queries::Projects::ProjectQuery
+                           :project_query
+                         when Symbol
+                           context_type
+                         when nil
+                           :global
+                         else
+                           raise "Unknown context: #{context_type}"
+                         end
+
+        @permissible_on.include?(context_symbol)
       end
 
       def grant_to_admin?

modules/boards/spec/features/menu_items/global_menu_item_spec.rb

@@ -32,6 +32,7 @@
 require "spec_helper"
 
 RSpec.describe "Global menu item for boards", :js, :with_cuprite do
+  shared_let(:project) { create(:project) }
   let(:boards_label) { I18n.t("boards.label_boards") }
 
   before do

modules/boards/spec/features/menu_items/top_menu_item_spec.rb

@@ -29,14 +29,14 @@
 require "spec_helper"
 
 RSpec.describe "Top menu item for boards", :js, :with_cuprite do
-  let(:user) { create(:admin) }
+  shared_let(:admin) { create(:admin) }
+  shared_let(:user) { create(:user) }
+  shared_let(:project) { create(:project) }
 
   let(:menu) { find(".op-app-menu a[title='#{I18n.t('label_modules')}']") }
   let(:boards) { I18n.t("boards.label_boards") }
 
-  before do
-    allow(User).to receive(:current).and_return user
-  end
+  current_user { admin }
 
   shared_examples_for "the boards menu item" do
     it "sends the user to the boards overview when clicked" do
@@ -52,8 +52,6 @@
   end
 
   context "when in the project settings" do
-    let!(:project) { create(:project) }
-
     before do
       visit "/projects/#{project.identifier}/settings/general"
     end
@@ -69,7 +67,7 @@
     it_behaves_like "the boards menu item"
 
     context "with missing permissions" do
-      let(:user) { create(:user) }
+      current_user { user }
 
       it "does not display the menu item" do
         within "#more-menu", visible: false do

modules/meeting/spec/controllers/meetings_controller_spec.rb

@@ -29,19 +29,11 @@
 require "#{File.dirname(__FILE__)}/../spec_helper"
 
 RSpec.describe MeetingsController do
-  let(:user) { create(:admin) }
-  let(:project) { create(:project) }
-  let(:other_project) { create(:project) }
+  shared_let(:user) { create(:admin) }
+  shared_let(:project) { create(:project) }
+  shared_let(:other_project) { create(:project) }
 
-  before do
-    allow(User).to receive(:current).and_return user
-
-    allow(Project).to receive(:find).and_return(project)
-
-    allow(controller).to receive(:authorize)
-    allow(controller).to receive(:authorize_global)
-    allow(controller).to receive(:check_if_login_required)
-  end
+  current_user { user }
 
   describe "GET" do
     describe "index" do
@@ -161,8 +153,6 @@
       let(:meeting_params) { base_meeting_params }
 
       before do
-        allow(Project).to receive(:find).and_return(project)
-
         post :create,
              params:
       end

modules/meeting/spec/features/meetings_global_menu_item_spec.rb

@@ -36,6 +36,7 @@
                :with_cuprite do
   shared_let(:user_without_permissions) { create(:user) }
   shared_let(:admin) { create(:admin) }
+  shared_let(:project) { create(:project) }
   shared_let(:meetings_label) { I18n.t(:label_meeting_plural) }
 
   let(:meetings_page) { Pages::Meetings::Index.new(project: nil) }

modules/reporting/spec/features/top_menu_item_spec.rb

@@ -29,6 +29,8 @@
 require "spec_helper"
 
 RSpec.describe "Top menu items", :js do
+  shared_let(:project) { create(:project) }
+
   let(:user) { create(:user) }
   let(:open_menu) { true }
 

modules/reporting/spec/workers/cost_query/export_job_spec.rb

@@ -31,7 +31,7 @@
 require "spec_helper"
 
 RSpec.describe CostQuery::ExportJob do
-  let(:user) { build_stubbed(:admin) }
+  let(:user) { build_stubbed(:user) }
   let(:project) { build_stubbed(:project) }
 
   let(:initial_filter_params) do
@@ -46,6 +46,10 @@
     }
   end
 
+  before do
+    mock_permissions_for(user, &:allow_everything)
+  end
+
   # Performs a cost export with the given extra filters.
   #
   # @param extra_filters [Hash] A hash of attribute names and operator/value

spec/controllers/activities_controller_spec.rb

@@ -30,6 +30,8 @@
 
 RSpec.describe ActivitiesController do
   shared_let(:admin) { create(:admin) }
+  shared_let(:project) { create(:project) }
+
   current_user { admin }
 
   before do

spec/controllers/news_controller_spec.rb

@@ -33,15 +33,10 @@
 
   include BecomeMember
 
-  let(:user) do
-    create(:admin)
-  end
-  let(:project) { create(:project) }
   let(:news) { create(:news) }
 
-  before do
-    allow(User).to receive(:current).and_return user
-  end
+  shared_let(:project) { create(:project) }
+  shared_current_user { create(:admin) }
 
   describe "#index" do
     it "renders index" do
@@ -101,7 +96,7 @@
   describe "#create" do
     context "with news_added notifications" do
       it "persists a news item" do
-        become_member(project, user)
+        become_member(project, current_user)
 
         post :create,
              params: {
@@ -117,7 +112,7 @@
         news = News.find_by!(title: "NewsControllerTest")
         expect(news).not_to be_nil
         expect(news.description).to eq "This is the description"
-        expect(news.author).to eq user
+        expect(news.author).to eq current_user
         expect(news.project).to eq project
       end
     end

spec/features/custom_fields/create_long_text_spec.rb

@@ -6,7 +6,7 @@
   let(:cf_page) { Pages::CustomFields.new }
   let(:editor) { Components::WysiwygEditor.new "#custom_field_form" }
   let(:type) { create(:type_task) }
-  let(:project) { create(:project, enabled_module_names: %i[work_package_tracking], types: [type]) }
+  let!(:project) { create(:project, enabled_module_names: %i[work_package_tracking], types: [type]) }
 
   let(:wp_page) { Pages::FullWorkPackageCreate.new project: }
 

spec/features/news/global_menu_item_spec.rb

@@ -34,6 +34,7 @@
 RSpec.describe "News global menu item spec", :js, :with_cuprite do
   shared_let(:admin) { create(:admin) }
   shared_let(:user_without_permissions) { create(:user) }
+  shared_let(:project) { create(:project) }
 
   before do
     login_as current_user

spec/features/projects/global_menu_item_spec.rb

@@ -32,10 +32,12 @@
 require "spec_helper"
 
 RSpec.describe "Projects global menu item", :js, :with_cuprite do
-  let(:user) { create(:user) }
+  shared_let(:user) { create(:user) }
+  shared_let(:admin) { create(:admin) }
+
+  current_user { user }
 
   before do
-    login_as user
     visit root_path
   end
 
@@ -65,7 +67,7 @@
     end
 
     context "with an admin user" do
-      let(:user) { create(:admin) }
+      current_user { admin }
 
       it "renders the archived filter as well" do
         within "#main-menu" do

spec/features/types/form_configuration_spec.rb

@@ -32,7 +32,7 @@
   shared_let(:admin) { create(:admin) }
   let(:type) { create(:type) }
 
-  let(:project) { create(:project, types: [type]) }
+  let!(:project) { create(:project, types: [type]) }
   let(:category) { create(:category, project:) }
   let(:work_package) do
     create(:work_package,

spec/features/work_packages/work_package_index_spec.rb

@@ -29,13 +29,12 @@
 require "spec_helper"
 
 RSpec.describe "Work Packages", "index view", :js, :with_cuprite do
-  let(:user) { create(:admin) }
-  let(:project) { create(:project, enabled_module_names: %w[work_package_tracking]) }
+  shared_let(:user) { create(:admin) }
+  shared_let(:project) { create(:project, enabled_module_names: %w[work_package_tracking]) }
+
   let(:wp_table) { Pages::WorkPackagesTable.new(project) }
 
-  before do
-    login_as(user)
-  end
+  current_user { user }
 
   context "within a global context" do
     before do