Merge branch 'main' into add-tooltips-to-fresh-comments

app/assets/javascripts/application.js

@@ -53,7 +53,7 @@
 //= require users.js
 //= require searchform.js
 //= require tagging.js
-//= require leaflet_helper
+//= require leafletHelper
 //= require grids.js
 //= require graph.js
 //= require simple-data-grapher.js

app/assets/javascripts/leaflet_helper.js → app/assets/javascripts/leafletHelper.js

@@ -163,4 +163,4 @@
       function peopleMap() {
          peopleLayerParser(map, markers_hash);
       }
-   }
+   }

app/assets/javascripts/timeago.js

@@ -1,6 +1,6 @@
 // Function by caiotarifa: https://gist.github.com/caiotarifa/30ae974f2293c761f3139dd194abd9e5
 //
-// using this function instead of distance_of_time_in_words in Ruby for the map popups calculation in leaflet_helper.js
+// using this function instead of distance_of_time_in_words in Ruby for the map popups calculation in leafletHelper.js
 
 var TimeAgo = (function() {
     var self = {};
@@ -60,4 +60,4 @@ var TimeAgo = (function() {
     };
 
     return self;
-  }());
+  }());

app/controllers/home_controller.rb

@@ -53,8 +53,14 @@ def dashboard_v2
       @blog = Tag.find_nodes_by_type('blog', 'note', 1).limit(1).first
       # Tags without the blog tag and everything tag to avoid double display
       exclude_tids = Tag.where(name: %w(blog everything)).pluck(:tid)
-      # Not all tags have notes, some are wikis. A text will be displayed to show this.
-      @pagy, @tag_subscriptions = pagy(current_user.subscriptions(:tag).includes(:tag).where.not(tid: exclude_tids))
+      @pagy, @tag_subscriptions = pagy(
+        TagSelection
+        .select('tag_selections.tid, term_data.name')
+        .where(user_id: current_user.id, following: true)
+        .where.not(tid: exclude_tids)
+        .joins("INNER JOIN term_data ON tag_selections.tid = term_data.tid")
+        .order("term_data.activity_timestamp DESC")
+      )
       @trending_tags = trending
       render template: 'dashboard_v2/dashboard'
     else

app/controllers/wiki_controller.rb

@@ -486,11 +486,16 @@ def update_node_attributes
         @node.main_image_id = img.id
         img.save
       end
-
       @node.save
+      update_tags if @node.valid?
     end
   end
 
+  def update_tags
+    tids = @node.tag.pluck(:tid)
+    Tag.update_tags_activity(tids, @node.nid)
+  end
+
   def author
     @user = User.find_by(name: params[:id])
     @title = @user.name

app/models/node.rb

@@ -668,9 +668,16 @@ def add_comment(params = {})
                     message_id: params[:message_id],
                     tweet_id: params[:tweet_id])
     c.save
+    tag_activity(c.cid) if c.valid?
     c
   end
 
+  def tag_activity(cid)
+    tids = tag.pluck(:tid)
+    comment_id = "c#{cid}"
+    Tag.update_tags_activity(tids, comment_id)
+  end
+
   def new_revision(params)
     title = params[:title] || self.title
     Revision.new(nid: id,
@@ -871,7 +878,6 @@ def add_tag(tagname, user)
 
             if node_tag.save
               saved = true
-              tag.run_count # update count of tag usage
               # send email notification if there are subscribers, status is OK, and less than 1 month old
               isStatusValid = status == 3 || status == 4
               isMonthOld = created < (DateTime.now - 1.month).to_i

app/models/node_tag.rb

@@ -8,7 +8,7 @@ class NodeTag < ApplicationRecord
   has_many :tag_selections, foreign_key: 'tid'
   accepts_nested_attributes_for :tag
 
-  after_create :update_count
+  after_create :update_count, :update_activity
   after_destroy :update_count
 
   def update_count
@@ -30,4 +30,10 @@ def name
   def description
     tag.description if tag&.description && !tag.description.empty?
   end
+
+  def update_activity
+    tag.activity_timestamp = DateTime.now
+    tag.latest_activity_nid = nid
+    tag.save
+  end
 end

app/models/tag.rb

@@ -436,6 +436,10 @@ def self.tag_frequency(limit)
     hash.sort_by { |_, v| v }.reverse.first(limit).to_h
   end
 
+  def self.update_tags_activity(tids = [], activity_id = nil)
+    Tag.where(tid: tids).update_all(activity_timestamp: DateTime.now, latest_activity_nid: activity_id)
+  end
+
   private
 
   def tids

app/views/comments/_form.html.erb

@@ -167,8 +167,8 @@
         };
       </script>
       <script src="/emoji.js" type="text/javascript"></script>
