Fix change node name do not sync to topic's node_name #1224

ruby-china · Nov 19, 2020 · 3b62898 · 3b62898
1 parent 4cd7fef
commit 3b62898
Show file tree

Hide file tree

Showing 17 changed files with 47 additions and 37 deletions.
diff --git a/app/controllers/admin/topics_controller.rb b/app/controllers/admin/topics_controller.rb
@@ -15,7 +15,7 @@ def index
         @topics = @topics.where("user_id = ?", u.try(:id))
       end
       @topics = @topics.order(id: :desc)
-      @topics = @topics.includes(:user).page(params[:page])
+      @topics = @topics.includes(:node, :user).page(params[:page])
     end
 
     def show

diff --git a/app/controllers/api/v3/topics_controller.rb b/app/controllers/api/v3/topics_controller.rb
@@ -37,7 +37,7 @@ def index
           @topics = @node.topics
         end
 
-        @topics = @topics.without_ban.fields_for_list.includes(:user).send(scope_method_by_type)
+        @topics = @topics.without_ban.fields_for_list.includes(:node, :user).send(scope_method_by_type)
         if %w[no_reply popular].index(params[:type])
           @topics = @topics.last_actived
         elsif params[:type] == "excellent"

diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
@@ -12,9 +12,9 @@ class TopicsController < ApplicationController
   def index
     @suggest_topics = []
     if params[:page].to_i <= 1
-      @suggest_topics = topics_scope.suggest.limit(3)
+      @suggest_topics = topics_scope.suggest.includes(:node).limit(3)
     end
-    @topics = topics_scope.without_suggest.last_actived.page(params[:page])
+    @topics = topics_scope.without_suggest.last_actived.includes(:node).page(params[:page])
     @page_title = t("menu.topics")
     @read_topic_ids = []
     if current_user

diff --git a/app/helpers/topics_helper.rb b/app/helpers/topics_helper.rb
@@ -55,7 +55,8 @@ def topic_close_tag(topic)
     content_tag(:i, "", title: "问题已解决／话题已结束讨论", class: "fa fa-check-circle", data: { toggle: "tooltip" })
   end
 
-  def render_node_name(name, id)
-    link_to(name, main_app.node_topics_path(id), class: "node")
+  def render_node_name(node)
+    return "" if node.blank?
+    link_to(node.name, main_app.node_topics_path(node.id), class: "node")
   end
 end
diff --git a/app/models/topic.rb b/app/models/topic.rb
@@ -45,7 +45,6 @@ class Topic < ApplicationRecord
     exclude_column_ids("node_id", ids)
   }
 
-  before_save { self.node_name = node.try(:name) || "" }
   before_create { self.last_active_mark = Time.now.to_i }
 
   def self.fields_for_list

diff --git a/app/views/admin/topics/index.html.erb b/app/views/admin/topics/index.html.erb
@@ -28,7 +28,7 @@
     <td>
       <%= link_to truncate(topic.title, length: 30), topic, target: "_blank" %>
     </td>
-    <td><%= topic.node_name %></td>
+    <td><%= topic.node&.name %></td>
     <td><%= user_name_tag(topic.user) %></td>
     <td><%= topic.replies_count %></td>
     <td>

diff --git a/app/views/api/v3/application/_topic.json.jbuilder b/app/views/api/v3/application/_topic.json.jbuilder
@@ -30,8 +30,9 @@
 if topic
   json.cache! ["v1.1", topic, defined?(detail)] do
     json.(topic, :id, :title, :created_at, :updated_at, :replied_at, :replies_count,
-          :node_name, :node_id, :last_reply_user_id, :last_reply_user_login,
+          :node_id, :last_reply_user_id, :last_reply_user_login,
           :grade, :likes_count, :suggested_at, :closed_at)
+    json.merge!(node_name: topic.node&.name)
     json.deleted topic.deleted_at.present?
     json.user do
       json.partial! "user", user: topic.user

diff --git a/app/views/notifications/_node_changed.html.erb b/app/views/notifications/_node_changed.html.erb
@@ -6,6 +6,6 @@
   相关信息已删除
 <% else %>
   <div class="media-heading">
-    你发布的话题 <%= topic_title_tag(topic) %> 由于内容原因被管理员移动到了 <%= render_node_name(node.name, node.id) %> 节点，请注意查看节点说明。
+    你发布的话题 <%= topic_title_tag(topic) %> 由于内容原因被管理员移动到了 <%= render_node_name(node) %> 节点，请注意查看节点说明。
   </div>
 <% end %>
