Migration issue

[rohandaxini]

Hi @vinistock Seems we have an issue with class name generated due to migration step for sail configuration settings.

When we run the step rails g sail my_desired_migration_name and let's say we name our migration as for_sail i.e. rails g sail for_sail then it generates following migration file.

class CreateSailSettings < ActiveRecord::Migration
  def change
    create_table :sail_settings do |t|
      t.string :name, null: false
      t.text :description
      t.string :value, null: false
      t.integer :cast_type, null: false, limit: 1
      t.timestamps
      t.index ["name"], name: "index_settings_on_name", unique: true
    end
  end
end

But when we run rake db:migrate then this throws an error as

NameError: uninitialized constant ForSail

Here the class name is CreateSailSettings which should be ideally ForSail in this case. I had to manually modify the class name i.e. top line in the migration to be class ForSail < ActiveRecord::Migration and then the migrations started working.

[rohandaxini]

@vinistock I also noticed that the page index.html.erb runs out with an error as undefined local variable or method "number_of_pages" when we try navigate the application to /sailsand when we have zero configuration in the table. Seems we need to handle such edge cases where user begins with zero settings then we should still show them Dashboard to start over from.

[vinistock]

@rohandaxini Indeed the migration name is static right now, which is the source of the issue.

Concerning the number_of_pages issue, I'd have to give it a check. It shouldn't happen even if the user has zero settings. Perhaps the helper module wasn't properly imported for some reason. I will try to reproduce it.

Anyhow, both issues need fixing.

[rohandaxini]

@vinistock

It shouldn't happen even if the user has zero settings. Perhaps the helper module wasn't properly imported for some reason. I will try to reproduce it.

Sure, let me know if you need any help to reproduce it. After I added one record manually from console say, for example, using the following snippet

Sail::Setting.create(name: :my_setting, cast_type: :integer, description: 'A very important setting', value: '15')

then I no longer face error of undefined local variable or method "number_of_pages" but now I am facing another issue at next step that says ActiveModel::MissingAttributeError (missing attribute: updated_at): though the table for Sail Setting has this column (attribute) properly created.

[rohandaxini]

@vinistock Refer attached screenshot

Seems it originates due to this line sail (1.2.2) app/controllers/sail/settings_controller.rb:7:in index'`

I tried debugging the gem and this is what I see on the console

(byebug) Setting.by_name(params[:query]).paginated(index_params[:page])
  CACHE (0.1ms)  SELECT  `sail_settings`.`name`, `sail_settings`.`description`, `sail_settings`.`value`, `sail_settings`.`cast_type` FROM `sail_settings` LIMIT 8 OFFSET 0
#<ActiveRecord::Relation [#<Sail::Setting id: nil, name: "my_setting", description: "A very important setting", value: "15", cast_type: 0>]>

Seems its trying to use updated_at attribute which the above query does not return in the result set.

[vinistock]

@rohandaxini I believe this was caused because the query didn't select the updated_at field and perhaps it was needed for fresh_when.

I pushed a commit in master to try to fix it, if you could please try to reproduce this again and see if it fully addressed the issue, then I can close this.

After resolving this, I will release a new version, since we had significant defects fixed.

Thanks a ton!

[rohandaxini]

@vinistock I pulled latest from master and tested on local.

1. Issue with updated_at is fixed now
2. I still face errors for number_of_pages despite having 1 settings in table that I added from the console. I then manually hardcoded value as 1 for @number_of_pages in file index.html.erb to load it so that I can test further. Seems we still need to fix it as I noticed that you have method number_of_pages in application helper but the instance variable is not used correctly in the erb file.
3. Despite doing step-2 above, I face issues with Sail Dashboard conflicting with style of navigation header of my app in which I am trying to integrate sail. Then I commented those haml snippet in my app locally and tried to load sail dashboard. This is what I see now, refer screenshot below.

4. I noticed one more issue. Using the latest code from master and after creating one setting from console, example

Sail::Setting.create(name: :my_setting, cast_type: :date, description: 'A very important setting', value: '2018-15-30')

Now, when I try to create another setting for integer or float then it rollbacks the record. I still need to investigate and debug this issue on why its happening. You might have more insights though. Here is an example for your reference

2.2.2 :034 > Sail::Setting.create(name: :my_setting, cast_type: :string, description: 'A very important setting', value: '15')
   (0.3ms)  BEGIN
  Sail::Setting Exists (0.4ms)  SELECT  1 AS one FROM `sail_settings` WHERE `sail_settings`.`name` = BINARY 'my_setting' LIMIT 1
   (0.2ms)  ROLLBACK

