Add document delete (#227)

* basic doc delete

* show deleted

---------

Co-authored-by: Vijay Swamidass <[email protected]>

Author: vswamidass-sfdc

app/controllers/base_documents_controller.rb

@@ -2,14 +2,21 @@
 
 class BaseDocumentsController < ApplicationController
   helper_method :can_manage_documents?
-  before_action :set_document, only: %i[show edit update]
+  before_action :set_document, only: %i[show edit update destroy restore]
 
   include Hashable
   include GptConcern
   include NeighborConcern
   # GET /documents or /documents.json
   def index
-    @documents = Document.includes(:library, :user)
+    # Handle show_deleted parameter to include deleted documents
+    @documents = if params[:show_deleted] == 'true'
+                   Document.unscoped.includes(:library, :user)
+                 elsif params[:show_deleted] == 'only'
+                   Document.unscoped.includes(:library, :user).where.not(deleted_date: nil)
+                 else
+                   Document.includes(:library, :user).not_deleted
+                 end
 
     # Apply cheap filters first for better performance
     library_id = params[:library_id]
@@ -136,6 +143,30 @@ def create
     end
   end
 
+  # DELETE /documents/1 or /documents/1.json
+  def destroy
+    authorize @document
+
+    @document.soft_delete!
+
+    respond_to do |format|
+      format.html { redirect_to @document, notice: 'Document was successfully deleted.' }
+      format.json { head :no_content }
+    end
+  end
+
+  # PATCH /documents/1/restore or /documents/1/restore.json
+  def restore
+    authorize @document, :destroy? # Use same authorization as destroy
+
+    @document.restore!
+
+    respond_to do |format|
+      format.html { redirect_to @document, notice: 'Document was successfully restored.' }
+      format.json { render :show, status: :ok, location: @document }
+    end
+  end
+
   private
 
   def can_manage_documents?
@@ -151,7 +182,7 @@ def can_manage_documents?
 
   # Use callbacks to share common setup or constraints between actions.
   def set_document
-    @document = Document.find(params[:id])
+    @document = Document.unscoped.find(params[:id])
   end
 
   # Only allow a list of trusted parameters through.

app/models/document.rb

@@ -62,6 +62,14 @@ class Document < ApplicationRecord
     scope = nearest_neighbors(:embedding, embedding, distance: 'euclidean')
     scope.order(updated_at: :desc).limit(limit)
   }
+
+  # Default scope to only show non-deleted documents
+  default_scope { where(deleted_date: nil) }
+
+  # Soft delete scopes
+  scope :not_deleted, -> { where(deleted_date: nil) }
+  scope :deleted, -> { where.not(deleted_date: nil) }
+  scope :with_deleted, -> { unscoped }
   belongs_to :library, counter_cache: true
   belongs_to :user
 
@@ -174,4 +182,21 @@ def update_search_vector
       "SELECT #{sanitized_sql} AS tsvector"
     ).first['tsvector']
   end
+
+  # Soft delete methods
+  def soft_delete!
+    update!(deleted_date: Time.current)
+  end
+
+  def restore!
+    update!(deleted_date: nil)
+  end
+
+  def deleted?
+    deleted_date.present?
+  end
+
+  def active?
+    !deleted?
+  end
 end

app/policies/document_policy.rb

@@ -16,6 +16,10 @@ def update?
     user.admin? || document.library.user_id == user.id || user_is_editor?
   end
 
+  def destroy?
+    user.admin? || document.library.user_id == user.id || user_is_editor?
+  end
+
   def flag?
     !user.nil? # Any logged-in user can flag documents
   end

app/views/documents/_document.html.erb

@@ -1,10 +1,34 @@
 <div class="flex justify-between items-center">
-  <h1 class="text-3xl font-semibold <%= @document.enabled ? 'text-stone-900' : 'text-gray-500' %>"><%= document.title %></h1>
+  <h1 class="text-3xl font-semibold <%= @document.enabled ? 'text-stone-900' : 'text-gray-500' %>">
+    <%= document.title %>
+    <% if document.deleted? %>
+      <span class="text-lg text-red-600 font-normal ml-2">(Deleted)</span>
+    <% end %>
+  </h1>
   <div class="flex space-x-2">
     <% if policy(@document).edit? %>
       <div class="text-gray-700">
-        <div class="flex items-center">
-          <%= render partial: 'shared/button_group', locals: { buttons: { "Edit" => edit_document_path } } %>
+        <div class="inline-flex rounded-md shadow-sm" role="group">
+          <% unless document.deleted? %>
+            <%= link_to "Edit", edit_document_path, class: "border text-sky-700 border-sky-500 py-3 px-5 bg-white text-sm #{'border-r-0' if policy(@document).destroy?} rounded-l-lg #{'rounded-r-lg' unless policy(@document).destroy?} hover:bg-sky-500 hover:text-sky-100" %>
+          <% end %>
+          <% if policy(@document).destroy? %>
+            <% if document.deleted? %>
+              <%= link_to "Undelete", restore_document_path(@document), 
+                          data: { 
+                            "turbo-method": "patch",
+                            "turbo-confirm": "Are you sure you want to restore this document?" 
+                          },
+                          class: "border text-green-700 border-green-300 py-3 px-5 bg-white text-sm rounded-lg hover:bg-green-50 hover:text-green-800" %>
+            <% else %>
+              <%= link_to "Delete", document_path(@document), 
+                          data: { 
+                            "turbo-method": "delete",
+                            "turbo-confirm": "Are you sure you want to delete this document?" 
+                          },
+                          class: "border text-red-700 border-red-300 py-3 px-5 bg-white text-sm #{'rounded-l-lg' if document.deleted?} rounded-r-lg hover:bg-red-50 hover:text-red-800" %>
+            <% end %>
+          <% end %>
         </div>
       </div>
     <% end %>

app/views/documents/show.html.erb

@@ -2,7 +2,23 @@
 
 <%= render partial: 'shared/breadcrumb', locals: { breadcrumbs: [['Home', root_path], [@document.library.name, library_path(@document.library)], ["Document", document_path(@document)]] } %>
 
-<% unless @document.enabled %>
+<% if @document.deleted? %>
+  <div class="bg-red-50 border-l-4 border-red-400 p-4 mb-4">
+    <div class="flex">
+      <div class="flex-shrink-0">
+        <svg class="h-5 w-5 text-red-400" viewBox="0 0 20 20" fill="currentColor">
+          <path fill-rule="evenodd" d="M9 2a1 1 0 000 2h2a1 1 0 100-2H9z" clip-rule="evenodd" />
+          <path fill-rule="evenodd" d="M4 5a2 2 0 012-2h8a2 2 0 012 2v6a2 2 0 01-2 2H6a2 2 0 01-2-2V5zm3 2a1 1 0 000 2h6a1 1 0 100-2H7zm0 4a1 1 0 100 2h6a1 1 0 100-2H7z" clip-rule="evenodd" />
+        </svg>
+      </div>
+      <div class="ml-3">
+        <p class="text-sm text-red-700">
+          <strong>Document is deleted.</strong> This document was deleted on <%= @document.deleted_date.strftime("%B %d, %Y at %I:%M %p") %> and will not appear in searches or question results.
+        </p>
+      </div>
+    </div>
+  </div>
+<% elsif !@document.enabled %>
   <div class="bg-amber-50 border-l-4 border-amber-400 p-4 mb-4">
     <div class="flex">
       <div class="flex-shrink-0">

app/views/shared/_doc_list.html.erb

@@ -31,7 +31,14 @@
                 </svg>
                 <%= document.questions_count %> references
               </div>
-              <% unless document.enabled %>
+              <% if document.deleted? %>
+                <div class="flex items-center text-red-600">
+                  <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="w-3 h-3 mr-1">
+                    <path stroke-linecap="round" stroke-linejoin="round" d="m14.74 9-.346 9m-4.788 0L9.26 9m9.968-3.21c.342.052.682.107 1.022.166m-1.022-.165L18.16 19.673a2.25 2.25 0 0 1-2.244 2.077H8.084a2.25 2.25 0 0 1-2.244-2.077L4.772 5.79m14.456 0a48.108 48.108 0 0 0-3.478-.397m-12 .562c.34-.059.68-.114 1.022-.165m0 0a48.11 48.11 0 0 1 3.478-.397m7.5 0v-.916c0-1.18-.91-2.164-2.09-2.201a51.964 51.964 0 0 0-3.32 0c-1.18.037-2.09 1.022-2.09 2.201v.916m7.5 0a48.667 48.667 0 0 0-7.5 0" />
+                  </svg>
+                  Deleted
+                </div>
+              <% elsif !document.enabled %>
                 <div class="flex items-center text-amber-600">
                   <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="w-3 h-3 mr-1">
                     <path stroke-linecap="round" stroke-linejoin="round" d="M12 9v3.75m-9.303 3.376c-.866 1.5.217 3.374 1.948 3.374h14.71c1.73 0 2.813-1.874 1.948-3.374L13.949 3.378c-.866-1.5-3.032-1.5-3.898 0L2.697 16.126ZM12 15.75h.007v.008H12v-.008Z" />

config/routes.rb

@@ -34,13 +34,20 @@
   resources :questions
   resources :documents do
     resources :comments, only: %i[create update destroy]
+    member do
+      patch :restore
+    end
   end
   resources :api_tokens
 
   # Nested Resources
   resources :libraries do
     resources :library_users
-    resources :documents
+    resources :documents do
+      member do
+        patch :restore
+      end
+    end
     member do
       get 'users'
       get 'download'

db/migrate/20251009002318_add_deleted_date_to_documents.rb

@@ -0,0 +1,5 @@
+class AddDeletedDateToDocuments < ActiveRecord::Migration[7.2]
+  def change
+    add_column :documents, :deleted_date, :datetime
+  end
+end

db/schema.rb

@@ -35,9 +35,9 @@
 
     context 'when the document saves successfully' do
       it 'enqueues an EmbedDocumentJob' do
-        expect {
+        expect do
           post :create, params: { document: valid_attributes }
-        }.to have_enqueued_job(EmbedDocumentJob).on_queue('default')
+        end.to have_enqueued_job(EmbedDocumentJob).on_queue('default')
       end
     end
 
@@ -64,6 +64,33 @@
       get :index, params: { contains: 'including keyword' }
       expect(assigns(:documents)).to match_array([Document.find_by(title: 'Second Document')])
     end
+
+    context 'with show_deleted parameter' do
+      let!(:active_document) { Document.create!(valid_attributes.merge(title: 'Active Document', document: 'Active document content')) }
+      let!(:deleted_document) { Document.create!(valid_attributes.merge(title: 'Deleted Document', document: 'Deleted document content')) }
+
+      before do
+        deleted_document.soft_delete!
+      end
+
+      it 'shows only active documents by default' do
+        get :index
+        expect(assigns(:documents)).to include(active_document)
+        expect(assigns(:documents)).not_to include(deleted_document)
+      end
+
+      it 'shows both active and deleted documents when show_deleted=true' do
+        get :index, params: { show_deleted: 'true' }
+        expect(assigns(:documents)).to include(active_document)
+        expect(assigns(:documents)).to include(deleted_document)
+      end
+
+      it 'shows only deleted documents when show_deleted=only' do
+        get :index, params: { show_deleted: 'only' }
+        expect(assigns(:documents)).not_to include(active_document)
+        expect(assigns(:documents)).to include(deleted_document)
+      end
+    end
   end
 
   describe 'POST #create' do