From aa53f1fa0936ec2f1306bf3c0058bee01e21703e Mon Sep 17 00:00:00 2001 From: Bruno Pagno Date: Mon, 28 Apr 2025 17:13:25 +0200 Subject: [PATCH] remove internal comments feature flag --- .github/workflows/pullpreview.yml | 1 - .../journals/item_component.html.erb | 2 +- .../journals/item_component/details.rb | 2 +- .../activities_tab/journals/new_component.rb | 3 +-- .../work_packages/create_note_contract.rb | 3 --- .../activities/base_activity_provider.rb | 4 ---- app/models/work_package/journalized.rb | 3 +-- config/initializers/feature_decisions.rb | 4 ---- config/initializers/permissions.rb | 12 ++++------- .../create_note_contract_spec.rb | 21 ++++--------------- .../work_packages_controller_spec.rb | 2 +- .../work_package/activities_spec.rb | 6 ++---- .../activity_tab_comment_editor_spec.rb | 3 +-- .../work_package/internal_comments_spec.rb | 3 +-- .../activities/fetcher_integration_spec.rb | 2 +- .../work_package_acts_as_journalized_spec.rb | 4 ++-- ...ctivities_by_work_package_resource_spec.rb | 2 +- 17 files changed, 21 insertions(+), 56 deletions(-) diff --git a/.github/workflows/pullpreview.yml b/.github/workflows/pullpreview.yml index 9555e9cde86..3396cbe311c 100644 --- a/.github/workflows/pullpreview.yml +++ b/.github/workflows/pullpreview.yml @@ -35,7 +35,6 @@ jobs: echo "OPENPROJECT_HSTS=false" >> .env.pullpreview echo "OPENPROJECT_NOTIFICATIONS_POLLING_INTERVAL=10000" >> .env.pullpreview echo "OPENPROJECT_FEATURE_WORK_PACKAGE_COMMENT_ID_URL_ACTIVE=true" >> .env.pullpreview - echo "OPENPROJECT_FEATURE_COMMENTS_WITH_RESTRICTED_VISIBILITY_ACTIVE=true" >> .env.pullpreview - name: Boot as BIM edition if: contains(github.ref, 'bim/') || contains(github.head_ref, 'bim/') run: | diff --git a/app/components/work_packages/activities_tab/journals/item_component.html.erb b/app/components/work_packages/activities_tab/journals/item_component.html.erb index bc1471f0d89..0123bbb33f0 100644 --- a/app/components/work_packages/activities_tab/journals/item_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component.html.erb @@ -73,7 +73,7 @@ Primer::Beta::Octicon.new( :"dot-fill", # color is set via CSS as requested by UI/UX Team classes: "work-packages-activities-tab-journals-item-component--notification-dot-icon", - size: (OpenProject::FeatureDecisions.internal_comments_active? ? :small : :medium), + size: :small, data: { test_selector: "op-journal-unread-notification", "op-ian-center-update-immediate": true } ) ) diff --git a/app/components/work_packages/activities_tab/journals/item_component/details.rb b/app/components/work_packages/activities_tab/journals/item_component/details.rb index 96e0c60a6ff..09cb9224117 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/details.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/details.rb @@ -193,7 +193,7 @@ module WorkPackages render(Primer::Beta::Octicon.new( :"dot-fill", # color is set via CSS as requested by UI/UX Team classes: "work-packages-activities-tab-journals-item-component-details--notification-dot-icon", - size: (OpenProject::FeatureDecisions.internal_comments_active? ? :small : :medium), + size: :small, data: { test_selector: "op-journal-unread-notification", "op-ian-center-update-immediate": true } )) end diff --git a/app/components/work_packages/activities_tab/journals/new_component.rb b/app/components/work_packages/activities_tab/journals/new_component.rb index 63a30878b68..0f122206934 100644 --- a/app/components/work_packages/activities_tab/journals/new_component.rb +++ b/app/components/work_packages/activities_tab/journals/new_component.rb @@ -60,8 +60,7 @@ module WorkPackages end def adding_internal_comment_allowed? - OpenProject::FeatureDecisions.internal_comments_active? && - work_package.project.enabled_internal_comments && + work_package.project.enabled_internal_comments && User.current.allowed_in_project?(:add_internal_comments, work_package.project) end diff --git a/app/contracts/work_packages/create_note_contract.rb b/app/contracts/work_packages/create_note_contract.rb index 9d35f098a86..2cc990e6054 100644 --- a/app/contracts/work_packages/create_note_contract.rb +++ b/app/contracts/work_packages/create_note_contract.rb @@ -40,9 +40,6 @@ module WorkPackages attribute :journal_internal do next unless model.journal_internal - unless OpenProject::FeatureDecisions.internal_comments_active? - errors.add(:journal_internal, :feature_disabled) - end unless allowed_in_project?(:add_internal_comments) errors.add(:journal_internal, :error_unauthorized) end diff --git a/app/models/activities/base_activity_provider.rb b/app/models/activities/base_activity_provider.rb index 1e5f644dc94..5c2ddef7fad 100644 --- a/app/models/activities/base_activity_provider.rb +++ b/app/models/activities/base_activity_provider.rb @@ -163,10 +163,6 @@ class Activities::BaseActivityProvider end def filter_for_visibility(query, user) - unless OpenProject::FeatureDecisions.internal_comments_active? - return query.where(journals_table[:internal].eq(false)) - end - query.where( projects_table[:id] .in(Project.allowed_to(user, :view_internal_comments).select(:id).arel) diff --git a/app/models/work_package/journalized.rb b/app/models/work_package/journalized.rb index 0901cefd0b8..7f7b946c249 100644 --- a/app/models/work_package/journalized.rb +++ b/app/models/work_package/journalized.rb @@ -34,8 +34,7 @@ module WorkPackage::Journalized included do acts_as_journalized journals_association_extension: proc { def internal_visible - if OpenProject::FeatureDecisions.internal_comments_active? && - proxy_association.owner.project.enabled_internal_comments && + if proxy_association.owner.project.enabled_internal_comments && User.current.allowed_in_project?(:view_internal_comments, proxy_association.owner.project) all else diff --git a/config/initializers/feature_decisions.rb b/config/initializers/feature_decisions.rb index 1a07fb7748c..0a103565628 100644 --- a/config/initializers/feature_decisions.rb +++ b/config/initializers/feature_decisions.rb @@ -60,7 +60,3 @@ OpenProject::FeatureDecisions.add :work_package_comment_id_url, description: "Introduced a new WP comment URL identifier structure " \ "`#comment-` replacing the old " \ "`#activity-`." - -OpenProject::FeatureDecisions.add :internal_comments, - description: "Enables submitting comments that are internal" \ - "and only a subset of users can see" diff --git a/config/initializers/permissions.rb b/config/initializers/permissions.rb index d7404f59602..a8e4ac20ddf 100644 --- a/config/initializers/permissions.rb +++ b/config/initializers/permissions.rb @@ -317,29 +317,25 @@ Rails.application.reloader.to_prepare do {}, permissible_on: %i[project], require: :loggedin, - dependencies: :view_work_packages, - visible: -> { OpenProject::FeatureDecisions.internal_comments_active? } + dependencies: :view_work_packages wpt.permission :add_internal_comments, {}, permissible_on: %i[project], require: :loggedin, - dependencies: %i[view_project view_internal_comments], - visible: -> { OpenProject::FeatureDecisions.internal_comments_active? } + dependencies: %i[view_project view_internal_comments] wpt.permission :edit_own_internal_comments, {}, permissible_on: %i[project], require: :loggedin, - dependencies: %i[view_project view_internal_comments], - visible: -> { OpenProject::FeatureDecisions.internal_comments_active? } + dependencies: %i[view_project view_internal_comments] wpt.permission :edit_others_internal_comments, {}, permissible_on: %i[project], require: :loggedin, - dependencies: %i[view_project view_internal_comments], - visible: -> { OpenProject::FeatureDecisions.internal_comments_active? } + dependencies: %i[view_project view_internal_comments] # WP attachments can be added with :edit_work_packages, this permission allows it without Edit WP as well. wpt.permission :add_work_package_attachments, diff --git a/spec/contracts/work_packages/create_note_contract_spec.rb b/spec/contracts/work_packages/create_note_contract_spec.rb index 6fe7b9c5cc9..c3d00949bf2 100644 --- a/spec/contracts/work_packages/create_note_contract_spec.rb +++ b/spec/contracts/work_packages/create_note_contract_spec.rb @@ -88,17 +88,7 @@ RSpec.describe WorkPackages::CreateNoteContract do work_package.journal_notes = "blubs" end - context "and journal_internal is true, and internal_comments_active? is disabled", - with_flag: { internal_comments_active: false } do - before do - work_package.journal_internal = true - end - - it_behaves_like "contract is invalid", journal_internal: :feature_disabled - end - - context "and journal_internal is true, and internal_comments_active? is enabled", - with_flag: { internal_comments_active: true } do + context "and journal_internal is true" do before do work_package.journal_internal = true end @@ -106,8 +96,7 @@ RSpec.describe WorkPackages::CreateNoteContract do it_behaves_like "contract is valid" end - context "and journal_internal is false, and internal_comments_active? is disabled", - with_flag: { internal_comments_active: false } do + context "and journal_internal is false" do before do work_package.journal_internal = false end @@ -115,8 +104,7 @@ RSpec.describe WorkPackages::CreateNoteContract do it_behaves_like "contract is valid" end - context "with journal_internal is true, internal_comments_active? is active but lacking permissions", - with_flag: { internal_comments_active: true } do + context "with journal_internal is true, but lacking permissions" do let(:permissions) { super() - [:add_internal_comments] } before do @@ -126,8 +114,7 @@ RSpec.describe WorkPackages::CreateNoteContract do it_behaves_like "contract is invalid", journal_internal: :error_unauthorized end - context "with journal_internal is false, internal_comments_active? is active and lacking permissions", - with_flag: { internal_comments_active: true } do + context "with journal_internal is false and lacking permissions" do let(:permissions) { super() - [:add_internal_comments] } before do diff --git a/spec/controllers/work_packages_controller_spec.rb b/spec/controllers/work_packages_controller_spec.rb index b1b5fdf1ec0..90cd592d432 100644 --- a/spec/controllers/work_packages_controller_spec.rb +++ b/spec/controllers/work_packages_controller_spec.rb @@ -313,7 +313,7 @@ RSpec.describe WorkPackagesController do end end - context "when there are internal comments", with_flag: { internal_comments: true } do + context "when there are internal comments" do render_views let(:admin) { create(:admin) } diff --git a/spec/features/activities/work_package/activities_spec.rb b/spec/features/activities/work_package/activities_spec.rb index 71246c0434b..60a34317fba 100644 --- a/spec/features/activities/work_package/activities_spec.rb +++ b/spec/features/activities/work_package/activities_spec.rb @@ -236,8 +236,7 @@ RSpec.describe "Work package activity", :js, :with_cuprite do end end - context "when a user cannot see internal comments", - with_flag: { internal_comments: true } do + context "when a user cannot see internal comments" do current_user { member } before do @@ -257,8 +256,7 @@ RSpec.describe "Work package activity", :js, :with_cuprite do end end - context "when a user can see internal comments", - with_flag: { internal_comments: true } do + context "when a user can see internal comments" do current_user { admin } before do diff --git a/spec/features/activities/work_package/activity_tab_comment_editor_spec.rb b/spec/features/activities/work_package/activity_tab_comment_editor_spec.rb index b0ad47a134d..3fab5263f17 100644 --- a/spec/features/activities/work_package/activity_tab_comment_editor_spec.rb +++ b/spec/features/activities/work_package/activity_tab_comment_editor_spec.rb @@ -32,8 +32,7 @@ require "spec_helper" RSpec.describe "Work package activity tab comment editor", :js, - :with_cuprite, - with_flag: { internal_comments: true } do + :with_cuprite do let(:project) { create(:project) } let(:admin) { create(:admin) } let(:work_package) { create(:work_package, project:, author: admin) } diff --git a/spec/features/activities/work_package/internal_comments_spec.rb b/spec/features/activities/work_package/internal_comments_spec.rb index 1de033c2c3f..d1d266b06fe 100644 --- a/spec/features/activities/work_package/internal_comments_spec.rb +++ b/spec/features/activities/work_package/internal_comments_spec.rb @@ -31,8 +31,7 @@ require "spec_helper" RSpec.describe "Work package internal comments", - :js, - with_flag: { internal_comments: true } do + :js do include InternalCommentsHelpers shared_let(:project) { create(:project, enabled_internal_comments: true) } diff --git a/spec/models/activities/fetcher_integration_spec.rb b/spec/models/activities/fetcher_integration_spec.rb index 1368ff5aa75..425b81199f4 100644 --- a/spec/models/activities/fetcher_integration_spec.rb +++ b/spec/models/activities/fetcher_integration_spec.rb @@ -50,7 +50,7 @@ RSpec.describe Activities::Fetcher, "integration" do .not_to include("budgets") end - describe "#events", with_flag: { internal_comments: true } do + describe "#events" do let(:event_user) { user } let(:work_package) { create(:work_package, project:, author: event_user) } let(:forum) { create(:forum, project:) } diff --git a/spec/models/work_package/work_package_acts_as_journalized_spec.rb b/spec/models/work_package/work_package_acts_as_journalized_spec.rb index 90218b4b044..e538e119c6e 100644 --- a/spec/models/work_package/work_package_acts_as_journalized_spec.rb +++ b/spec/models/work_package/work_package_acts_as_journalized_spec.rb @@ -902,7 +902,7 @@ RSpec.describe WorkPackage do login_as user end - context "when internal_comments is enabled", with_flag: { internal_comments: true } do + context "when internal_comments is enabled" do context "and setting is enabled for the project" do before do work_package.project.enabled_internal_comments = true @@ -952,7 +952,7 @@ RSpec.describe WorkPackage do end end - context "when internal_comments is disabled", with_flag: { internal_comments: false } do + context "when internal_comments is disabled" do before do mock_permissions_for(user) do |mock| mock.allow_in_project(:view_internal_comments, project: work_package.project) diff --git a/spec/requests/api/v3/activities_by_work_package_resource_spec.rb b/spec/requests/api/v3/activities_by_work_package_resource_spec.rb index 594dd3b3120..9876ba410a5 100644 --- a/spec/requests/api/v3/activities_by_work_package_resource_spec.rb +++ b/spec/requests/api/v3/activities_by_work_package_resource_spec.rb @@ -51,7 +51,7 @@ RSpec.describe API::V3::Activities::ActivitiesByWorkPackageAPI do # rubocop:disa allow(User).to receive(:current).and_return(current_user) end - describe "GET /api/v3/work_packages/:id/activities", with_flag: { internal_comments: true } do + describe "GET /api/v3/work_packages/:id/activities" do context "when activities do not include internal journals" do before do get api_v3_paths.work_package_activities work_package.id