Apply feedback

app/controllers/api/base_controller.rb

@@ -27,9 +27,8 @@ def authenticate
       true
     end
 
-    def current_organization(value = nil)
-      @current_organization ||=
-        ApiKey.find_by(value:)&.organization || Organization.find_by(api_key: value)
+    def current_organization(api_key_value = nil)
+      @current_organization ||= ApiKey.find_by(value: api_key_value)&.organization
     end
 
     def set_context_source

app/models/api_key.rb

@@ -7,19 +7,16 @@ class ApiKey < ApplicationRecord
 
   before_create :set_value
 
-  def generate_value
-    value = SecureRandom.uuid
-    api_key = ApiKey.find_by(value:)
-
-    return generate_value if api_key.present?
-
-    value
-  end
+  validates :value, uniqueness: true
+  validates :value, presence: true, on: :update
 
   private
 
   def set_value
-    self.value = generate_value
+    loop do
+      self.value = SecureRandom.uuid
+      break unless self.class.exists?(value:)
+    end
   end
 end
 
@@ -36,6 +33,7 @@ def set_value
 # Indexes
 #
 #  index_api_keys_on_organization_id  (organization_id)
+#  index_api_keys_on_value            (value) UNIQUE
 #
 # Foreign Keys
 #

app/services/organizations/create_service.rb

@@ -8,7 +8,9 @@ def initialize(params)
     end
 
     def call
-      organization = Organization.new(params)
+      organization = Organization.new(
+        params.slice(:name, :document_numbering)
+      )
 
       ActiveRecord::Base.transaction do
         organization.save!

app/services/users_service.rb

@@ -32,6 +32,7 @@ def register(email, password, organization_name)
 
       result.organization = Organizations::CreateService
         .call(name: organization_name, document_numbering: 'per_organization')
