Add UI #70
Add UI #70
Conversation
VladislavSokov
commented
Jun 24, 2024
•
VladislavSokov
commented
- add icons to the buttons
- move logic from controller to a service
- refactor with "each db connection"
- intro new env var for "disabled"
|
|
lib/actual_db_schema/engine.rb
Outdated
|
|
||
| initializer "actual_db_schema.append_routes", after: "add_routing_paths" do |app| | ||
| app.routes.append do | ||
| mount ActualDbSchema::Engine => "/actual_db_schema" |
There was a problem hiding this comment.
| mount ActualDbSchema::Engine => "/actual_db_schema" | |
| mount ActualDbSchema::Engine => "/rails" |
Can we do this? Won't it conflict with rails mailers previews?
| @manual_mode = manual_mode || manual_mode_default? | ||
| super() | ||
| super(context: context) |
There was a problem hiding this comment.
Here that makes sense since the signature is different from base, unlike in others.
lib/actual_db_schema.rb
Outdated
| class << self | ||
| attr_accessor :config, :failed | ||
| end | ||
|
|
||
| self.failed = [] | ||
| self.config = { | ||
| enabled: Rails.env.development?, | ||
| auto_rollback_disabled: ENV["ACTUAL_DB_SCHEMA_AUTO_ROLLBACK_DISABLED"].present? | ||
| enabled: Rails.env.development? || ENV["ACTUAL_DB_SCHEMA_ENABLED"].present?, |
There was a problem hiding this comment.
This option is not related to this PR, please extract it to separate one.
| @@ -1,3 +1,6 @@ | |||
| ## [0.7.6] - 2024-07-02 | |||
| - Added UI | |||
| def index; end | ||
|
|
||
| def show | ||
| render :not_found, status: 404 unless migration |
There was a problem hiding this comment.
What does it render now if no migration is found? Please check. And also add tests.
|
|
||
| def routes_setup | ||
| @routes = @app.routes | ||
| Rails.application.routes.draw do |
There was a problem hiding this comment.
Still, why we should init routes here and why they are not initiated during engine loading? Maybe the problem is that the engine is not being loaded in tests? Then we should try to load it in tests?
These lines signify that the engine is not being loaded in tests and those lines are not covered by tests. And this is bad as the tests are fragile.
| end | ||
|
|
||
| def branch_for(version) | ||
| metadata.fetch(version, {})[:branch] || "unknown" | ||
| end | ||
|
|
||
| def metadata |
There was a problem hiding this comment.
Can you please move these lines back so that we don't have the not relevant diff in git?
|
Manual testing results:
2. If no phantom migrations the table looks clunky
Show some text saying no phantom migrations instead of the empty table? Or fix the table look so it's clear what's going on.
|
I think this is a great idea |