diff --git a/app/views/topics/_topic.html.erb b/app/views/topics/_topic.html.erb
@@ -6,7 +6,7 @@
     <div class="infos media-body">
       <div class="title media-heading">
         <%= link_to(main_app.topic_path(topic), title: topic.title) do %>
-          <%= raw content_tag(:span, topic.node_name, class: 'node') %>
+          <%= raw content_tag(:span, topic.node&.name, class: 'node') %>
           <%= topic.title %>
         <% end %>
         <% if suggest %>

diff --git a/app/views/topics/_topic_info.html.erb b/app/views/topics/_topic_info.html.erb
@@ -1,7 +1,7 @@
 <div class="card-header media clearfix">
   <div class="media-body">
     <h1 class="media-heading">
-      <%= render_node_name(topic.node_name, topic.node_id) %>
+      <%= render_node_name(node) %>
       <span class="title">
         <%= topic.title %>
       </span>
@@ -10,7 +10,7 @@
     <% content_for :header do %>
       <div class="navbar-topic-title">
         <a href="#" class="topic-title pull-left" title="<%= topic.title %>" data-type="top">
-          <h1><span class="node"><%= topic.node_name %></span> <span class="title"><%= topic.title %><%= topic_close_tag(topic) %></span></h1>
+          <h1><span class="node"><%= node&.name %></span> <span class="title"><%= topic.title %><%= topic_close_tag(topic) %></span></h1>
         </a>
       </div>
     <% end %>

diff --git a/app/views/topics/node_feed.builder b/app/views/topics/node_feed.builder
@@ -3,9 +3,9 @@
 xml.instruct! :xml, version: "1.0"
 xml.rss version: "2.0" do
   xml.channel do
-    xml.title t("rss.recent_node_topics_title", name: Setting.app_name, node_name: @node.name)
+    xml.title t("rss.recent_node_topics_title", name: Setting.app_name, node_name: @node&.name)
     xml.link root_url
-    xml.description t("rss.recent_node_topics_description", name: Setting.app_name, node_name: @node.name)
+    xml.description t("rss.recent_node_topics_description", name: Setting.app_name, node_name: @node&.name)
     @topics.each do |topic|
       xml.item do
         xml.title topic.title

diff --git a/app/views/topics/show.html.erb b/app/views/topics/show.html.erb
@@ -22,7 +22,7 @@
 <div class="row">
   <div class="col-lg-9">
     <div class="topic-detail card">
-      <%= render partial: "topics/topic_info", locals: { topic: @topic } %>
+      <%= render partial: "topics/topic_info", locals: { topic: @topic, node: @node } %>
       <% if @topic.excellent? %>
         <div class="label-awesome">
           <i class="fa fa-award awesome"></i> 本帖已被设为精华帖！

diff --git a/db/migrate/20201119010515_remove_node_name_from_topics.rb b/db/migrate/20201119010515_remove_node_name_from_topics.rb
@@ -0,0 +1,5 @@
+class RemoveNodeNameFromTopics < ActiveRecord::Migration[6.1]
+  def change
+    remove_column :topics, :node_name
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_10_16_074728) do
+ActiveRecord::Schema.define(version: 2020_11_19_010515) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -291,7 +291,6 @@
     t.integer "last_reply_id"
     t.integer "last_reply_user_id"
     t.string "last_reply_user_login"
-    t.string "node_name"
     t.string "who_deleted"
     t.integer "last_active_mark"
     t.boolean "lock_node", default: false

