Merge branch 'limitable' into 'master'

Reuse Limitable concern with shared examples

See merge request gitlab-org/gitlab!26400

doc/development/application_limits.md

@@ -15,10 +15,6 @@ limits](https://about.gitlab.com/handbook/product/#introducing-application-limit
 
 ## Development
 
-The merge request to [configure maximum number of webhooks per
-project](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/20730/diffs) is a
-good example about configuring application limits.
-
 ### Insert database plan limits
 
 In the `plan_limits` table, you have to create a new column and insert the
@@ -78,12 +74,22 @@ can be used to validate that a model does not exceed the limits. It ensures
 that the count of the records for the current model does not exceed the defined
 limit.
 
-NOTE: **Note:** The name (pluralized) of the plan limit introduced in the
-database (`project_hooks`) must correspond to the name of the model we are
-validating (`ProjectHook`).
+NOTE: **Note:** You must specify the limit scope of the object being validated
+and the limit name if it's different from the pluralized model name.
 
 ```ruby
 class ProjectHook
   include Limitable
+
+  self.limit_name = 'project_hooks' # Optional as ProjectHook corresponds with project_hooks
+  self.limit_scope = :project
+end
+```
+
+To test the model, you can include the shared examples.
+
+```ruby
+it_behaves_like 'includes Limitable concern' do
+  subject { build(:project_hook, project: create(:project)) }
 end
 ```

ee/app/models/concerns/ee/limitable.rb

-# frozen_string_literal: true
-
-module EE
-  module Limitable
-    extend ActiveSupport::Concern
-
-    included do
-      validate :validate_plan_limit_not_exceeded, on: :create
-    end
-
-    private
-
-    def validate_plan_limit_not_exceeded
-      return unless project
-
-      limit_name = self.class.name.demodulize.tableize
-      relation = self.class.where(project: project)
-
-      if project.actual_limits.exceeded?(limit_name, relation)
-        errors.add(:base, _("Maximum number of %{name} (%{count}) exceeded") %
-          { name: limit_name.humanize(capitalize: false), count: project.actual_limits.public_send(limit_name) }) # rubocop:disable GitlabSecurity/PublicSend
-      end
-    end
-  end
-end

ee/app/models/concerns/limitable.rb

+# frozen_string_literal: true
+
+module Limitable
+  extend ActiveSupport::Concern
+
+  included do
+    class_attribute :limit_scope
+    class_attribute :limit_name
+    self.limit_name = self.name.demodulize.tableize
+
+    validate :validate_plan_limit_not_exceeded, on: :create
+  end
+
+  private
+
+  def validate_plan_limit_not_exceeded
+    scope_relation = self.public_send(limit_scope) # rubocop:disable GitlabSecurity/PublicSend
+    return unless scope_relation
+
+    relation = self.class.where(limit_scope => scope_relation)
+
+    if scope_relation.actual_limits.exceeded?(limit_name, relation)
+      errors.add(:base, _("Maximum number of %{name} (%{count}) exceeded") %
+        { name: limit_name.humanize(capitalize: false), count: scope_relation.actual_limits.public_send(limit_name) }) # rubocop:disable GitlabSecurity/PublicSend
+    end
+  end
+end

ee/app/models/ee/project_hook.rb

@@ -8,6 +8,7 @@ module EE
       include CustomModelNaming
       include Limitable
 
+      self.limit_scope = :project
       self.singular_route_key = :hook
     end
   end

ee/app/models/hooks/group_hook.rb

 # frozen_string_literal: true
 
-class GroupHook < ProjectHook
+class GroupHook < WebHook
   include CustomModelNaming
   include TriggerableHooks
+  include Limitable
 
+  self.limit_name = 'group_hooks'
+  self.limit_scope = :group
   self.singular_route_key = :hook
 
   triggerable_hooks [
@@ -20,22 +23,9 @@ class GroupHook < ProjectHook
 
   belongs_to :group
 
-  clear_validators!
   validates :url, presence: true, addressable_url: true
-  validate :validate_group_hook_limits_not_exceeded, on: :create
 
   def pluralized_name
     _('Group Hooks')
   end
-
-  private
-
-  def validate_group_hook_limits_not_exceeded
-    return unless group
-
-    if group.actual_limits.exceeded?(:group_hooks, GroupHook.where(group: group))
-      errors.add(:base, _("Maximum number of group hooks (%{count}) exceeded") %
-        { count: group.actual_limits.group_hooks })
-    end
-  end
 end

ee/spec/factories/plan_limits.rb

@@ -4,5 +4,9 @@
 FactoryBot.define do
   factory :plan_limits do
     plan
+
+    trait :default_plan do
+      plan factory: :default_plan
+    end
   end
 end

ee/spec/models/ee/project_hook_spec.rb

@@ -3,34 +3,7 @@
 require 'spec_helper'
 
 describe ProjectHook do
-  subject(:project_hook) { build(:project_hook, project: project) }
-
-  let(:gold_plan) { create(:gold_plan) }
-  let(:plan_limits) { create(:plan_limits, plan: gold_plan) }
-  let(:namespace) { create(:namespace) }
-  let(:project) { create(:project, namespace: namespace) }
-  let!(:subscription) { create(:gitlab_subscription, namespace: namespace, hosted_plan: gold_plan) }
-
-  context 'without plan limits configured' do
-    it 'can create new project hooks' do
-      expect { project_hook.save }.to change { described_class.count }
-    end
-  end
-
-  context 'with plan limits configured' do
-    before do
-      plan_limits.update(project_hooks: 1)
-    end
-
-    it 'can create new project hooks' do
-      expect { project_hook.save }.to change { described_class.count }
-    end
-
-    it 'cannot create new project hooks exceding the plan limits' do
-      create(:project_hook, project: project)
-
-      expect { project_hook.save }.not_to change { described_class.count }
-      expect(project_hook.errors[:base]).to contain_exactly('Maximum number of project hooks (1) exceeded')
-    end
+  it_behaves_like 'includes Limitable concern' do
+    subject { build(:project_hook, project: create(:project)) }
   end
 end

ee/spec/models/hooks/group_hook_spec.rb

@@ -3,39 +3,11 @@
 require 'spec_helper'
 
 describe GroupHook do
-  describe "Associations" do
+  describe 'associations' do
     it { is_expected.to belong_to :group }
   end
 
-  describe 'validations' do
-    subject(:group_hook) { build(:group_hook, group: group) }
-
-    let(:gold_plan) { create(:gold_plan) }
-    let(:plan_limits) { create(:plan_limits, plan: gold_plan) }
-    let(:group) { create(:group) }
-    let!(:subscription) { create(:gitlab_subscription, namespace: group, hosted_plan: gold_plan) }
-
-    context 'without plan limits configured' do
-      it 'can create new group hooks' do
-        expect { group_hook.save }.to change { described_class.count }
-      end
-    end
-
-    context 'with plan limits configured' do
-      before do
-        plan_limits.update(group_hooks: 1)
-      end
-
-      it 'can create new group hooks' do
-        expect { group_hook.save }.to change { described_class.count }
-      end
-
-      it 'cannot create new group hooks exceding the plan limits' do
-        create(:group_hook, group: group)
-
-        expect { group_hook.save }.not_to change { described_class.count }
-        expect(group_hook.errors[:base]).to contain_exactly('Maximum number of group hooks (1) exceeded')
-      end
-    end
+  it_behaves_like 'includes Limitable concern' do
+    subject { build(:group_hook, group: create(:group)) }
   end
 end

ee/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'includes Limitable concern' do
+  describe 'validations' do
+    let(:plan_limits) { create(:plan_limits, :default_plan) }
+
+    it { is_expected.to be_a(Limitable) }
+
+    context 'without plan limits configured' do
+      it 'can create new group hooks' do
+        expect { subject.save }.to change { described_class.count }
+      end
+    end
+
+    context 'with plan limits configured' do
+      before do
+        plan_limits.update(subject.class.limit_name => 1)
+      end
+
+      it 'can create new models' do
+        expect { subject.save }.to change { described_class.count }
+      end
+
+      context 'with an existing model' do
+        before do
+          subject.dup.save
+        end
+
+        it 'cannot create new models exceding the plan limits' do
+          expect { subject.save }.not_to change { described_class.count }
+          expect(subject.errors[:base]).to contain_exactly("Maximum number of #{subject.class.limit_name.humanize(capitalize: false)} (1) exceeded")
+        end
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -12051,9 +12051,6 @@ msgstr ""
 msgid "Maximum number of comments exceeded"
 msgstr ""
 
-msgid "Maximum number of group hooks (%{count}) exceeded"
-msgstr ""
-
 msgid "Maximum number of mirrors that can be synchronizing at the same time."
 msgstr ""