Open nathanpalmer opened 5 years ago
We are running into this issue as well. We use the same setup, and get the same errors.
Any updates on this?
👋
How have folks been solving this?
My hacky solution for now is to copy the definitions into both places...
@shageman We don't have any hacky solutions, have been adding the views in the main app and not the engine, unfortunately.
Today we ended up fixing this by allowing a root_path
override on all the helpers. Example
create_view :order_calculated_costs, version: 2, root_path: Database::Engine.root
or
update_view :product_stats, version: 2, revert_to_version: 1, root_path: Database::Engine.root
This allows you to pass in an extra parameter, if you are calling these from a rails engine. Seems to work well in my tests, even when the engine is referenced from another engine. I couldn't see a way to auto-detect this behavior so this was our next best option.
It includes some minor override parameters like this
If the scenic team is interested is accepting it as a PR I can put it together.
@nathanpalmer do you have a fork with the fix for this issue? Running into it myself but I really want to use scenic inside an engine.
I just submitted a PR with the changes needed to override the root.
@nathanpalmer Nice! Hoping this gets merged in soon. This is a much needed change for all who are using rails engines and using scenic to create db views.
@nathanpalmer @derekprior hey guys any updates on this? We've been using scenic for a while inside an engine, and this change would just make our lives so much greater. Eternal thanks would be given back after merging it :D
@nathanpalmer Any updates on this?
@alexanderfrankel There is nothing I can do from my side to move this along. The PR is out #287, but will need a response from the scenic team to move forward.
@nathanpalmer Ok thanks for the update.
@derekprior It would be a huge help to get this PR merged and this issue resolved. Thanks for all your work on this project!
My hesitancy here is based in my unfamiliarity with engines. I don't want to commit to a solution only to find out it has headaches down the road. Since I don't know headaches with engines or even best practices with them, I'm a bit at a loss.
Can anyone point to prior art for gems that load files being used in an engine?
@derekprior Thanks for your response; however, @nathanpalmer's solution here: https://github.com/scenic-views/scenic/pull/287 has nothing to do with Rails engines specifically. It just adds more flexibility by allowing the location of the file containing the view definition to be explicitly defined.
This solution could be useful for other scenarios outside of using Rails engines if someone wants to define their views in a different file location other than db/views
I am running into this, as well. However, I tend to prefer running railties:install:migrations
to pull migrations into the main application from the engines and scenic doesn't even know it's inside an engine in the first place (as it wants to look in the dummy application's db folder).
Ideally, views would go in db/views
instead of spec/dummy/db/views
and when you run railties:install:migrations
, it would pull the views into the application that is running the command.
In the case of the engine, the views would start in db/views
(alongside migrations in db/migrate
) and then run from there for the engine. Upstream, running railties:install:migrations
would pull in both migrations AND views in similar relation to each other - just look at what railties:install:migrations
does and the views should live adjacent to the migrations.
As Derek mentioned, the use case is unfortunately foreign to us and we’d like to see examples of gems that do similar things to accommodate being run inside of an engine.
It seems like there are at least three of you in this thread-are any of you able to share examples of gems you’re using in engines within you app that do this? It would be really helpful to have some examples we could poke through to see this.
Something like Devise, which has migrations it creates, or another gem with migration generators beyond installing itself like Scenic does would be great. Thanks!
Sorry, I mean further than those specific gems, if there are code examples you can track down that would be useful.
I'm not entirely sure what you're looking for. It seems to me that views should be pulled in when bundle exec rake railties:install:migrations
is run. If I knew how to do that, I would have hooked it up in our project myself. I settled on a workaround in our codebase currently, but it's not ideal. Even if it's a new rake command that mimics railties:install:migrations
pull in views from scenic, that's still better than the current state where there is no way to pull views into an application from the engine they live in.
@calebthompson there is a book on the topic: https://github.com/shageman/component-based-rails-applications-book
You can also find a whole host of blog posts and other resources on this approach at https://cbra.info
PS: Best to look through the resources... I am biased, both of the above links point to stuff I build/wrote 🙂
I am also running into issues using this in an engine.
engine/[engine_name]/app/[engine_name]/[engine_name]/[engine_name.rb]
which defines table_name_prefix
. I deleted this file since table_name_prefix
is already defined within rails engines by the engines themselves.rails [engine_name]:install:migrations
does not copy over the view files. I just manually copy, for now.rails db:migrate
did not find the *.sql files for the views because it's being run from within engine/[engine_name]/test/dummy/
. This one I monkey patched.My current monkey patch is this:
engine/[engine_name]/lib/[engine_name.rb]
I already had require "scenic"
and I added a require "scenic_monkeypatch"
engine/[engine_name]/lib/scenic_monkeypatch.rb
with the following contents:class Scenic::Definition
def path
if Module.const_defined?(:Dummy) &&
ARGV.length > 0 &&
ARGV[0] == 'db:migrate'
File.join("..", "..", "db", "views", filename)
else
File.join("db", "views", filename)
end
end
end
Note: Dummy is the actual name of the rails app that engines generate for testing purposes so I just check for the existence of that module as a way to know that the current ruby process (e.g. test, migrate, etc) is running within the engine itself and not within the host app.
For those of you unfamiliar with monkey patching, this simply re-opens the class that is defined within the scenic gem and overwrites a method. This is what it is overwriting: https://github.com/scenic-views/scenic/blob/3600a485797fe1dbf30d152cc60b6d2318f04c48/lib/scenic/definition.rb#L22
(I'll add to this comment as I run into other issues and fix them.)
I just ran into this issue as well. Added scenic inside an engine, and it tries to find the definition from the main application. I might try to solve it with a variation of the monkeypatch above. When it comes to the engine's dummy app, I solved that by just symlinking the views folder in the dummy.
This PR would have definitely been nice to have! https://github.com/scenic-views/scenic/pull/287
Yep just tried the monkeypatch and it worked, my variation is
module Scenic
class Definition
def full_path
<EngineName>::Engine.root.join(path)
end
end
end
Seems like the easiest approach for now, despite being a monkeypatch. It would be great to have a standard supported way to do this though.
Yep just tried the monkeypatch and it worked, my variation is
module Scenic class Definition def full_path <EngineName>::Engine.root.join(path) end end end
Seems like the easiest approach for now, despite being a monkeypatch. It would be great to have a standard supported way to do this though.
@jamesyp where did you put this file?
@gregkonush I had to do a little PR digging to find it, but I had this monkeypatch in engines/<engine_name>/lib/scenic_monkeypatch.rb
, and these requires in engines/<engine_name>/lib/<engine_name>.rb
:
require 'scenic'
require 'scenic_monkeypatch'
This worked at the time and I was actually using it in a production codebase, but we don't have the monkeypatch anymore. We "flattened" the engine it was originally in and promoted most of its functionality to the root Rails app (unrelated to Scenic or anything in this issue directly), so no more need for the patch. It'll probably still work but I can't easily check in the project anymore, so can't 100% promise it'll still work for you too.
@gregkonush I had to do a little PR digging to find it, but I had this monkeypatch in
engines/<engine_name>/lib/scenic_monkeypatch.rb
, and these requires inengines/<engine_name>/lib/<engine_name>.rb
:require 'scenic' require 'scenic_monkeypatch'
This worked at the time and I was actually using it in a production codebase, but we don't have the monkeypatch anymore. We "flattened" the engine it was originally in and promoted most of its functionality to the root Rails app (unrelated to Scenic or anything in this issue directly), so no more need for the patch. It'll probably still work but I can't easily check in the project anymore, so can't 100% promise it'll still work for you too.
Thank you, @jamesyp! This monkey patch still works
@gregkonush I had to do a little PR digging to find it, but I had this monkeypatch in
engines/<engine_name>/lib/scenic_monkeypatch.rb
, and these requires inengines/<engine_name>/lib/<engine_name>.rb
:require 'scenic' require 'scenic_monkeypatch'
This worked at the time and I was actually using it in a production codebase, but we don't have the monkeypatch anymore. We "flattened" the engine it was originally in and promoted most of its functionality to the root Rails app (unrelated to Scenic or anything in this issue directly), so no more need for the patch. It'll probably still work but I can't easily check in the project anymore, so can't 100% promise it'll still work for you too.
This also worked for me although I had to put the require in <engine_name>/<engine_name>.rb
like so:
require '<engine_name>/scenic_monkeypatch'
This all works though, and the views now live in <engine_name>/db/views
and get migrated both in the engine schema and the apps that the engine is added to, which is ace.
However, I do have an issue with trying to use rails generate scenic:view my_view
where it does the following when run:
❯ rails generate scenic:view my_view
create spec/dummy/db/views
conflict db/views/my_view_v01.sql
This is despite my_view
is actually at version 5 so it isn't detecting what is already existing. Can work around by creating the view and migration manually, but thought I'd flag here too.
However, I do have an issue with trying to use
rails generate scenic: view my_view
Ah, looks like this is the problem as it also uses Rails.root.join("db", "views")
We would like to use scenic from within a rails engine. However, the path to the view is not coming up correctly. In my example I have this.
The
products
is the rails engine that is referenced by the main app by this.and to get rails to recognize regular migrations this is put in the
engine.rb
Failing in the component directory
From inside the
components/products
directory when I run arake db:migrate
it seems to be looking for the view intest/dummy/db/views
instead ofdb/views
.20181206151701 CreateExpandedDeals: migrating
``` == 20181206151701 CreateExpandedDeals: migrating ============================== -- create_view(:expanded_deals) rake aborted! StandardError: An error has occurred, this and all later migrations canceled: No such file or directory @ rb_sysopen - /Users/nathan/Source/test_app/components/products/test/dummy/db/views/expanded_deals_v01.sql /Users/nathan/.rvm/gems/ruby-2.3.4/gems/scenic-1.4.1/lib/scenic/definition.rb:10:in `read' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/scenic-1.4.1/lib/scenic/definition.rb:10:in `to_sql' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/scenic-1.4.1/lib/scenic/statements.rb:142:in `definition' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/scenic-1.4.1/lib/scenic/statements.rb:36:in `create_view' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:849:in `block in method_missing' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:818:in `block in say_with_time' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:818:in `say_with_time' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:838:in `method_missing' /Users/nathan/Source/test_app/components/products/db/migrate/20181206151701_create_expanded_deals.rb:3:in `change' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:792:in `exec_migration' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:776:in `block (2 levels) in migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:775:in `block in migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/connection_adapters/abstract/connection_pool.rb:408:in `with_connection' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:774:in `migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:953:in `migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:1230:in `block in execute_migration_in_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:1298:in `block in ddl_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/connection_adapters/abstract/database_statements.rb:235:in `block in transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/connection_adapters/abstract/transaction.rb:194:in `block in within_new_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/connection_adapters/abstract/transaction.rb:191:in `within_new_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/connection_adapters/abstract/database_statements.rb:235:in `transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/transactions.rb:210:in `transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:1298:in `ddl_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:1229:in `execute_migration_in_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:1201:in `block in migrate_without_lock' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:1200:in `each' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:1200:in `migrate_without_lock' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:1148:in `block in migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:1317:in `with_advisory_lock' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:1148:in `migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:1007:in `up' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/migration.rb:985:in `migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/tasks/database_tasks.rb:171:in `migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.6.1/lib/active_record/railties/databases.rake:58:in `block (2 levels) inFailing in the main app
And if I try to run the migration from the main app itself it's looking for it in
db/views
instead ofcomponents/products/db/views
.20181206151701 CreateExpandedDeals: migrating
``` == 20181206151701 CreateExpandedDeals: migrating ============================== -- create_view(:expanded_deals) rake aborted! StandardError: An error has occurred, this and all later migrations canceled: No such file or directory @ rb_sysopen - /Users/nathan/Source/test_app/db/views/expanded_deals_v01.sql /Users/nathan/.rvm/gems/ruby-2.3.4/gems/scenic-1.4.1/lib/scenic/definition.rb:10:in `read' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/scenic-1.4.1/lib/scenic/definition.rb:10:in `to_sql' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/scenic-1.4.1/lib/scenic/statements.rb:142:in `definition' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/scenic-1.4.1/lib/scenic/statements.rb:36:in `create_view' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:849:in `block in method_missing' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:818:in `block in say_with_time' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:818:in `say_with_time' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:838:in `method_missing' /Users/nathan/Source/test_app/components/products/db/migrate/20181206151701_create_expanded_deals.rb:3:in `change' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:792:in `exec_migration' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:776:in `block (2 levels) in migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:775:in `block in migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:408:in `with_connection' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:774:in `migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:953:in `migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:1230:in `block in execute_migration_in_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:1298:in `block in ddl_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/connection_adapters/abstract/database_statements.rb:225:in `block in transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/connection_adapters/abstract/transaction.rb:194:in `block in within_new_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/connection_adapters/abstract/transaction.rb:191:in `within_new_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/connection_adapters/abstract/database_statements.rb:225:in `transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/transactions.rb:210:in `transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:1298:in `ddl_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:1229:in `execute_migration_in_transaction' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:1201:in `block in migrate_without_lock' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:1200:in `each' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:1200:in `migrate_without_lock' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:1148:in `block in migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:1317:in `with_advisory_lock' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:1148:in `migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:1007:in `up' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/migration.rb:985:in `migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/tasks/database_tasks.rb:171:in `migrate' /Users/nathan/.rvm/gems/ruby-2.3.4/gems/activerecord-5.1.2/lib/active_record/railties/databases.rake:58:in `block (2 levels) inPossible fixes?
From looking around PR #237 might fix this, but since that was submitted in February I wasn't sure if it would be merged in any time soon. I haven't seen any feedback from the original author, though @calebthompson did comment on it a day or so ago.