diff --git a/app/controllers/map_controller.rb b/app/controllers/map_controller.rb
index 95acd6d086..846a09181c 100644
--- a/app/controllers/map_controller.rb
+++ b/app/controllers/map_controller.rb
@@ -20,7 +20,7 @@ def show
# redirect_old_urls
- impressionist(@node.drupal_node_counter)
+ impressionist(@node)
@title = @node.title
@tags = @node.tags
@tagnames = @tags.collect(&:name)
diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index c7a9f70e68..2567dc7f9f 100644
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -79,7 +79,7 @@ def show
alert_and_redirect_moderated
- impressionist(@node.drupal_node_counter)
+ impressionist(@node, 'show', :unique => [:ip_address])
@title = @node.latest.title
@tags = @node.tags
@tagnames = @tags.collect(&:name)
@@ -305,8 +305,7 @@ def popular
@notes = DrupalNode.research_notes
.limit(20)
.where(status: 1)
- .order("node_counter.totalcount DESC")
- .includes(:drupal_node_counter)
+ .order("views DESC")
@unpaginated = true
render :template => 'notes/index'
end
diff --git a/app/controllers/questions_controller.rb b/app/controllers/questions_controller.rb
index e1ead3f21c..fbc4bf6c56 100644
--- a/app/controllers/questions_controller.rb
+++ b/app/controllers/questions_controller.rb
@@ -30,7 +30,7 @@ def show
alert_and_redirect_moderated
- impressionist(@node.drupal_node_counter)
+ impressionist(@node)
@title = @node.latest.title
@tags = @node.power_tag_objects('question')
@tagnames = @tags.collect(&:name)
@@ -81,8 +81,7 @@ def popular
@title = "Popular Questions"
@questions = DrupalNode.questions.where(status: 1)
sort_question_by_tags
- @questions = @questions.order('node_counter.totalcount DESC')
- .includes(:drupal_node_counter)
+ @questions = @questions.order('views DESC')
.limit(20)
@wikis = DrupalNode.limit(10)
diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb
index 9834398111..e98e709447 100644
--- a/app/controllers/wiki_controller.rb
+++ b/app/controllers/wiki_controller.rb
@@ -63,7 +63,7 @@ def show
set_sidebar :tags, @tagnames, {:videos => true}
@wikis = Tag.find_pages(@node.slug_from_path,30) if @node.has_tag('chapter') || @node.has_tag('tabbed:wikis')
- impressionist(@node.drupal_node_counter)
+ impressionist(@node, 'show', :unique => [:ip_address])
@revision = @node.latest
@title = @revision.title
end
@@ -288,7 +288,7 @@ def index
order_string = "node_revisions.timestamp DESC"
end
- @wikis = DrupalNode.includes(:drupal_node_revision, :drupal_node_counter)
+ @wikis = DrupalNode.includes(:drupal_node_revision)
.group('node_revisions.nid')
.order(order_string)
.where("node_revisions.status = 1 AND node.status = 1 AND (type = 'page' OR type = 'tool' OR type = 'place')")
@@ -300,12 +300,11 @@ def index
def popular
@title = I18n.t('wiki_controller.popular_wiki_pages')
@wikis = DrupalNode.limit(40)
- .order("node_counter.totalcount DESC")
+ .order("views DESC")
.joins(:drupal_node_revision)
.group('node_revisions.nid')
.order("node_revisions.timestamp DESC")
.where("node.status = 1 AND node_revisions.status = 1 AND node.nid != 259 AND (type = 'page' OR type = 'tool' OR type = 'place')")
- .includes(:drupal_node_counter)
render :template => "wiki/index"
end
diff --git a/app/models/drupal_node.rb b/app/models/drupal_node.rb
index ddfd3a02fe..ad84b0875a 100644
--- a/app/models/drupal_node.rb
+++ b/app/models/drupal_node.rb
@@ -16,7 +16,7 @@ def validate(record)
class DrupalNode < ActiveRecord::Base
include NodeShared # common methods for node-like models
- attr_accessible :title, :uid, :status, :type, :vid, :cached_likes, :comment, :path, :slug
+ attr_accessible :title, :uid, :status, :type, :vid, :cached_likes, :comment, :path, :slug, :views
self.table_name = 'node'
self.primary_key = 'nid'
@@ -41,7 +41,9 @@ def updated_month
updated_at.strftime('%B %Y')
end
-
+ # FriendlyId is not being used; see
+ # https://github.com/publiclab/plots2/pull/687 and
+ # https://github.com/publiclab/plots2/pull/600
extend FriendlyId
friendly_id :friendly_id_string, use: [:slugged, :history]
@@ -65,7 +67,6 @@ def friendly_id_string
# wasn't working to tie it to .vid, manually defining below
# has_one :drupal_main_image, :foreign_key => 'vid', :dependent => :destroy
# has_many :drupal_content_field_image_gallery, :foreign_key => 'nid'
- has_one :drupal_node_counter, :foreign_key => 'nid', :dependent => :destroy
has_many :drupal_upload, :foreign_key => 'nid', :dependent => :destroy
has_many :drupal_files, :through => :drupal_upload
has_many :drupal_node_community_tag, :foreign_key => 'nid', :dependent => :destroy
@@ -167,15 +168,24 @@ def set_changed_and_created
self['changed'] = DateTime.now.to_i
end
- # determines URL ("slug"), initializes the view counter, and sets up a created timestamp
+ # determines URL ("slug"), and sets up a created timestamp
def setup
self['created'] = DateTime.now.to_i
self.save
- DrupalNodeCounter.new({:nid => self.id}).save
end
public
+ # the counter_cache does not currently work: views column is not updated for some reason
+ # https://github.com/publiclab/plots2/issues/1196
+ is_impressionable counter_cache: true, column_name: :views
+
+ def totalviews
+ # disabled as impressionist is not currently updating counter_cache; see above
+ #self.views + self.legacy_views
+ self.impressionist_count(:filter => :ip_address) + self.legacy_views
+ end
+
def self.weekly_tallies(type = "note", span = 52, time = Time.now)
weeks = {}
(0..span).each do |week|
@@ -473,12 +483,6 @@ def tagnames
self.tags.collect(&:name)
end
- # view count
- def totalcount
- DrupalNodeCounter.new({:nid => self.id}).save if self.drupal_node_counter.nil?
- self.drupal_node_counter.totalcount
- end
-
def edit_path
if self.type == "page" || self.type == "tool" || self.type == "place"
path = "/wiki/edit/" + self.path.split("/").last
diff --git a/app/models/drupal_node_counter.rb b/app/models/drupal_node_counter.rb
index 302d709c01..610c418c52 100644
--- a/app/models/drupal_node_counter.rb
+++ b/app/models/drupal_node_counter.rb
@@ -4,5 +4,4 @@ class DrupalNodeCounter < ActiveRecord::Base
belongs_to :drupal_node, :foreign_key => 'nid', :dependent => :destroy
- is_impressionable :counter_cache => true, :column_name => :totalcount, :unique => :ip_address
end
diff --git a/app/models/tag.rb b/app/models/tag.rb
index 12ed40f95c..8321c0bde9 100644
--- a/app/models/tag.rb
+++ b/app/models/tag.rb
@@ -62,9 +62,9 @@ def belongs_to(current_user, nid)
def self.find_top_nodes_by_type(tagname, type = "wiki", limit = 10)
DrupalNode.where(type: type)
.where('term_data.name = ?', tagname)
- .order("node_counter.totalcount DESC")
+ .order("node.views DESC")
.limit(limit)
- .include(:drupal_node_counter, :drupal_node_community_tag, :tag)
+ .include(:drupal_node_community_tag, :tag)
end
# finds recent nodes - should drop "limit" and allow use of chainable .limit()
@@ -109,10 +109,10 @@ def self.find_nodes_by_type_with_all_tags(tagnames, type = "note", limit = 10)
def self.find_popular_notes(tagname, views = 20, limit = 10)
DrupalNode.where(type: "note")
- .where('term_data.name = ? AND node_counter.totalcount > (?)', tagname, views)
+ .where('term_data.name = ? AND node.views > (?)', tagname, views)
.order("node.nid DESC")
.limit(limit)
- .include(:drupal_node_counter, :drupal_node_community_tag, :tag)
+ .include(:drupal_node_community_tag, :tag)
end
def self.exists?(tagname,nid)
diff --git a/app/views/dashboard/_node_meta.html.erb b/app/views/dashboard/_node_meta.html.erb
index 7e2df1d5c1..5aa4dd01a7 100644
--- a/app/views/dashboard/_node_meta.html.erb
+++ b/app/views/dashboard/_node_meta.html.erb
@@ -9,7 +9,7 @@
<% else %>
| <%= node.comment_count %>
<% end %>
- | <%= number_with_delimiter(node.totalcount) %>
+ | <%= number_with_delimiter(node.totalviews) %>
| <%= node.likes %>
<% if params[:mod] %>| <% end %>
diff --git a/app/views/map/show.html.erb b/app/views/map/show.html.erb
index d38f57df5b..fcbb70aab3 100644
--- a/app/views/map/show.html.erb
+++ b/app/views/map/show.html.erb
@@ -38,7 +38,7 @@
Published by <%= @node.author.name %>
<% end %>
<%= @node.lat %> N, <%= @node.lon %> E
- <%= @node.totalcount %> views
+ <%= @node.totalviews %> views
<% if @node.map.field_ground_resolution_value %>Ground resolution: <%= @node.map.field_ground_resolution_value %> cm/px
<% end %>
Capture date: <%= @node.map.captured_on.to_s %>
Publication date: <%= @node.map.published_on.to_s %>
diff --git a/app/views/notes/_notes.html.erb b/app/views/notes/_notes.html.erb
index 81c7be03db..59cf5ef9d8 100644
--- a/app/views/notes/_notes.html.erb
+++ b/app/views/notes/_notes.html.erb
@@ -31,7 +31,7 @@
<%= t('notes._notes.last_edit_by') %> target="_blank"<% end %> href="/profile/<%= node.latest.author.name %>"><%= node.latest.author.name %>
<%= distance_of_time_in_words(Time.at(node.latest.timestamp), Time.current, false, :scope => :'datetime.time_ago_in_words') %>
<% end %>
- | <%= number_with_delimiter(node.totalcount) %> <%= t('notes._notes.views') %>
+ | <%= number_with_delimiter(node.totalviews) %> <%= t('notes._notes.views') %>
| <%= node.likes %>
diff --git a/app/views/notes/_tagged_notes.html.erb b/app/views/notes/_tagged_notes.html.erb
index a74beef777..1e0b028c62 100644
--- a/app/views/notes/_tagged_notes.html.erb
+++ b/app/views/notes/_tagged_notes.html.erb
@@ -8,7 +8,7 @@
<%= raw t('notes._tagged_notes.by_author_time_ago', :url1 => "/notes/author/"+node.author.name, :author => node.author.name, :time => time_ago_in_words(node.created_at)) %>
| <%= node.comment_count %>
- | <%= number_with_delimiter(node.totalcount) %> <%= t('notes._tagged_notes.views') %>
+ | <%= number_with_delimiter(node.totalviews) %> <%= t('notes._tagged_notes.views') %>
| <%= node.likes %>
<%= raw strip_tags(sanitize(RDiscount.new(node.body).to_html)).truncate(150) %>
diff --git a/app/views/notes/show.html.erb b/app/views/notes/show.html.erb
index d3e2cd580f..2f85233c0f 100644
--- a/app/views/notes/show.html.erb
+++ b/app/views/notes/show.html.erb
@@ -45,7 +45,7 @@
<%= @node.created_at.to_s(:long) %>
- | <%= number_with_delimiter(@node.totalcount) %> <%= t('notes.show.views') %>
+ | <%= number_with_delimiter(@node.totalviews) %> <%= t('notes.show.views') %>
<% if @node.comments %>
|
<%= @node.comments.length %> <%= t('notes.show.comments') %>
diff --git a/app/views/questions/show.html.erb b/app/views/questions/show.html.erb
index fcabef7737..ed1a8f3580 100644
--- a/app/views/questions/show.html.erb
+++ b/app/views/questions/show.html.erb
@@ -30,7 +30,7 @@
<%= @node.author.name %> asked on <%= @node.created_at.to_s(:long) %> <% if @node.status == 0 || @node.status == 4 %>| UNPUBLISHED<% end %>
- <%= number_with_delimiter(@node.totalcount) %> views | answers | shortlink
+
<%= number_with_delimiter(@node.totalviews) %> views | answers | shortlink
diff --git a/app/views/tag/blog.html.erb b/app/views/tag/blog.html.erb
index 97c769379d..797a8342a9 100644
--- a/app/views/tag/blog.html.erb
+++ b/app/views/tag/blog.html.erb
@@ -47,7 +47,7 @@
| <%= distance_of_time_in_words(node.created_at, Time.current, false, :scope => :'datetime.time_ago_in_words') %>
| <%= node.comment_count %>
<% if params[:controller] == "notes" && params[:action] == "popular" %>
- | <%= number_with_delimiter(node.totalcount) %> <%= t('tag.blog.views') %>
+ | <%= number_with_delimiter(node.totalviews) %> <%= t('tag.blog.views') %>
<% else %>
| <%= node.likes %>
<% end %>
diff --git a/app/views/wiki/_wikis.html.erb b/app/views/wiki/_wikis.html.erb
index 5fa2eb4ce4..72368b8df7 100644
--- a/app/views/wiki/_wikis.html.erb
+++ b/app/views/wiki/_wikis.html.erb
@@ -9,7 +9,7 @@
<%= distance_of_time_in_words(Time.at(wiki.latest.created_at), Time.current, false, :scope => :'datetime.time_ago_in_words') %> <%= raw t('wiki._wikis.by') %>
<%= wiki.latest.author.name %> |
<%= wiki.revisions.length %> |
- <%= number_with_delimiter(wiki.totalcount) %> |
+ <%= number_with_delimiter(wiki.totalviews) %> |
<%= number_with_delimiter(wiki.cached_likes) %> |
<% end %>
diff --git a/app/views/wiki/show.html.erb b/app/views/wiki/show.html.erb
index 824c77db18..121529b810 100644
--- a/app/views/wiki/show.html.erb
+++ b/app/views/wiki/show.html.erb
@@ -34,7 +34,7 @@
- <%= number_with_delimiter(@node.totalcount) %> <%= t('wiki.show.views') %> |
+ <%= number_with_delimiter(@node.totalviews) %> <%= t('wiki.show.views') %> |
<%= raw t('wiki.show.last_edited', :url1 => "/profile/"+@node.latest.author.name, :author => @node.latest.author.name, :time => time_ago_in_words(@node.latest.created_at)) %>
| <%= t('wiki.show.shortlink') %>
diff --git a/db/migrate/20170111153001_node_pageviews.rb b/db/migrate/20170111153001_node_pageviews.rb
new file mode 100644
index 0000000000..dea49902a5
--- /dev/null
+++ b/db/migrate/20170111153001_node_pageviews.rb
@@ -0,0 +1,17 @@
+class NodePageviews < ActiveRecord::Migration
+ def up
+ add_column :node, :legacy_views, :integer, default: 0
+ add_column :node, :views, :integer, default: 0
+ DrupalNodeCounter.all.each do |counter|
+ n = DrupalNode.find_by_nid(counter.nid)
+ n.update_attribute('views', counter.totalcount) if n
+ end
+ # later we'll need to: drop_table :node_counter
+ end
+
+ def down
+ # we don't backwards-migrate totalcount from views
+ remove_column :node, :legacy_views
+ remove_column :node, :views
+ end
+end
diff --git a/db/schema.rb.example b/db/schema.rb.example
index 4480d4bc79..3164a25b1d 100644
--- a/db/schema.rb.example
+++ b/db/schema.rb.example
@@ -11,7 +11,7 @@
#
# It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 20161230142222) do
+ActiveRecord::Schema.define(:version => 20170111153001) do
create_table "answer_selections", :force => true do |t|
t.integer "user_id"
@@ -231,6 +231,8 @@ ActiveRecord::Schema.define(:version => 20161230142222) do
t.string "path"
t.integer "main_image_id"
t.string "slug"
+ t.integer "legacy_views", :default => 0
+ t.integer "views", :default => 0
end
add_index "node", ["changed"], :name => "node_changed"
diff --git a/test/fixtures/node_counter.yml b/test/fixtures/node_counter.yml
deleted file mode 100644
index de4720f206..0000000000
--- a/test/fixtures/node_counter.yml
+++ /dev/null
@@ -1,59 +0,0 @@
-one:
- nid: 1
-
-two:
- nid: 2
-
-three:
- nid: 3
-
-four:
- nid: 4
-
-five:
- nid: 5
-
-six:
- nid: 6
-
-seven:
- nid: 7
-
-eight:
- nid: 8
-
-nine:
- nid: 9
-
-ten:
- nid: 10
-
-eleven:
- nid: 11
-
-twelve:
- nid: 12
-
-thirteen:
- nid: 13
-
-fourteen:
- nid: 14
-
-fifteen:
- nid: 15
-
-sixteen:
- nid: 16
-
-seventeen:
- nid: 17
-
-eighteen:
- nid: 18
-
-nineteen:
- nid: 19
-
-twenty:
- nid: 20
diff --git a/test/functional/features_controller_test.rb b/test/functional/features_controller_test.rb
index cc77f96d5d..7a95c15132 100644
--- a/test/functional/features_controller_test.rb
+++ b/test/functional/features_controller_test.rb
@@ -10,9 +10,14 @@
class FeaturesControllerTest < ActionController::TestCase
def setup
+ Timecop.freeze # account for timestamp change
activate_authlogic
end
+ def teardown
+ Timecop.return
+ end
+
test "cannot see /features if not logged in" do
get :index
diff --git a/test/functional/notes_controller_test.rb b/test/functional/notes_controller_test.rb
index 64ce8cede2..0192db1288 100644
--- a/test/functional/notes_controller_test.rb
+++ b/test/functional/notes_controller_test.rb
@@ -48,6 +48,7 @@ def teardown
end
test "show note" do
+
note = node(:blog)
note.add_tag('activity:nonexistent', note.author) # testing responses display
assert_equal 'nonexistent', note.power_tag('activity')
@@ -61,6 +62,48 @@ def teardown
assert_select "#other-activities", false
end
+ test "notes record views with unique ips" do
+ note = node(:blog)
+ # clear impressions so we get a unique view
+ Impression.delete_all
+ assert_equal 0, note.views
+ assert_equal 0, Impression.count
+
+ # this assertion didn't work due to a bug in:
+ # https://github.com/publiclab/plots2/issues/1196
+ # assert_difference 'note.views', 1 do
+ assert_difference 'Impression.count', 1 do
+
+ get :show,
+ author: note.author.name,
+ date: Time.at(note.created).strftime("%m-%d-%Y"),
+ id: note.title.parameterize
+
+ end
+
+ 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
+ get :show,
+ author: note.author.name,
+ date: Time.at(note.created).strftime("%m-%d-%Y"),
+ id: note.title.parameterize
+ end
+
+ assert_equal 2, note.totalviews
+
+ # same IP won't add to views twice
+ assert_difference 'note.totalviews', 0 do
+
+ get :show,
+ author: note.author.name,
+ date: Time.at(note.created).strftime("%m-%d-%Y"),
+ id: note.title.parameterize
+
+ end
+ end
+
test "redirect normal user to tagged blog page" do
note = node(:one)
blog = node(:blog)
diff --git a/test/functional/tag_controller_test.rb b/test/functional/tag_controller_test.rb
index 5d82fef5f8..039d621611 100644
--- a/test/functional/tag_controller_test.rb
+++ b/test/functional/tag_controller_test.rb
@@ -311,14 +311,28 @@ def setup
assert_equal 'spectrometry', tag.parent
assert_equal '', tag2.parent
node(:blog).add_tag('spectrometry', rusers(:bob))
+ assert node(:blog).has_tag_without_aliasing('spectrometry')
get :show, id: 'spectrometry'
- assert_equal 2, assigns(:notes).length
- assert assigns(:notes).first.has_tag_without_aliasing('spectrometer')
- assert_false assigns(:notes).first.has_tag_without_aliasing('spectrometry')
- assert_false assigns(:notes).last.has_tag_without_aliasing('spectrometer')
- assert assigns(:notes).last.has_tag_without_aliasing('spectrometry')
+ # order of timestamps during testing (almost same timestamps) was causing testing irregularities
+ notes = assigns(:notes).sort_by { |a| a.title }.reverse
+
+ assert_equal 2, notes.length
+ assert_equal [1,13], notes.collect(&:nid)
+ assert_equal [node(:one).title, "Blog post"], notes.collect(&:title)
+
+ # should be the first node, nid=1
+ assert_equal node(:one).title, notes.first.title
+ assert_equal ["spectrometer"], notes.first.tags.collect(&:name)
+ assert notes.first.has_tag_without_aliasing('spectrometer')
+ assert_false notes.first.has_tag_without_aliasing('spectrometry')
+
+ # should be the blog node, nid=13
+ assert_equal "Blog post", notes.last.title
+ assert_equal ["spectrometry"], notes.last.tags.collect(&:name)
+ assert_false notes.last.has_tag_without_aliasing('spectrometer')
+ assert notes.last.has_tag_without_aliasing('spectrometry')
end
test "does not show things tagged with parent tag" do
diff --git a/test/functional/wiki_controller_test.rb b/test/functional/wiki_controller_test.rb
index 0a9bbdb6f5..f7503e4ee0 100644
--- a/test/functional/wiki_controller_test.rb
+++ b/test/functional/wiki_controller_test.rb
@@ -59,10 +59,18 @@ def teardown
assert_response :success
end
- test "should get wiki page" do
- get :show, id: node(:about).id
+ test "should get wiki page and record unique views" do
+ Impression.delete_all # clear uniques
+ assert_equal 0, node(:about).views
+ assert_equal 0, Impression.count
- assert_response :success
+ assert_difference 'Impression.count', 1 do
+
+ get :show, id: node(:about).slug
+
+ assert_response :success
+
+ end
end
test "should get root-level (/about) wiki page" do
diff --git a/test/integration/node_unique_views_test.rb b/test/integration/node_unique_views_test.rb
new file mode 100644
index 0000000000..411c30cf5d
--- /dev/null
+++ b/test/integration/node_unique_views_test.rb
@@ -0,0 +1,40 @@
+require 'test_helper'
+
+class NodeInsertExtrasTest < ActionDispatch::IntegrationTest
+
+ test "should get wiki page and record unique views" do
+ Impression.delete_all # clear uniques
+ assert_equal 0, node(:about).views
+ assert_equal 0, Impression.count
+
+ assert_difference 'Impression.count', 1 do
+
+ get "wiki/#{node(:about).slug}"
+
+ assert_response :success
+
+ end
+
+ assert_difference 'node(:about).totalviews', 0 do
+ assert_difference 'Impression.count', 0 do
+
+ get "wiki/#{node(:about).slug}"
+ assert_response :success
+
+ end
+ end
+
+ assert_equal '127.0.0.1', Impression.last.ip_address
+ assert Impression.last.update_attributes(ip_address: '0.0.0.0')
+
+ assert_difference 'node(:about).totalviews', 1 do
+ assert_difference 'Impression.count', 1 do
+
+ get "wiki/#{node(:about).slug}"
+ assert_response :success
+
+ end
+ end
+ end
+
+end