-      <%# if this script runs on the edit form, it will blank out the value of #text-input-edit-123 %>
-      <script src="/assets/atwho_autocomplete.js" type="text/javascript"></script>
+      <!-- if this script runs on the edit form, it will blank out the value of #text-input-edit-123 -->
+      <script src="/assets/atWhoAutoComplete.js" type="text/javascript"></script>
     <% end %>
     <div class="control-group">
       <button 

app/views/dashboard_v2/_sidebar.html.erb

@@ -1,4 +1,4 @@
-<%= javascript_include_tag('async_tag_subscriptions.js') %>
+<%= javascript_include_tag('asyncTagSubscriptions.js') %>
 <%# TODO Location not working, will be fixed after working on https://github.com/publiclab/plots2/issues/8566 %>
 <% if current_user %>
   <div style="margin: 2.2rem 0;">

app/views/dashboard_v2/_topicCard.html.erb

@@ -2,26 +2,27 @@
 <% subscriptions ||= @tag_subscriptions  # allow overriding with local variable %>
 <% shown_nids = [@blog.nid] %>
 <% subscriptions.each do |subscription| %>
-  <% notes = Tag.find_nodes_by_type(subscription.tag.name, type = 'note', limit = 3).where.not(nid: shown_nids)  %>
+  <% tag_name = subscription.tagname %>
+  <% notes = Tag.find_nodes_by_type(tag_name, type = 'note', limit = 3).where.not(nid: shown_nids)  %>
     <% if notes.present?  %> <%# Topics without notes will not be displayed. If pagination shows many empty pages, refer to https://github.com/publiclab/plots2/issues/8964#issuecomment-763303076 %>
       <div class="card-view pt-5">
           <div class="card shadow-sm p-2 mb-3 bg-white rounded" >
             <div class="card-header mt-2" style=" padding:0.8em; background-color: inherit; border:none;">
             <%# Unfollow section %>
-              <% if current_user && current_user.following(subscription.tag.name) %>
+              <% if current_user && current_user.following(tag_name) %>
                 <a class="ellipsis pull-right" data-toggle="dropdown" style="cursor:pointer" aria-expanded="true"><i class="fa fa-ellipsis-h" style="color:#ccc;font-size:18px;margin-right:10px;"></i></a>
                 <div class="dropdown-menu">
                   <div class="dropdown-item">
-                    <a style="margin-top:-8px;" data-method="delete" data-confirm="Are you sure you'd like to unfollow this topic?" rel="tooltip" title="<%= translation('tag.show.unfollow') %>" href="/unsubscribe/tag/<%= subscription.tag.name %>">
+                    <a style="margin-top:-8px;" data-method="delete" data-confirm="Are you sure you'd like to unfollow this topic?" rel="tooltip" title="<%= translation('tag.show.unfollow') %>" href="/unsubscribe/tag/<%= tag_name %>">
                       <%= translation('tag.show.unfollow_text') %>
                     </a>
                   </div>
                 </div>
               <% end %>
             <%# Topic name %>
-              <h4 style="display: inline-block;"><a href="/tag/<%= subscription.tag.name %>"><%= subscription.tag.name %></a></h4>
+              <h4 style="display: inline-block;"><a href="/tag/<%= tag_name %>"><%= tag_name %></a></h4>
             <%# Follower count  %>
