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

Added ignore_deleted option #958

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ As such, _Breaking Changes_ are major. _Features_ would map to either major or m
* Features
* [@mizukami234 @junmoka Make table names configurable](https://github.com/mbleigh/acts-as-taggable-on/pull/910)
* [@damianlegawiec Rails 6.0.0.beta1 support](https://github.com/mbleigh/acts-as-taggable-on/pull/937)
* [@rolandoalvarado Added ignore_deleted option](https://github.com/mbleigh/acts-as-taggable-on/pull/958)
* Fixes
* [@tonyta Avoid overriding user-defined columns cache methods](https://github.com/mbleigh/acts-as-taggable-on/pull/911)
* [@hengwoon tags_count only need to join on the taggable's table if using STI](https://github.com/mbleigh/acts-as-taggable-on/pull/904)
Expand Down
16 changes: 16 additions & 0 deletions db/migrate/7_add_deleted_at_on_taggings.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
if ActiveRecord.gem_version >= Gem::Version.new('5.0')
class AddDeletedAtOnTaggings < ActiveRecord::Migration[4.2]; end
else
class AddDeletedAtOnTaggings < ActiveRecord::Migration; end
end
AddDeletedAtOnTaggings.class_eval do
def self.up
add_column(ActsAsTaggableOn.taggings_table, :deleted_at, :datetime)
add_index(ActsAsTaggableOn.taggings_table, :deleted_at) unless index_exists?(ActsAsTaggableOn.taggings_table, :deleted_at)
end

def self.down
index_exists?(ActsAsTaggableOn.taggings_table, :deleted_at) && remove_index(ActsAsTaggableOn.taggings_table, :deleted_at)
column_exists?(ActsAsTaggableOn.taggings_table, :deleted_at) && remove_column(ActsAsTaggableOn.taggings_table, :deleted_at)
end
end
1 change: 1 addition & 0 deletions gemfiles/activerecord_6.0.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ when "mysql"
else
gem 'sqlite3'
end
gem 'rspec-rails', '~> 4.0.0.beta2'

group :local_development do
gem "guard"
Expand Down
1 change: 1 addition & 0 deletions gemfiles/activerecord_7.0.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ when "mysql"
else
gem 'sqlite3'
end

group :local_development do
gem "guard"
gem "guard-rspec"
Expand Down
2 changes: 1 addition & 1 deletion lib/acts_as_taggable_on/tag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class Tag < ::ActiveRecord::Base
### VALIDATIONS:

validates_presence_of :name
validates_uniqueness_of :name, if: :validates_name_uniqueness?, case_sensitive: true
validates_uniqueness_of :name, case_sensitive: false, if: :validates_name_uniqueness?
validates_length_of :name, maximum: 255

# monkey patch this method if don't need name uniqueness validation
Expand Down
11 changes: 11 additions & 0 deletions lib/acts_as_taggable_on/taggable/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,17 @@ def all_#{tags_type}_list
def dirtify_tag_list(tagging)
attribute_will_change! tagging.context.singularize+"_list"
end

def duplicated_mutations_from_database
unless defined?(@mutations_from_database)
@mutations_from_database = nil
end
@mutations_from_database ||= if defined?(@attributes)
ActiveModel::AttributeMutationTracker.new(@attributes)
else
NullMutationTracker.instance
end
end
RUBY
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ def on_conditions(tag, tagging_alias)
.and(tagging_alias[:tagger_type].eq(owner.class.base_class.to_s))
end

if options[:ignore_deleted].present?
on_condition = on_condition.and(tagging_alias[:deleted_at].eq(nil))
end

on_condition
end

Expand All @@ -76,6 +80,10 @@ def match_all_on_conditions
on_condition = on_condition.and(tagging_arel_table[:context].eq(options[:on]))
end

if options[:ignore_deleted].present?
on_condition = on_condition.and(tagging_arel_table[:deleted_at].eq(nil))
end

on_condition
end

Expand Down Expand Up @@ -105,7 +113,8 @@ def order_conditions

def tagging_alias(tag)
alias_base_name = taggable_model.base_class.name.downcase
adjust_taggings_alias("#{alias_base_name[0..11]}_taggings_#{ActsAsTaggableOn::Utils.sha_prefix(tag)}")
random_string = SecureRandom.hex(4)
adjust_taggings_alias("#{alias_base_name[0..11]}_taggings_#{ActsAsTaggableOn::Utils.sha_prefix(random_string)}")
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ def at_least_one_tag
.and(tagging_arel_table[:tagger_type].eq(owner.class.base_class.to_s))
end

if options[:ignore_deleted].present?
exists_contition = exists_contition.and(tagging_arel_table[:deleted_at].eq(nil))
end

exists_contition
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ def match_all_on_conditions
on_condition = on_condition.and(tagging_arel_table[:context].eq(options[:on]))
end

if options[:ignore_deleted].present?
on_condition = on_condition.and(tagging_arel_table[:deleted_at].eq(nil))
end

on_condition
end

Expand Down
11 changes: 10 additions & 1 deletion lib/acts_as_taggable_on/tagging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ class Tagging < ::ActiveRecord::Base #:nodoc:
validates_presence_of :context
validates_presence_of :tag_id

validates_uniqueness_of :tag_id, scope: [:taggable_type, :taggable_id, :context, :tagger_id, :tagger_type]
validates_uniqueness_of :tag_id, case_sensitive: false, scope: [:taggable_type, :taggable_id, :context, :tagger_id, :tagger_type]

before_destroy :mark_as_deleted, if: :has_deleted_at?
after_destroy :remove_unused_tags

private
Expand All @@ -34,5 +35,13 @@ def remove_unused_tags
end
end
end

def mark_as_deleted
self.update_columns(deleted_at: Time.current)
end

def has_deleted_at?
ActiveRecord::Base.connection.column_exists?(ActsAsTaggableOn.taggings_table, :deleted_at)
end
end
end
19 changes: 9 additions & 10 deletions spec/acts_as_taggable_on/tag_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
end

it 'should find both tags' do
expect(ActsAsTaggableOn::Tag.named_like_any(%w(awesome epic)).count).to eq(3)
expect(ActsAsTaggableOn::Tag.named_like_any(%w(awesome epic)).count).to eq(2)
end
end
end
Expand Down Expand Up @@ -146,11 +146,11 @@
include_context 'without unique index'
end

it 'should find by name case sensitive' do
it 'should not find by name case sensitive' do
ActsAsTaggableOn.strict_case_match = true
expect {
ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name('AWESOME')
}.to change(ActsAsTaggableOn::Tag, :count).by(1)
}.not_to change(ActsAsTaggableOn::Tag, :count)
end
end

Expand All @@ -169,7 +169,7 @@
ActsAsTaggableOn.strict_case_match = true
expect {
expect(ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name('AWESOME', 'awesome').map(&:name)).to eq(%w(AWESOME awesome))
}.to change(ActsAsTaggableOn::Tag, :count).by(1)
}.not_to change(ActsAsTaggableOn::Tag, :count)
end
end

Expand Down Expand Up @@ -265,12 +265,12 @@
include_context 'without unique index'
end

it 'should find by name case sensitively' do
it 'should not find by name case sensitively' do
expect {
ActsAsTaggableOn::Tag.find_or_create_with_like_by_name('AWESOME')
}.to change(ActsAsTaggableOn::Tag, :count)
}.not_to change(ActsAsTaggableOn::Tag, :count)

expect(ActsAsTaggableOn::Tag.last.name).to eq('AWESOME')
expect(ActsAsTaggableOn::Tag.last.name).to eq('awesome')
end
end

Expand All @@ -281,11 +281,10 @@

it 'should have a named_scope named(something) that matches exactly' do
uppercase_tag = ActsAsTaggableOn::Tag.create(name: 'Cool')
@tag.name = 'cool'
@tag.save!

expect(ActsAsTaggableOn::Tag.named('cool')).to include(@tag)
expect(ActsAsTaggableOn::Tag.named('cool')).to_not include(uppercase_tag)
expect(ActsAsTaggableOn::Tag.named('cool')).not_to include(@tag)
expect(ActsAsTaggableOn::Tag.named('Cool')).to include(uppercase_tag)
end
end

Expand Down
41 changes: 40 additions & 1 deletion spec/acts_as_taggable_on/taggable_spec.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
# encoding: utf-8
require 'spec_helper'
require 'db/migrate/7_add_deleted_at_on_taggings.rb'

shared_examples_for 'without deleted_at column' do
prepend_before(:all) { AddDeletedAtOnTaggings.down }
append_after(:all) { AddDeletedAtOnTaggings.up }
end

describe 'Taggable To Preserve Order' do
before(:each) do
@taggable = OrderedTaggableModel.new(name: 'Bob Jones')
end


it 'should have tag associations' do
[:tags, :colours].each do |type|
expect(@taggable.respond_to?(type)).to be_truthy
Expand Down Expand Up @@ -550,6 +555,40 @@
expect(model.tag_list.sort).to eq(%w(ruby rails programming).sort)
end

context 'with ignore_deleted option' do
let(:bob) { TaggableModel.create(name: 'Bob', tag_list: 'lazy, happier') }
let(:frank) { TaggableModel.create(name: 'Frank', tag_list: 'lazy, happier') }
let(:steve) { TaggableModel.create(name: 'Steve', tag_list: 'lazy, happier') }

before(:example) do
bob
frank
steve
end

context 'when deleted_at is not present' do
include_context 'without deleted_at column'

it 'should be able to find tagged with ignore_deleted option' do
bob.taggings.first.destroy!
taggables = TaggableModel.tagged_with('lazy, happier')
expect(taggables).to include(frank, steve)
expect(taggables).not_to include(bob)
expect(bob.taggings.first.deleted_at).to be(nil)
end
end

context 'when deleted_at is present' do
it 'should be able to find tagged with ignore_deleted option' do
bob.taggings.first.destroy!
taggables = TaggableModel.tagged_with('lazy, happier', ignore_deleted: true)
expect(taggables).to include(frank, steve)
expect(taggables).not_to include(bob)
expect(bob.taggings.first.deleted_at).to be_within(10).of(Time.current.utc)
end
end
end

context 'Duplicates' do
context 'should not create duplicate taggings' do
let(:bob) { TaggableModel.create(name: 'Bob') }
Expand Down
4 changes: 2 additions & 2 deletions spec/internal/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@
# Limit is created to prevent MySQL error on index
# length for MyISAM table type: http://bit.ly/vgW2Ql
t.string :context, limit: 128

t.string :tenant , limit: 128

t.datetime :deleted_at
t.datetime :created_at
end
add_index ActsAsTaggableOn.taggings_table,
['tag_id', 'taggable_id', 'taggable_type', 'context', 'tagger_id', 'tagger_type'],
['tag_id', 'taggable_id', 'taggable_type', 'context', 'tagger_id', 'tagger_type', 'deleted_at'],
unique: true, name: 'taggings_idx'
add_index ActsAsTaggableOn.taggings_table, :tag_id , name: 'index_taggings_on_tag_id'

Expand Down