Commit 396c4931 authored by Markus Koller's avatar Markus Koller Committed by Robert Speicher

Add license check for group wikis

When we originally started working on this, it wasn't clear yet if it
would be a paid feature. Shortly afterwards we decided to put it into
EE Starter and moved the code into `ee/`, but forgot to add the license
checks.

- Rename feature flag to `group_wikis_feature_flag` to align with the
  feature name in the `License `model, which is plural as well.
  We have to use a different name for the FF because licensed features
  need to default to on.

- `EE::Group#wiki_access_level` is removed since this was only added
  to support the shared examples for wiki policies, which also expect
  the `private` access level to be supported. In practice we don't
  support this yet for group wikis, so now we just skip these specs
  instead.

- The license and feature flag checks are centralized in the group
  policy, so we only test these special cases there, and otherwise
  just call our new `stub_group_wikis` helper to test the general
  case of the feature not being available.
parent ea9078dd
...@@ -378,16 +378,6 @@ module EE ...@@ -378,16 +378,6 @@ module EE
end end
end end
def wiki_access_level
# TODO: Remove this method once we implement group-level features.
# https://gitlab.com/gitlab-org/gitlab/-/issues/208412
if ::Feature.enabled?(:group_wiki, self)
::ProjectFeature::ENABLED
else
::ProjectFeature::DISABLED
end
end
def owners_emails def owners_emails
owners.pluck(:email) owners.pluck(:email)
end end
......
...@@ -19,6 +19,7 @@ class License < ApplicationRecord ...@@ -19,6 +19,7 @@ class License < ApplicationRecord
group_activity_analytics group_activity_analytics
group_bulk_edit group_bulk_edit
group_webhooks group_webhooks
group_wikis
issuable_default_templates issuable_default_templates
issue_weights issue_weights
iterations iterations
......
...@@ -10,19 +10,19 @@ module EE ...@@ -10,19 +10,19 @@ module EE
with_scope :subject with_scope :subject
condition(:ldap_synced) { @subject.ldap_synced? } condition(:ldap_synced) { @subject.ldap_synced? }
condition(:epics_available) { @subject.feature_available?(:epics) } condition(:epics_available) { feature_available?(:epics) }
condition(:iterations_available) { @subject.feature_available?(:iterations) } condition(:iterations_available) { feature_available?(:iterations) }
condition(:subepics_available) { @subject.feature_available?(:subepics) } condition(:subepics_available) { feature_available?(:subepics) }
condition(:contribution_analytics_available) do condition(:contribution_analytics_available) do
@subject.feature_available?(:contribution_analytics) feature_available?(:contribution_analytics)
end end
condition(:cycle_analytics_available) do condition(:cycle_analytics_available) do
@subject.feature_available?(:cycle_analytics_for_groups) feature_available?(:cycle_analytics_for_groups)
end end
condition(:group_merge_request_analytics_available) do condition(:group_merge_request_analytics_available) do
@subject.feature_available?(:group_merge_request_analytics) feature_available?(:group_merge_request_analytics)
end end
condition(:group_repository_analytics_available) do condition(:group_repository_analytics_available) do
...@@ -30,7 +30,7 @@ module EE ...@@ -30,7 +30,7 @@ module EE
end end
condition(:group_activity_analytics_available) do condition(:group_activity_analytics_available) do
@subject.feature_available?(:group_activity_analytics) && ::Feature.enabled?(:group_activity_analytics, @subject, default_enabled: true) feature_available?(:group_activity_analytics) && ::Feature.enabled?(:group_activity_analytics, @subject, default_enabled: true)
end end
condition(:can_owners_manage_ldap, scope: :global) do condition(:can_owners_manage_ldap, scope: :global) do
...@@ -46,11 +46,11 @@ module EE ...@@ -46,11 +46,11 @@ module EE
end end
condition(:security_dashboard_enabled) do condition(:security_dashboard_enabled) do
@subject.feature_available?(:security_dashboard) feature_available?(:security_dashboard)
end end
condition(:prevent_group_forking_available) do condition(:prevent_group_forking_available) do
@subject.feature_available?(:group_forking_protection) feature_available?(:group_forking_protection)
end end
condition(:needs_new_sso_session) do condition(:needs_new_sso_session) do
...@@ -62,11 +62,11 @@ module EE ...@@ -62,11 +62,11 @@ module EE
end end
condition(:dependency_proxy_available) do condition(:dependency_proxy_available) do
@subject.feature_available?(:dependency_proxy) feature_available?(:dependency_proxy)
end end
condition(:cluster_deployments_available) do condition(:cluster_deployments_available) do
@subject.feature_available?(:cluster_deployments) feature_available?(:cluster_deployments)
end end
condition(:group_saml_enabled) do condition(:group_saml_enabled) do
...@@ -74,7 +74,7 @@ module EE ...@@ -74,7 +74,7 @@ module EE
end end
condition(:group_timelogs_available) do condition(:group_timelogs_available) do
@subject.feature_available?(:group_timelogs) feature_available?(:group_timelogs)
end end
with_scope :global with_scope :global
...@@ -88,15 +88,15 @@ module EE ...@@ -88,15 +88,15 @@ module EE
end end
condition(:commit_committer_check_available) do condition(:commit_committer_check_available) do
@subject.feature_available?(:commit_committer_check) feature_available?(:commit_committer_check)
end end
condition(:reject_unsigned_commits_available) do condition(:reject_unsigned_commits_available) do
@subject.feature_available?(:reject_unsigned_commits) feature_available?(:reject_unsigned_commits)
end end
condition(:push_rules_available) do condition(:push_rules_available) do
@subject.feature_available?(:push_rules) feature_available?(:push_rules)
end end
condition(:over_storage_limit, scope: :subject) { @subject.over_storage_limit? } condition(:over_storage_limit, scope: :subject) { @subject.over_storage_limit? }
...@@ -246,7 +246,7 @@ module EE ...@@ -246,7 +246,7 @@ module EE
end end
desc "Group has wiki disabled" desc "Group has wiki disabled"
condition(:wiki_disabled, score: 32) { !feature_available?(:wiki) } condition(:wiki_disabled, score: 32) { !feature_available?(:group_wikis) }
rule { wiki_disabled }.policy do rule { wiki_disabled }.policy do
prevent(*create_read_update_admin_destroy(:wiki)) prevent(*create_read_update_admin_destroy(:wiki))
...@@ -304,18 +304,16 @@ module EE ...@@ -304,18 +304,16 @@ module EE
super super
end end
# TODO: Extract this into a helper shared with ProjectPolicy, once we implement group-level features. # TODO: Once we implement group-level feature toggles, see if we can refactor
# the shared logic in ProjectPolicy and GroupPolicy.
# https://gitlab.com/gitlab-org/gitlab/-/issues/208412 # https://gitlab.com/gitlab-org/gitlab/-/issues/208412
def feature_available?(feature) def feature_available?(feature)
return false unless feature == :wiki if feature == :group_wikis
# TODO: Remove this special case when we remove the feature flag
case subject.wiki_access_level # https://gitlab.com/gitlab-org/gitlab/-/issues/207888
when ::ProjectFeature::DISABLED ::Feature.enabled?(:group_wikis_feature_flag, subject) && subject.feature_available?(feature)
false
when ::ProjectFeature::PRIVATE
admin? || access_level >= ::ProjectFeature.required_minimum_access_level(feature)
else else
true subject.feature_available?(feature)
end end
end end
......
--- ---
name: group_wiki name: group_wikis_feature_flag
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29176 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29176
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/207888 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/207888
group: group::knowledge group: group::knowledge
......
...@@ -3,18 +3,19 @@ ...@@ -3,18 +3,19 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Groups::WikisController do RSpec.describe Groups::WikisController do
include WikiHelpers
it_behaves_like 'wiki controller actions' do it_behaves_like 'wiki controller actions' do
let(:container) { create(:group, :public) } let(:container) { create(:group, :public) }
let(:routing_params) { { group_id: container } } let(:routing_params) { { group_id: container } }
before do before do
container.add_owner(user) container.add_owner(user)
stub_feature_flags(group_wiki: container)
end end
context 'when the feature flag is disabled' do context 'when the feature is not available' do
before do before do
stub_feature_flags(group_wiki: false) stub_group_wikis(false)
end end
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
......
...@@ -5,6 +5,7 @@ require 'spec_helper' ...@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe 'Group navbar' do RSpec.describe 'Group navbar' do
include NavbarStructureHelper include NavbarStructureHelper
include WaitForRequests include WaitForRequests
include WikiHelpers
include_context 'group navbar structure' include_context 'group navbar structure'
...@@ -14,7 +15,7 @@ RSpec.describe 'Group navbar' do ...@@ -14,7 +15,7 @@ RSpec.describe 'Group navbar' do
before do before do
group.add_maintainer(user) group.add_maintainer(user)
stub_feature_flags(group_iterations: false) stub_feature_flags(group_iterations: false)
stub_feature_flags(group_wiki: false) stub_group_wikis(false)
sign_in(user) sign_in(user)
insert_package_nav(_('Kubernetes')) insert_package_nav(_('Kubernetes'))
...@@ -216,7 +217,7 @@ RSpec.describe 'Group navbar' do ...@@ -216,7 +217,7 @@ RSpec.describe 'Group navbar' do
context 'when group wiki is available' do context 'when group wiki is available' do
before do before do
stub_feature_flags(group_wiki: true) stub_group_wikis(true)
insert_after_nav_item( insert_after_nav_item(
_('Analytics'), _('Analytics'),
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::GitAccessWiki do RSpec.describe Gitlab::GitAccessWiki do
include WikiHelpers
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :wiki_repo) } let(:project) { create(:project, :wiki_repo) }
let(:changes) { ['6f6d7e7ed 570e7b2ab refs/heads/master'] } let(:changes) { ['6f6d7e7ed 570e7b2ab refs/heads/master'] }
...@@ -76,7 +78,7 @@ RSpec.describe Gitlab::GitAccessWiki do ...@@ -76,7 +78,7 @@ RSpec.describe Gitlab::GitAccessWiki do
context 'when wiki feature is disabled' do context 'when wiki feature is disabled' do
before do before do
stub_feature_flags(group_wiki: false) stub_group_wikis(false)
end end
it_behaves_like 'forbidden git access' do it_behaves_like 'forbidden git access' do
...@@ -100,7 +102,7 @@ RSpec.describe Gitlab::GitAccessWiki do ...@@ -100,7 +102,7 @@ RSpec.describe Gitlab::GitAccessWiki do
context 'when wiki feature is disabled' do context 'when wiki feature is disabled' do
before do before do
stub_feature_flags(group_wiki: false) stub_group_wikis(false)
end end
it_behaves_like 'forbidden git access' do it_behaves_like 'forbidden git access' do
......
...@@ -1065,20 +1065,37 @@ RSpec.describe GroupPolicy do ...@@ -1065,20 +1065,37 @@ RSpec.describe GroupPolicy do
end end
it_behaves_like 'model with wiki policies' do it_behaves_like 'model with wiki policies' do
let_it_be(:container) { create(:group) } include WikiHelpers
let_it_be(:container) { create(:group_with_plan, plan: :bronze_plan) }
let_it_be(:user) { owner } let_it_be(:user) { owner }
# We don't have feature toggles on groups yet, so we currently simulate
# this by toggling the feature flag instead.
def set_access_level(access_level) def set_access_level(access_level)
allow(container).to receive(:wiki_access_level).and_return(access_level) case access_level
when ProjectFeature::ENABLED
stub_feature_flags(group_wikis_feature_flag: true)
when ProjectFeature::DISABLED
stub_feature_flags(group_wikis_feature_flag: false)
when ProjectFeature::PRIVATE
skip('Access level private is not supported yet for group wikis, see https://gitlab.com/gitlab-org/gitlab/-/issues/208412')
end
end end
context 'when the feature flag is disabled on this group' do
before do before do
stub_feature_flags(group_wiki: true) stub_feature_flags(group_wikis_feature_flag: create(:group))
end
it 'does not include the wiki permissions' do
expect_disallowed(*wiki_permissions[:all])
end
end end
context 'when the feature flag is disabled' do context 'when the feature is not licensed on this group' do
before do before do
stub_feature_flags(group_wiki: false) allow(container).to receive(:feature_available?).with(:group_wikis).and_return(false)
end end
it 'does not include the wiki permissions' do it 'does not include the wiki permissions' do
......
...@@ -13,6 +13,7 @@ require 'spec_helper' ...@@ -13,6 +13,7 @@ require 'spec_helper'
# because they are 3 edge cases of using wiki pages. # because they are 3 edge cases of using wiki pages.
RSpec.describe API::Wikis do RSpec.describe API::Wikis do
include WikiHelpers
include WorkhorseHelpers include WorkhorseHelpers
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
...@@ -31,7 +32,7 @@ RSpec.describe API::Wikis do ...@@ -31,7 +32,7 @@ RSpec.describe API::Wikis do
context 'when group wiki is disabled' do context 'when group wiki is disabled' do
before do before do
stub_feature_flags(group_wiki: false) stub_group_wikis(false)
end end
context 'when user is guest' do context 'when user is guest' do
...@@ -125,7 +126,7 @@ RSpec.describe API::Wikis do ...@@ -125,7 +126,7 @@ RSpec.describe API::Wikis do
context 'when wiki is disabled' do context 'when wiki is disabled' do
before do before do
stub_feature_flags(group_wiki: false) stub_group_wikis(false)
end end
context 'when user is guest' do context 'when user is guest' do
...@@ -250,7 +251,7 @@ RSpec.describe API::Wikis do ...@@ -250,7 +251,7 @@ RSpec.describe API::Wikis do
context 'when wiki is disabled' do context 'when wiki is disabled' do
before do before do
stub_feature_flags(group_wiki: false) stub_group_wikis(false)
end end
context 'when user is guest' do context 'when user is guest' do
...@@ -342,7 +343,7 @@ RSpec.describe API::Wikis do ...@@ -342,7 +343,7 @@ RSpec.describe API::Wikis do
context 'when wiki is disabled' do context 'when wiki is disabled' do
before do before do
stub_feature_flags(group_wiki: false) stub_group_wikis(false)
end end
context 'when user is guest' do context 'when user is guest' do
...@@ -482,7 +483,7 @@ RSpec.describe API::Wikis do ...@@ -482,7 +483,7 @@ RSpec.describe API::Wikis do
context 'when wiki is disabled' do context 'when wiki is disabled' do
before do before do
stub_feature_flags(group_wiki: false) stub_group_wikis(false)
end end
context 'when user is guest' do context 'when user is guest' do
...@@ -613,7 +614,7 @@ RSpec.describe API::Wikis do ...@@ -613,7 +614,7 @@ RSpec.describe API::Wikis do
context 'when wiki is disabled' do context 'when wiki is disabled' do
before do before do
stub_feature_flags(group_wiki: false) stub_group_wikis(false)
end end
context 'when user is guest' do context 'when user is guest' do
......
...@@ -210,9 +210,6 @@ RSpec.describe 'layouts/nav/sidebar/_group' do ...@@ -210,9 +210,6 @@ RSpec.describe 'layouts/nav/sidebar/_group' do
allow(view).to receive(:current_user).and_return(current_user) allow(view).to receive(:current_user).and_return(current_user)
allow(view).to receive(:can?).with(current_user, :read_wiki, group).and_return(can_read_wiki) allow(view).to receive(:can?).with(current_user, :read_wiki, group).and_return(can_read_wiki)
# TODO can be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/207888
stub_feature_flags(group_wiki: true)
end end
describe 'when wiki is available to user' do describe 'when wiki is available to user' do
......
...@@ -4,6 +4,7 @@ require 'spec_helper' ...@@ -4,6 +4,7 @@ require 'spec_helper'
RSpec.describe 'Group navbar' do RSpec.describe 'Group navbar' do
include NavbarStructureHelper include NavbarStructureHelper
include WikiHelpers
include_context 'group navbar structure' include_context 'group navbar structure'
...@@ -49,7 +50,7 @@ RSpec.describe 'Group navbar' do ...@@ -49,7 +50,7 @@ RSpec.describe 'Group navbar' do
insert_package_nav(_('Kubernetes')) insert_package_nav(_('Kubernetes'))
stub_feature_flags(group_iterations: false) stub_feature_flags(group_iterations: false)
stub_feature_flags(group_wiki: false) stub_group_wikis(false)
group.add_maintainer(user) group.add_maintainer(user)
sign_in(user) sign_in(user)
end end
......
...@@ -3,6 +3,11 @@ ...@@ -3,6 +3,11 @@
module WikiHelpers module WikiHelpers
extend self extend self
def stub_group_wikis(enabled)
stub_feature_flags(group_wikis_feature_flag: enabled)
stub_licensed_features(group_wikis: enabled)
end
def wait_for_svg_to_be_loaded(example = nil) def wait_for_svg_to_be_loaded(example = nil)
# Ensure the SVG is loaded first before clicking the button # Ensure the SVG is loaded first before clicking the button
find('.svg-content .js-lazy-loaded') if example.nil? || example.metadata.key?(:js) find('.svg-content .js-lazy-loaded') if example.nil? || example.metadata.key?(:js)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment