Stop using assistant slug in url (#588)

app/controllers/messages_controller.rb

@@ -71,7 +71,7 @@ def set_conversation
   end
 
   def set_assistant
-    @assistant = Current.user.assistants_including_deleted.find_by(slug: params[:assistant_id])
+    @assistant = Current.user.assistants_including_deleted.find_by(id: params[:assistant_id])
     @assistant ||= @conversation.latest_message_for_version(@version).assistant
   end
 

app/controllers/settings/assistants_controller.rb

@@ -38,7 +38,7 @@ def destroy
   private
 
   def set_assistant
-    @assistant = Current.user.assistants.find_by(slug: params[:id])
+    @assistant = Current.user.assistants.find_by(id: params[:id])
     if @assistant.nil?
       redirect_to new_settings_assistant_url, notice: "The assistant was deleted", status: :see_other
     end

app/models/assistant.rb

@@ -1,5 +1,6 @@
 class Assistant < ApplicationRecord
   include Export
+  include Slug
 
   MAX_LIST_DISPLAY = 5
 
@@ -23,8 +24,6 @@ class Assistant < ApplicationRecord
 
   delegate :api_name, to: :language_model, prefix: true, allow_nil: true
 
-  before_validation :set_default_slug
-
   def initials
     return nil if name.blank?
 
@@ -38,33 +37,7 @@ def to_s
     name
   end
 
-  def to_param
-    slug
-  end
-
   def language_model_api_name=(api_name)
     self.language_model = LanguageModel.for_user(user).find_by(api_name:)
   end
-
-  private
-
-  # Set the slug to the name, downcased, with non-word characters replaced with "-"
-  # and trailing "-" removed.
-  # If the slug is not unique for the user, append "-2", "-3", etc.
-  def set_default_slug
-    return if slug.present?
-    return if name.blank?
-
-    base_slug = name.downcase.gsub(/[^a-z0-9]+/, "-").gsub(/-$/, "")
-
-    existing_base_slugs = user.assistants.where("slug LIKE ?", "#{base_slug}%").pluck(:slug)
-    largest_slug_number = existing_base_slugs.map { |slug| slug.split("--").last.to_i }.max
-    self.slug = if largest_slug_number.present?
-      "#{base_slug}--#{largest_slug_number + 1}"
-    elsif existing_base_slugs.any?
-      "#{base_slug}--1"
-    else
-      base_slug
-    end
-  end
 end

app/models/assistant/slug.rb

@@ -0,0 +1,31 @@
+module Assistant::Slug
+  extend ActiveSupport::Concern
+
+  included do
+    before_validation :set_default_slug
+    before_save :clear_slug_on_delete
+  end
+
+  private
+
+  def clear_slug_on_delete
+    self.slug = nil if deleted_at_changed? && deleted_at_was.nil? && deleted_at.present?
+  end
+
+  def set_default_slug
+    return if slug.present?
+    return if name.blank?
+
+    base_slug = name.downcase.gsub(/[^a-z0-9]+/, "-").gsub(/-$/, "")
+
+    existing_base_slugs = user.assistants.where("slug LIKE ?", "#{base_slug}%").pluck(:slug)
+    largest_slug_number = existing_base_slugs.map { |slug| slug.split("--").last.to_i }.max
+    self.slug = if largest_slug_number.present?
+      "#{base_slug}--#{largest_slug_number + 1}"
+    elsif existing_base_slugs.any?
+      "#{base_slug}--1"
+    else
+      base_slug
+    end
+  end
+end

test/models/assistant/slug_test.rb

@@ -0,0 +1,59 @@
+require "test_helper"
+
+class Assistant::SlugTest < ActiveSupport::TestCase
+  test "sets a default slug" do
+    samantha = assistants(:samantha)
+    assert_equal "samantha", samantha.slug
+
+    keith_gpt4 = assistants(:keith_gpt4)
+    assert_equal "gpt-4o", keith_gpt4.slug
+    keith_gpt4.slug = nil
+    keith_gpt4.save!
+    assert_equal "openai-gpt-4o", keith_gpt4.slug
+
+    lm = language_models(:gpt_best)
+    user = users(:keith)
+    same_name1 = user.assistants.create!(language_model: lm, name: "Best OpenAI Model")
+    assert_equal "best-openai-model", same_name1.slug
+    same_name2 = user.assistants.create!(language_model: lm, name: "Best OpenAI Model")
+    assert_equal "best-openai-model--1", same_name2.slug
+    same_name3 = user.assistants.create!(language_model: lm, name: "Best OpenAI Model")
+    assert_equal "best-openai-model--2", same_name3.slug
+
+    similar_name = user.assistants.create!(language_model: lm, name: "Best OpenAI Model 2")
+    assert_equal "best-openai-model-2", similar_name.slug
+    similar_name2 = user.assistants.create!(language_model: lm, name: "Best OpenAI Model 2")
+    assert_equal "best-openai-model-2--1", similar_name2.slug
+  end
+
+  test "ensure all fixtures have a slug defined since it will not be set automatically" do
+    Assistant.all.each do |assistant|
+      assert_not_nil assistant.slug, "Assistant #{assistant.name} does not have a slug defined in the fixture"
+    end
+  end
+
+  test "clears slug when assistant is deleted" do
+    assistant = assistants(:samantha)
+    original_slug = assistant.slug
+    assert_not_nil original_slug
+
+    assistant.deleted!
+    assert_nil assistant.reload.slug
+
+    # Create a new assistant with the same name
+    new_assistant = assistant.user.assistants.create!(
+      name: assistant.name,
+      language_model: assistant.language_model,
+      tools: assistant.tools
+    )
+    assert_equal original_slug, new_assistant.slug
+  end
+
+  test "does not clear slug when other attributes change" do
+    assistant = assistants(:samantha)
+    original_slug = assistant.slug
+
+    assistant.update!(name: "New Name")
+    assert_equal original_slug, assistant.reload.slug
+  end
+end

test/models/assistant_test.rb

@@ -1,166 +1,25 @@
 require "test_helper"
 
 class AssistantTest < ActiveSupport::TestCase
-  test "has an associated user" do
-    assert_instance_of User, assistants(:samantha).user
-  end
-
-  test "has associated conversations" do
-    assert_instance_of Conversation, assistants(:samantha).conversations.first
-  end
-
-  test "has supports_images?" do
-    assert assistants(:samantha).supports_images?
-    refute assistants(:keith_gpt3).supports_images?
-  end
-
-  test "has associated documents" do
-    assert_instance_of Document, assistants(:samantha).documents.first
-  end
-
-  test "has associated runs" do
-    assert_instance_of Run, assistants(:samantha).runs.first
-  end
-
-  test "has associated steps" do
-    assert_instance_of Step, assistants(:samantha).steps.first
-  end
-
-  test "has associated messages (through conversations)" do
-    assert_instance_of Message, assistants(:samantha).messages.first
-  end
-
-  test "has associated language_model" do
-    assert_instance_of LanguageModel, assistants(:samantha).language_model
-  end
-
-  test "tools is an array of objects" do
-    assert_instance_of Array, assistants(:samantha).tools
-  end
-
-  test "simple create works and tool defaults to empty array" do
-    a = nil
-    assert_nothing_raised do
-      a = Assistant.create!(
-        user: users(:keith),
-        language_model: language_models(:gpt_4o),
-        name: "abc"
-      )
-    end
-  end
-
-  test "assert execption occures when external ids are not unique" do
-    Assistant.create!(user: users(:keith), language_model: language_models(:gpt_4o), name: "new", external_id: "1")
-    assert_raise ActiveRecord::RecordNotUnique do
-      Assistant.create!(user: users(:rob), language_model: language_models(:gpt_4o), name: "new", external_id: "1")
-    end
-  end
-
-  test "associations are deleted upon destroy" do
-    assistant = assistants(:samantha)
-    conversation_count = assistant.conversations.count * -1
-    message_count = assistant.conversations.map { |c| c.messages.length }.sum * -1
-    document_count = (assistant.documents.count+assistant.conversations.sum { |c| c.messages.sum { |m| m.documents.count }}) * -1
-    run_count = assistant.runs.count * -1
-    step_count = assistant.steps.count * -1
-
-    assert_difference "Message.count", message_count do
-      assert_difference "Conversation.count", conversation_count do
-        assert_difference "Document.count", document_count do
-          assert_difference "Run.count", run_count do
-            assert_difference "Step.count", step_count do
-              assistant.destroy!
-            end
-          end
-        end
-      end
-    end
-  end
-
-  test "associations are left intact upon soft delete" do
-    assistant = assistants(:samantha)
-
-    assert_no_difference "Message.count" do
-      assert_no_difference "Conversation.count" do
-        assert_no_difference "Document.count" do
-          assert_no_difference "Run.count" do
-            assert_no_difference "Step.count" do
-              assistant.deleted!
-            end
-          end
-        end
-      end
-    end
-    refute_nil assistant.deleted_at
-  end
-
-  test "associations are not deleted upon soft delete" do
-    assert_no_difference "Message.count" do
-      assert_no_difference "Conversation.count" do
-        assert_no_difference "Document.count" do
-          assert_no_difference "Run.count" do
-            assert_no_difference "Step.count" do
-              assistants(:samantha).deleted!
-            end
-          end
-        end
-      end
-    end
-  end
-
-  test "initials returns a single letter" do
-    assert_equal "S", assistants(:samantha).initials
-  end
-
-  test "initials returns a single two letters for two-word names" do
-    assistants(:samantha).update!(name: "Samantha Jones")
-    assert_equal "SJ", assistants(:samantha).initials
-  end
-
-  test "initials will split on - and return two characters" do
-    assert_equal "G4", assistants(:rob_gpt4).initials
-  end
-
-  test "language model validated" do
-    record = Assistant.new
-    refute record.valid?
-    assert record.errors[:language_model].present?
-  end
-
-  test "name validated" do
-    record = Assistant.new
-    refute record.valid?
-    assert record.errors[:name].present?
-  end
-
-  test "sets a default slug" do
+  test "initials" do
     samantha = assistants(:samantha)
-    assert_equal "samantha", samantha.slug
+    assert_equal "S", samantha.initials
 
     keith_gpt4 = assistants(:keith_gpt4)
-    assert_equal "gpt-4o", keith_gpt4.slug
-    keith_gpt4.slug = nil
-    keith_gpt4.save!
-    assert_equal "openai-gpt-4o", keith_gpt4.slug
+    assert_equal "OG", keith_gpt4.initials
 
-    lm = language_models(:gpt_best)
-    user = users(:keith)
-    same_name1 = user.assistants.create!(language_model: lm, name: "Best OpenAI Model")
-    assert_equal "best-openai-model", same_name1.slug
-    same_name2 = user.assistants.create!(language_model: lm, name: "Best OpenAI Model")
-    assert_equal "best-openai-model--1", same_name2.slug
-    same_name3 = user.assistants.create!(language_model: lm, name: "Best OpenAI Model")
-    assert_equal "best-openai-model--2", same_name3.slug
+    keith_gpt3 = assistants(:keith_gpt3)
+    assert_equal "G3", keith_gpt3.initials
+  end
 
-    similar_name = user.assistants.create!(language_model: lm, name: "Best OpenAI Model 2")
-    assert_equal "best-openai-model-2", similar_name.slug
-    similar_name2 = user.assistants.create!(language_model: lm, name: "Best OpenAI Model 2")
-    assert_equal "best-openai-model-2--1", similar_name2.slug
+  test "to_s" do
+    samantha = assistants(:samantha)
+    assert_equal "Samantha", samantha.to_s
   end
 
-  test "ensure all fixtures have a slug defined since it will not be set automatically" do
-    Assistant.all.each do |assistant|
-      assert_not_nil assistant.slug, "Assistant #{assistant.name} does not have a slug defined in the fixture"
-    end
+  test "language_model_api_name=" do
+    assistant = assistants(:samantha)
+    assistant.language_model_api_name = "gpt-4o"
+    assert_equal language_models(:gpt_4o), assistant.language_model
   end
 end