Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop using assistant slug in url #588

Merged
merged 2 commits into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/messages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/settings/assistants_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 1 addition & 28 deletions app/models/assistant.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class Assistant < ApplicationRecord
include Export
include Slug

MAX_LIST_DISPLAY = 5

Expand All @@ -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?

Expand All @@ -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
31 changes: 31 additions & 0 deletions app/models/assistant/slug.rb
Original file line number Diff line number Diff line change
@@ -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
59 changes: 59 additions & 0 deletions test/models/assistant/slug_test.rb
Original file line number Diff line number Diff line change
@@ -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
167 changes: 13 additions & 154 deletions test/models/assistant_test.rb
Original file line number Diff line number Diff line change
@@ -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
Loading