Can you try to use sail gem (and your latest code from master) on your local in one of your sample app and see if it all works for you just fine?

[vinistock]

@rohandaxini I've created a new application from scratch and included Sail, but couldn't reproduce the number_of_pages issue. I took the following steps and things worked fine

1. rails new test_app --database=mysql
2. added Sail to gemfile
3. added mount in routes.rb
4. ran rails g sail create_settings and rake db:migrate
5. visited /sail

Results were as expected. The dashboard was, of course, empty, but I got no errors.

I will push out a new release for the moment due to the fixed bugs and if we manage to accurately reproduce this we can work on a fix.

[rohandaxini]

I will push out a new release for the moment due to the fixed bugs and if we manage to accurately reproduce this we can work on a fix.

Sounds good, let's go for it 👍
Seems we have made good progress in past few days 😊

Results were as expected. The dashboard was, of course, empty, but I got no errors.

@vinistock Actually it works all fine in an empty new application. But when you try to integrate sail with an existing app that has a lot of code, has many routes like say for example mount Resque::Server.new, at: "/resque" and mount_griddler, that has a lot of styles and active_admin then it seems sail is not loading properly or conflicting with something. I also need to debug this more on why sail is not working properly to integrate with existing app.

I can give you access to the app with which I am trying to integrate sail to try with - so do you want to test in that app?

[vinistock]

@rohandaxini that would help. If you can provide a sample application that reproduces the error, I will take a detailed look and try to find the source of the problem.

Thanks for all the help you've been providing for the project!

[rohandaxini]

@vinistock I invited you to this repo now https://github.com/Kiprosh/knowbuddy
Please note that this is a playground application where many members have contributed just to learn. So hope you don't judge the app 😉

[rohandaxini]

@vinistock I tried debugging this issue and found that the app in which I am using sail is unable to load this ApplicationHelper from sail gem.

module Sail
  module ApplicationHelper
    def number_of_pages
      @number_of_pages ||= (Sail::Setting.count.to_f / Sail::Setting::SETTINGS_PER_PAGE).ceil
    end
  end
end

To do so, I had to explicitly add this line in my Knowbuddy applications ApplicationController

helper Sail::Engine.helpers

This fixed the issue for me.

I read about it here https://edgeapi.rubyonrails.org/classes/Rails/Engine.html#label-Isolated+engine%27s+helpers
Please refer section Isolated engine's helpers

Is there a better way to fix it? Or is my debugging and above mentioned solution fine?

Also, one more thought - Why don't we move this method number_of_pages to SettingsControllerinstead of having a dedicated helper for it? Reason: Its only used in one view which is views/sail/settings/index.html.erb

def number_of_pages
  @number_of_pages ||= (Sail::Setting.count.to_f / Sail::Setting::SETTINGS_PER_PAGE).ceil
end

[vinistock]

@rohandaxini I don't see any problems with moving it to the controller. However, I will take a look at the sample application you provided first.

If you read the section from the article you mentioned, it says you'd only need to add the helper statement if you're trying to use helpers from an isolated engine inside your main app. In other words, you shouldn't need to explicitly include the helper since it is only used within the engine itself.

Could be a setup issue. If I can't find the source of the problem, I will put together a pull request and move it to the controller.

[rohandaxini]

@vinistock Agree, but as I was facing ongoing issue with number_of_pages method while trying to load sail dashboard on /sail path so I tried these various options just to verify if anything is getting loaded properly or not.

Yeah, I think you can yourself debug it once and then we can decide to move it to the controller.

Another issue I am facing now is that Sail Dashboard does not load properly and shows js asset errors as follows

ActionController::RoutingError (No route matches [GET] "/sail/mini-profiler-resources/includes.js"):

and the UI appears broken as 👇

Let me know if it works for you?

[rohandaxini]

@vinistock Did you got chance to check above mentioned issue and screenshot ?

[vinistock]

@rohandaxini I haven't had the chance to clone your app and try to figure out what's going on just yet. As soon as I can, I will take a look at it.

I have tried using Sail in existing applications and brand new applications and so far got no errors. I'd have to dig into your app to understand what's happening.

[rohandaxini]

@vinistock Sure no problem, thanks for your help.

[vinistock]

@rohandaxini I was finally able to reproduce the pagination bug you reported. It should be fixed by #30.

I will push out a minor release with the fixes as soon as possible and then close this issue.

Thanks for pointing it out!

