Merge pull request #1522 from Ivanov-Anton/1521-Network-prefixes-chec…

…k-number-length

allow to create network_prefixes with the same prefix

app/models/system/network_prefix.rb

@@ -14,7 +14,7 @@
 #
 # Indexes
 #
-#  network_prefixes_prefix_key        (prefix) UNIQUE
+#  network_prefixes_prefix_key        (prefix,number_min_length,number_max_length) UNIQUE
 #  network_prefixes_prefix_range_idx  (((prefix)::prefix_range)) USING gist
 #  network_prefixes_uuid_key          (uuid) UNIQUE
 #
@@ -51,8 +51,9 @@ class System::NetworkPrefix < ApplicationRecord
               if: :number_max_length
             }
 
-  validates :prefix, uniqueness: { allow_blank: true }, presence: true
+  validates :prefix, presence: true
   validates :network, presence: true
+  validate :validate_prefix_uniqueness, if: :prefix?, on: %i[create update]
 
   scope :number_contains, lambda { |prefix|
     where('prefix_range(sys.network_prefixes.prefix)@>prefix_range(?)', prefix.to_s)
@@ -83,4 +84,19 @@ def self.ransackable_scopes(_auth_object = nil)
       :number_contains
     ]
   end
+
+  private
+
+  def validate_prefix_uniqueness
+    return if find_conflicting_prefixes.empty?
+
+    errors.add(:prefix, :taken)
+    errors.add(:number_max_length, "length with #{prefix} prefix already taken")
+    errors.add(:number_min_length, "length with #{prefix} prefix already taken")
+  end
+
+  def find_conflicting_prefixes
+    base_query = System::NetworkPrefix.where(prefix:, number_max_length:, number_min_length:)
+    persisted? ? base_query.where.not(id: id) : base_query
+  end
 end

db/migrate/20240725132743_change_unique_index_on_the_network_prefixes.rb

@@ -0,0 +1,21 @@
+class ChangeUniqueIndexOnTheNetworkPrefixes < ActiveRecord::Migration[7.0]
+  disable_ddl_transaction!
+
+  def up
+    execute <<~SQL.squish
+      ALTER TABLE ONLY sys.network_prefixes
+      DROP CONSTRAINT network_prefixes_prefix_key;
+
+      CREATE UNIQUE INDEX network_prefixes_prefix_key ON sys.network_prefixes USING btree (prefix, number_min_length, number_max_length);
+    SQL
+  end
+
+  def down
+    execute <<~SQL.squish
+      DROP INDEX IF EXISTS network_prefixes_prefix_key;
+
+      ALTER TABLE ONLY sys.network_prefixes
+      ADD CONSTRAINT network_prefixes_prefix_key UNIQUE (prefix);
+    SQL
+  end
+end

db/structure.sql

@@ -39032,14 +39032,6 @@ ALTER TABLE ONLY sys.network_prefixes
     ADD CONSTRAINT network_prefixes_pkey PRIMARY KEY (id);
 
 
---
--- Name: network_prefixes network_prefixes_prefix_key; Type: CONSTRAINT; Schema: sys; Owner: -
---
-
-ALTER TABLE ONLY sys.network_prefixes
-    ADD CONSTRAINT network_prefixes_prefix_key UNIQUE (prefix);
-
-
 --
 -- Name: network_prefixes network_prefixes_uuid_key; Type: CONSTRAINT; Schema: sys; Owner: -
 --
@@ -39846,6 +39838,13 @@ CREATE INDEX "index_sys.cdr_exports_on_customer_account_id" ON sys.cdr_exports U
 CREATE UNIQUE INDEX "index_sys.cdr_exports_on_uuid" ON sys.cdr_exports USING btree (uuid);
 
 
+--
+-- Name: network_prefixes_prefix_key; Type: INDEX; Schema: sys; Owner: -
+--
+
+CREATE UNIQUE INDEX network_prefixes_prefix_key ON sys.network_prefixes USING btree (prefix, number_min_length, number_max_length);
+
+
 --
 -- Name: network_prefixes_prefix_range_idx; Type: INDEX; Schema: sys; Owner: -
 --
