Skip to content

Commit

Permalink
Restore using node.views for unique impressions count
Browse files Browse the repository at this point in the history
This should improve performance of various page loads, as it avoids
running COUNT() queries on the impressions table, and instead uses the
views value, which the impressions gem keeps up-to-date.
  • Loading branch information
Alon Pe'er committed Mar 24, 2019
1 parent 64b0207 commit 828f8df
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 9 deletions.
6 changes: 2 additions & 4 deletions app/models/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,10 @@ def setup

public

is_impressionable counter_cache: true, column_name: :views
is_impressionable counter_cache: true, column_name: :views, unique: :ip_address

def totalviews
# this doesn't filter out duplicate ip addresses as the line below does:
# self.views + self.legacy_views
impressionist_count(filter: :ip_address) + legacy_views
views + legacy_views
end

def self.weekly_tallies(type = 'note', span = 52, time = Time.now)
Expand Down
6 changes: 3 additions & 3 deletions test/functional/notes_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def teardown
assert_equal '0.0.0.0', Impression.last.ip_address
Impression.last.update_attribute('ip_address', '0.0.0.1')

assert_difference 'note.totalviews', 1 do
assert_difference 'note.reload.totalviews', 1 do
get :show,
params: {
author: note.author.name,
Expand All @@ -90,10 +90,10 @@ def teardown
}
end

assert_equal 2, note.totalviews
assert_equal 2, note.reload.totalviews

# same IP won't add to views twice
assert_difference 'note.totalviews', 0 do
assert_difference 'note.reload.totalviews', 0 do
get :show,
params: {
author: note.author.name,
Expand Down
4 changes: 2 additions & 2 deletions test/integration/node_unique_views_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class NodeInsertExtrasTest < ActionDispatch::IntegrationTest
assert_response :success
end

assert_difference 'nodes(:about).totalviews', 0 do
assert_difference 'nodes(:about).reload.totalviews', 0 do
assert_difference 'Impression.count', 0 do
get "/wiki/#{nodes(:about).slug}"
assert_response :success
Expand All @@ -22,7 +22,7 @@ class NodeInsertExtrasTest < ActionDispatch::IntegrationTest
assert_equal '127.0.0.1', Impression.last.ip_address
assert Impression.last.update_attributes(ip_address: '0.0.0.0')

assert_difference 'nodes(:about).totalviews', 1 do
assert_difference 'nodes(:about).reload.totalviews', 1 do
assert_difference 'Impression.count', 1 do
get "/wiki/#{nodes(:about).slug}"
assert_response :success
Expand Down

0 comments on commit 828f8df

Please sign in to comment.