-              <p style="display: inline-block; color: #808080;"><a style="text-decoration: underline; color: #808080;" href="/tag/<%= subscription.tag.name %>"><%= Tag.follower_count(subscription.tag.name) %> <%= translation('tag.show.following') %></a></p>
+              <p style="display: inline-block; color: #808080;"><a style="text-decoration: underline; color: #808080;" href="/tag/<%= tag_name %>"><%= Tag.follower_count(tag_name) %> <%= translation('tag.show.following') %></a></p>
             </div>
             <div class="card-body" style="padding:0.8em;">
             <%# First 3 notes in the Topic  %>
@@ -48,15 +49,15 @@
             </div>
 
             <div class="card-footer" style="background-color: inherit; border:none;">
-              <% note_count = post_count(subscription.tag.name, 'note') %>
-              <% wiki_count = post_count(subscription.tag.name, 'page') %>
-              <a style="padding-top:15px;text-decoration:underline;color:#808080;display:inline-block;" href="/tag/<%= subscription.tag.name %>"><%= note_count %> <% if note_count > 1 %> <%= translation('tag.index.post').pluralize %> <% else %> <%= translation('tag.index.post') %> <% end %></a> |
-              <a style="text-decoration:underline;color:#808080;display:inline-block;" href="/wiki/<%= subscription.tag.name %>"><%= wiki_count %><% if wiki_count > 1 %> <%= translation('tag.index.wiki').pluralize %> <% else %> <%= translation('tag.index.wiki') %> <% end %></a>
+              <% note_count = post_count(tag_name, 'note') %>
+              <% wiki_count = post_count(tag_name, 'page') %>
+              <a style="padding-top:15px;text-decoration:underline;color:#808080;display:inline-block;" href="/tag/<%= tag_name %>"><%= note_count %> <% if note_count > 1 %> <%= translation('tag.index.post').pluralize %> <% else %> <%= translation('tag.index.post') %> <% end %></a> |
+              <a style="text-decoration:underline;color:#808080;display:inline-block;" href="/wiki/<%= tag_name %>"><%= wiki_count %><% if wiki_count > 1 %> <%= translation('tag.index.wiki').pluralize %> <% else %> <%= translation('tag.index.wiki') %> <% end %></a>
               <div id="new-post" style="float:right;margin:10px 0 10px 10px;">
                 <% if current_user %>
-                  <a rel="tooltip" title="<%= translation('sidebar._post_button.share_your_work') %>" data-placement="bottom" href="/post?tags=<%= subscription.tag.name %>" class="btn btn-primary btn-sm requireLogin"><%= translation('tag.show.new_post') %> <i class="fa fa-plus fa-white"></i></a>
+                  <a rel="tooltip" title="<%= translation('sidebar._post_button.share_your_work') %>" data-placement="bottom" href="/post?tags=<%= tag_name %>" class="btn btn-primary btn-sm requireLogin"><%= translation('tag.show.new_post') %> <i class="fa fa-plus fa-white"></i></a>
                 <% else %>
-                  <a class="btn btn-primary btn-sm index-follow-buttons follow-btn-remote requireLogin" href="/subscribe/tag/<%= subscription.tag.name %>" data-remote="true"><%= translation('tag.index.follow') %></a>
+                  <a class="btn btn-primary btn-sm index-follow-buttons follow-btn-remote requireLogin" href="/subscribe/tag/<%= tag_name %>" data-remote="true"><%= translation('tag.index.follow') %></a>
                 <% end %>
               </div>
             </div>

app/views/editor/post.html.erb

@@ -62,15 +62,16 @@
     url = { :controller => "notes", :action => "create", :id => params[:id] }
   end
   %>
-  <%= form_for :node, :as => :drupal_node, :url => url do |f| %>
-    <% if f.error_messages != "" %><div class="alert alert-danger"><%= f.error_messages :header_message => "Your note couldn't be saved." %></div><% end %>
-  <% end %>
   <div class="card bg-light">
+    <div class="card-body">
+      <%= form_for :node, :as => :drupal_node, :url => url do |f| %>
+        <%= render partial: 'layouts/errorMessages', locals: { model: @node } if @node.present? %>
+      <% end %>
+
     <div id="legacy-editor-container" class="card-body">
       <%= form_for :revision, :as => :revision, :url => url, :html => {:class => "form well legacy-form row"} do |f| %>
 
