Commit 55d15fe7 authored by Arturo Herrero's avatar Arturo Herrero

Reuse Limitable concern with shared examples

- Move Limitable concern under the right directory
- Create shared_examples to reuse in any model
- Use Limitable in GroupHook class
parent 51d962d2
...@@ -15,10 +15,6 @@ limits](https://about.gitlab.com/handbook/product/#introducing-application-limit ...@@ -15,10 +15,6 @@ limits](https://about.gitlab.com/handbook/product/#introducing-application-limit
## Development ## 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 ### Insert database plan limits
In the `plan_limits` table, you have to create a new column and insert the 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 ...@@ -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 that the count of the records for the current model does not exceed the defined
limit. limit.
NOTE: **Note:** The name (pluralized) of the plan limit introduced in the NOTE: **Note:** You must specify the limit scope of the object being validated
database (`project_hooks`) must correspond to the name of the model we are and the limit name if it's different from the pluralized model name.
validating (`ProjectHook`).
```ruby ```ruby
class ProjectHook class ProjectHook
include Limitable 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 end
``` ```
# 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
# 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
...@@ -8,6 +8,7 @@ module EE ...@@ -8,6 +8,7 @@ module EE
include CustomModelNaming include CustomModelNaming
include Limitable include Limitable
self.limit_scope = :project
self.singular_route_key = :hook self.singular_route_key = :hook
end end
end end
......
# frozen_string_literal: true # frozen_string_literal: true
class GroupHook < ProjectHook class GroupHook < WebHook
include CustomModelNaming include CustomModelNaming
include TriggerableHooks include TriggerableHooks
include Limitable
self.limit_name = 'group_hooks'
self.limit_scope = :group
self.singular_route_key = :hook self.singular_route_key = :hook
triggerable_hooks [ triggerable_hooks [
...@@ -20,22 +23,9 @@ class GroupHook < ProjectHook ...@@ -20,22 +23,9 @@ class GroupHook < ProjectHook
belongs_to :group belongs_to :group
clear_validators!
validates :url, presence: true, addressable_url: true validates :url, presence: true, addressable_url: true
validate :validate_group_hook_limits_not_exceeded, on: :create
def pluralized_name def pluralized_name
_('Group Hooks') _('Group Hooks')
end 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 end
...@@ -4,5 +4,9 @@ ...@@ -4,5 +4,9 @@
FactoryBot.define do FactoryBot.define do
factory :plan_limits do factory :plan_limits do
plan plan
trait :default_plan do
plan factory: :default_plan
end
end end
end end
...@@ -3,34 +3,7 @@ ...@@ -3,34 +3,7 @@
require 'spec_helper' require 'spec_helper'
describe ProjectHook do describe ProjectHook do
subject(:project_hook) { build(:project_hook, project: project) } it_behaves_like 'includes Limitable concern' do
subject { build(:project_hook, project: create(: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
end end
end end
...@@ -3,39 +3,11 @@ ...@@ -3,39 +3,11 @@
require 'spec_helper' require 'spec_helper'
describe GroupHook do describe GroupHook do
describe "Associations" do describe 'associations' do
it { is_expected.to belong_to :group } it { is_expected.to belong_to :group }
end end
describe 'validations' do it_behaves_like 'includes Limitable concern' do
subject(:group_hook) { build(:group_hook, group: group) } subject { build(:group_hook, group: create(: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
end end
end end
# 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
...@@ -12051,9 +12051,6 @@ msgstr "" ...@@ -12051,9 +12051,6 @@ msgstr ""
msgid "Maximum number of comments exceeded" msgid "Maximum number of comments exceeded"
msgstr "" msgstr ""
msgid "Maximum number of group hooks (%{count}) exceeded"
msgstr ""
msgid "Maximum number of mirrors that can be synchronizing at the same time." msgid "Maximum number of mirrors that can be synchronizing at the same time."
msgstr "" msgstr ""
......
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