[rohandaxini]

@vinistock Glad to hear 👍
Sure that sounds good, let me know once done and ill again test it on my local.

[vinistock]

@rohandaxini Give version 1.4.2 a try and see if it works fully

[rohandaxini]

Sure @vinistock ill give it a try and let you know. Thanks.

[rohandaxini]

@vinistock Now, I am facing the following issue while navigating to http://localhost:3000/sail
Did you try from scratch now on v1.4.2 ?

[vinistock]

That is really weird. As you can see on line 18, Sail tests to see if the main app has a root_path defined before trying to invoke it.

I tested it with the dummy app inside spec and everything works fine. I don't understand how it would get past this check, if root_path is undefined.

main_app.respond_to?("root_path")

[rohandaxini]

@vinistock Yeah it's weird. I noticed that sails routes.rb correctly has root 'settings#index' so then ideally it root_path should work without any problems.

Is it mandatory to add this configuration in one of the initializers?

Sail.configure do |config|
  config.cache_life_span = 10.minutes # How long to cache the Sail::Setting.get response for
  config.array_separator = ';'        # Default separator for array settings
  config.dashboard_auth_lambda = nil  # Defines an authorization lambda to access the dashboard as a before action. Rendering or redirecting is included here if desired.
  config.back_link_path = 'root_path' # Path method as string for the "Main app" button in the dashboard. Any non-existent path will make the button disappear
end

I tried it and still same error. Seems like the mounting problems?

I will debug more.

[rohandaxini]

@vinistock When I run specs for the gem, I am facing these errors

NameError:
  uninitialized constant Sail
# ./spec/controllers/sail/settings_controller_spec.rb:1:in `<top (required)>'

[vinistock]

@rohandaxini the root path it looks for is from the main app and not the engine. That is, your application should have the root_path defined. However, Sail checks to see if the app has a root_path defined or not, so that shouldn't be a problem.

Concerning the specs, I have no idea why that error would happen. In order to run the specs, you need google-chrome and chromedriver installed for the feature specs, but that wouldn't explain that error.

These errors are very strange. How is your environment set up? Which OS, which Ruby version, do you use RVM?

[rohandaxini]

@vinistock Thanks for details.
Here is an additional information you requested.

OS = macOS High Sierra v10.13.6
Ruby version = tried on ruby-2.2.2 and ruby-2.4.3
RVM = Yes I do use rvm

[vinistock]

@rohandaxini and which Rails version is your app using? It could be that Sail requires a certain version of Rails and I'm not aware of that.

[rohandaxini]

@vinistock Its Rails 4.2.4 that the application is using in which I am trying Sail.

[vinistock]

@rohandaxini I believe #34 may fix the issue for older Rails applications. Could you please help me verify it?

I tried creating a Rails 4 app on my machine and I'm getting some weird errors. Just checkout to that branch and point to the local version of Sail in your app's Gemfile like so

gem "sail", path: "PATH_TO_SAIL_CLONE"

[rohandaxini]

@vinistock Good news, Your PR #34 fixed the issues. I am not facing any errors related to routing ot root_path now.

Seem's the setting page is not loaded properly now and logs have these errors

Started GET "/sail/mini-profiler-resources/includes.js?v=898a13ca6797c6bc1fee313e17d388b0" for ::1 at 2018-12-04 10:37:26 +0530

ActionController::RoutingError (No route matches [GET] "/sail/mini-profiler-resources/includes.js"):

Screenshot 👇

When I try to create new settings then also it shows similar errors and does not create settings due to error

[2018-12-04 10:37:27] ERROR Errno::ECONNRESET: Connection reset by peer @ io_fillbuf - fd:19
	/Users/rohan/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/webrick/httpserver.rb:80:in `eof?'
	/Users/rohan/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/webrick/httpserver.rb:80:in `run'
	/Users/rohan/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/webrick/server.rb:294:in `block in start_thread'

Do you recommend to not use Sails gem with Rails4 apps and instead try them only in Rails5 apps or any other specific Ruby versions ?

[vinistock]

@rohandaxini Sail needs to support Rails 4. There are still many applications that haven't migrated to Rails 5.

I believe the missing styles and javascript errors might be due to async loading, I will dig into that and try to figure out the problem.

And the server issue must be associated with using the YAML file to create settings, I suppose. These seem like separate issues, I will create two new bugs to track this. Thanks a ton for your help, I wouldn't have found these issues on my own.

[rohandaxini]

@vinistock Sounds good. I am glad I could help and contribute on Sail 😃