diff --git a/public/packs-test/manifest.json b/public/packs-test/manifest.json
@@ -1,41 +1,41 @@
 {
-  "admin.css": "/packs-test/css/admin-e44e9b0a.css",
-  "admin.js": "/packs-test/js/admin-ffcf61e7b3c79ae446a5.js",
+  "admin.css": "/packs-test/css/admin-0ed548df.css",
+  "admin.js": "/packs-test/js/admin-25870bfbbbbc46f82207.js",
   "application.js": "/packs-test/js/application-afb598184845268b886d.js",
   "entrypoints": {
     "admin": {
       "css": [
-        "/packs-test/css/admin-e44e9b0a.css"
+        "/packs-test/css/admin-0ed548df.css"
       ],
       "js": [
-        "/packs-test/js/admin-ffcf61e7b3c79ae446a5.js"
+        "/packs-test/js/admin-25870bfbbbbc46f82207.js"
       ]
     },
     "application": {
       "js": [
-        "/packs-test/js/vendors-ab58bfd6f6f0318f2f8c.chunk.js",
+        "/packs-test/js/vendors-844fc2a4f67f0408e857.chunk.js",
         "/packs-test/js/application-afb598184845268b886d.js"
       ]
     },
     "front": {
       "css": [
-        "/packs-test/css/front-920436d3.css"
+        "/packs-test/css/front-8980e88a.css"
       ],
       "js": [
-        "/packs-test/js/front-c53a5cbd9c08e94d7f27.js"
+        "/packs-test/js/front-3c4c8f6478a9c6266925.js"
       ]
     },
     "turbolinks-app": {
       "css": [
-        "/packs-test/css/turbolinks-app-653f8ab1.css"
+        "/packs-test/css/turbolinks-app-7b10bc0a.css"
       ],
       "js": [
-        "/packs-test/js/turbolinks-app-1e26708b9ea93cb5371f.js"
+        "/packs-test/js/turbolinks-app-b545e576e4b5d8545c21.js"
       ]
     }
   },
-  "front.css": "/packs-test/css/front-920436d3.css",
-  "front.js": "/packs-test/js/front-c53a5cbd9c08e94d7f27.js",
+  "front.css": "/packs-test/css/front-8980e88a.css",
+  "front.js": "/packs-test/js/front-3c4c8f6478a9c6266925.js",
   "media/webfonts/fa-brands-400.eot": "/packs-test/media/webfonts/fa-brands-400-0fea2496.eot",
   "media/webfonts/fa-brands-400.svg": "/packs-test/media/webfonts/fa-brands-400-e33e2cf6.svg",
   "media/webfonts/fa-brands-400.ttf": "/packs-test/media/webfonts/fa-brands-400-ec82f282.ttf",
@@ -46,7 +46,7 @@
   "media/webfonts/fa-solid-900.ttf": "/packs-test/media/webfonts/fa-solid-900-10ecefc2.ttf",
   "media/webfonts/fa-solid-900.woff": "/packs-test/media/webfonts/fa-solid-900-46fdbd2d.woff",
   "media/webfonts/fa-solid-900.woff2": "/packs-test/media/webfonts/fa-solid-900-3ceb50e7.woff2",
-  "turbolinks-app.css": "/packs-test/css/turbolinks-app-653f8ab1.css",
-  "turbolinks-app.js": "/packs-test/js/turbolinks-app-1e26708b9ea93cb5371f.js",
-  "vendors.js": "/packs-test/js/vendors-ab58bfd6f6f0318f2f8c.chunk.js"
+  "turbolinks-app.css": "/packs-test/css/turbolinks-app-7b10bc0a.css",
+  "turbolinks-app.js": "/packs-test/js/turbolinks-app-b545e576e4b5d8545c21.js",
+  "vendors.js": "/packs-test/js/vendors-844fc2a4f67f0408e857.chunk.js"
 }
diff --git a/test/helpers/topics_helper_test.rb b/test/helpers/topics_helper_test.rb
@@ -62,4 +62,14 @@ class TopicsHelperTest < ActionView::TestCase
       assert_equal "<a data-id=\"#{topic.id}\" class=\"follow active\" href=\"#\"><i class=\"fa fa-eye\"></i> 订阅</a>", topic_follow_tag(topic)
     end
   end
+
+  test "render_node_name" do
+    node = create :node
+
+    # should return empty when node is nil
+    assert_equal "", render_node_name(nil)
+
+    # should return a link with node name
+    assert_equal "<a class=\"node\" href=\"/topics/node#{node.id}\">#{node.name}</a>", render_node_name(node)
+  end
 end
diff --git a/test/models/topic_test.rb b/test/models/topic_test.rb
@@ -26,11 +26,6 @@ class TopicTest < ActiveSupport::TestCase
     assert_equal last_active_mark_was, topic.last_active_mark
   end
 
-  test "should get node name" do
-    node = create :node
-    assert_equal node.name, create(:topic, node: node).node_name
-  end
-
   test "should update after reply" do
     reply = create :reply, topic: topic, user: user
     refute_nil topic.last_active_mark