janko / rodauth-rails

Rails integration for Rodauth authentication framework
https://github.com/jeremyevans/rodauth
MIT License
584 stars 40 forks source link

DB is not rodauth's db #3

Closed HoneyryderChuck closed 4 years ago

HoneyryderChuck commented 4 years ago

I'm having a bit of inconsistency in my tests, and I think it's due to this: https://github.com/janko/rodauth-rails/blob/master/lib/generators/rodauth/templates/config/initializers/sequel.rb

In this example, DB is the sequel connection loading the activerecord connection. However, this might not be the same as the rodauth db, because rodauth defines db as Sequel::DATABASES.first, and that might not be DB. This config thereby assumes there's only one sequel db loaded, and that might not be the case (I'm testing roda and rails integrations).

As a change proposal, I'd suggest maybe defining the db constant in rodauth app:

class RodauthApp < Rodauth::Rails::App

  configure do
    enable :login, :http_basic_auth, :oauth

    db DB
    rails_controller { RodauthController }
...

Alternatively, one could define the DB directly in the rodauth app file, unless there's real need to expose the DB constant in initializer code.

janko commented 4 years ago

Thanks for the feedback. I think a good solution would be not to generate the Sequel initializer if a Sequel connection already exists (i.e. if Sequel::DATABASES is not empty). Because if Sequel is already initialized, that probably means the user wants to use that connection for Rodauth. I'll go ahead and make that change.

The initializer code could theoretically go in Rodauth's #post_configure method, but I prefer to keep it in an initializer, because I want users to know how Sequel is configured, in case they want to tweak it in case something is not working correctly.