Skip to content

Commit

Permalink
Add better error messages for Ownership uniqueness validation
Browse files Browse the repository at this point in the history
  • Loading branch information
martinemde committed Oct 9, 2023
1 parent 122c723 commit b170e9e
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 12 deletions.
15 changes: 14 additions & 1 deletion app/models/ownership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Ownership < ApplicationRecord
belongs_to :authorizer, class_name: "User"
has_many :api_key_rubygem_scopes, dependent: :destroy

validates :user_id, uniqueness: { scope: :rubygem_id }
validate :validate_unique_user

delegate :name, to: :user, prefix: :owner
delegate :name, to: :authorizer, prefix: true, allow_nil: true
Expand Down Expand Up @@ -72,4 +72,17 @@ def unconfirmed?
def safe_destroy
destroy if unconfirmed? || rubygem.owners.many?
end

def validate_unique_user
return unless rubygem && user
ownerships = persisted? ? Ownership.where.not(id: id) : Ownership
other = ownerships.find_by(rubygem:, user:)
return unless other

if other.confirmed?
errors.add :user_id, "is already an owner of this gem"
else
errors.add :user_id, "is already invited to be an owner of this gem"
end
end
end
36 changes: 25 additions & 11 deletions test/models/ownership_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,6 @@ class OwnershipTest < ActiveSupport::TestCase
should have_db_index %i[user_id rubygem_id]
should have_many(:api_key_rubygem_scopes).dependent(:destroy)

context "with ownership" do
setup do
@ownership = create(:ownership)
create(:version, rubygem: @ownership.rubygem)
end

subject { @ownership }

should validate_uniqueness_of(:user_id).scoped_to(:rubygem_id)
end

context "by_indexed_gem_name" do
setup do
@ownership = create(:ownership)
Expand Down Expand Up @@ -108,6 +97,31 @@ class OwnershipTest < ActiveSupport::TestCase
refute_predicate ownership, :valid?
assert_contains ownership.errors[:rubygem], "must exist"
end

should "not create with a duplicate unconfirmed user and rubygem" do
existing_ownership = create(:ownership, :unconfirmed)
ownership = build(:ownership, user: existing_ownership.user, rubygem: existing_ownership.rubygem)

refute_predicate ownership, :valid?
assert_contains ownership.errors[:user_id], "is already invited to be an owner of this gem"
end

should "not create with a duplicate confirmed user and rubygem" do
existing_ownership = create(:ownership)
ownership = build(:ownership, user: existing_ownership.user, rubygem: existing_ownership.rubygem)

refute_predicate ownership, :valid?
assert_contains ownership.errors[:user_id], "is already an owner of this gem"
end

should "not update to a duplicate confirmed user and rubygem" do
existing_ownership = create(:ownership)
ownership = create(:ownership, :unconfirmed, rubygem: existing_ownership.rubygem)
ownership.user = existing_ownership.user

refute_predicate ownership, :valid?
assert_contains ownership.errors[:user_id], "is already an owner of this gem"
end
end

context "#valid_confirmation_token?" do
Expand Down

0 comments on commit b170e9e

Please sign in to comment.