Merge pull request #79 from amatsuda/list_polymorphic

Refactor List to be a model

Author: amatsuda

app/controllers/messages_controller.rb

@@ -6,7 +6,7 @@ def index
     if (list_name = params[:list_name])
       @list = List.find_by_name list_name
 
-      messages = Message.with_recursive(parent_and_children: [Message.where(list_id: @list.id, parent_id: nil).order(:id).limit(100), Message.joins('inner join parent_and_children on messages.parent_id = parent_and_children.id')])
+      messages = Message.with_recursive(parent_and_children: [Message.where(list_id: @list, parent_id: nil).order(:id).limit(100), Message.joins('inner join parent_and_children on messages.parent_id = parent_and_children.id')])
         .joins('inner join parent_and_children on parent_and_children.id = messages.id')
       @messages = compose_tree(messages)
     elsif (query = params[:q])
@@ -22,8 +22,8 @@ def index
 
   # GET /messages/ruby-dev/1
   def show
-    list = List.find_by_name(params[:list_name])
-    @message = Message.find_by(list_id: list.id, list_seq: params[:list_seq])
+    @list = List.find_by_name(params[:list_name])
+    @message = Message.find_by(list_id: @list, list_seq: params[:list_seq])
   end
 
   private

app/models/list.rb

@@ -1,8 +1,11 @@
 class List
+  include ActiveModel::Model
+
   def initialize(name, id)
     @name = name
     @id = id
   end
+
   attr_reader :name, :id
 
   # Ordered by the established dates. ruby-list was started in 1995.
@@ -24,4 +27,12 @@ def find_by_id(id)
 
     alias find find_by_id
   end
+
+  def to_param
+    name
+  end
+
+  def persisted?
+    true
+  end
 end

app/models/message.rb

@@ -48,13 +48,13 @@ def from_mail(mail, list, list_seq)
 
     # mail.in_reply_to returns strange Array object in some cases (?), so let's use the raw value
     parent_message_id_header = extract_message_id_from_in_reply_to(mail.header[:in_reply_to]&.value)
-    self.parent_id = Message.where(list_id: list.id, message_id_header: parent_message_id_header).pick(:id) if parent_message_id_header
+    self.parent_id = Message.where(list_id: list, message_id_header: parent_message_id_header).pick(:id) if parent_message_id_header
     if !self.parent_id && (String === mail.references)
-      self.parent_id = Message.where(list_id: list.id, message_id_header: mail.references).pick(:id)
+      self.parent_id = Message.where(list_id: list, message_id_header: mail.references).pick(:id)
     end
     if !self.parent_id && (Array === mail.references)
       mail.references.compact.each do |ref|
-        break if (self.parent_id = Message.where(list_id: list.id, message_id_header: ref).pick(:id))
+        break if (self.parent_id = Message.where(list_id: list, message_id_header: ref).pick(:id))
       end
     end
 
@@ -130,6 +130,11 @@ def list
     @list ||= List.find(list_id)
   end
 
+  def to_param
+    #NOTE: This value isn't unique system-wide. Ideally, this should return a combination of list_name and list_seq
+    list_seq
+  end
+
   def count_recursively(count = 0)
     count + 1 + (children&.sum(&:count_recursively) || 0)
   end

app/views/messages/_thread.html.erb

@@ -1,6 +1,6 @@
 <div class="thread-message" style="margin-left: <%= depth * 6 %>px;" <% if (depth == 0) && message.children&.any? %>data-controller="thread"<% end %>>
   <% if depth == 0 %>
-    <%= link_to "/#{list.name}/#{message.list_seq}", class: 'message-item block bg-white dark:bg-gray-800 rounded-lg shadow-md border border-gray-200 dark:border-gray-700 overflow-hidden hover:shadow-lg transition-shadow', data: {turbo_frame: 'message_content', turbo_action: 'advance', action: 'click->message-list#select'} do %>
+    <%= link_to [list, message], class: 'message-item block bg-white dark:bg-gray-800 rounded-lg shadow-md border border-gray-200 dark:border-gray-700 overflow-hidden hover:shadow-lg transition-shadow', data: {turbo_frame: 'message_content', turbo_action: 'advance', action: 'click->message-list#select'} do %>
       <div class="p-5">
         <div class="flex items-start justify-between gap-4">
           <div class="flex-1 min-w-0">
@@ -35,7 +35,7 @@
       </div>
     <% end %>
   <% else %>
-    <%= link_to "/#{list.name}/#{message.list_seq}", class: 'message-item block py-2 border-l-2 border-gray-200 dark:border-gray-700 pl-4 hover:border-red-400 dark:hover:border-red-500 transition-colors', data: {turbo_frame: 'message_content', turbo_action: 'advance', action: 'click->message-list#select'} do %>
+    <%= link_to [list, message], class: 'message-item block py-2 border-l-2 border-gray-200 dark:border-gray-700 pl-4 hover:border-red-400 dark:hover:border-red-500 transition-colors', data: {turbo_frame: 'message_content', turbo_action: 'advance', action: 'click->message-list#select'} do %>
       <div class="flex items-start gap-2 text-sm">
         <svg class="w-4 h-4 text-gray-400 dark:text-gray-500 mt-0.5 flex-shrink-0" fill="none" stroke="currentColor" viewBox="0 0 24 24">
           <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M3 10h10a8 8 0 018 8v2M3 10l6 6m-6-6l6-6"></path>

app/views/messages/search.html.erb

@@ -27,10 +27,9 @@
 
 <% @messages.each do |message| %>
   <div class="search-result">
-    <% list_name = List.find(message.list_id).name %>
     <h2 class="subject">
-      <span class="prefix"><%= list_name %>:<%= message.list_seq %></span>
-      <%= link_to without_list_prefix(message.subject), "/#{list_name}/#{message.list_seq}" %>
+      <span class="prefix"><%= message.list.name %>:<%= message.list_seq %></span>
+      <%= link_to without_list_prefix(message.subject), [message.list, message] %>
     </h2>
     <div class="snippet"><%= search_snippet(message.body, params[:q]) %></div>
   </div>

config/routes.rb

@@ -1,7 +1,8 @@
 Rails.application.routes.draw do
-  get '/:list_name/:list_seq', to: 'messages#show'
-  get '/:list_name/', to: 'messages#index'
-
+  get '/:list_name/', to: 'messages#index', as: :list
+  resources :list, only: [], path: '', param: :name do
+    resources :messages, only: :show, path: '', param: :list_seq
+  end
   get '/attachments/:encoded_key/*filename' => 'attachments#show', as: :attachment
 
   # Define your application routes per the DSL in https://guides.rubyonrails.org/routing.html