+        .raise_if_error!
         .organization
 
       result.membership = Membership.create!(

db/migrate/20241021140054_create_api_keys.rb

@@ -5,7 +5,7 @@ def up
     create_table :api_keys, id: :uuid do |t|
       t.references :organization, type: :uuid, null: false, foreign_key: true
 
-      t.string :value, null: false
+      t.string :value, null: false, index: {unique: true}
 
       t.timestamps
     end

spec/factories/api_keys.rb

@@ -2,6 +2,6 @@
 
 FactoryBot.define do
   factory :api_key do
-    organization { association(:organization) }
+    organization { association(:organization, api_keys: []) }
   end
 end

spec/factories/organizations.rb

@@ -8,6 +8,8 @@
     email { Faker::Internet.email }
     email_settings { ['invoice.finalized', 'credit_note.created'] }
 
+    api_keys { [association(:api_key)] }
+
     transient do
       webhook_url { Faker::Internet.url }
     end
@@ -16,8 +18,6 @@
       if evaluator.webhook_url
         organization.webhook_endpoints.create!(webhook_url: evaluator.webhook_url)
       end
-
-      organization.api_keys.create!
     end
   end
 end

spec/models/api_key_spec.rb

@@ -5,57 +5,53 @@
 RSpec.describe ApiKey, type: :model do
   it { is_expected.to belong_to(:organization) }
 
-  describe '#save' do
-    subject { api_key.save! }
+  describe 'validations' do
+    describe 'of value uniqueness' do
+      before { create(:api_key) }
 
-    before do
-      allow(api_key).to receive(:set_value).and_call_original
-      subject
+      it { is_expected.to validate_uniqueness_of(:value) }
     end
 
-    context 'with a new record' do
-      let(:api_key) { build(:api_key) }
+    describe 'of value presence' do
+      subject { api_key }
+
+      context 'with a new record' do
+        let(:api_key) { build(:api_key) }
 
-      it 'calls #set_value' do
-        expect(api_key).to have_received(:set_value)
+        it { is_expected.not_to validate_presence_of(:value) }
       end
-    end
 
-    context 'with a persisted record' do
-      let(:api_key) { create(:api_key) }
+      context 'with a persisted record' do
+        let(:api_key) { create(:api_key) }
 
-      it 'does not call #set_value' do
-        expect(api_key).not_to have_received(:set_value)
+        it { is_expected.to validate_presence_of(:value) }
       end
     end
   end
 
-  describe '#set_value' do
-    subject { api_key.send(:set_value) }
+  describe '#save' do
+    subject { api_key.save! }
 
-    let(:api_key) { build(:api_key) }
-    let(:unique_value) { SecureRandom.uuid }
+    context 'with a new record' do
+      let(:api_key) { build(:api_key) }
+      let(:used_value) { create(:api_key).value }
+      let(:unique_value) { SecureRandom.uuid }
 
-    before { allow(api_key).to receive(:generate_value).and_return(unique_value) }
+      before do
+        allow(SecureRandom).to receive(:uuid).and_return(used_value, unique_value)
+      end
 
-    it 'sets result of #generate_value to the value' do
-      expect { subject }.to change(api_key, :value).to unique_value
+      it 'sets the value' do
+        expect { subject }.to change(api_key, :value).to unique_value
+      end
     end
-  end
-
-  describe '#generate_value' do
-    subject { api_key.generate_value }
 
-    let(:api_key) { build(:api_key) }
-    let(:used_value) { create(:api_key).value }
-    let(:unique_value) { SecureRandom.uuid }
-
-    before do
-      allow(SecureRandom).to receive(:uuid).and_return(used_value, unique_value)
-    end
+    context 'with a persisted record' do
+      let(:api_key) { create(:api_key) }
 
-    it 'returns unique value between all ApiKeys' do
-      expect(subject).to eq unique_value
+      it 'does not change the value' do
+        expect { subject }.not_to change(api_key, :value)
+      end
     end
   end
 end

spec/models/organization_spec.rb

@@ -142,43 +142,26 @@
   describe '#save' do
     subject { organization.save! }
 
-    before do
-      allow(organization).to receive(:generate_document_number_prefix).and_call_original # rubocop:disable RSpec/SubjectStub
-      subject
-    end
-
     context 'with a new record' do
       let(:organization) { build(:organization) }
 
-      it 'calls #generate_document_number_prefix' do
-        expect(organization).to have_received(:generate_document_number_prefix)
+      it 'sets document number prefix of organization' do
+        subject
+
+        expect(organization.document_number_prefix)
+          .to eq "#{organization.name.first(3).upcase}-#{organization.id.last(4).upcase}"
       end
     end
 
     context 'with a persisted record' do
       let(:organization) { create(:organization) }
 
-      it 'does not call #generate_document_number_prefix' do
-        expect(organization).not_to have_received(:generate_document_number_prefix)
+      it 'does not change document number prefix of organization' do
+        expect { subject }.not_to change(organization, :document_number_prefix)
       end
     end
   end
 
-  describe '#generate_document_number_prefix' do
-    subject { organization.send(:generate_document_number_prefix) }
-
-    let(:organization) { create(:organization) }
-    let(:prefix) { "#{organization.name.first(3).upcase}-#{organization.id.last(4).upcase}" }
-
-    before { organization.update!(document_number_prefix: 'invalid') }
-
-    it 'sets document number prefix of organization' do
-      expect { subject }
-        .to change(organization, :document_number_prefix)
-        .to prefix
-    end
-  end
-
   describe 'Premium integrations scopes' do
     it "returns the organization if the premium integration is enabled" do
       Organization::PREMIUM_INTEGRATIONS.each do |integration|

spec/requests/api/base_controller_spec.rb

@@ -31,20 +31,10 @@ def create
     end
 
     context 'with valid authorization header' do
-      context 'when authorization token matching api key' do
-        let(:token) { api_key.value }
+      let(:token) { api_key.value }
 
-        it 'returns success response' do
-          expect(response).to have_http_status(:success)
-        end
-      end
-
-      context 'when authorization token matching organization api_key' do
-        let(:token) { create(:organization, api_key: 'api_key').api_key }
-
-        it 'returns success response' do
-          expect(response).to have_http_status(:success)
-        end
+      it 'returns success response' do
+        expect(response).to have_http_status(:success)
       end
     end
 

spec/services/organizations/create_service_spec.rb

@@ -7,7 +7,12 @@
     subject(:service_result) { described_class.call(params) }
 
     context 'with valid params' do
-      let(:params) { attributes_for(:organization) }
+      let(:params) do
+        {
+          name: Faker::Company.name,
+          document_numbering: Organization::DOCUMENT_NUMBERINGS.sample.to_s
+        }
+      end
 
       it 'creates an organization with provided params' do
         expect { service_result }.to change(Organization, :count).by(1)