Skip to content

Commit

Permalink
Simplifies Flag cache even further
Browse files Browse the repository at this point in the history
  • Loading branch information
kikito committed Aug 31, 2015
1 parent cfc7c43 commit 1696598
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 37 deletions.
2 changes: 1 addition & 1 deletion app/controllers/debates_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def show
set_debate_votes(@debate)
@comments = @debate.root_comments.recent.page(params[:page]).for_render
all_comments = @debate.comment_threads
current_ability.flag_cache = Flag::Cache.new(current_user, all_comments)
current_ability.flag_cache = Flag.build_cache(current_user, all_comments)
end

def new
Expand Down
2 changes: 1 addition & 1 deletion app/models/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def initialize(user)
end

def flagged?(flaggable)
is_flagged = flag_cache && flag_cache.flagged?(flaggable)
is_flagged = flag_cache && flag_cache[flaggable.id]
is_flagged.nil? ? Flag.flagged?(@user, flaggable) : is_flagged
end

Expand Down
20 changes: 6 additions & 14 deletions app/models/flag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,12 @@ def self.flagged?(user, flaggable)
!! by_user_and_flaggable(user, flaggable).try(:first)
end

class Cache
attr_accessor :user

def initialize(user, flaggables)
@user = user
@cache = {}
flags = Flag.by_user_and_flaggables(user, flaggables)
flags.each{ |flag| @cache[flag.flaggable_id] = true }
flaggables.each{ |flaggable| @cache[flaggable.id] = !! @cache[flaggable.id] }
end

def flagged?(flaggable)
@cache[flaggable.id]
end
def self.build_cache(user, flaggables)
hash = {}
flags = Flag.by_user_and_flaggables(user, flaggables)
flags.each{ |flag| hash[flag.flaggable_id]= true }
flaggables.each{ |flaggable| hash[flaggable.id]= !! hash[flaggable.id] }
hash
end

class AlreadyFlaggedError < StandardError
Expand Down
4 changes: 2 additions & 2 deletions spec/models/ability_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,14 @@

it 'calls Flag.flagged? when the flag_cache is set, but it does not know the flaggable' do
ability.flag_cache = double(:cache, user: user)
expect(ability.flag_cache).to receive(:flagged?).with(comment).and_return(nil)
expect(ability.flag_cache).to receive(:[]).with(comment.id).and_return(nil)
expect(Flag).to receive(:flagged?).with(user, comment).and_return(false)
ability.flagged?(comment)
end

it 'calls flag_cache.flagged? when the flag_cache is set and it knows the flaggable' do
ability.flag_cache = double(:cache, user: user)
expect(ability.flag_cache).to receive(:flagged?).with(comment).and_return(true)
expect(ability.flag_cache).to receive(:[]).with(comment.id).and_return(true)
expect(Flag).to_not receive(:flagged?)
ability.flagged?(comment)
end
Expand Down
32 changes: 13 additions & 19 deletions spec/models/flag_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,32 +53,26 @@
end
end

describe Flag::Cache do
describe '.create_cache' do
let(:user) { create(:user) }
let(:debate1) { create(:debate) }
let(:debate2) { create(:debate) }

it 'accepts a user and a collection of flaggables' do
expect{ Flag::Cache.new(user, [debate2, debate1]) }.to_not raise_error
it 'accepts a user & collection of flaggables' do
expect{ Flag.build_cache(user, [debate2, debate1]) }.to_not raise_error
end

describe '#flagged?' do
it 'returns false if the item was not flagged by the user' do
cache = Flag::Cache.new(user, [debate1, debate2])
expect(cache.flagged?(debate1)).to eq(false)
end

it 'returns true if the item was flagged by the user' do
Flag.flag!(user, debate1)
cache = Flag::Cache.new(user, [debate1, debate2])
expect(cache.flagged?(debate1)).to eq(true)
expect(cache.flagged?(debate2)).to eq(false)
end
it 'returns nil if the item was not used for building the cache' do
cache = Flag.build_cache(user, [debate2])
expect(cache[debate1.id]).to eq(nil)
end

it 'returns nil if the item was not used for building the cache' do
cache = Flag::Cache.new(user, [debate2])
expect(cache.flagged?(debate1)).to eq(nil)
end
it 'returns true if the item was flagged by the user, false if not' do
Flag.flag!(user, debate1)
cache = Flag.build_cache(user, [debate1, debate2])
expect(cache[debate1.id]).to eq(true)
expect(cache[debate2.id]).to eq(false)
end
end

end

0 comments on commit 1696598

Please sign in to comment.