-
Notifications
You must be signed in to change notification settings - Fork 32
Handle schema_dump_path
being unavailable in older Rails versions
#264
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,7 @@ class Server | |
|
||
extend T::Sig | ||
|
||
sig { params(model_name: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) } | ||
sig { params(model_name: String).returns(T::Hash[Symbol, T.untyped]) } | ||
def resolve_database_info_from_model(model_name) | ||
const = ActiveSupport::Inflector.safe_constantize(model_name) | ||
unless const && const < ActiveRecord::Base && !const.abstract_class? | ||
|
@@ -35,14 +35,18 @@ def resolve_database_info_from_model(model_name) | |
} | ||
end | ||
|
||
schema_file = ActiveRecord::Tasks::DatabaseTasks.schema_dump_path(const.connection.pool.db_config) | ||
|
||
{ | ||
info = { | ||
result: { | ||
columns: const.columns.map { |column| [column.name, column.type] }, | ||
schema_file: ::Rails.root + schema_file, | ||
}, | ||
} | ||
|
||
if ActiveRecord::Tasks::DatabaseTasks.respond_to?(:schema_dump_path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's possible we can make this also work in Rails 6 and earlier, if someone wants to open a PR. |
||
info[:result][:schema_file] = | ||
ActiveRecord::Tasks::DatabaseTasks.schema_dump_path(const.connection.pool.db_config) | ||
|
||
end | ||
info | ||
rescue => e | ||
{ | ||
error: e.message, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
# typed: true | ||
# frozen_string_literal: true | ||
|
||
require "test_helper" | ||
require "ruby_lsp/ruby_lsp_rails/server" | ||
|
||
class ServerTest < ActiveSupport::TestCase | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @vinistock since the server is in a different process, this needs to be tested differently from the other aspects in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pushed a commit to show how that would look. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is fine, but let's put the server in an instance variable instead of doing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is fine, but let's put the server in an instance variable instead of doing |
||
setup do | ||
@server = RubyLsp::Rails::Server.new | ||
end | ||
|
||
test "returns nil if model doesn't exist" do | ||
response = @server.resolve_database_info_from_model("Foo") | ||
assert_nil(response.fetch(:result)) | ||
end | ||
|
||
test "returns nil if class is not a model" do | ||
response = @server.resolve_database_info_from_model("Time") | ||
assert_nil(response.fetch(:result)) | ||
end | ||
|
||
test "returns nil if class is an abstract model" do | ||
response = @server.resolve_database_info_from_model("ApplicationRecord") | ||
assert_nil(response.fetch(:result)) | ||
end | ||
|
||
test "handles older Rails version which don't have `schema_dump_path`" do | ||
ActiveRecord::Tasks::DatabaseTasks.send(:alias_method, :old_schema_dump_path, :schema_dump_path) | ||
ActiveRecord::Tasks::DatabaseTasks.undef_method(:schema_dump_path) | ||
response = @server.resolve_database_info_from_model("User") | ||
assert_nil(response.fetch(:result)[:schema_file]) | ||
ensure | ||
ActiveRecord::Tasks::DatabaseTasks.send(:alias_method, :schema_dump_path, :old_schema_dump_path) | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realised that we always return a hash.