-        <% if f.error_messages != "" %><div class="alert alert-danger"><%= f.error_messages :header_message => "Your note couldn't be saved." %></div><% end %>
-
+        <%= render partial: 'layouts/errorMessages', locals: { model: @revision } if @revision.present? %>
         <%= render :partial => "editor/main_image" %>
 
         <div class="col-lg-9 order-lg-1">
@@ -118,8 +119,8 @@
           <p class="col-lg-12">By publishing, you agree to <a target="_blank" href="/licenses">open source your work</a> so that others may use it.</p>
           <br />
         </div>
+        <% end %>
 
-      <% end %>
     </div>
   </div>
 </div>

app/views/images/new.html.erb

@@ -3,7 +3,7 @@
 
   <h2>Upload an image</h2>
 
-  <%= f.error_messages %>
+  <%= render partial: 'layouts/errorMessages', locals: { model: @image } %>
 
   <p><%= f.file_field :photo %></p>
 

app/views/tag/index.html.erb

@@ -1,4 +1,4 @@
-<%= javascript_include_tag('async_tag_subscriptions.js') %>
+<%= javascript_include_tag('asyncTagSubscriptions.js') %>
 <div class="col-md-6">
   <h1 style="padding-top:40px;font-family:Junction Light;"><%= translation('tag.index.popular_tags') %></h1>
 

config/initializers/assets.rb

@@ -33,8 +33,8 @@
   'user_tags.css.scss',
   'wiki.css',
   'horsey.js',
-  'atwho_autocomplete.js',
-  'async_tag_subscriptions.js',
+  'atWhoAutoComplete.js',
+  'asyncTagSubscriptions.js',
   'cable.js',
   'spam2.css',
   'spam2.js',

db/migrate/20210208151828_add_activity_timestamp_to_tag.rb

@@ -0,0 +1,5 @@
+class AddActivityTimestampToTag < ActiveRecord::Migration[5.2]
+  def change
+    add_column :term_data, :activity_timestamp, :datetime
+  end
+end

db/migrate/20210210155509_add_latest_activity_nid_to_tag.rb

@@ -0,0 +1,5 @@
+class AddLatestActivityNidToTag < ActiveRecord::Migration[5.2]
+  def change
+    add_column :term_data, :latest_activity_nid, :string
+  end
+end

test/fixtures/node_tags.yml

@@ -279,3 +279,8 @@ question_with_multiple_comments: # another question for testing comments
   tid: 35
   uid: 2
   nid: 40
+
+sub_tag:
+  tid: 37
+  nid: 1
+  uid: 2

test/functional/comment_controller_test.rb

@@ -44,9 +44,18 @@ def teardown
 
   test 'should create note comments' do
     UserSession.create(users(:bob))
+    node = nodes(:one)
     assert_difference 'Comment.count' do
-      post :create, params: { id: nodes(:one).nid, body: '[notes:awesome]' }, xhr: true
+      post :create, params: { id: node.nid, body: '[notes:awesome]' }, xhr: true
     end
+
+    # latest_activity_nid for the node should be updated with the comment id in the format 'c_comment_id'
+    cid = Comment.where(comment: '[notes:awesome]').first.cid
+    expected_activity_nid = "c#{cid}"
+    node_tid = node.tag.first.tid
+    returned_activity_nid = Tag.where(tid: node_tid).first.latest_activity_nid
+
+    assert_equal expected_activity_nid, returned_activity_nid
     assert_response :success
     assert_not_nil :comment
     assert_template partial: 'notes/_comment'

test/functional/home_controller_test.rb

@@ -141,4 +141,79 @@ def setup
     get :dashboard_v2
     assert_includes response.body, alert
   end
