Counter fixes (#1180)

* counter fixes

* more totalcount excising

* completed count > views changes, I believe

* schema changes

* db schema tweak

* schema example tweak

* more controller changes

* controller changes for impressionable

* inconsistent test resolution

* test addition for inconsistencies

* test tweaks

* test tweak

* test fix

* additional

* fixed!?

* testing node.views incrementing in controllers with impressionist gem

* attempt to clear impressions to test unique ips

* further attempts

* workaround for node.views counter_cache, see #1196

* Timecop test fixes

* changes for wiki tests as well

* clear uniques before wiki test

* test fixes via new integration test

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)

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

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)

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
 

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

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

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)

app/views/dashboard/_node_meta.html.erb

@@ -9,7 +9,7 @@
   <% else %> 
     | <a href="<%= node.path %>#comments"><i class="fa fa-comment-o"></i> <%= node.comment_count %></a>
   <% end %>
-    <span class="hidden-xs hidden-sm">| <i class="fa fa-eye"></i> <%= number_with_delimiter(node.totalcount) %></span>
+    <span class="hidden-xs hidden-sm">| <i class="fa fa-eye"></i> <%= number_with_delimiter(node.totalviews) %></span>
     | <i style="<% if node.likes > 0 %>color:#db4;<% end %>" class="fa fa-star-o"></i> <%= node.likes %>
   <% if params[:mod] %>| <a href="#"><i class="fa fa-ban"></i></a><% end %>
 </span>

app/views/map/show.html.erb

@@ -38,7 +38,7 @@
         <p><b>Published by</b> <a href="/profile/<%= @node.author.name %>"><%= @node.author.name %></a></p>
       <% end %>
       <p><a href="https://maps.google.com/maps?t=h&ll=<%= @node.lat %>,<%= @node.lon %>"><%= @node.lat %> N, <%= @node.lon %> E</a></p>
-      <p><%= @node.totalcount %> views</p>
+      <p><%= @node.totalviews %> views</p>
       <% if @node.map.field_ground_resolution_value %><p><b>Ground resolution: </b><%= @node.map.field_ground_resolution_value %> cm/px</p><% end %>
       <p><b>Capture date:</b> <%= @node.map.captured_on.to_s %></p> 
       <p><b>Publication date:</b> <%= @node.map.published_on.to_s %></p> 

app/views/notes/_notes.html.erb

@@ -31,7 +31,7 @@
             <%= t('notes._notes.last_edit_by') %> <a <% if @widget %>target="_blank"<% end %> href="/profile/<%= node.latest.author.name %>"><%= node.latest.author.name %></a>
             <%= distance_of_time_in_words(Time.at(node.latest.timestamp), Time.current, false, :scope => :'datetime.time_ago_in_words') %>
           <% end %>
-          | <i class="fa fa-eye"></i> <%= number_with_delimiter(node.totalcount) %> <span class="hidden-xs hidden-sm"><%= t('notes._notes.views') %></span>
+          | <i class="fa fa-eye"></i> <%= number_with_delimiter(node.totalviews) %> <span class="hidden-xs hidden-sm"><%= t('notes._notes.views') %></span>
           | <i style="<% if node.likes > 0 %>color:#db4;<% else %>color:#888;<% end %>" class="fa fa-star-o"></i> <%= node.likes %>
         </small></p>
 

app/views/notes/_tagged_notes.html.erb

@@ -8,7 +8,7 @@
       <p style="color:#888;"><small>
         <%= 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)) %>
         | <a href="<%= node.path %>#comments"><i style="color:#888;" class="fa fa-comment-alt"></i> <%= node.comment_count %></a>
-        | <%= number_with_delimiter(node.totalcount) %> <%= t('notes._tagged_notes.views') %>
+        | <%= number_with_delimiter(node.totalviews) %> <%= t('notes._tagged_notes.views') %>
         | <i style="color:#888;" class="fa fa-star-o"></i> <%= node.likes %>
       </small></p>
       <p class="hidden-lg"><%= raw strip_tags(sanitize(RDiscount.new(node.body).to_html)).truncate(150) %></p>

app/views/notes/show.html.erb

@@ -45,7 +45,7 @@
         <%= @node.created_at.to_s(:long) %>
       </span>
       <span class="hidden-xs hidden-sm hidden-print">
-        | <%= number_with_delimiter(@node.totalcount) %> <%= t('notes.show.views') %>
+        | <%= number_with_delimiter(@node.totalviews) %> <%= t('notes.show.views') %>
         <% if @node.comments %>
           | <i class="fa fa-comment"></i>
           <a href="#comments"> <%= @node.comments.length %> <%= t('notes.show.comments') %> </a>

app/views/questions/show.html.erb

@@ -30,7 +30,7 @@
   </div>
   <div class="inline">
   	<p><a href="/profile/<%= @node.author.name %>"><%= @node.author.name %></a> asked on <%= @node.created_at.to_s(:long) %> <% if @node.status == 0 || @node.status == 4 %>| <span class="label label-danger">UNPUBLISHED</span><% end %> <br />
-    <small><i class="fa fa-eye"></i> <%= number_with_delimiter(@node.totalcount) %> <span class="hidden-xs hidden-sm hidden-print ">views</span> | <i class="fa fa-comments"></i> <a href="#answers"><span id="short-comment-count"><%= @node.answers.length %></span> answers</a> | <a href="/q/<%= @node.id %>">shortlink</a></small></p>
+    <small><i class="fa fa-eye"></i> <%= number_with_delimiter(@node.totalviews) %> <span class="hidden-xs hidden-sm hidden-print ">views</span> | <i class="fa fa-comments"></i> <a href="#answers"><span id="short-comment-count"><%= @node.answers.length %></span> answers</a> | <a href="/q/<%= @node.id %>">shortlink</a></small></p>
   </div>
 
   <hr style="margin-top:10px;" />

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') %>
             | <a href="<%= node.path %>#comments"><i style="color:#888;" class="fa fa-comment-alt"></i> <%= node.comment_count %></a>
             <% 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 %>
             | <i style="color:#888;" class="fa fa-star-o"></i> <%= node.likes %>
             <% end %>

app/views/wiki/_wikis.html.erb

@@ -9,7 +9,7 @@
       <td><%= 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') %> 
       <a href="/profile/<%= wiki.latest.author.name %>"><%= wiki.latest.author.name %></a></td>
       <td><%= wiki.revisions.length %></td>
-      <td class="hidden-xs"><%= number_with_delimiter(wiki.totalcount) %></td>
+      <td class="hidden-xs"><%= number_with_delimiter(wiki.totalviews) %></td>
       <td><%= number_with_delimiter(wiki.cached_likes) %></td>
     </tr>
   <% end %>

app/views/wiki/show.html.erb

@@ -34,7 +34,7 @@
     <div class="hidden-print">
       <div class="pull-right" style="padding-top:8px;">
         <span class="hidden-sm hidden-xs">
-          <%= 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)) %>
           | <a href="/n/<%= @node.id %>"><%= t('wiki.show.shortlink') %></a>
         </span>

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

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"

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