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

Support multiple db/views paths #237

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
18 changes: 17 additions & 1 deletion lib/scenic/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,25 @@ def initialize(name, version)
end

def to_sql
File.read(full_path).tap do |content|
File.read(find_definition).tap do |content|
derekprior marked this conversation as resolved.
Show resolved Hide resolved
if content.empty?
raise "Define view query in #{path} before migrating."
end
end
end

def find_definition
derekprior marked this conversation as resolved.
Show resolved Hide resolved
definition = views_paths.flat_map do |directory|
derekprior marked this conversation as resolved.
Show resolved Hide resolved
Dir.glob("#{directory}/**/#{filename}")
end.first
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
end.first
end

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be a find, but then it would only return the directory that the view exists under. This actually resolves the first matching file that is found under each view_path directory. Plus it could be possibly be nested.


unless definition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be more consistent like this:

Suggested change
unless definition
if definition.empty?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/IfUnlessModifier: Favor modifier unless usage when having a single-line body. Another good alternative is the usage of control flow &&/||.

raise "Unable to locate #{filename} in #{views_paths}"
end

definition
end

def full_path
Rails.root.join(path)
end
Expand All @@ -28,6 +40,10 @@ def version

private

def views_paths
Rails.application.config.paths["db/views"].expanded
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

end

def filename
"#{@name}_v#{version}.sql"
end
Expand Down
2 changes: 2 additions & 0 deletions lib/scenic/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ module Scenic
# @see Scenic.load
class Railtie < Rails::Railtie
initializer "scenic.load" do
Rails.application.config.paths.add("db/views")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


ActiveSupport.on_load :active_record do
Scenic.load
end
Expand Down
Empty file.
1 change: 1 addition & 0 deletions spec/fixtures/db_views/searches_v01.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
SELECT text 'Hi' as greeting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SELECT text 'Hi' as greeting
SELECT text 'Hi' as greeting

51 changes: 44 additions & 7 deletions spec/scenic/definition_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,51 @@

module Scenic
describe Definition do
describe "find_definition" do
derekprior marked this conversation as resolved.
Show resolved Hide resolved
it "raises an error if file cant be found" do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

definition = Definition.new("not_valid", 1)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


expect {
derekprior marked this conversation as resolved.
Show resolved Hide resolved
definition.find_definition
}.to raise_error RuntimeError, /Unable to locate not_valid_v01.sql/
end

it "looks inside Rails.root db/views" do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

definition = Definition.new("not_valid", 1)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


expect {
derekprior marked this conversation as resolved.
Show resolved Hide resolved
definition.find_definition
}.to raise_error RuntimeError, /spec\/dummy\/db\/views/
end

it "looks inside configured additional paths" do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

with_views_fixtures do
definition = Definition.new("empty_view", 1)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


expect(definition.find_definition).to be
end
end
end

describe "to_sql" do
it "returns the content of a view definition" do
sql_definition = "SELECT text 'Hi' as greeting"
allow(File).to receive(:read).and_return(sql_definition)

definition = Definition.new("searches", 1)
with_views_fixtures do
definition = Definition.new("searches", 1)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


expect(definition.to_sql).to eq sql_definition
expect(definition.to_sql).to eq sql_definition
end
end

it "raises an error if the file is empty" do
allow(File).to receive(:read).and_return("")
definition = Definition.new("empty_view", 1)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


expect do
Definition.new("searches", 1).to_sql
end.to raise_error RuntimeError
with_views_fixtures do
expect do
definition.to_sql
end.to raise_error RuntimeError
end
end
end

Expand Down Expand Up @@ -52,5 +81,13 @@ module Scenic
expect(definition.version).to eq "15"
end
end

def with_views_fixtures
original = Rails.application.config.paths["db/views"].to_a
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

Rails.application.config.paths["db/views"] << File.expand_path("../../fixtures/db_views", __FILE__)
derekprior marked this conversation as resolved.
Show resolved Hide resolved
yield
ensure
Rails.application.config.paths["db/views"] = original
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

end
end
end