+
+  test 'topic order change after note is created v2/dashboard' do
+    current_user = users(:bob)
+    UserSession.create(current_user)
+    get :dashboard_v2
+    expected_first_topic = assigns[:tag_subscriptions].last.tag
+
+    node = Node.create!(type: 'note', title:'Topic Order Note', uid: current_user.id, status: 1)
+    Revision.create(nid: node.id, uid: current_user.id, title: node.title,  body: 'Topic order Note')
+    node.add_tag(expected_first_topic.name, current_user)
+
+    get :dashboard_v2
+    expected_first_topic.reload
+    new_first_topic = assigns[:tag_subscriptions].first.tag
+
+    # In some cases, the previous first tag might still be the first tag because nodes can have multiple tags.
+    # Checking the latest_activity_node ensures that the most recent activity was bumped up.
+    assert_equal expected_first_topic.latest_activity_nid, new_first_topic.latest_activity_nid
+  end
+
+  test 'topic order change after wiki is created v2/dashboard' do
+    current_user = users(:bob)
+    UserSession.create(current_user)
+    get :dashboard_v2
+    expected_first_topic = assigns[:tag_subscriptions].last.tag
+
+    node = Node.create!(type: 'wiki', title:'Topic Order Wiki', uid: current_user.id, status: 1)
+    Revision.create(nid: node.id, uid: current_user.id, title: node.title,  body: 'Topic order Wiki')
+    node.add_tag(expected_first_topic.name, current_user)
+
+    get :dashboard_v2
+    expected_first_topic.reload
+    new_first_topic = assigns[:tag_subscriptions].first.tag
+
+    # In some cases, the previous first tag might still be the first tag because nodes can have multiple tags.
+    # Checking the latest_activity_node ensures that the most recent activity was bumped up.
+    assert_equal expected_first_topic.latest_activity_nid, new_first_topic.latest_activity_nid
+  end
+
+  test 'topic order change after wiki is edited v2/dashboard' do
+    current_user = users(:bob)
+    UserSession.create(current_user)
+    get :dashboard_v2
+    # User is subscribed to :awesome node_tag which contains a wiki node
+    node_tag = node_tags(:awesome2)
+    expected_first_topic = assigns[:tag_subscriptions].where(tid: node_tag.tid).first.tag
+    node_tag.node.latest.body = "Wiki update"
+
+    get :dashboard_v2
+    expected_first_topic.reload
+    new_first_topic = assigns[:tag_subscriptions].first.tag
+
+    # In some cases, the previous first tag might still be the first tag because nodes can have multiple tags.
+    # Checking the latest_activity_node ensures that the most recent activity was bumped up.
+    assert_equal expected_first_topic.latest_activity_nid, new_first_topic.latest_activity_nid
+  end
+
+  test 'topic order change after comment is created v2/dashboard' do
+    current_user = users(:bob)
+    UserSession.create(current_user)
+    get :dashboard_v2
+    expected_first_topic = assigns[:tag_subscriptions].last.tag
+
+    node_tag = NodeTag.where(tid: expected_first_topic.tid).first
+    node = node_tag.node
+    node.add_comment(subject: 'node comment', uid: current_user.uid, body: 'node body')
+
+    get :dashboard_v2
+    expected_first_topic.reload
+    new_first_topic = assigns[:tag_subscriptions].first.tag
+
+    # In some cases, the previous first tag might still be the first tag because nodes can have multiple tags.
+    # Checking the latest_activity_node ensures that the most recent activity was bumped up.
+    assert_equal expected_first_topic.latest_activity_nid, new_first_topic.latest_activity_nid
+  end
 end

test/functional/notes_controller_test.rb

@@ -278,6 +278,9 @@ def teardown
       email = ActionMailer::Base.deliveries.last
       assert_equal '[PublicLab] ' + title + ' (#' + Node.last.id.to_s + ') ', email.subject
       assert_equal 1, Node.last.status
+      # Tag count should increase
+      tag = Node.last.tag.first
+      assert_equal 1, tag.count
       assert_redirected_to '/notes/' + users(:jeff).username + '/' + Time.now.strftime('%m-%d-%Y') + '/' + title.parameterize
     end
   end
@@ -747,6 +750,12 @@ def teardown
            body: 'Some text.',
            tags: 'event'
            }
+      # latest_activity_nid should be updated with the node nid created in all tags
+      nid = Tag.where(name: 'event').first.latest_activity_nid.to_i
+      node = Node.where(nid: nid).first
+
+      assert_equal node.title, title + lang.to_s
+      assert_equal node.latest.body, 'Some text.'
 
       assert_equal I18n.t('notes_controller.research_note_published'), flash[:notice]
     end