@@ -41166,6 +41165,7 @@ INSERT INTO "public"."schema_migrations" (version) VALUES
 ('20240702142447'),
 ('20240704100545'),
 ('20240721201110'),
+('20240725132743'),
 ('20240725135654');
 
 

spec/factories/system/network_prefixes.rb

@@ -14,7 +14,7 @@
 #
 # Indexes
 #
-#  network_prefixes_prefix_key        (prefix) UNIQUE
+#  network_prefixes_prefix_key        (prefix,number_min_length,number_max_length) UNIQUE
 #  network_prefixes_prefix_range_idx  (((prefix)::prefix_range)) USING gist
 #  network_prefixes_uuid_key          (uuid) UNIQUE
 #

spec/models/system/network_prefix_spec.rb

@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+
+RSpec.describe System::NetworkPrefix, type: :model do
+  describe '#validate_prefix_uniqueness' do
+    subject { network_prefix.errors.messages }
+
+    let(:network_prefix_attrs) { { prefix: '9', number_min_length: 1, number_max_length: 1 } }
+
+    context 'when no conflicting prefixes exist' do
+      let(:network_prefix) { FactoryBot.build(:network_prefix, prefix: '1', number_min_length: 12) }
+
+      before do
+        FactoryBot.create(:network_prefix, network_prefix_attrs)
+        network_prefix.valid?
+      end
+
+      it 'does not add any validation errors' do
+        expect(subject[:number_max_length]).to eq []
+        expect(subject[:number_min_length]).to eq []
+        expect(subject[:prefix]).to eq []
+      end
+    end
+
+    context 'when conflicting prefixes exist' do
+      let(:network_prefix) { FactoryBot.build(:network_prefix, network_prefix_attrs) }
+
+      before do
+        FactoryBot.create(:network_prefix, network_prefix_attrs)
+        network_prefix.valid?
+      end
+
+      it 'adds errors for prefix, number_max_length, and number_min_length' do
+        expect(subject[:prefix]).to contain_exactly('has already been taken')
+        expect(subject[:number_max_length]).to contain_exactly('length with 9 prefix already taken')
+        expect(subject[:number_min_length]).to contain_exactly('length with 9 prefix already taken')
+      end
+    end
+
+    context 'when create network_prefix with "prefix" = nil' do
+      subject { network_prefix.valid? }
+
+      let(:network_prefix) { FactoryBot.build(:network_prefix, prefix: nil) }
+
+      it 'should not execute validation' do
+        expect(network_prefix).not_to receive(:validate_prefix_uniqueness)
+        subject
+      end
+    end
+
+    context 'when update record and there is conflicting record' do
+      subject { record.update(prefix: '2') }
+
+      let!(:record) { FactoryBot.create(:network_prefix, prefix: '1') }
+
+      before { FactoryBot.create(:network_prefix, prefix: '2') }
+
+      it 'should add error validation' do
+        subject
+
+        expect(record.errors[:prefix]).to contain_exactly('has already been taken')
+        expect(record.errors[:number_max_length]).to contain_exactly('length with 2 prefix already taken')
+        expect(record.errors[:number_min_length]).to contain_exactly('length with 2 prefix already taken')
+      end
+    end
+  end
+
+  describe 'network_prefixes_prefix_key' do
+    subject { network_prefix.save!(validate: false) }
+
+    let(:network_prefix) { FactoryBot.build(:network_prefix, prefix: '1') }
+
+    before { FactoryBot.create(:network_prefix, prefix: '1') }
+
+    it 'should raise PG error with "network_prefixes_prefix_key" index' do
+      expect { subject }.to raise_error(ActiveRecord::RecordNotUnique).with_message(/network_prefixes_prefix_key/)
+    end
+  end
+end