Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validates_uniqueness_of does not use "where deleted_at is null" #114

Closed
codeodor opened this issue Feb 13, 2014 · 18 comments
Closed

validates_uniqueness_of does not use "where deleted_at is null" #114

codeodor opened this issue Feb 13, 2014 · 18 comments

Comments

@codeodor
Copy link
Contributor

It's trivial in Rails 4 for the client code to fix by adding :conditions but that's not an option in Rails 3.

Is there any interest in a fix for this, either by adding a new method (maybe append _without_deleted to the name, or fixing it to work to be consistent with the rest of the queries it modifies?

@radar
Copy link
Collaborator

radar commented Mar 12, 2014

@codeodor Can you show me a situation with this failing?

@codeodor
Copy link
Contributor Author

Sure, I added tests to each branch that show it failing.

Employer has a new column name on which we use validates_uniqueness_of in the model.

The test creates a record, destroys it, and creates a new instance with the same name, and asserts whether the record is valid. At present, it is not.

@kidlab
Copy link

kidlab commented Mar 15, 2014

I encountered this problem too.
Here is my models:

class User < ActiveRecord::Base
  acts_as_paranoid

  has_many :favorites, dependent: :destroy
end

class Book < ActiveRecord::Base
  acts_as_paranoid

  has_many :favorites, dependent: :destroy
end

class Favorite < ActiveRecord::Base
  acts_as_paranoid

  belongs_to :user
  belongs_to :book

  validates :user, :book, presence: true
  validates_uniqueness_of :book_id, scope: :user_id
end

This code below will add book to user's favorite then remove it by soft-deleting. After that I cannot add book to favorite any more, because validates_uniqueness_of :book_id, scope: :user_id does not use paranoid scope.

user = User.first
book = Book.first

# Add my favorite book
f = Favorite.create(user: user, book: book)
# => #<Favorite id: 1, user_id: 1, book_id: 1, deleted_at: nil, created_at: "2014-03-15 17:53:50", updated_at: "2014-03-15 17:53:50">

# Remove from favorite
f.destroy

# Re-add book to favorite
f = Favorite.new(user: user, book: book)
f.valid?
# => false

f.errors
# => #<ActiveModel::Errors:0x007f2a8922acf8 @base=#<Favorite id: nil, user_id: 1, book_id: 1, deleted_at: nil, created_at: nil, updated_at: nil>, @messages={:book_id=>["has already been taken"]}>

@smostovoy
Copy link

+1. https:/goncalossilva/acts_as_paranoid has validates_uniqueness_of_without_deleted. IMHO it should be default. @radar do you plan anything?

codeodor added a commit to codeodor/paranoia that referenced this issue Mar 16, 2014
codeodor added a commit to codeodor/paranoia that referenced this issue Mar 16, 2014
@codeodor
Copy link
Contributor Author

@radar I added fixes to the rails3 and rails4 branches (of my fork, and thus the pull requests I made) where I had previously only included failing tests.

I am not sure how you feel about it, but I don't really like the idea of validates_uniqueness_of_without_deleted since the rest of the gem operates transparently.

Because of that, I used alias_method_chain to call the build_relation method both Rails 3 and 4 call when finding the existing records, and made sure we add relation.and(klass.quoted_table_name + ".#{klass.paranoia_column} IS NULL") to the relation. That fixes the tests.

@codeodor
Copy link
Contributor Author

FWIW, I looked into the Rails4 branch failure on Travis CI. One is due to (I think) assert_nothing_raised being gone on the Rails 4.1 tests. I have no idea why the 4.0.2 test is failing -- It works for me locally on 1.9.3 and 2.0.0.

I'm not sure if that's something I broke, or if it's been broken for a while.

@dvdplm
Copy link

dvdplm commented Jun 9, 2014

Any updates on this? :/

@dvdplm
Copy link

dvdplm commented Jun 9, 2014

PR on the PR: codeodor#1

@codeodor
Copy link
Contributor Author

codeodor commented Jun 9, 2014

Thanks, merged it into mine.

@radar
Copy link
Collaborator

radar commented Oct 11, 2014

Is there any interest in getting that PR merged back here? If so, please submit it as a PR.

@radar radar closed this as completed Oct 11, 2014
@codeodor
Copy link
Contributor Author

@radar #121 and #122 are pull requests for the Rails 4 and 3 branches that fix this issue.

codeodor added a commit to codeodor/paranoia that referenced this issue Jun 18, 2015
codeodor added a commit to codeodor/paranoia that referenced this issue Jun 18, 2015
@monkbroc
Copy link

Thanks for the fix @codeodor! I just hit this in my application and was able to pull the rails4 branch and it's fixed! 👍

@fwgroup-steven
Copy link

Do I have to use 'validates_uniqueness_of' because using:

validates :name, uniqueness: { scope: :user_id, message: :name_already_taken }

Doesn't work

@codeodor
Copy link
Contributor Author

@fwgroup-steven I'm not sure what Rails does under the hood, but I did change UniquenessValidator so I would assume they use the same thing in both places.

You might check the version you're running though. These changes were accepted into the repository ?(June 20, 2015) after the latest released version on RubyGems (June 17, 2015):

image

@fwgroup-steven
Copy link

@codeodor Thanks for your response. I just realised I had an out of date version, now updated to 2.1.3. But it's still not working :(

I have:

class Account < ActiveRecord::Base
  validates_uniqueness_of :name, scope: :user_id
end

But I get: Name has already been taken

Do you where I can start looking to figure out what is wrong?

@codeodor
Copy link
Contributor Author

@fwgroup-steven Unfortunately that version was released before these changes were merged, so you would need to take a look at the commit and patch it in your app, or use gem 'paranoia', github: 'radar/paranoia', branch: 'whicheverIsAppropriate' in your Gemfile to have it use the version here.

diego-silva pushed a commit to diego-silva/paranoia that referenced this issue Oct 23, 2015
@sur
Copy link

sur commented Jun 9, 2016

I think the readme file needs to be updated.

gem "paranoia", "~> 2.1.5" this solves the problem.

sebwinemaker pushed a commit to eatsprig/paranoia that referenced this issue Aug 3, 2016
@ikbenben
Copy link

ikbenben commented Sep 18, 2018

@codeodor @radar We seem to be experiencing this issue in version 1.3.4 (which is the latest version of the gem for Rails 3). @codeodor mentioned pull requests for both rails 3 and rails 4 but the following does not work:

validates_uniqueness_of :platform_id, :scope => [:trainer_id]

I've tried to use validates_uniqueness_of_without_deleted but that method does not exist: undefined method validates_uniqueness_of_without_deleted' for #Class:0x00000005ee9058`

Updated:
I also tried to use gem "paranoia", github: "rubysherpas/paranoia", branch: "rails3" as suggested in the README as I noticed the rails3 branch was last updated in Jun 2015 while the v1.3.4 tag was created in Jan 2015. However I'm unable to use this format as the dependancy is in an internal gem and it is not possible to reference a github gem from a gemspec (apparently)

s.add_dependency "paranoia", github: "rubysherpas/paranoia", branch: "rails3"

so it looks like a new release, v1.3.5 needs to be released with the latest rails3 branch if possible

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants