Fix quip sync and job management (#88)

* queue mgmt

* fix doc schedule

* only sync if source url changes

Author: vswamidass-sfdc

app/controllers/delayed_jobs_controller.rb

@@ -4,7 +4,27 @@ class DelayedJobsController < ApplicationController
   before_action :set_job, only: [:destroy]
 
   def index
-    @jobs = Delayed::Job.all
+    @priority_filter = params[:priority]
+
+    @jobs = case @priority_filter
+            when 'high'
+              Delayed::Job.where('priority < ?', 5)
+            when 'low'
+              Delayed::Job.where('priority >= ?', 5)
+            else
+              Delayed::Job.all
+            end
+
+    @jobs = @jobs.order(priority: :asc, run_at: :desc)
+  end
+
+  def run_now
+    @job = Delayed::Job.find(params[:id])
+
+    authorize @job, :update?
+    
+    @job.update(run_at: Time.now)
+    redirect_to delayed_jobs_path, notice: 'Job has been scheduled to run immediately.'
   end
 
   def destroy

app/jobs/sync_quip_doc_job.rb

@@ -2,53 +2,93 @@ class SyncQuipDocJob < ApplicationJob
   queue_as :default
 
   def perform(_doc_id)
-    begin
-      document = Document.find(_doc_id)
-    rescue ActiveRecord::RecordNotFound => e
-      # Handle the case where the document is not found
-      Rails.logger.error("Document with id #{_doc_id} not found: #{e.message}")
-      return # Exit early since there's nothing to sync
-    end
+    document = fetch_document(_doc_id)
+    return unless document
 
-    # Log or handle cases where the document has no quip_url
     if document.source_url.blank?
-      Rails.logger.warn("Document with id #{_doc_id} has no source URL.")
+      log_warning("Document with id #{_doc_id} has no source URL.")
       return
     end
 
-    begin
-      # Initialize the Quip client
-      quip_client = Quip::Client.new(access_token: ENV.fetch('QUIP_TOKEN'))
-      uri = URI.parse(document.source_url)
-      path = uri.path.sub(%r{^/}, '') # Removes the leading /
-      quip_thread = quip_client.get_thread(path)
-
-      # Convert Quip HTML content to Markdown
-      markdown_quip = ReverseMarkdown.convert(quip_thread['html'])
-
-      document.document = markdown_quip
-      document.synced_at = DateTime.current
-      document.last_sync_result = 'SUCCESS'
-      document.save!
-      # Reschedule the job to run again in 24 hours
-      SyncQuipDocJob.set(wait: 24.hours, priority: 10).perform_later(_doc_id)
-      return
-    rescue ActiveRecord::RecordInvalid => e
-      # Handle save! failures (validation errors)
-      Rails.logger.error("Failed to save document with id #{_doc_id}: #{e.record.errors.full_messages.join(', ')}")
-      document.document = "Document save failed: #{e.record.errors.full_messages.join(', ')}"
-    rescue Quip::Error => e
-      # Handle Quip-specific errors
-      Rails.logger.error("Quip API error while fetching document from #{document.source_url}: #{e.message}")
-      document.document = "Error from Quip: #{e.message}"
-    rescue StandardError => e
-      # Handle any other unforeseen errors
-      Rails.logger.error("Unexpected error during sync for document id #{_doc_id}: #{e.message}")
-      document.document = "#{e.message}"
-    end
+    success = sync_document_with_quip(document)
+    update_sync_status(document, success:)
+    schedule_next_sync(document.id, success:)
+  end
+
+  private
+
+  def fetch_document(doc_id)
+    Document.find(doc_id)
+  rescue ActiveRecord::RecordNotFound => e
+    log_error("Document with id #{doc_id} not found: #{e.message}")
+    nil
+  end
+
+  def sync_document_with_quip(document)
+    quip_client = initialize_quip_client
+    quip_thread = fetch_quip_thread(document.source_url, quip_client)
+    return false unless quip_thread
+
+    update_document_from_quip(document, quip_thread)
+    true
+  rescue ActiveRecord::RecordInvalid => e
+    handle_save_error(document, e)
+    false
+  rescue Quip::Error => e
+    handle_quip_error(document, e)
+    false
+  rescue StandardError => e
+    handle_unexpected_error(document, e)
+    false
+  end
+
+  def initialize_quip_client
+    Quip::Client.new(access_token: ENV.fetch('QUIP_TOKEN'))
+  end
+
+  def fetch_quip_thread(source_url, quip_client)
+    uri = URI.parse(source_url)
+    path = uri.path.sub(%r{^/}, '')
+    quip_client.get_thread(path)
+  rescue Quip::Error => e
+    log_error("Quip API error while fetching document from #{source_url}: #{e.message}")
+    nil
+  end
+
+  def update_document_from_quip(document, quip_thread)
+    markdown_quip = ReverseMarkdown.convert(quip_thread['html'])
+    document.update!(document: markdown_quip, synced_at: DateTime.current, last_sync_result: 'SUCCESS')
+  end
+
+  def handle_save_error(document, exception)
+    log_error("Failed to save document with id #{document.id}: #{exception.record.errors.full_messages.join(', ')}")
+    document.update(document: "Document save failed: #{exception.record.errors.full_messages.join(', ')}")
+  end
+
+  def handle_quip_error(document, exception)
+    log_error("Quip API error for document id #{document.id}: #{exception.message}")
+    document.update(document: "Error from Quip: #{exception.message}")
+  end
+
+  def handle_unexpected_error(document, exception)
+    log_error("Unexpected error during sync for document id #{document.id}: #{exception.message}")
+    document.update(document: exception.message)
+  end
+
+  def update_sync_status(document, success:)
+    document.update(synced_at: DateTime.current, last_sync_result: success ? 'SUCCESS' : 'FAILED')
+  end
+
+  def schedule_next_sync(doc_id, success:)
+    delay = success ? 24.hours : 3.hours
+    SyncQuipDocJob.set(wait: delay, priority: 10).perform_later(doc_id)
+  end
+
+  def log_error(message)
+    Rails.logger.error(message)
+  end
 
-    document.last_sync_result = 'FAILED'
-    document.save
-    SyncQuipDocJob.set(wait: 30.minutes, priority: 10).perform_later(_doc_id)
+  def log_warning(message)
+    Rails.logger.warn(message)
   end
 end

app/models/document.rb

@@ -87,19 +87,18 @@ def token_count_must_be_less_than
   # Sync the document with Quip if source_url is present, contains 'quip.com',
   def sync_quip_doc_if_needed
     return unless source_url.present? && source_url.include?('quip.com')
-
-    return unless synced_at.nil? # only schedule if it is for the initial sync
+    return unless new_record? || saved_change_to_source_url? # Only schedule if new or source_url changed
 
     # TODO: improve this dup protection
-    return if last_sync_result == 'SCHEDULED' || last_sync_result == 'FAILED' # Prevent duplicate
-
     self.last_sync_result = 'SCHEDULED'
 
-    SyncQuipDocJob.set(wait: 5.seconds, priority: 10).perform_later(id) # Add delay to prevent race condition with schedule jobs
+    SyncQuipDocJob.set(wait: 5.seconds, priority: 10).perform_later(id) # Add delay to prevent race condition with scheduled jobs
   end
 
   # Schedule the EmbedDocumentJob with a delay based on the number of jobs in the queue
   def schedule_embed_document_job
+    return unless document.present?
+
     total_jobs = Delayed::Job.count
     delay_seconds = total_jobs * 3 # 3-second delay per job in the queue
 

app/policies/delayed/backend/active_record/job_policy.rb

@@ -11,6 +11,10 @@ def initialize(user, job)
           @job = job
         end
 
+        def update?
+          user.admin?
+        end
+
         def destroy?
           user.admin?
         end

app/views/delayed_jobs/index.html.erb

@@ -1,4 +1,9 @@
 <h1 class="text-2xl font-bold my-4">Delayed Jobs (<%= @jobs.count %>)</h1>
+<div class="flex space-x-4 mb-4">
+  <%= link_to 'All', delayed_jobs_path, class: "px-4 py-2 border border-gray-300 rounded #{'bg-sky-800 text-white' if @priority_filter.nil?} hover:bg-gray-500 hover:text-white" %>
+  <%= link_to 'High Priority (< 5)', delayed_jobs_path(priority: 'high'), class: "px-4 py-2 border border-gray-300 rounded #{'bg-sky-800 text-white' if @priority_filter == 'high'} hover:bg-gray-500 hover:text-white" %>
+  <%= link_to 'Low Priority (>= 5)', delayed_jobs_path(priority: 'low'), class: "px-4 py-2 border border-gray-300 rounded #{'bg-sky-800 text-white' if @priority_filter == 'low'} hover:bg-gray-500 hover:text-white" %>
+</div>
 
 <div class="flex flex-wrap -m-2">
   <% @jobs.each do |job| %>
@@ -29,7 +34,10 @@
         <div class="mb-2">
           <strong>Created At:</strong> <%= time_ago_in_words(job.created_at) %> ago
         </div>
-        <%= button_to 'Delete', delayed_job_path(job), method: :delete, data: { confirm: 'Are you sure?' }, class: "mt-auto bg-red-500 hover:bg-red-700 text-white font-bold py-2 px-4 rounded" %>
+        <div class="flex space-x-2 mt-auto">
+          <%= button_to 'Run Now', run_now_delayed_job_path(job), method: :post, class: "bg-white hover:bg-green-700 hover:text-white text-green-500 font-bold py-2 px-4 rounded border " %>
+          <%= button_to 'Delete', delayed_job_path(job), method: :delete, data: { confirm: 'Are you sure?' }, class: "bg-white hover:bg-red-700 hover:text-white text-red-500 font-bold py-2 px-4 rounded border" %>
+        </div>
       </div>
     </div>
   <% end %>

config/routes.rb

@@ -38,7 +38,11 @@
     end
   end
 
-  resources :delayed_jobs, only: %i[index destroy]
+  resources :delayed_jobs, only: %i[index destroy] do
+    member do
+      post :run_now
+    end
+  end
 
   # API Routes - Setting default format to JSON
   namespace :api, defaults: { format: :json } do